Владелец подчинённого справочника типизируется по метаданным - #4346
Merged
Conversation
Contributor
📝 WalkthroughWalkthroughChangesCatalog owner type resolution
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CatalogMetadata
participant ConfigurationTypesProvider
participant CatalogOwnerTypesRegistrar
participant TypeRegistry
participant PlatformDeclarations
CatalogMetadata->>ConfigurationTypesProvider: register catalog metadata
ConfigurationTypesProvider->>CatalogOwnerTypesRegistrar: register catalog owner members
CatalogOwnerTypesRegistrar->>TypeRegistry: resolve owner reference types
TypeRegistry-->>CatalogOwnerTypesRegistrar: return registered owner types
CatalogOwnerTypesRegistrar->>TypeRegistry: register specialized Владелец members
ConfigurationTypesProvider->>PlatformDeclarations: retain platform declarations for untyped attributes
Possibly related PRs
🚥 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: 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/registry/ConfigurationTypesProvider.java`:
- Around line 708-723: Update registerOwnerMembers so owner reference types are
resolved only after all catalog reference types have been registered, using a
deferred or second-pass registration flow. Ensure catalogs processed before
their owners still register the Владелец override, and add a regression test
covering that metadata ordering.
🪄 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: ee5177b7-7995-4259-9763-7ffb64e8bf39
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java
Contributor
sfaqer
force-pushed
the
feat/catalog-owner-typing
branch
from
August 1, 2026 05:44
31e83fd to
36dc5f9
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java (1)
46-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for owner order and multiple owners.
Force a fixture order where the subordinate catalog is processed before its owner. The current test can pass with the previous eager implementation when the owner registers first.
Add a catalog with multiple owners. Assert that
Владелецcontains every owner reference type.As per coding guidelines, “Always run tests before submitting changes and maintain or improve test coverage using appropriate test frameworks (JUnit, AssertJ, Mockito).”
🤖 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/registry/CatalogOwnerTypeTest.java` around lines 46 - 104, Extend the catalog-owner test fixture and tests to cover both registration order and multiple owners: ensure the subordinate catalog is processed before its owner, then verify memberTypes for Владелец still resolves the owner reference lazily. Add a catalog with multiple owners and assert that its Владелец member contains every expected owner reference type, using the existing CatalogOwnerTypeTest helpers and AssertJ style.Source: Coding guidelines
src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json (1)
2823-2827: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUpdate both locale docs for the catalog
Владелец/Ownerproperty.This registry entry changes the type contract for the catalog reference’s owner property. Add/adjust coverage in
docs/anddocs/en/thatВладелец/Ownerresolves asНеопределено/Undefinedfor non-subordinate catalogs, and resolves to the catalog object type for subordinate catalogs.🤖 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/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json` around lines 2823 - 2827, Update the documentation entries for the catalog Владелец/Owner property in both docs/ and docs/en/ to describe its conditional type: Неопределено/Undefined for non-subordinate catalogs and the catalog object type for subordinate catalogs. Keep the locale-specific terminology consistent with the registry entry.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.
Inline comments:
In
`@src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json`:
- Around line 3481-3488: Update the “Владелец” property definition in the
СправочникСсылка built-in type registry to include accessMode set to READ,
matching the other standard properties in the block and keeping the property
non-writable.
- Around line 3481-3488: Update the “Владелец” property declaration in the
builtin platform types registry to replace the generic description field with
localized descriptionRu and descriptionEn fields, preserving the existing
Russian text and providing its English equivalent so localized hover and
documentation output retain the owner description.
- Around line 2823-2827: Update the `Владелец` property declaration in
`builtin-platform-types.json` so its `description` states that the owner
reference applies only to subordinate catalogs, matching the corresponding
reference declaration; preserve the existing `returnType` and other metadata.
---
Nitpick comments:
In
`@src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json`:
- Around line 2823-2827: Update the documentation entries for the catalog
Владелец/Owner property in both docs/ and docs/en/ to describe its conditional
type: Неопределено/Undefined for non-subordinate catalogs and the catalog object
type for subordinate catalogs. Keep the locale-specific terminology consistent
with the registry entry.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java`:
- Around line 46-104: Extend the catalog-owner test fixture and tests to cover
both registration order and multiple owners: ensure the subordinate catalog is
processed before its owner, then verify memberTypes for Владелец still resolves
the owner reference lazily. Add a catalog with multiple owners and assert that
its Владелец member contains every expected owner reference type, using the
existing CatalogOwnerTypeTest helpers and AssertJ style.
🪄 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: 38ff23d9-2734-4e9a-9170-38a997eed402
⛔ Files ignored due to path filters (4)
src/test/resources/metadata/catalogOwners/Catalogs/Справочник1.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Catalogs/Справочник2.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Configuration.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Languages/Русский.xmlis excluded by!src/test/resources/**
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.jsonsrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java
sfaqer
force-pushed
the
feat/catalog-owner-typing
branch
2 times, most recently
from
August 1, 2026 07:21
7cc25c8 to
940e22c
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java (1)
73-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a test for a catalog with multiple owners.
The PR requires a union type when
Catalog.getOwners()returns multiple owners. The current tests only assert one owner type. A broken implementation that keeps only the first owner will pass.Add metadata with two owners. Assert both qualified reference types, preferably with
containsExactlyInAnyOrder(...).🤖 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/registry/CatalogOwnerTypeTest.java` around lines 73 - 85, Extend the catalog test metadata to define a catalog with two owners, then add a test covering its owner member through the relevant type path. Assert both qualified owner reference types with containsExactlyInAnyOrder, ensuring the implementation preserves every value returned by Catalog.getOwners() rather than only the first.
🤖 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/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.java`:
- Around line 73-85: Extend the catalog test metadata to define a catalog with
two owners, then add a test covering its owner member through the relevant type
path. Assert both qualified owner reference types with
containsExactlyInAnyOrder, ensuring the implementation preserves every value
returned by Catalog.getOwners() rather than only the first.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b4961db5-a11b-4c76-aff0-d4882f7e22f8
⛔ Files ignored due to path filters (4)
src/test/resources/metadata/catalogOwners/Catalogs/Справочник1.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Catalogs/Справочник2.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Configuration.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Languages/Русский.xmlis excluded by!src/test/resources/**
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypesRegistrar.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.jsonsrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderHelpersTest.java
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json
…нным В синтакс-помощнике стандартный реквизит Владелец объявлен типом Неопределено — и для неподчинённого справочника это правда. У подчинённого владельцы известны из Catalog.getOwners(), оттуда и берётся тип (несколько владельцев дают объединение). Заодно бестиповый стандартный реквизит из mdclasses больше не регистрируется: он перекрывал объявление платформы, у которой есть и тип, и описание.
sfaqer
force-pushed
the
feat/catalog-owner-typing
branch
from
August 1, 2026 07:54
940e22c to
6b74679
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypesRegistrar.java (1)
56-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove call-order rationale out of this Javadoc.
The Javadoc explains that
getChildrenByMdoRef()returns an immutable hash map whose iteration order JDK randomizes across runs, and that this is why owner types resolve lazily inside the member source. This is information about caller-side traversal order, not about the contract ofregisterOwnerMembersitself.The coding guidelines require Javadoc to describe the contract (parameters, result, invariants, side effects) but not callers or call order; such details belong in the caller, tests,
package-info, or documentation. The test file already places equivalent reasoning correctly in a comment onPATH_TO_METADATA(CatalogOwnerTypeTest.java, lines 46-55), which duplicates this explanation.Trim this Javadoc to the contract (what the method does and the "why lazy" invariant), and keep the traversal-order rationale only in the test comment or a
package-info.java.As per coding guidelines, "Javadoc классов и методов должен описывать контракт: параметры, результат, инварианты и побочные эффекты, но не вызывающие стороны, порядок вызовов или CLI-сценарии; сведения о вызовах хранить у вызывающей стороны, в тестах,
package-infoили документации."🤖 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/registry/CatalogOwnerTypesRegistrar.java` around lines 56 - 76, The Javadoc for registerOwnerMembers contains caller traversal-order details that do not belong in the method contract. Trim it to describe the direct owner-type replacement for subordinate catalogs, handling multiple owners as a union, and the invariant that owner types resolve lazily within the member source; remove references to getChildrenByMdoRef(), hash-map iteration, configuration traversal, and registration timing.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.
Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypesRegistrar.java`:
- Around line 56-76: The Javadoc for registerOwnerMembers contains caller
traversal-order details that do not belong in the method contract. Trim it to
describe the direct owner-type replacement for subordinate catalogs, handling
multiple owners as a union, and the invariant that owner types resolve lazily
within the member source; remove references to getChildrenByMdoRef(), hash-map
iteration, configuration traversal, and registration timing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 75274779-82be-4d84-8356-544ba44c807d
⛔ Files ignored due to path filters (4)
src/test/resources/metadata/catalogOwners/Catalogs/Справочник1.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Catalogs/Справочник2.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Configuration.xmlis excluded by!src/test/resources/**src/test/resources/metadata/catalogOwners/Languages/Русский.xmlis excluded by!src/test/resources/**
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypesRegistrar.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.jsonsrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/CatalogOwnerTypeTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderHelpersTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProviderHelpersTest.java
- src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.java
|
nixel2007
approved these changes
Aug 1, 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
2 participants
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.
В синтакс-помощнике стандартный реквизит
Владелецобъявлен типомНеопределено— и для неподчинённого справочника это правда. У подчинённого владельцы известны изCatalog.getOwners(), оттуда и берётся тип; несколько владельцев дают объединение.Заодно закрыт хвост, из-за которого это не работало бы и с типом: бестиповый стандартный реквизит из mdclasses больше не регистрируется. Дедуп членов идёт по (вид, имя), и такой член затирал объявление платформы вместе с его типом и описанием. Общее правило — не отдавать член, тип которого из метаданных не выводится.
Выделено из #4337, чтобы не тащить в ветку типов форм.
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests