feat(types): «Перем» без сведений — «Неопределено», единый резолвер висячих комментариев - #4348
Closed
nixel2007 wants to merge 14 commits into
Closed
feat(types): «Перем» без сведений — «Неопределено», единый резолвер висячих комментариев#4348nixel2007 wants to merge 14 commits into
nixel2007 wants to merge 14 commits into
Conversation
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:
📝 WalkthroughWalkthroughThe change replaces comment-based type resolvers with ChangesType inference updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Variable
participant ExpressionTypeInferencer
participant TrailingCommentTypeResolver
participant TypeRegistry
participant SymbolTypeIndex
Variable->>ExpressionTypeInferencer: infer declaration or assignment type
ExpressionTypeInferencer->>TrailingCommentTypeResolver: resolve trailing comment
TrailingCommentTypeResolver->>TypeRegistry: resolve direct type
TrailingCommentTypeResolver->>SymbolTypeIndex: resolve см. reference
SymbolTypeIndex-->>TrailingCommentTypeResolver: return referenced type
TrailingCommentTypeResolver-->>ExpressionTypeInferencer: return TypeSet
ExpressionTypeInferencer-->>Variable: merge inferred types and Неопределено
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 3
🤖 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/inferencer/ExpressionTypeInferencer.java`:
- Around line 960-1008: Update the documentation for the type-inference behavior
implemented by declaredByPerem and assignedBeforeAnyUse, covering Перем
variables, module initialization, and ПриСозданииОбъекта; add equivalent
explanations to both localized documentation trees, docs/ and docs/en/, using
the existing documentation structure and terminology.
- Around line 993-1005: Replace the unconditional success logic in
assignedBeforeAnyUse with control-flow analysis that verifies a module variable
assignment dominates and reaches every use, including module-level reads and all
constructor branches; return true only when no use can observe an unassigned
value. Add tests covering a read before a later module assignment and an
assignment confined to a conditional ПриСозданииОбъекта branch.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeRef.java`:
- Around line 190-203: Update TypeSet.map to preserve all decorations when
rebuilding mapped references: remap element types, local fields, and lazy
decorations using each source ref’s mapped ref, and merge decoration entries
when multiple refs canonicalize to the same target. Keep returning the original
typeSet when no references change, while ensuring changed results retain the
complete decorated TypeSet structure rather than calling TypeSet.of(rebuilt)
alone.
🪄 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 Plus
Run ID: e9ae7415-8f80-46a1-a7db-18f71fbdc5fc
⛔ Files ignored due to path filters (5)
src/test/resources/types/FlowSensitiveVariableTypes.bslis excluded by!src/test/resources/**src/test/resources/types/InlineTypeComment.bslis excluded by!src/test/resources/**src/test/resources/types/ModuleBodyFlow.osis excluded by!src/test/resources/**src/test/resources/types/ModuleVariableFlow.bslis excluded by!src/test/resources/**src/test/resources/types/PeremTypeComment.bslis excluded by!src/test/resources/**
📒 Files selected for processing (21)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/CLAUDE.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/MemberTypeFromCommentResolver.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.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/inferencer/VariableCommentTypeResolver.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeRef.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/TypeRegistry.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ExpressionTypeInferencerSelfPropertyFallbackTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/FlowSensitiveVariableTypeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleVariableFlowTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/autumn/AutumnDependencyInjectionInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistrySpecializationTest.java
💤 Files with no reviewable changes (2)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/MemberTypeFromCommentResolver.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/VariableCommentTypeResolver.java
sfaqer
reviewed
Aug 1, 2026
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/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.java`:
- Around line 214-230: Update TypeSet.mapRefs to recursively apply the mapper to
nested TypeSet values in elementTypes and the types held by LocalField, and
include those nested results when determining whether to return this. Preserve
unchanged nested values where possible, while ensuring canonicalized element and
field references are returned even when the parent refs are unchanged; add
coverage for both nested cases.
🪄 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 Plus
Run ID: abcf29ae-ee8d-426d-b329-5c43c5d13b81
⛔ Files ignored due to path filters (1)
src/test/resources/types/ModuleBodyFlowEnglishConstructor.osis excluded by!src/test/resources/**
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.java
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the types subsystem to better match the “Типизация кода” recommendation: variables declared via Перем without additional type info now infer as Неопределено, trailing type comments are resolved by a single shared resolver, and type specializations canonicalize TypeRefs to avoid duplicate refs for the same qualified name.
Changes:
- Treat
Перем-declared variables with no other type signals as having value/typeНеопределено, with flow-sensitive handling for module-body / constructor assignments. - Replace two overlapping trailing-comment resolvers with
TrailingCommentTypeResolverand extend// см. …support to assignment-line comments too. - Canonicalize refs after structural specialization (incl. decorations) so unions don’t show the same type twice.
Reviewed changes
Copilot reviewed 32 out of 32 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.java | New unified resolver for trailing type comments and // см. … references (declaration + assignment-line). |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/MemberTypeFromCommentResolver.java | Removed (merged into TrailingCommentTypeResolver). |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/VariableCommentTypeResolver.java | Removed (merged into TrailingCommentTypeResolver). |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java | Adds Неопределено for Перем without other info; uses unified trailing resolver; adds “assigned before any use” logic. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.java | Adds mapRefs to canonicalize refs (incl. decorations) after specialization. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java | Specialization now accepts a canonicalizer and applies it to return/parameter refs (incl. decorations). |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java | Passes canonicalizer during specialization and adds canonicalRef helper. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/RegisterTypesRegistrar.java | Uses canonicalizer when specializing register-based members. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/PlaceholderBinder.java | Uses canonicalizer when binding placeholders into members. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProvider.java | Switches module member typing from removed resolver to TrailingCommentTypeResolver. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProvider.java | Switches property typing from removed resolver to TrailingCommentTypeResolver. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java | Updates documentation reference to the new unified trailing resolver. |
| src/main/java/com/github/_1c_syntax/bsl/languageserver/types/CLAUDE.md | Updates subsystem documentation to reflect the new resolver name/responsibilities. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.java | New assertions: Перем without other info → Неопределено; // см. … on Перем resolves constructor-return structure fields. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleVariableFlowTest.java | Flow tests updated for Неопределено baseline + new cases for module-body initialization behavior. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.java | Adds constructor/module-body assignment scenarios that eliminate or preserve Неопределено depending on unconditionality. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/FlowSensitiveVariableTypeTest.java | Adds merge behavior test: branch-only assignment must preserve Неопределено path. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.java | Adds support tests for // см. … on assignment-line (local + cross-module). |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ExpressionTypeInferencerSelfPropertyFallbackTest.java | Updates expectation: unassigned Перем now yields Неопределено (not empty). |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProviderTest.java | Updates mocks/imports to use TrailingCommentTypeResolver. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/autumn/AutumnDependencyInjectionInferenceTest.java | Adjusts expectations: “not injected” now leaves Неопределено instead of empty. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistrySpecializationTest.java | Adds regression test: specialized members must use canonical TypeRef from registry. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSetTest.java | Adds tests for mapRefs moving/merging decorations and returning this when no changes. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptorSpecializeTest.java | Updates to new specialize(bindings, canonicalizer) signature. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptorFactoryTest.java | Updates to new specialize(bindings, canonicalizer) signature. |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java | Updates hover content expectations: module/local vars show Тип: Неопределено. |
| src/test/resources/types/PeremTypeComment.bsl | Adds fixtures for no-comment Перем and // см. … reference in Перем declaration. |
| src/test/resources/types/InlineTypeComment.bsl | Adds fixtures for assignment-line // см. … references. |
| src/test/resources/types/ModuleVariableFlow.bsl | Adds fixtures for module-body initialization (unconditional vs conditional). |
| src/test/resources/types/ModuleBodyFlow.os | Adds fixtures for constructor initialization paths and “no assignment” behavior. |
| src/test/resources/types/ModuleBodyFlowEnglishConstructor.os | New fixture: English-named constructor OnObjectCreate() path. |
| src/test/resources/types/FlowSensitiveVariableTypes.bsl | Adds fixture for “declared, assigned only in branch” merge behavior. |
Comment on lines
+236
to
+239
| // Ленивые декорации приводятся при вычислении: заглядывать в них здесь означало бы | ||
| // считать то, ради отсрочки чего они и заведены. | ||
| mapKeys(lazyElements, mapper, LazyTypeSet::combine, UnaryOperator.identity()), | ||
| mapNestedKeys(lazyFields, mapper, LazyField::merge, UnaryOperator.identity()) |
Member
Author
There was a problem hiding this comment.
Верно, комментарий врал. Поправил в 565ad0becc: ленивая декорация теперь оборачивается — new LazyTypeSet(lazy.key(), () -> lazy.get().mapRefs(mapper)). Форсить источник для этого не нужно, приведение применяется к тому, что он вернёт при чтении, а ключ у обёртки прежний, поэтому равенство и combine не меняются. Набор с ленивыми декорациями теперь пересобирается всегда: изменилось ли их содержимое, сравнением по ключу не узнать.
Тест TypeSetTest.mapRefsAppliesToLazyDecorationOnRead проверяет обе стороны: при приведении источник не вычисляется, а при чтении ссылка приходит канонической.
Member
Author
There was a problem hiding this comment.
@coderabbitai проверь адекватность и корректность решения
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
type_set_test="$(fd '^TypeSetTest\.java$' src test 2>/dev/null | head -n1 || true)"
lazy_type_set="$(fd '^LazyTypeSet\.java$' src 2>/dev/null | head -n1 || true)"
lazy_field="$(fd '^LazyField\.java$' src 2>/dev/null | head -n1 || true)"
printf '%s\n' '=== outlines ==='
for file in "$lazy_type_set" "$lazy_field" "$type_set_test"; do
if [ -n "$file" ]; then
printf '%s\n' "--- $file"
ast-grep outline "$file" --items all
fi
done
printf '%s\n' '=== target test and lazy contracts ==='
rg -n -A90 -B15 'mapRefsAppliesToLazyDecorationOnRead|class LazyTypeSet|record LazyField|combine\(|equals\(|hashCode\(' \
"$lazy_type_set" "$lazy_field" "$type_set_test" 2>/dev/null || trueLength of output: 24826
@nixel2007, решение адекватно и корректно.
mapLazy не вычисляет исходный LazyTypeSet. Он создаёт обёртку с тем же key() и применяет mapRefs(mapper) только после вызова get().
Сохранение key() корректно. LazyTypeSet.equals, hashCode и combine используют ключ. Поэтому объединение ленивых источников сохраняет прежнюю семантику.
Принудительная пересборка TypeSet при наличии ленивых декораций также корректна. Равенство LazyTypeSet не включает resolver, поэтому сравнение карт не может обнаружить изменение результата, которое создаёт обёртка.
Тест mapRefsAppliesToLazyDecorationOnRead проверяет два необходимых свойства: mapRefs не форсит источник, а результат чтения содержит каноническую ссылку. Отдельный тест для lazyFields мог бы сделать покрытие симметричным, но текущая реализация использует тот же mapLazy; это не является дефектом.
You are interacting with an AI system.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.java:216
- Javadoc для
mapRefsговорит, что метод вернёт исходный набор, если «ни одна не изменилась», но реализация всегда пересобирает набор при наличииlazyElements/lazyFields(даже если все уже известные ссылки/ключи не изменились), чтобы обернуть ленивые декорации. Лучше уточнить контракт в@return, чтобы он соответствовал фактическому поведению.
* @param mapper преобразование ссылки.
* @return набор с преобразованными ссылками; исходный, если ни одна не изменилась.
*/
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.java (1)
149-159: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve defined types through
resolveSet.Defined types are resolved by
TypeRegistry.resolveSet(String), notresolve(String, FileType), and the current accumulation collects onlyTypeRefvalues. Accumulate the resolvedTypeSetdirectly and preserve its metadata.🤖 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/TrailingCommentTypeResolver.java` around lines 149 - 159, Update the defined-type resolution logic in TrailingCommentTypeResolver to use TypeRegistry.resolveSet(String) instead of resolve(String, FileType). Accumulate the returned TypeSet values directly, combining them into the result while preserving their metadata; do not collect TypeRef values or reconstruct the set with TypeSet.of.Source: Learnings
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.java (1)
62-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd contract Javadoc for
TrailingCommentTypeResolver.Add class-level Javadoc that describes the supported trailing comments, the
TypeSet.EMPTYresult for unresolved input, and mutation behavior.As per coding guidelines, public Java APIs require contract Javadoc.
Suggested Javadoc
+/** + * Resolves types declared in trailing comments on variable declarations and assignments. + * + * <p>Returns {`@link` TypeSet#EMPTY} when no declared type or link resolves. + */ `@Component` `@RequiredArgsConstructor` public class TrailingCommentTypeResolver implements VariableTypeSource {🤖 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/TrailingCommentTypeResolver.java` around lines 62 - 64, Add class-level contract Javadoc to TrailingCommentTypeResolver describing the supported trailing comments, that unresolved input returns TypeSet.EMPTY, and the resolver’s mutation behavior. Keep the existing annotations and VariableTypeSource implementation unchanged.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.java`:
- Around line 149-159: Update the defined-type resolution logic in
TrailingCommentTypeResolver to use TypeRegistry.resolveSet(String) instead of
resolve(String, FileType). Accumulate the returned TypeSet values directly,
combining them into the result while preserving their metadata; do not collect
TypeRef values or reconstruct the set with TypeSet.of.
---
Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.java`:
- Around line 62-64: Add class-level contract Javadoc to
TrailingCommentTypeResolver describing the supported trailing comments, that
unresolved input returns TypeSet.EMPTY, and the resolver’s mutation behavior.
Keep the existing annotations and VariableTypeSource implementation unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 61723773-21dd-4337-8566-55dc2c707b6f
⛔ Files ignored due to path filters (2)
src/test/resources/types/ModuleBodyFlow.osis excluded by!src/test/resources/**src/test/resources/types/ModuleVariableFlow.bslis excluded by!src/test/resources/**
📒 Files selected for processing (14)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/TrailingCommentTypeResolver.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSet.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/PlaceholderBinder.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/RegisterTypesRegistrar.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleVariableFlowTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptorFactoryTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptorSpecializeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeSetTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/PeremTypeCommentInferenceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/InlineTypeCommentInferenceTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ModuleBodyFlowTest.java
Contributor
Test Results 3 936 files ± 0 3 936 suites ±0 1h 7m 32s ⏱️ + 2m 4s Results for commit e96ce6d. ± Comparison against base commit 50d66d6. This pull request removes 1 and adds 23 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java (1)
485-521: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winUpdate the stale Javadoc for
applyFields.The Javadoc at Line 486 still states that fields are attached "на головной {
@linkTypeRef}" (onto the headTypeRef). The code below it no longer does this unconditionally: Lines 498-503 redirect field attachment tocollectionElement(headRef, ...)when the head type is a collection, and only fall back toheadReffor structure-like types. Line 519 confirms this by wrapping the result throughbase.withElement(headRef, result)for the collection case.Update the Javadoc to describe the current contract: fields go to the collection's resolved element type when one exists, and to the head type only when it is structure-like or has no known element.
📝 Proposed Javadoc update
/** * Если у описания типа есть {`@link` TypeDescription#fields() поля} * (декларация структуры/ТЗ ключами через {`@code` * Поле - Тип}), - * навесить их на головной {`@link` TypeRef}. Поле, типизированное см.-ссылкой + * навесить их на элемент коллекции (строку таблицы/дерева, пару «ключ-значение» + * и т.п.), полученный через {`@link` `#collectionElement`}, либо на головной + * {`@link` TypeRef} для структуроподобных типов. Поле, типизированное см.-ссылкой * на локальную функцию, навешивается лениво ({`@link` TypeSet#withLazyField}) — * для поддержки рекурсивных структур. */As per coding guidelines, "Javadoc классов и методов должен описывать контракт: параметры, результат, инварианты и побочные эффекты" and general Java guideline to "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/types/index/SymbolTypeIndex.java` around lines 485 - 521, Update the Javadoc for applyFields to describe that fields attach to the resolved collection element type when collectionElement returns one, while falling back to the head TypeRef for structure-like types or types without a known element; retain the notes about lazy local-function references and recursive structures.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.java`:
- Around line 485-521: Update the Javadoc for applyFields to describe that
fields attach to the resolved collection element type when collectionElement
returns one, while falling back to the head TypeRef for structure-like types or
types without a known element; retain the notes about lazy local-function
references and recursive structures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 72397369-8216-4f2b-a2ed-129ad310d018
⛔ Files ignored due to path filters (1)
src/test/resources/types/ValueTableColumnsFields.bslis excluded by!src/test/resources/**
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/index/SymbolTypeIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/ValueTableColumnsFieldsInferenceTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java
nixel2007
commented
Aug 1, 2026
| * @param variable переменная. | ||
| * @return {@code true}, если переменная объявлена записью {@code Перем}. | ||
| */ | ||
| private static boolean declaredByPerem(VariableSymbol variable) { |
Member
Author
There was a problem hiding this comment.
Переименовал в declaredByVar — смеси языков в идентификаторе больше нет.
nixel2007
commented
Aug 1, 2026
| */ | ||
| private static boolean declaredByPerem(VariableSymbol variable) { | ||
| var kind = variable.getKind(); | ||
| return kind == VariableKind.LOCAL || kind == VariableKind.MODULE; |
Member
Author
There was a problem hiding this comment.
Global тоже объявляется через Перем
Member
Author
There was a problem hiding this comment.
Верно, добавил VariableKind.GLOBAL в предикат. Сейчас этот вид, правда, нигде не проставляется — в VariableSymbolComputer переменная уровня модуля получает MODULE, а GLOBAL только проверяется (WorkspaceSymbolIndex, DocumentSymbolProvider, UseLessForEachDiagnostic). Но правило от этого не зависит: объявлена записью «Перем» — значит до присваивания там «Неопределено».
nixel2007
commented
Aug 1, 2026
| var enclosingMethod = enclosingMethod(symbolTree, position); | ||
| var runsBeforeAnyUse = enclosingMethod.isEmpty() | ||
| || Methods.isOscriptClassConstructorName(enclosingMethod.get().getName()); | ||
| if (runsBeforeAnyUse && unconditional(owner, position)) { |
Member
Author
There was a problem hiding this comment.
А по cfg нет смысла собрать? Как это делают другие части инференсера
Member
Author
There was a problem hiding this comment.
Смысл есть, и точность выше. Сейчас проверка синтаксическая — обход предков узла до Если/Пока/Для/Попытка, — и она консервативна: случай, где присваивание есть в обеих ветках,
Если Условие Тогда
Кэш = Новый Соответствие;
Иначе
Кэш = Новый Массив;
КонецЕсли;она считает условным, хотя после КонецЕсли значение присвоено на любом пути. На графе это ровно доминирование выхода, и такой случай разбирался бы правильно.
Инфраструктура рядом: VariableFlowAnalyzer и FlowLayout уже работают поверх ControlFlowGraphIndex, у FlowLayout есть отображение позиции в оператор и его вершину. Загвоздка в том, что declaredTypes отвечает про переменную без привязки к телу и вызывается до расчёта по потоку, поэтому граф нужного тела придётся доставать отдельно (тело мы и так находим — это либо метод, либо тело модуля).
Делать в этом PR или отдельной задачей? PR уже довольно широкий, а правка заметная — она меняет и место, откуда берётся ответ, и стоимость: на каждую переменную модуля без объявленных типов добавится обращение к индексу графов.
nixel2007
commented
Aug 1, 2026
| */ | ||
| @Component | ||
| @RequiredArgsConstructor | ||
| public class TrailingCommentTypeResolver implements VariableTypeSource { |
Member
Author
There was a problem hiding this comment.
А почему вообще только trailing комментарий разбирается? Что не так с комментарием над самой переменной?
Member
Author
There was a problem hiding this comment.
Ничего не так — просто не разбирался. Сделал, и заодно выяснилась причина, по которой само оно не работало: комментарий, дописанный в конце предыдущей строки кода, парсер отдаёт и как описание следующего объявления — по строкам он ведь стоит выше. На фикстуре это сразу поймал тест:
Перем ПараметрыВызова; // Булево
Перем БезКомментария; // ← получал тип Булево от соседаОтличить можно по началу описания: своё начинается со своей строки, чужое висячее — после кода. С этой проверкой оба случая работают, висячий комментарий остаётся приоритетнее. Класс переименован в CommentTypeResolver — он больше не только про висячий комментарий. Тест PeremTypeCommentInferenceTest.moduleVarWithTypeCommentAboveDeclaration.
nixel2007
force-pushed
the
feat/typification-spec-conformance
branch
from
August 1, 2026 16:36
1a77971 to
f03d7c7
Compare
This was referenced Aug 1, 2026
nixel2007
force-pushed
the
feat/typification-spec-conformance
branch
from
August 2, 2026 06:13
e96ce6d to
36546af
Compare
Шаблон вида «СправочникСсылка.<Имя справочника>» приходит из синтакс-помощника платформенным, а конфигурационные типы регистрируются своим видом. Структурная специализация сохраняла вид шаблона, поэтому за одним именем оказывались две ссылки, различающиеся только видом: объединение наборов их не схлопывало и тип показывался дважды. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
…елено» По методической рекомендации «Типизация кода» переменная, объявленная записью «Перем» и без иных сведений о типе, содержит «Неопределено» — это её значение, а не отсутствие сведений. В точке слияния путей оно остаётся, если хотя бы один путь до присваивания не дошёл. Исключения, где это состояние наблюдать неоткуда: присваивание в теле модуля (оно отрабатывает раньше любой процедуры) и в «ПриСозданииОбъекта» (конструктор выполняется при создании объекта). Заодно два резолвера висячих комментариев — у члена и у переменной — объединены в «TrailingCommentTypeResolver»: они разбирали один и тот же вид комментария и разошлись в возможностях, ссылку «// см. Функция» умел только первый. Теперь она работает и в объявлении переменной, и в строке присваивания. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Сборка результата через «TypeSet.of» оставляла одни ссылки: поля объекта и типы элементов коллекции отбрасывались. Приведение переехало в «TypeSet.mapRefs», который переносит декорации на новую ссылку, а если несколько исходных ссылок сходятся в одну каноническую — сливает их. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Сравнение со строкой «ПриСозданииОбъекта» не видело английского написания. Используется общая проверка «Methods.isOscriptClassConstructorName», а метод в позиции присваивания ищется спуском по дереву символов, а не перебором методов. Объявленное о переменной расчёт по потоку спрашивает многократно за один запрос, а у переменной модуля за ответом стоит обход индекса ссылок — ответ запоминается на время запроса, как это уже сделано для операторов-мутаторов. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
…еским Перегруз без приведения оставлял дверь, через которую в members попадали неканонические ссылки: подстановка сохраняет вид шаблона, поэтому платформенный шаблон давал платформенную ссылку и на конфигурационный тип. Приведение получили и остальные места специализации — разворачивание generic-членов, регистры и подстановка единственного плейсхолдера. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Присваивание внутри условия, цикла или попытки может не выполниться, поэтому доказательством того, что состояние от объявления наблюдать неоткуда, оно не является — ни в теле модуля, ни в конструкторе. Приведение ссылок к каноническим заодно заходит внутрь декораций: типы элементов коллекции и типы полей объекта приводятся тем же преобразованием. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Ленивая декорация оборачивается: преобразование применяется к тому, что её источник вернёт при чтении, форсить его для этого не нужно. Ключ у обёртки прежний, поэтому равенство и слияние ленивых ссылок не меняются. Заодно замечания статического анализа: объявления рядом с использованием, явные типы параметров лямбд, ссылки на методы вместо лямбд. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Колонки, объявленные звёздочками в описании возвращаемого значения («ТаблицаЗначений: * Колонка - Тип»), навешивались на саму таблицу. Обращения «Таблица.Колонка» в 1С нет — колонки видны у строки, поэтому строка, полученная из «Добавить()» или обходом «Для Каждого», о них не знала и каждое обращение к колонке становилось находкой «нет метода или свойства». Описанные колонки навешиваются на тип элемента таблицы — так же, как это уже делает разбор «Колонки.Добавить» рядом с кодом. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Правило распространено с таблицы значений на все коллекции: собственные свойства бывают только у структуроподобных типов, у остальных звёздочки в описании описывают элемент. Так работают дерево значений, табличная часть, коллекции формы и соответствие, у которого «Ключ» и «Значение» — свойства пары, а не самого соответствия; обращения к нему через точку в 1С нет. Дерево значений само не обходится, его строки лежат в отдельной коллекции «Строки» — уточнение строки переезжает внутрь неё. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Правило «поля описания принадлежат элементу» сплошным быть не может: у коллекции элементов формы и у выборки из результата запроса обращение по имени свойства к ним самим законное, и перенос ломал его. Список типов, у которых собственных свойств нет, задан явно: таблица и дерево значений, табличная часть, коллекции формы, соответствие. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Новые места специализации из типов форм переведены на единственную сигнатуру с приведением — как и все остальные. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Разбирался только висячий комментарий, хотя парсер отдаёт типы и у описания над записью «Перем». Причина, по которой это не работало само собой: комментарий, дописанный в конце предыдущей строки кода, парсер отдаёт и как описание следующего объявления — по строкам он стоит выше. Отличается по началу: своё описание начинается со своей строки, чужое висячее — после кода. Висячий комментарий остаётся приоритетнее: он стоит вплотную к объявлению. Класс переименован — он больше не только про висячий комментарий. Заодно по замечаниям ревью: глобальная переменная тоже объявляется записью «Перем», поэтому попадает под то же правило, а предикат назван без смеси языков. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Компенсация была нужна, пока комментарий предыдущей строки попадал в описание следующего символа. Утечка исправлена в сборе комментариев (#4360), и различать своё описание по началу строки больше не требуется. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
nixel2007
force-pushed
the
feat/typification-spec-conformance
branch
from
August 2, 2026 06:15
36546af to
6e60e7d
Compare
Member
Author
|
Разнёс на стек из трёх независимых правок — так их проще смотреть и вливать по одной:
Конечное состояние стека посимвольно совпадает с тем, что было здесь. |
|
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
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.
Что сделано
Правки системы типов, закрывающие расхождения с методической рекомендацией «Типизация кода»,
и тесты на них в компонентах системы типов. Сверка ведётся на тестовом полигоне (#4345);
сюда вынесены только код и тесты — полигона в ветке нет.
1. Значение переменной, объявленной записью
ПеремПо рекомендации переменная, объявленная через
Переми без иных сведений о типе, содержитНеопределено— это её значение, а не отсутствие сведений.Исключения, где
Неопределеноот объявления наблюдать неоткуда:ПриСозданииОбъекта— конструктор выполняется при создании объекта,до обращения к его полям.
2. Один резолвер висячих комментариев вместо двух
MemberTypeFromCommentResolver(типизирующий комментарий члена) иVariableCommentTypeResolver(типизирующий комментарий переменной) разбирали один и тот жевид комментария и разошлись в возможностях: ссылку
// см. Функцияумел только первый.Объединены в
TrailingCommentTypeResolver; ссылка// см. …теперь работает и в объявлениипеременной, и в строке присваивания — как в описании члена.
3. Приведение ссылок после структурной специализации
Шаблон
СправочникСсылка.<Имя справочника>приходит из синтакс-помощника платформенным, аконфигурационные типы регистрируются своим видом. Специализация сохраняла вид шаблона, из-за
чего за одним именем оказывались две ссылки, различающиеся только видом, и объединение
наборов их не схлопывало — тип показывался дважды. Ссылки специализированных членов
приводятся к зарегистрированным в реестре.
Тесты
Новые:
PeremTypeCommentInferenceTest,ModuleVariableFlowTest,ModuleBodyFlowTest,FlowSensitiveVariableTypeTest,InlineTypeCommentInferenceTest,TypeRegistrySpecializationTest.Обновлены под новое поведение:
ExpressionTypeInferencerSelfPropertyFallbackTest,AutumnDependencyInjectionInferenceTest,VariableSymbolMarkupContentBuilderTest(у
Перембез сведений hover теперь показываетТип: Неопределено).🤖 Generated with Claude Code
https://claude.ai/code/session_01MnWpDdNSEFohPZHG6voe6y
Summary by CodeRabbit
New Features
Bug Fixes
Неопределеноinstead of an empty type.