Резолв типа ресивера-модуля и позиционный API TypeService (#3991)#3993
Merged
Conversation
Автодополнение/signature help на вызове метода общего модуля (ОбщегоНазначения.Метод()) не работали: тип ресивера не выводился. Root cause: ссылка на голое имя общего модуля резолвится ReferenceIndexReferenceFinder'ом в ModuleSymbol, а inferSymbol(ModuleSymbol) → inferModuleAsType знал только OneScript-library-модули (по URI в oScriptLibraryIndex) и для BSL-модуля конфигурации возвращал EMPTY. Поэтому findTypes на ресивере давал пусто, и члены/сигнатура не находились. Фикс точечный: inferModuleAsType дополнительно берёт зарегистрированный конфигурационный тип модуля по URI документа из ConfigurationModuleMembersProvider (там общий модуль уже зарегистрирован как CONFIGURATION-тип). Этого достаточно — completion и signature help чинятся через штатный findTypes, без правок в провайдерах. Помечен арх. долг (TODO + задачи): оба типа модулей уже лежат в GlobalScopeProvider по имени, и резолв стоит свести к одному источнику вместо двух URI-индексов. Тесты: findTypes на ресивере-модуле (root-cause), completion mid-call, signature help на методе общего модуля. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vider Рефактор (задача #1): инференсер больше не обращается к двум URI-ключевым подсистемным индексам (oScriptLibraryIndex + ConfigurationModuleMembersProvider) ради восстановления типа ресивера-модуля. Тип теперь берётся из единого обратного индекса URI→TypeRef в GlobalScopeProvider. Почему так: GlobalScopeProvider уже владеет обоими типами модулей по ИМЕНИ (общий модуль — PLATFORM_GLOBAL_PROPERTY, library — LIBRARY_MODULE), но у inferSymbol(ModuleSymbol) на руках URI, а не имя — не хватало обратного lookup'а у самого авторитета, и его обходили через чужие индексы. - GlobalScopeProvider: indexModuleType/removeModuleType/moduleTypeByUri(URI). - Писатели — те же провайдеры регистрации модулей, синхронно рядом с регистрацией имени (URI у них на руках): общий/менеджер/объектный модуль (ConfigurationModuleMembersProvider) и library-модуль (OScriptModuleMembersProvider); снятие library — в его unregister(uri). - ExpressionTypeInferencer.inferModuleAsType — один вызов moduleTypeByUri; удалены зависимости OScriptLibraryIndex и ConfigurationModuleMembersProvider (минус 2 коллаборатора, задача #2), снят мёртвый typeForUri (#3991). Race-safety: обновления синхронные (event-листенеры не @async), в тех же хендлерах, ConcurrentHashMap; переименование — перезапись по URI; удаление library — remove(uri). Общий модуль без листенера удаления и reindex библиотек протекают ровно как существующий name-индекс (паритет, не регресс). Тест: lifecycle обратного индекса (register→present, unregister→cleared). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
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:
📝 WalkthroughWalkthroughRenames member lookup APIs to memberAt/membersAt, adds Reference-driven type entry points (typesAt, expressionTypesAt), changes hover builders to accept Reference, indexes module types by URI in GlobalScopeProvider, updates provider/diagnostic/inlay flows, and migrates many tests. ChangesType Resolution and Hover Markup Refactoring
Sequence Diagram(s)sequenceDiagram
participant Editor
participant CompletionProvider
participant SignatureHelpProvider
participant TypeService
participant GlobalScopeProvider
Editor->>CompletionProvider: request completion at dot
CompletionProvider->>TypeService: receiverTypesAt(documentContext, position)
Editor->>SignatureHelpProvider: request signature help inside call
SignatureHelpProvider->>TypeService: expressionTypesAt(documentContext, argStart)
SignatureHelpProvider->>TypeService: receiverTypesAt(documentContext, memberPos)
TypeService->>GlobalScopeProvider: moduleTypeByUri(uri) (when needed)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
2 times, most recently
from
6acc243 to
d80c81a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java (1)
36-41:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix the JavaDoc target type.
The class and
getSymbolClass()handleAnnotationParamSymbol, but the JavaDoc still saysAnnotationSymbol. That leaves the public contract misleading right where this API was changed.📝 Proposed fix
-/** - * Построитель контента для всплывающего окна для {`@link` AnnotationSymbol}. - */ +/** + * Построитель контента для всплывающего окна для {`@link` AnnotationParamSymbol}. + */As per coding guidelines, "Write JavaDoc for public APIs, include comments for complex logic, and keep documentation up to date with code changes".
🤖 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/AnnotationParamSymbolMarkupContentBuilder.java` around lines 36 - 41, Update the class JavaDoc to reference the correct target type: replace the incorrect {`@link` AnnotationSymbol} with {`@link` AnnotationParamSymbol} (the class AnnotationParamSymbolMarkupContentBuilder and its getSymbolClass() already operate on AnnotationParamSymbol) so the public documentation matches the actual API and implementation.src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.java (1)
108-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve the legacy
returnType()fallback for platform-member references.Lines 108-112 only read
descriptor.returnTypes().ExpressionTypeInferencer#inferDereferencestill falls back todescriptor.returnType()for members that have not been migrated yet, sotypesAt(reference)now returns empty for those same references. That makes the new reference-based path weaker than the existing AST path and can drop hover/type info for legacy platform members.Proposed fix
if (reference.symbol() instanceof PlatformMemberSymbol platformMember) { - var returnTypes = platformMember.getDescriptor().returnTypes(); + var descriptor = platformMember.getDescriptor(); + var returnTypes = descriptor.returnTypes(); if (returnTypes != null && !returnTypes.isEmpty()) { return returnTypes; } + var returnType = descriptor.returnType(); + if (returnType != null && !returnType.equals(TypeRef.UNKNOWN)) { + return TypeSet.of(returnType); + } }🤖 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/TypeService.java` around lines 108 - 112, The platform-member branch only uses platformMember.getDescriptor().returnTypes() and drops legacy single returnType(), so restore the fallback: when reference.symbol() is a PlatformMemberSymbol and descriptor.returnTypes() is null or empty, call platformMember.getDescriptor().returnType(), convert it into the expected collection (if non-null/non-empty) and return that instead; update the code around the PlatformMemberSymbol check in TypeService (the reference.symbol() instanceof PlatformMemberSymbol platformMember block) to prefer returnTypes() but fall back to returnType() to match ExpressionTypeInferencer#inferDereference behavior.
🤖 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/oscript/OScriptModuleMembersProvider.java`:
- Around line 130-135: The code currently calls
globalScopeProvider.indexModuleType(uri, ref) unconditionally causing CLASS
entries to overwrite MODULE refs; change OScriptModuleMembersProvider so you
only call indexModuleType for module entries (e.g., check libraryEntry.kind() ==
OScriptLibraryIndex.EntryKind.MODULE or libraryEntry.kind() !=
OScriptLibraryIndex.EntryKind.CLASS) before invoking indexModuleType(uri, ref),
leaving the existing CLASS-specific handling (the subsequent if
(libraryEntry.kind() == OScriptLibraryIndex.EntryKind.CLASS) block) unchanged.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java`:
- Around line 333-336: The helper referenceTo currently builds a Reference using
symbol.getSelectionRange(), causing tests to exercise declaration-site hover;
change tests to use a usage-site Reference when asserting inference by
constructing the Reference from the actual hovered occurrence range (the
read/write occurrence) instead of symbol.getSelectionRange(). Add a second
helper (e.g., referenceToUsage or referenceTo(DocumentContext, Range)) or
overload referenceTo to accept the occurrence Range and use
Reference.of(documentContext.getSymbolTree().getModule(), symbol, new
Location(documentContext.getUri().toString(), occurrenceRange)); update
inference-focused assertions to call that usage-site helper so
VariableSymbolMarkupContentBuilder and typeService.typesAt(reference) are
exercised with the position-sensitive reference.
---
Outside diff comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java`:
- Around line 36-41: Update the class JavaDoc to reference the correct target
type: replace the incorrect {`@link` AnnotationSymbol} with {`@link`
AnnotationParamSymbol} (the class AnnotationParamSymbolMarkupContentBuilder and
its getSymbolClass() already operate on AnnotationParamSymbol) so the public
documentation matches the actual API and implementation.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.java`:
- Around line 108-112: The platform-member branch only uses
platformMember.getDescriptor().returnTypes() and drops legacy single
returnType(), so restore the fallback: when reference.symbol() is a
PlatformMemberSymbol and descriptor.returnTypes() is null or empty, call
platformMember.getDescriptor().returnType(), convert it into the expected
collection (if non-null/non-empty) and return that instead; update the code
around the PlatformMemberSymbol check in TypeService (the reference.symbol()
instanceof PlatformMemberSymbol platformMember block) to prefer returnTypes()
but fall back to returnType() to match ExpressionTypeInferencer#inferDereference
behavior.
🪄 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: 2f3087f3-3416-41c7-bd28-51bad13a31e3
⛔ Files ignored due to path filters (2)
src/test/resources/types/CommonModuleMidCallCompletion.bslis excluded by!src/test/resources/**src/test/resources/types/CommonModuleSignatureHelp.bslis excluded by!src/test/resources/**
📒 Files selected for processing (41)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AssignToReadOnlyPropertyDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/platform/PlatformMemberCalls.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorCallMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/KeywordSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilderConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/PlatformMemberSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/PlatformMethodCallInlayHintSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SignatureHelpProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/references/PlatformMemberReferenceFinder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolStructureRenderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SignatureHelpCommonModuleTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonModuleCallInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceBenchmarkTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceDelegationTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/autumn/AutumnDependencyInjectionInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptLibraryFileSystemWatcherTest.java
Comment on lines
+333
to
+336
| private static Reference referenceTo(DocumentContext documentContext, SourceDefinedSymbol symbol) { | ||
| return Reference.of(documentContext.getSymbolTree().getModule(), symbol, | ||
| new Location(documentContext.getUri().toString(), symbol.getSelectionRange())); | ||
| } |
Contributor
There was a problem hiding this comment.
Use a usage-site Reference in the inference-focused tests.
Line 334 always builds the Reference from symbol.getSelectionRange(), so the updated tests still exercise hover from the declaration site. That misses the main contract change here: VariableSymbolMarkupContentBuilder now delegates inferred content to typeService.typesAt(reference), which is position-sensitive. A regression in hover for an actual read/write occurrence could slip through while these tests stay green. Please build the Reference from the concrete hovered occurrence in the inference assertions, or add a second helper for usage-site references.
🤖 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/hover/VariableSymbolMarkupContentBuilderTest.java`
around lines 333 - 336, The helper referenceTo currently builds a Reference
using symbol.getSelectionRange(), causing tests to exercise declaration-site
hover; change tests to use a usage-site Reference when asserting inference by
constructing the Reference from the actual hovered occurrence range (the
read/write occurrence) instead of symbol.getSelectionRange(). Add a second
helper (e.g., referenceToUsage or referenceTo(DocumentContext, Range)) or
overload referenceTo to accept the occurrence Range and use
Reference.of(documentContext.getSymbolTree().getModule(), symbol, new
Location(documentContext.getUri().toString(), occurrenceRange)); update
inference-focused assertions to call that usage-site helper so
VariableSymbolMarkupContentBuilder and typeService.typesAt(reference) are
exercised with the position-sensitive reference.
2d9a813 to
7adb999
Compare
Contributor
Test Results 2 964 files +12 2 964 suites +12 1h 17m 2s ⏱️ +13s Results for commit 3f25f28. ± Comparison against base commit 1e5d9e4. This pull request removes 10 and adds 9 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
7adb999 to
decf892
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/KeyValueIndexAccessInferenceTest.java (1)
38-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale JavaDoc reference to removed
inferAtPosition.The class JavaDoc still links
{@linkTypeService#inferAtPosition}, but the test now exercisesexpressionTypesAtand that API was renamed/removed in this PR. This is now a broken javadoc link.As per coding guidelines: "keep documentation up to date with code changes".📝 Proposed doc fix
- * результат {`@link` TypeService#inferAtPosition} напрямую — на уровне TypeSet, + * результат {`@link` TypeService#expressionTypesAt} напрямую — на уровне TypeSet,🤖 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/types/KeyValueIndexAccessInferenceTest.java` around lines 38 - 44, The class JavaDoc in KeyValueIndexAccessInferenceTest references the removed method TypeService#inferAtPosition, causing a broken link; update the doc to reference the current API (e.g. TypeService#expressionTypesAt) or remove the {`@link`} entirely and adjust the narrative to state that the test exercises expressionTypesAt (or the actual current method name) directly at the TypeSet level so the javadoc matches the code under test.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainAndIndexAccessInferenceTest.java (1)
89-89: ⚡ Quick winTypeService still exposes the old positional API.
TypeServicedeclarespublic TypeSet expressionTypesAt(DocumentContext documentContext, Position position), so the tests’ continued use ofexpressionTypesAt(...)shouldn’t break compilation. If the intent was to standardize on the consolidatedtypesAt(...)/receiverTypesAt(...)APIs, consider updating the test helpers for consistency.🤖 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/types/ChainAndIndexAccessInferenceTest.java` at line 89, Tests are calling the legacy TypeService.expressionTypesAt(DocumentContext, Position) API while the project aims to standardize on the consolidated typesAt(...) / receiverTypesAt(...) APIs; update the test helper in ChainAndIndexAccessInferenceTest to call the new TypeService.typesAt(...) or TypeService.receiverTypesAt(...) (whichever matches the test intent) instead of expressionTypesAt, passing the same DocumentContext and adjusted positional arguments, and update any helper wrapper method that currently delegates to expressionTypesAt to delegate to the new API so tests and TypeService usage are consistent.
🤖 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/providers/SignatureHelpProvider.java`:
- Around line 140-142: Update the JavaDoc link to point to the actual method
used: replace the incorrect {`@link` TypeService#typesAt(DocumentContext,
org.eclipse.lsp4j.Position)} with the correct {`@link`
TypeService#expressionTypesAt(DocumentContext, org.eclipse.lsp4j.Position)} (or
{`@link` TypeService#expressionTypesAt(DocumentContext, Position)}) so the JavaDoc
references the expressionTypesAt method that is invoked by
SignatureHelpProvider.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionMethodReturnsTest.java`:
- Line 113: The test calls TypeService.expressionTypesAt(DocumentContext,
Position) but TypeService still exposes that method and it’s used internally by
receiverSegmentTypes(...), so either update the test to use the intended public
API (typesAt(...) or receiverTypesAt(...)) or keep expressionTypesAt(...)
documented as an internal API; specifically, decide whether expressionTypesAt
should remain public or be made package-private/removed, then update the test in
CollectionMethodReturnsTest to call the appropriate method
(TypeService.typesAt(...) or TypeService.receiverTypesAt(...)) or adjust
TypeService (change visibility of expressionTypesAt or delegate
typesAt/receiverTypesAt to it) and update the PR description/docs to reflect the
chosen public surface (TypeService.expressionTypesAt, TypeService.typesAt,
TypeService.receiverTypesAt, and receiverSegmentTypes).
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceTest.java`:
- Line 143: The test currently calls typeRegistry.getMembers(arrayRef) directly,
bypassing FileType-aware filtering; update the test in TypeServiceTest to use
the public API TypeService.getMembers(TypeRef, FileType) instead (e.g., replace
the direct call to typeRegistry.getMembers(arrayRef) with a call to
typeService.getMembers(arrayRef, <appropriate FileType>)), ensuring you pass the
correct FileType used by the test so member filtering behavior is exercised via
TypeService rather than TypeRegistry.
---
Outside diff comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/KeyValueIndexAccessInferenceTest.java`:
- Around line 38-44: The class JavaDoc in KeyValueIndexAccessInferenceTest
references the removed method TypeService#inferAtPosition, causing a broken
link; update the doc to reference the current API (e.g.
TypeService#expressionTypesAt) or remove the {`@link`} entirely and adjust the
narrative to state that the test exercises expressionTypesAt (or the actual
current method name) directly at the TypeSet level so the javadoc matches the
code under test.
---
Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainAndIndexAccessInferenceTest.java`:
- Line 89: Tests are calling the legacy
TypeService.expressionTypesAt(DocumentContext, Position) API while the project
aims to standardize on the consolidated typesAt(...) / receiverTypesAt(...)
APIs; update the test helper in ChainAndIndexAccessInferenceTest to call the new
TypeService.typesAt(...) or TypeService.receiverTypesAt(...) (whichever matches
the test intent) instead of expressionTypesAt, passing the same DocumentContext
and adjusted positional arguments, and update any helper wrapper method that
currently delegates to expressionTypesAt to delegate to the new API so tests and
TypeService usage are consistent.
🪄 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: ff27aebb-158d-4f46-bef3-3391c1cbd9ed
📒 Files selected for processing (95)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AssignToReadOnlyPropertyDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/platform/PlatformMemberCalls.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorCallMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/KeywordSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilderConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/PlatformMemberSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/PlatformMethodCallInlayHintSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SignatureHelpProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/references/PlatformMemberReferenceFinder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolStructureRenderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccessChainOnUnionTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccumulatePatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ArrayChainAccessTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BigBslFixtureTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BinaryOperatorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BooleanExpressionsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CatalogAttributeMembersTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainAndIndexAccessInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CircularVariableInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionMethodReturnsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonBslPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonModuleCallInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ComprehensiveInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConfigurationManagerChainInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConstructorVariantsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ControlFlowPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CrossModuleCallsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/DereferenceVariationsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/DynamicConstructorTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ElementTypesInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ExpressionTreePatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ForEachPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/GlobalEnumPropertyInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/GlobalScopeInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InferEdgeCasesTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/JsDocVariantsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/KeyValueIndexAccessInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/LiteralInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MapAndFixedStructureInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MethodCallPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MixedAssignmentsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MoreBinaryOpsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MorePlatformInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptDirectivesTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ParameterJsDocInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PlatformCollectionsChainTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PrimitiveConversionTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/QueryAndDateLiteralsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnTypeInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeCrossModuleRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeLocalMethodRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeMetadataRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SignatureInheritanceInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SkippedArgAndChainDereferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StreamingApiPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureConstructorEdgeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureConstructorKeysInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureInsertFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureLocalFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionConstructorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceBenchmarkTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceDelegationTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/UnaryExpressionPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableColumnsFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableTypedColumnsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/VariableTrackingPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/autumn/AutumnDependencyInjectionInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/MultiWorkspaceTypeRegistryTest.java
🚧 Files skipped from review as they are similar to previous changes (29)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceBenchmarkTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/PlatformMemberReferenceFinder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/PlatformMemberSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/platform/PlatformMemberCalls.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolStructureRenderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorCallMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/KeywordSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AssignToReadOnlyPropertyDiagnostic.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/PlatformMethodCallInlayHintSupplier.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilderConfiguration.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonModuleCallInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/autumn/AutumnDependencyInjectionInferenceTest.java
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
decf892 to
aa6eac7
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.java (2)
397-420: 💤 Low valueConsider narrowing the exception handler or logging suppressed exceptions.
The broad
catch (Exception e)at line 417 silently swallows all exceptions. While this provides robustness, it could mask unexpected bugs (e.g.,NullPointerExceptionfrom an unexpectednullingetContentList()). Consider catching onlyIndexOutOfBoundsExceptionor at minimum logging the exception atDEBUGlevel for diagnostics.🤖 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/TypeService.java` around lines 397 - 420, In receiverEndBeforeDot, replace the broad catch (Exception e) with a narrower handler (e.g., catch IndexOutOfBoundsException and/or NullPointerException) or rethrow unexpected exceptions, and/or log the caught exception at DEBUG using the existing logger so unexpected nulls from documentContext.getContentList() or similar issues are visible; update the catch to only handle the specific bounds/null cases and include a debug log of the exception and context (position) before returning Optional.empty().
117-122: 💤 Low valueClarify the purpose of the empty-string resolve call.
Line 118 calls
typeRegistry.resolve("")with an empty string, which appears to be a side-effect-only bootstrap trigger. This pattern is unclear and could confuse future maintainers. Consider extracting this into a named method likeensureTypeRegistryBootstrapped()or adding a comment explaining why this call is necessary here.🤖 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/TypeService.java` around lines 117 - 122, Replace the unclear side-effect call typeRegistry.resolve("") in TypeService.expressionTypesAt with a self-documenting helper (e.g. ensureTypeRegistryBootstrapped()) or add an inline comment explaining the bootstrap side-effect; implement ensureTypeRegistryBootstrapped() as a private method on TypeService that performs the resolve call (and any future initialization) so callers clearly show intent and maintenance becomes easier, then call that helper from expressionTypesAt before invoking ExpressionAtPosition.findExpressionTree.
🤖 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/types/TypeService.java`:
- Around line 397-420: In receiverEndBeforeDot, replace the broad catch
(Exception e) with a narrower handler (e.g., catch IndexOutOfBoundsException
and/or NullPointerException) or rethrow unexpected exceptions, and/or log the
caught exception at DEBUG using the existing logger so unexpected nulls from
documentContext.getContentList() or similar issues are visible; update the catch
to only handle the specific bounds/null cases and include a debug log of the
exception and context (position) before returning Optional.empty().
- Around line 117-122: Replace the unclear side-effect call
typeRegistry.resolve("") in TypeService.expressionTypesAt with a
self-documenting helper (e.g. ensureTypeRegistryBootstrapped()) or add an inline
comment explaining the bootstrap side-effect; implement
ensureTypeRegistryBootstrapped() as a private method on TypeService that
performs the resolve call (and any future initialization) so callers clearly
show intent and maintenance becomes easier, then call that helper from
expressionTypesAt before invoking ExpressionAtPosition.findExpressionTree.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5abce692-3f46-4b85-be7f-e895052ee0f6
📒 Files selected for processing (96)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/AssignToReadOnlyPropertyDiagnostic.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/platform/PlatformMemberCalls.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorCallMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/KeywordSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilderConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/PlatformMemberSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/PlatformMethodCallInlayHintSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SignatureHelpProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/references/PlatformMemberReferenceFinder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolStructureRenderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccessChainOnUnionTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccumulatePatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ArrayChainAccessTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BigBslFixtureTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BinaryOperatorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/BooleanExpressionsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CatalogAttributeMembersTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainAndIndexAccessInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CircularVariableInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionMethodReturnsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonBslPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonModuleCallInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ComprehensiveInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConfigurationManagerChainInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConstructorVariantsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ControlFlowPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CrossModuleCallsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/DereferenceVariationsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/DynamicConstructorTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ElementTypesInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ExpressionTreePatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ForEachPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/GlobalEnumPropertyInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/GlobalScopeInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InferEdgeCasesTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/JsDocVariantsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/KeyValueIndexAccessInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/LiteralInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MapAndFixedStructureInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MethodCallPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MixedAssignmentsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MoreBinaryOpsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/MorePlatformInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptDirectivesTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ParameterJsDocInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PlatformCollectionsChainTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PrimitiveConversionTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/QueryAndDateLiteralsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnTypeInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeCrossModuleRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeLocalMethodRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeMetadataRefInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SignatureInheritanceInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/SkippedArgAndChainDereferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StreamingApiPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureConstructorEdgeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureConstructorKeysInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureInsertFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureLocalFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionConstructorInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceBenchmarkTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceDelegationTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/UnaryExpressionPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableColumnsFieldsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableTypedColumnsInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/VariableTrackingPatternsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/autumn/AutumnDependencyInjectionInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/MultiWorkspaceTypeRegistryTest.java
💤 Files with no reviewable changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java
✅ Files skipped from review due to trivial changes (3)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeCrossModuleRefInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/HoverProvider.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderTest.java
🚧 Files skipped from review as they are similar to previous changes (78)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ControlFlowPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ParameterJsDocInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnTypeInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeMetadataRefInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableTypedColumnsInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/StreamingApiPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/references/PlatformMemberReferenceFinder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptDirectivesTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/DynamicConstructorTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/InferEdgeCasesTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/PrimitiveConversionTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccessChainOnUnionTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConstructorVariantsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceBenchmarkTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/VariableTrackingPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ArrayChainAccessTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/MapAndFixedStructureInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/BigBslFixtureTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ConstructorCallMarkupContentBuilder.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/AccumulatePatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/JsDocVariantsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/platform/PlatformMemberCalls.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionMethodReturnsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CrossModuleCallsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/SeeLocalMethodRefInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/MorePlatformInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/UnaryExpressionPatternsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ExpressionTreePatternsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/PlatformMemberSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/MethodCallPatternsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/inlayhints/PlatformMethodCallInlayHintSupplier.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ElementTypesInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/QueryAndDateLiteralsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonModuleCallInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/KeywordSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/ModuleSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/MixedAssignmentsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/OScriptPatternsTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/MoreBinaryOpsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableColumnsFieldsInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/SyntheticSymbolMarkupContentBuilderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureLocalFieldsInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ComprehensiveInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MethodSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/KeyValueIndexAccessInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/BinaryOperatorInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CommonBslPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/PlatformCollectionsChainTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeDescriptionConstructorInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/MarkupContentBuilderConfiguration.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/StructureInsertFieldsInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/GlobalEnumPropertyInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationSymbolMarkupContentBuilderTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/autumn/AutumnDependencyInjectionInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/MultiWorkspaceTypeRegistryTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ForEachPatternsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/TypeServiceDelegationTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SignatureHelpProvider.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolStructureRenderTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ConfigurationManagerChainInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/AnnotationParamSymbolMarkupContentBuilderTest.java
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
aa6eac7 to
9c77ab9
Compare
Позиционная поверхность TypeService для консьюмеров сведена к трём явным операциям с чёткой семантикой: - receiverTypesAt(DC, Position) — тип ресивера доступа к члену (именованный символ / mid-call / висячая точка разбираются внутри); - expressionTypesAt(DC, Position) — тип ВСЕГО охватывающего выражения в позиции (для типов аргументов вызова и инференс-тестов; позиционно нечувствителен внутри выражения — для `Г + А.Б.В` это тип суммы); - typesAt(Reference) — тип значения по ссылке (hover). Консьюмеры: CompletionProvider.dotCompletion и SignatureHelpProvider — на receiverTypesAt; типы аргументов (SignatureHelp, InlayHint) — на expressionTypesAt. Удалён несвязный findTypes(SourceDefinedSymbol) («тип символа» — некогерентное понятие). Ховер reference-ориентирован: контракт билдера getContent(Reference) (без дублирующего symbol-параметра), дженерик MarkupContentBuilder<T> убран. Удалены методы-делегаты, использовавшиеся только тестами (findGlobalContext, getGlobalContextNames, одно-арг getMembers/getDescription/getConstructors/resolve); тесты переведены на TypeRegistry/GlobalScopeProvider напрямую. findMemberAt → memberAt, findMembersAt → membersAt. В SignatureHelpProvider удалён мёртвый fallback на голое имя OneScript-модуля (receiverTypesAt его покрывает). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
9c77ab9 to
3f25f28
Compare
Member
Author
|
@claude review pls |
|
Claude finished @nixel2007's task in 2m 25s —— View job Code ReviewОбщий вывод: стек хорошо структурирован, чистый API, тесты покрывают новые сценарии. Ниже — одна реальная ошибка и несколько мелких проблем. Баг:
|
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
895ae8b to
ce5ea63
Compare
Member
Author
Ответ по ревью (CodeRabbit + Claude)Спасибо за разбор. Изменения — в коммите Исправлено
Отклонено (с обоснованием)
|
- OScriptModuleMembersProvider: indexModuleType только для роли MODULE — у dual-role .os-файла роль CLASS больше не перетирает тип модуля в moduleTypeByUri (+ red-тест OScriptModuleMembersProviderTest) - ExpressionTypeInferencer.inferDereference: удалена недостижимая ветка returnType()-fallback (returnType() производный от returnTypes(): при пустом returnTypes всегда UNKNOWN), заодно убраны лишние null/isEmpty - SignatureHelpProvider: javadoc-ссылка typesAt → expressionTypesAt - AnnotationParamSymbolMarkupContentBuilder: javadoc AnnotationSymbol → AnnotationParamSymbol (+ убран ставший лишним импорт) - TypeServiceTest: члены через публичный typeService.getMembers(ref, fileType) вместо прямого typeRegistry.getMembers - актуализированы устаревшие упоминания inferAtPosition/findMemberAt/ findMembersAt в комментариях и javadoc → expressionTypesAt/memberAt/membersAt Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
nixel2007
force-pushed
the
refactor/typeservice-positional-api
branch
from
ce5ea63 to
c4aab4f
Compare
|
7 tasks
sfaqer
added a commit
that referenced
this pull request
Jun 3, 2026
…зиты.<имя> Проверяет наружный эффект MetadataCollectionSpecializer через TypeService — именно тот API, который видят consumer'ы после реархитектуры #3993 (expressionTypesAt(DC, Position) для типа охватывающего выражения). Регистрация специализаций уже покрыта unit-тестами в registry, здесь — резолв выражения целиком на каждом шаге цепочки, на тестовой metadata-фикстуре (Документ1 с реквизитами и табчастью). Тип Метаданные приходит из платформенного bsl-context, поэтому тест требует HBK — включается переменной BSL_LANGUAGE_SERVER_RUN_HBK_TESTS.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Стек из трёх логически связанных изменений (фикс → рефакторинг), все в области вывода типов по позиции.
1. Фикс #3991 — автодополнение/signature help на члене общего модуля
ОбщегоНазначения.Метод()не резолвил тип ресивера. Root cause: голое имя общего модуля резолвится вModuleSymbol, аinferSymbol(ModuleSymbol)знал только OneScript-library-модули и для BSL-модуля конфигурации возвращал пусто. Фикс:inferModuleAsTypeберёт зарегистрированный конфигурационный тип модуля. Чинит и completion, и signature help через штатныйfindTypes.2. Единый источник типа модуля-значения
inferModuleAsTypeлазил в два URI-ключевых подсистемных индекса (oScriptLibraryIndex+ConfigurationModuleMembersProvider). Введён обратный индексURI → типвGlobalScopeProvider(единый авторитет), наполняемый провайдерами регистрации модулей синхронно. Инференсер теряет 2 зависимости. Обновления race-safe (синхронные листенеры,ConcurrentHashMap), удаление library — вunregister(uri).3. Реархитектура позиционного API TypeService
Поверхность для консьюмеров сведена к двум самодостаточным точкам входа —
typesAt(...)(тип значения в позиции; перегрузка поReference) иreceiverTypesAt(...)(тип ресивера доступа к члену: именованный/mid-call/висячая точка — без оркестрации в провайдерах). Удалён некогерентныйfindTypes(SourceDefinedSymbol). Ховер переведён на reference-ориентированный контрактMarkupContentBuilder.getContent(Reference)(без дублирующего symbol-параметра, дженерик убран).findMemberAt/findMembersAt→memberAt/membersAt.Все тесты зелёные (142 сьюта по затронутым областям: types/providers/hover/inlay/semantictokens/diagnostics).
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
New Features
Tests