feat(documentLink): кликабельные ссылки «См.» в описаниях методов#4110
Conversation
Добавлен SeeReferenceDocumentLinkSupplier: сканирует doc-комментарии методов на ссылки вида `// См. ИмяМетода` и `// См. ОбщийМодуль.Метод` (а также английское `See`), разрешает их в местоположение целевого метода и формирует DocumentLink над текстом ссылки. Ссылки на метод того же модуля разрешаются через SymbolTree, ссылки на метод общего модуля — через Configuration.findCommonModule и ServerContext.getDocument. Неразрешимые ссылки пропускаются (без висячих ссылок). Цель ссылки указывается сразу (URI#Lстрока,колонка). Co-Authored-By: Claude Fable 5 <[email protected]>
📝 WalkthroughWalkthroughAdds ChangesSee-Reference Document Link Feature
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
| public List<DocumentLink> getDocumentLinks(DocumentContext documentContext) { | ||
| var documentLinks = new ArrayList<DocumentLink>(); | ||
|
|
||
| for (MethodSymbol method : documentContext.getSymbolTree().getMethods()) { |
There was a problem hiding this comment.
У переменных тоже есть описание, и там могут быть линки
There was a problem hiding this comment.
Готово — сапплаер теперь обходит описания и методов, и переменных через общий Describable.getDescription().getLinks() (метод лежит на базовом SourceDefinedSymbolDescription, общем для MethodSymbol и VariableSymbol). Добавлен тест: Перем с // См. <ref> в doc-комментарии даёт ссылку. (b915198)
|
|
||
| var moduleName = reference.substring(0, separatorIndex); | ||
| var methodName = reference.substring(separatorIndex + 1); | ||
| return resolveCommonModuleMethod(documentContext, moduleName, methodName); |
There was a problem hiding this comment.
Зачем??? Есть же TypeService
There was a problem hiding this comment.
Проверил по коду: на этой ветке (от develop, без #4111) TypeService кейс не покрывает. Все его точки входа — по Position/Reference/TypeRef/MemberDescriptor; единственный строковый метод resolve(String, FileType) отдаёт TypeRef, а не источниковый символ/Location. Ссылка «См.» — голая строка из doc-комментария: её нет ни в AST как вызова, ни в индексе ссылок. Цепочка findCommonModule → getMdoRef → getDocument → getMethodSymbol — штатная идиома репозитория (та же в ReferenceIndexFiller, MissingCommonModuleMethodDiagnostic, ScheduledJobHandlerDiagnostic), не самопал; зафиксировал причину в javadoc. Правильный инструмент — TypeService.definingSymbol, который добавляет #4111: как только он вмержится, переведу резолв на него (или ребейзну #4110 поверх). Если ты имел в виду другой метод TypeService — подскажи, переключусь.
…TypeService Сапплаер «См.» теперь обходит описания не только методов, но и переменных: оба символа Describable, getLinks() лежит на общем SourceDefinedSymbolDescription. Резолв цели не переведён на TypeService: на этой ветке он умеет резолвить только по позиции/Reference или возвращать TypeRef по имени, но не источниковый символ/Location по строковому имени из doc-комментария. Причина зафиксирована в javadoc resolveTarget; идиома findCommonModule → getDocument сохранена. Co-Authored-By: Claude Fable 5 <[email protected]>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/DocumentLinkProviderTest.java`:
- Around line 65-86: The assertion chain in
testProviderAggregatesSeeReferenceLinks() has a compilation error because
ListAssert does not support chaining after anyMatch(). Split the single
assertion into two separate assertions: one that uses anyMatch to verify at
least one extracted target contains "`#L`", and another that verifies the
documentLinks list contains "http://example.com". The anyMatch check should be
performed on the extracted list, and the contains check should be performed
separately on the original documentLinks list or on a re-extracted list.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2b336772-886f-484c-b220-4170974ed0e9
⛔ Files ignored due to path filters (1)
src/test/resources/documentlink/seeReferenceDocumentLinkSupplier.bslis excluded by!src/test/resources/**
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/documentlink/SeeReferenceDocumentLinkSupplier.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/documentlink/SeeReferenceDocumentLinkSupplierTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/DocumentLinkProviderTest.java
| @Test | ||
| void testProviderAggregatesSeeReferenceLinks() { | ||
| // given | ||
| var content = """ | ||
| // См. ДругойМетод. http://example.com | ||
| Процедура Тест() Экспорт | ||
| КонецПроцедуры | ||
|
|
||
| Процедура ДругойМетод() Экспорт | ||
| КонецПроцедуры | ||
| """; | ||
| var documentContext = TestUtils.getDocumentContext(content); | ||
|
|
||
| // when | ||
| var documentLinks = documentLinkProvider.getDocumentLinks(documentContext); | ||
|
|
||
| // then | ||
| assertThat(documentLinks) | ||
| .extracting(DocumentLink::getTarget) | ||
| .anyMatch(target -> target.contains("#L")) | ||
| .contains("http://example.com"); | ||
| } |
There was a problem hiding this comment.
Fix the invalid assertion chain.
Lines 82-85 contain a compilation error. AssertJ's ListAssert (returned by extracting) does not have an anyMatch method that returns the assertion for chaining. The assertion must be split into separate checks:
🐛 Proposed fix
// then
- assertThat(documentLinks)
- .extracting(DocumentLink::getTarget)
- .anyMatch(target -> target.contains("`#L`"))
- .contains("http://example.com");
+ assertThat(documentLinks)
+ .extracting(DocumentLink::getTarget)
+ .anyMatch(target -> target.contains("`#L`"));
+ assertThat(documentLinks)
+ .extracting(DocumentLink::getTarget)
+ .contains("http://example.com");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Test | |
| void testProviderAggregatesSeeReferenceLinks() { | |
| // given | |
| var content = """ | |
| // См. ДругойМетод. http://example.com | |
| Процедура Тест() Экспорт | |
| КонецПроцедуры | |
| Процедура ДругойМетод() Экспорт | |
| КонецПроцедуры | |
| """; | |
| var documentContext = TestUtils.getDocumentContext(content); | |
| // when | |
| var documentLinks = documentLinkProvider.getDocumentLinks(documentContext); | |
| // then | |
| assertThat(documentLinks) | |
| .extracting(DocumentLink::getTarget) | |
| .anyMatch(target -> target.contains("#L")) | |
| .contains("http://example.com"); | |
| } | |
| `@Test` | |
| void testProviderAggregatesSeeReferenceLinks() { | |
| // given | |
| var content = """ | |
| // См. ДругойМетод. http://example.com | |
| Процедура Тест() Экспорт | |
| КонецПроцедуры | |
| Процедура ДругойМетод() Экспорт | |
| КонецПроцедуры | |
| """; | |
| var documentContext = TestUtils.getDocumentContext(content); | |
| // when | |
| var documentLinks = documentLinkProvider.getDocumentLinks(documentContext); | |
| // then | |
| assertThat(documentLinks) | |
| .extracting(DocumentLink::getTarget) | |
| .anySatisfy(target -> assertThat(target).contains("`#L`")); | |
| assertThat(documentLinks) | |
| .extracting(DocumentLink::getTarget) | |
| .contains("http://example.com"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/DocumentLinkProviderTest.java`
around lines 65 - 86, The assertion chain in
testProviderAggregatesSeeReferenceLinks() has a compilation error because
ListAssert does not support chaining after anyMatch(). Split the single
assertion into two separate assertions: one that uses anyMatch to verify at
least one extracted target contains "`#L`", and another that verifies the
documentLinks list contains "http://example.com". The anyMatch check should be
performed on the extracted list, and the contains check should be performed
separately on the original documentLinks list or on a re-extracted list.
|
1 participant
Что сделано
Добавлен
SeeReferenceDocumentLinkSupplier(@Component implements DocumentLinkSupplier), который делает кликабельными перекрёстные ссылкиСм.(англ.See) в doc-комментариях методов. Клик по ссылке ведёт к объявлению целевого метода.Что линкуется
// См. ДругойМетод// См. ОбщийМодуль.МетодSeeподдерживается «из коробки» (общийSEE_KEYWORDлексера описанийbsl-parser).DocumentLink формируется строго над текстом ссылки (без ключевого слова
См.и без завершающей точки) — точныйRangeберётся изHyperlink.range()парсера описаний.Как разрешаются цели
Сапплаер не вводит новых индексов и переиспользует существующую инфраструктуру:
MethodSymbol.getDescription().getLinks()→Hyperlink(текст ссылки + диапазон уже в координатах документа);DocumentContext.getSymbolTree().getMethodSymbol(имя);Configuration.findCommonModule(имя)→MdoReference→ServerContext.getDocument(mdoRef, ModuleType.CommonModule)→getSymbolTree().getMethodSymbol(метод).Цель указывается сразу (eager
target) в формеURI#Lстрока,колонка—documentLink/resolveне требуется. Неразрешимые ссылки пропускаются, висячие (битые) ссылки не создаются.Тесты
SeeReferenceDocumentLinkSupplierTest: ссылка того же модуля → линк с точным диапазоном и целью; ссылка на общий модуль → линк на метод общего модуля; несуществующий метод → ничего (без висячих ссылок).DocumentLinkProviderTest: проверка агрегации — провайдер отдаёт «См.»-ссылки рядом с существующими (URL из комментариев).*DocumentLink*тесты зелёные (15 шт.),compileJava— BUILD SUCCESSFUL.🤖 Generated with Claude Code
Summary by CodeRabbit