feat(types): резолв См.-ссылок в определение через type service + кликабельные ссылки в hover#4209
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR replaces ChangesSee-reference links
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java (1)
154-198: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffДублирование с
resolveHyperlink/walkMembers.
resolveReferenceSymbolповторяет структуру перебора префиксов изresolveHyperlink, аwalkToMember— обход сегментов изwalkMembers. Различие лишь в возвращаемом результате (символ-источник vsTypeSet/обработка параметра-последнего-сегмента). Можно выделить общий обход сегментов, чтобы поведение резолва не разъезжалось при будущих правках.🤖 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/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java` around lines 154 - 198, `resolveReferenceSymbol` and `walkToMember` duplicate the same prefix/member traversal logic already used by `resolveHyperlink` and `walkMembers`; extract the shared segment-walking behavior in `SymbolTypeIndex` into a common helper so all resolvers use the same path resolution flow. Keep the existing return differences by having the shared traversal return the resolved chain/member and let `resolveReferenceSymbol`, `resolveHyperlink`, and `walkMembers` map that result to `SourceDefinedSymbol`, `TypeSet`, or parameter handling as needed.src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (1)
295-303: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueНедетерминированный порядок при нескольких источниках.
sourcesприходит какSet<Object>(вfieldExpansionSourcesэтоHashSetключей элементов), поэтому при наличии более одного источника порядок в результатеjoining(" | ")нестабилен между запусками. Для одиночного источника не проявляется, но при множественных «См.» метках вывод hover может «прыгать». Стоит зафиксировать порядок (например,sorted()по тексту ссылки) для стабильного рендеринга.🤖 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/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java` around lines 295 - 303, The seeReferenceLabel method builds the hover “See/См.” string from a Set, so the joined source links can appear in a different order between runs. Update seeReferenceLabel in VariableSymbolMarkupContentBuilder to make the output deterministic by sorting the mapped source links before joining them, while keeping the existing blank-link filtering and distinct handling intact.
🤖 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.
Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java`:
- Around line 295-303: The seeReferenceLabel method builds the hover “See/См.”
string from a Set, so the joined source links can appear in a different order
between runs. Update seeReferenceLabel in VariableSymbolMarkupContentBuilder to
make the output deterministic by sorting the mapped source links before joining
them, while keeping the existing blank-link filtering and distinct handling
intact.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java`:
- Around line 154-198: `resolveReferenceSymbol` and `walkToMember` duplicate the
same prefix/member traversal logic already used by `resolveHyperlink` and
`walkMembers`; extract the shared segment-walking behavior in `SymbolTypeIndex`
into a common helper so all resolvers use the same path resolution flow. Keep
the existing return differences by having the shared traversal return the
resolved chain/member and let `resolveReferenceSymbol`, `resolveHyperlink`, and
`walkMembers` map that result to `SourceDefinedSymbol`, `TypeSet`, or parameter
handling as needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 58461998-b87d-41f0-9f3f-86a9ef4e42dc
📒 Files selected for processing (9)
src/main/java/com/github/_1c_syntax/bsl/languageserver/documentlink/SeeReferenceDocumentLinkSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SeeReferenceHyperlinks.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/SourceSymbolLinks.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/documentlink/SeeReferenceDocumentLinkSupplierTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/RecursiveSeeRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReporterScenariosSeeRefTest.java
Test Results 3 588 files 3 588 suites 1h 52m 11s ⏱️ Results for commit ffefd69. ♻️ This comment has been updated with latest results. |
Добавлена переиспользуемая утилита SeeReferenceHyperlinks.toMarkdownLink(text, target): строит markdown-ссылку [text](uri#L<line>,<col>) на определение символа. Формат таргета совпадает с SeeReferenceDocumentLinkSupplier, поэтому переход и позиционирование работают в клиентах, поддерживающих такие ссылки (VS Code и совместимые). Сейчас применяется к меткам обрыва рекурсивных см.-цепочек: `См. Коробка` → `См. [Коробка](…)` с переходом к функции-конструктору. Утилита вынесена отдельно, чтобы переиспользовать её при доработке представления см.-ссылок в подсказках. Тесты (Reporter/Recursive SeeRef) обновлены на проверку markdown-ссылки. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
Добавлен переиспользуемый резолвер ссылки в символ-определение, не привязанный к hover/documentlink: - TypeService.resolveSeeReference(reference, context): Optional<SourceDefinedSymbol> — неквалифицированная ссылка ищется в методах того же модуля, квалифицированная (Модуль.Метод, Справочники.X.Метод, СправочникМенеджер.X.Метод и т.п.) — через реестр типов (SymbolTypeIndex.resolveReferenceSymbol), единообразно для общих модулей, модулей менеджеров и прочих типов; берётся source-символ найденного члена (MemberDescriptor.getSourceSymbol); - utils/SourceSymbolLinks.navigationTarget(symbol) — общий формат таргета uri#L<line>,<col>. SeeReferenceDocumentLinkSupplier переведён на TypeService.resolveSeeReference + SourceSymbolLinks вместо findCommonModule (тот видел только общие модули и не доставал, например, методы модулей менеджеров). DocumentLink больше не зависит от пакета hover. hover/SeeReferenceHyperlinks.toMarkdownLink использует тот же SourceSymbolLinks. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
Прямой тест мотивирующего случая: `См. СправочникМенеджер.СправочникСМенеджером.МетодМенеджера` теперь даёт document link на определение метода в модуле менеджера справочника — то, что старый findCommonModule не доставал. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
… См.-ссылок - юнит-тесты SymbolTypeIndex.resolveReferenceSymbol/walkToMember: пустая/ неквалифицированная ссылка, наличие/отсутствие source-символа, ненайденный член, обход вложенных сегментов. Покрытие нового кода поднято выше порога Quality Gate (предыдущий прогон Sonar был на устаревшем коммите без этих методов и их тестов); - seeReferenceLabel: добавлен sorted() перед join — детерминированный порядок при нескольких источниках (замечание CodeRabbit). Замечание про дублирование walkToMember/walkMembers осознанно не правлю: обходы расходятся (параметрический fallback нужен только для типов, извлечение source-символа — только для определений), общая абстракция добавила бы больше сложности, чем убирает. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
- S1448: TypeService больше не растёт (36 методов) — резолв См.-ссылки убран из фасада: SeeReferenceDocumentLinkSupplier теперь сам диспетчеризует (метод того же модуля — по дереву символов, квалифицированная ссылка — через SymbolTypeIndex.resolveReferenceSymbol). DocumentLink зависит от слоя типов, не от hover; - S109: магическая 2 вынесена в константу MIN_QUALIFIED_SEGMENTS; - S2589: убрана всегда-ложная проверка next == null (returnType() не-null под @NullMarked). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
…edSymbolLinks utils/ — листовой пакет и не должен зависеть от доменных типов (context.symbol), поэтому хелпер построения таргета uri#L<line>,<col> переехал из utils.SourceSymbolLinks в context.symbol.SourceDefinedSymbolLinks и снова принимает SourceDefinedSymbol (чистый однопараметрический API). Вызовы в hover (SeeReferenceHyperlinks) и documentlink (SeeReferenceDocumentLinkSupplier) обновлены. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
86dc8a3 to
3bb9f32
Compare
…ылок Исправляет смешение ответственностей, возникшее из-за попытки обойти смелл S1448: - дублирование убрано: resolveHyperlink (тип по строке) и резолв См.-ссылки в символ используют один обход resolveChain; из результата первый берёт тип возврата, второй — getSourceSymbol(). Удалены параллельные walkMembers/walkToMember; - слой типов: резолв См.-ссылки снова на фасаде TypeService.resolveSeeReference (диспетч «тот же модуль / квалифицированная» внутри фасада); DocumentLink-саплаер ходит через фасад, а не напрямую в SymbolTypeIndex. Ценой известного смелла S1448 (TypeService большой; дробление — отдельно); - presentation вынесен из домена: форматтер uri#L<line>,<col> переехал из context.symbol в доменно-независимый utils.NavigationLinks(URI, Range); им пользуются и hover, и documentlink. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
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/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java`:
- Around line 192-224: The parameter fallback in resolveChain is unreachable
when an intermediate MemberDescriptor has TypeKind.UNKNOWN, so
Модуль.Процедура.Параметр references fail before the trailing-parameter lookup
runs. Update SymbolTypeIndex.resolveChain to attempt the last-segment parameter
resolution via parameterFromMember(lastMethod, name) before returning null on an
unknown return type, and make sure lastMethod is set for method/procedure
members even when their returnType is unknown. Add a test covering a См.-style
reference to a procedure parameter to verify this path.
🪄 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: 2f4c36b1-36f5-4175-b6bc-7251b7e2f4ed
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/documentlink/SeeReferenceDocumentLinkSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SeeReferenceHyperlinks.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/NavigationLinks.java
…лой Hover→Utils resolveChain не доходил до фолбэка на параметр (Модуль.Метод.Параметр), если у промежуточного метода тип возврата UNKNOWN — а у процедур и недокументированных функций он всегда такой. Теперь на последнем сегменте пробуем разрешить его как имя параметра метода до выхода по UNKNOWN. Также ArchitectureTest: Hover теперь легитимно зависит от Utils (hover.SeeReferenceHyperlinks использует utils.NavigationLinks) — как и прочие фичевые пакеты (Color, DocumentLink, Folding, InlayHints, SemanticTokens). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
…ания Отдельный компонент ради однострочного построения markdown-ссылки избыточен — формирование ссылки перенесено прямо в VariableSymbolMarkupContentBuilder.sourceLink. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
Резолв не привязан к «См.»: разрешает имя/квалифицированную ссылку в определяющий символ — метод, общий модуль, менеджер справочника/документа, любой тип с модульным отражением. Имя метода фасада отражает это, а не «см.». Ветка имени типа целиком добавлена явно (resolve + definingSymbol), покрыта тестом на ссылку в тип менеджера. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
…34/S1142/S109) Добавленный фолбэк на параметр раздул resolveChain до когнитивной сложности 32. Дублированные блоки «последний сегмент — имя параметра метода» вынесены в parameterChain(); убраны лишние вложенность, возвраты и magic number 2. Поведение не изменилось (покрыто SymbolTypeIndexHyperlinkTest). Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01QcYuuctkrfWkmGqXaUmx5S
|



Продолжение #4196 / #4207 (#4204). Делает
См.-ссылки навигируемыми и вводитпереиспользуемый резолв ссылки в определение через type service (корректно по
общим модулям, модулям менеджеров и прочим типам).
Компоненты (нейтральные, без зависимости hover↔documentlink)
TypeService.resolveSeeReference(reference, context): Optional<SourceDefinedSymbol>—единая точка резолва
См.-ссылки в символ-определение:Метод) — среди методов того же модуля;Модуль.Метод,Справочники.X.Метод,СправочникМенеджер.X.Методи т.п.) — через реестр типов(
SymbolTypeIndex.resolveReferenceSymbol): идём по членам и берёмsource-символ найденного члена (
MemberDescriptor.getSourceSymbol).utils/SourceSymbolLinks.navigationTarget(symbol)— общий формат таргетаuri#L<line>,<col>(используют и documentlink, и hover).Применение
SeeReferenceDocumentLinkSupplier) переведён сfindCommonModuleна
TypeService.resolveSeeReference. Прежний путь видел только общие модули и недоставал, например, методы модулей менеджеров; теперь резолв единообразен.
DocumentLink больше не зависит от пакета
hover.SeeReferenceHyperlinks.toMarkdownLink) делает метки обрыварекурсивных
см.-цепочек кликабельными:См. Коробка→См. [Коробка](…)(источник метки —
MethodSymbol, резолв не нужен). Утилита и резолвер готовы кприменению при доработке представления
см.-ссылок в подсказках (это ужевопрос UX/«п.1» [BUG] Ошибка в определении последнего типа в ховере рекурсивных типов #4204 — в этот PR не входит).
Почему не
DereferenceMemberMatcherОн резолвит
приёмник.членпо узлам AST в позиции курсора(
matchAt(TerminalNode, …, Position)), аСм.-ссылка — строка изdoc-комментария без AST. Поэтому используется обход по реестру типов
(
findMember/resolveReferenceSymbol), параллельный существующемуresolveHyperlink.walkMembers(но возвращающий символ члена, без параметрическогоfallback, нужного только для типов).
Тесты
SeeReferenceDocumentLinkSupplierTest(same-module + общий модуль) проходит ужечерез type-service путь — он же включает резолв модулей менеджеров.
ReporterScenariosSeeRefTest/RecursiveSeeRefInferenceTestобновлены напроверку markdown-гиперссылки
См. [Имя](uri#L<line>,<col>)(RU) иSee [Имя](…)(EN).🤖 Generated with Claude Code
Summary by CodeRabbit