fix(types): поля структуры из возврата функции (в т.ч. межмодульно) + bump bsl-parser 0.36.0#4122
Merged
Merged
Conversation
0.36.0 чинит разбор многострочного составного типа поля в описании
метода (std453 п.5.3): раньше continuation-строка типа "разрезала"
структуру возвращаемого значения на второй тип верхнего уровня, и
автокомплит после точки не показывал часть ключей структуры.
Добавлен инференс-тест: тип переменной, присвоенной из вызова функции
с таким JsDoc, выводится как единый Структура со всеми полями (а не
{Структура, Строка}); поле с многострочным типом несёт оба типа.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
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:
📝 WalkthroughWalkthroughIntroduces ChangesLocalField model, TypeSet refactoring, and return structure metadata handling
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
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 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 |
![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: 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/types/ReturnStructureMultilineFieldTypeInferenceTest.java`:
- Around line 75-77: The indexOf call for the marker variable on the line
assigning to markerStart can return -1 if the marker is not found in the content
string. Add an explicit guard immediately after the indexOf assignment that
checks if markerStart equals -1, and if so, throw an exception with a
descriptive message that includes the missing marker value. This prevents
subsequent calculations using targetOffset and lineStart from producing
misleading results that could hide fixture regressions.
🪄 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: b45b0377-2e92-4389-bf0f-5a5b6ec0ebd3
⛔ Files ignored due to path filters (1)
src/test/resources/types/ReturnStructureMultilineFieldType.bslis excluded by!src/test/resources/**
📒 Files selected for processing (2)
build.gradle.ktssrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnStructureMultilineFieldTypeInferenceTest.java
Проверка на реальном JsDoc ЭлектроннаяПодписьКлиентСервер.НовыеСвойстваПодписи (а не синтетическом): после фикса разбора многострочного составного типа поля (bsl-parser 0.36.0) completion после точки и hover по переменной показывают все поля структуры, включая объявленные многострочным типом. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
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/RealReturnStructureCompletionTest.java`:
- Around line 57-58: The code calls indexOf to find marker strings and
immediately uses the returned offsets to compute positions without checking if
the markers were actually found. If indexOf returns -1 (when the marker is
missing), it gets used in arithmetic operations that produce misleading offset
values passed to positionAtOffset. Add explicit precondition assertions
immediately after each indexOf call to verify that the found offset is not -1
before using it to calculate the position, ensuring that missing marker strings
cause clear test failures rather than subtle misleading 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: 5118647b-06aa-4bc2-a279-63eebe9c6f8c
⛔ Files ignored due to path filters (1)
src/test/resources/types/RealSignaturePropertiesReturn.bslis excluded by!src/test/resources/**
📒 Files selected for processing (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/RealReturnStructureCompletionTest.java
…зова Вызов экспортной функции другого общего модуля (ОбщийМодуль.Метод()) резолвился через MemberDescriptor, чей returnType — лишь головной TypeRef без localFields. Поэтому поля структуры/ТЗ, объявленные в JsDoc, терялись, и автокомплит/hover по точке от результата межмодульного вызова не видели ключей (в отличие от вызова внутри того же модуля). inferDereference теперь для member-метода с source-символом MethodSymbol берёт полный тип возврата из SymbolTypeIndex.getDeclaredReturnTypes (с localFields) — тем же путём, что и одномодульный вызов. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
nixel2007
changed the title
Contributor
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/types/CrossModuleStructureReturnInferenceTest.java`:
- Around line 72-77: The helper method computes a position based on a marker in
the content without validating that the marker was actually found. When
indexOf(marker) returns -1 (marker not found), the subsequent calculations for
targetOffset, lineStart, line, and charInLine become invalid and may query the
wrong location silently. After the indexOf(marker) call, add an explicit
assertion or check to verify that markerStart is not -1, and fail fast with a
clear error message if the marker is missing from the content.
🪄 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: 2d4b34cf-4d2a-4564-b1ae-8beb2e241c8b
⛔ Files ignored due to path filters (2)
src/test/resources/metadata/designer/CommonModules/ОбщегоНазначения/Ext/Module.bslis excluded by!src/test/resources/**src/test/resources/types/CrossModuleStructureReturn.bslis excluded by!src/test/resources/**
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CrossModuleStructureReturnInferenceTest.java
Contributor
Test Results 3 432 files 3 432 suites 1h 40m 45s ⏱️ Results for commit 72c7ff1. |
Поля «открытого» объекта (localFields) теперь несут не только типы, но и текстовое описание из doc-комментария (LocalField(types, description)). Источник — SymbolTypeIndex.applyFields (секции Параметры/Возвращаемое значение); описание течёт через union/withField в тип переменной. Потребители: hover по переменной-структуре и по отдельному полю (X.Поле), а также documentation элемента автокомплита — теперь показывают описание поля, в т.ч. для локальной переменной из возврата функции (раньше описания подмешивались только для переменных-параметров). getLocalFields(ref) возвращает Map<String, LocalField> (единый источник, без параллельных структур); рантайм-ключи (.Вставить/колонки) — с пустым описанием через 3-арг withField. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Гард на indexOf == -1 в хелперах вычисления позиции по маркеру: при отсутствии маркера тест падает с понятным сообщением, а не считает ложную позицию и не прячет регресс фикстуры за невнятным ассертом. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/RealReturnStructureCompletionTest.java (1)
58-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard marker lookups before offset math.
Each
indexOf(...)result is used immediately to build an offset. If a marker is absent, the test can fail at an unrelated position instead of failing with a clear cause. Add explicit assertions after every marker lookup.Proposed patch
@@ void completionAfterDotExposesAllStructureFields() { @@ - var afterDot = content.indexOf("Свойства.", content.indexOf("X = Свойства.")) + "Свойства.".length(); + var assignOffset = content.indexOf("X = Свойства."); + assertThat(assignOffset).isGreaterThanOrEqualTo(0); + var dotOffset = content.indexOf("Свойства.", assignOffset); + assertThat(dotOffset).isGreaterThanOrEqualTo(0); + var afterDot = dotOffset + "Свойства.".length(); @@ void hoverOnVariableExposesStructureFields() { @@ - var varOffset = content.indexOf("Свойства = НовыеСвойстваПодписи()") + 1; + var assignmentOffset = content.indexOf("Свойства = НовыеСвойстваПодписи()"); + assertThat(assignmentOffset).isGreaterThanOrEqualTo(0); + var varOffset = assignmentOffset + 1; @@ void hoverOnFieldExposesFieldDescription() { @@ - var dotField = content.indexOf(".Подпись") + 1; + var fieldOffset = content.indexOf(".Подпись"); + assertThat(fieldOffset).isGreaterThanOrEqualTo(0); + var dotField = fieldOffset + 1; @@ void completionItemCarriesFieldDescription() { @@ - var afterDot = content.indexOf("Свойства.", content.indexOf("X = Свойства.")) + "Свойства.".length(); + var assignOffset = content.indexOf("X = Свойства."); + assertThat(assignOffset).isGreaterThanOrEqualTo(0); + var dotOffset = content.indexOf("Свойства.", assignOffset); + assertThat(dotOffset).isGreaterThanOrEqualTo(0); + var afterDot = dotOffset + "Свойства.".length();Also applies to: 83-83, 106-106, 125-125
🤖 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/RealReturnStructureCompletionTest.java` at line 58, Add explicit assertions to guard each indexOf marker lookup before performing offset arithmetic. In the RealReturnStructureCompletionTest class, locate all lines where indexOf is used to find markers and immediately assert the result is not -1 (indicating the marker was found). This applies at multiple sites: line 58 where indexOf is used to find "X = Свойства." and "Свойства.", line 83, line 106, and line 125. For each indexOf call used in offset calculations (like the afterDot assignment and similar patterns), insert an assertion immediately after the lookup to verify the marker exists before using the result in arithmetic operations, ensuring test failures clearly indicate missing markers rather than failing at unrelated positions due to incorrect offsets.
🤖 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.
Duplicate comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/RealReturnStructureCompletionTest.java`:
- Line 58: Add explicit assertions to guard each indexOf marker lookup before
performing offset arithmetic. In the RealReturnStructureCompletionTest class,
locate all lines where indexOf is used to find markers and immediately assert
the result is not -1 (indicating the marker was found). This applies at multiple
sites: line 58 where indexOf is used to find "X = Свойства." and "Свойства.",
line 83, line 106, and line 125. For each indexOf call used in offset
calculations (like the afterDot assignment and similar patterns), insert an
assertion immediately after the lookup to verify the marker exists before using
the result in arithmetic operations, ensuring test failures clearly indicate
missing markers rather than failing at unrelated positions due to incorrect
offsets.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 009bb8af-7fcb-4f57-a596-63cdd73b6b50
📒 Files selected for processing (10)
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/DereferenceMemberMatcher.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/LocalField.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/RealReturnStructureCompletionTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnStructureMultilineFieldTypeInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSetTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ReturnStructureMultilineFieldTypeInferenceTest.java
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
|
This was referenced Jun 21, 2026
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.
Исходный репорт:
НовыеСвойстваПодписи = ЭлектроннаяПодписьКлиентСервер.НовыеСвойстваПодписи();→ после точки нет автокомплита/hover ключей структуры, а в hover нет описаний полей.Тремя слоями. Все чинятся.
1. Парсинг многострочного составного типа поля (bsl-parser 0.36.0)
Бамп
bsl-parser0.35.0→0.36.0(#390). Continuation-строка типа поля (std453 п.5.3) «разрезала» структуру возврата на второй тип верхнего уровня.2. Потеря полей при МЕЖМОДУЛЬНОМ вызове
ОбщийМодуль.Метод()резолвился черезMemberDescriptorс голымTypeRefбезlocalFields.inferDereferenceтеперь для member-метода с source-символомMethodSymbolберёт полный тип изSymbolTypeIndex.getDeclaredReturnTypes(с полями) — как одномодульный вызов.3. Текстовые описания полей в hover/автокомплите
TypeSet.localFieldsтеперь несётLocalField(types, description)— единое представление (без параллельных карт). Описания берутся вSymbolTypeIndex.applyFieldsиз JsDoc (Параметры/Возвращаемое значение) и текут черезunion/withField. Потребители: hover по переменной-структуре, hover по полю (X.Поле), documentation автокомплита — раньше описания подмешивались только для переменных-параметров.getLocalFields(ref)→Map<String, LocalField>(единый источник).Архитектура согласована с пользователем и проверена ревью-агентом (вариант A: единый rich-аксессор).
Тесты
ReturnStructureMultilineFieldTypeInferenceTest(синтетика),RealReturnStructureCompletionTest(реальный JsDoc БСП: completion + hover + описания).CrossModuleStructureReturnInferenceTest.TypeSetTest(withField c описанием, union-merge — первое непустое).🤖 Generated with Claude Code
Summary by CodeRabbit
Структураreturn fields, cross-module structure inference, and real-document dot-completion/hover behavior.TypeSetunions/merges.