Skip to content

feat(semantictokens): не красить собственные реквизиты конфигурации как defaultLibrary#4132

Merged
nixel2007 merged 2 commits into
developfrom
worktree-issue-3996-semantic-tokens-defaultlibrary
Jun 16, 2026
Merged

feat(semantictokens): не красить собственные реквизиты конфигурации как defaultLibrary#4132
nixel2007 merged 2 commits into
developfrom
worktree-issue-3996-semantic-tokens-defaultlibrary

Conversation

@nixel2007

@nixel2007 nixel2007 commented Jun 16, 2026

Copy link
Copy Markdown
Member

Closes #3996

Проблема

PlatformMemberPropertyAccessSemanticTokensSupplier (PR #3989) вешал модификатор LSP defaultLibrary на все разрезолвленные свойства. Но defaultLibrary означает «символ из стандартной библиотеки/платформы». Это верно для свойств платформенных типов и стандартных реквизитов (Наименование, Код, Ссылка, …), но не для собственных реквизитов конфигурации (Контрагент.ИНН), которые завёл разработчик.

Х = Объект.Наименование; // стандартный реквизит → Property + DefaultLibrary (ок)
У = Объект.ИНН;          // собственный реквизит → теперь просто Property

Что сделано

  1. MemberDescriptor — добавлен признак standardLibrary (по умолчанию true, чтобы все платформенные источники остались как есть). Все compat-конструкторы и with…-методы проносят флаг. Заодно убран избыточный шорткат withLocalizedNames (был сахаром над withBilingualName(BilingualString.of(...))) — это удержало число методов записи под лимитом Sonar S1448.
  2. ConfigurationTypesProvider — дискриминатор attribute instanceof StandardAttribute в buildAttributeMembers (тот же, что на :978): стандартные реквизиты → true, собственные → false. Общие реквизиты (buildCommonAttributeMembers) и имена/колонки табличных частей → false.
  3. PlatformMemberPropertyAccessSemanticTokensSupplierdefaultLibrary вешается только при standardLibrary, иначе просто Property.

Тесты

PlatformMemberPropertyAccessSemanticTokensSupplierTest:

  • testStandardAttributeColoredAsDefaultLibraryОбъект.НаименованиеProperty + DefaultLibrary;
  • testOwnAttributeColoredAsPlainPropertyОбъект.Реквизит1Property без модификаторов.

Прогон: semantictokens / types / hover / completion — зелёные.

Пункт 3 issue (симметрия для методов) — отдельно, нужно согласовать

Не входит в этот PR. Экспортные методы общих модулей (ОбщегоНазначения.X) через PlatformMemberMethodCallSemanticTokensSupplier не проходят — их красит MethodCallSemanticTokensSupplier по ReferenceIndex, а сюда попадают только методы платформенных типов (для них defaultLibrary корректен). Чтобы снять defaultLibrary с методов общих модулей, нужно трогать MethodCallSemanticTokensSupplier — предлагаю вынести в отдельный issue/PR. Согласны?

Аналогично вне scope (домен GlobalScope / generic-expansion): значения перечислений, предопределённые, поля записей регистров.

Summary by CodeRabbit

Release Notes

  • New Features

    • Improved semantic highlighting: standard library and platform attributes are now visually distinguished from custom configuration attributes.
  • Tests

    • Added test coverage verifying that built-in attributes receive distinct semantic tokens from user-defined attributes.

…ак defaultLibrary

Модификатор LSP `defaultLibrary` означает «символ из стандартной
библиотеки/платформы». Это корректно для свойств платформенных типов и
стандартных реквизитов объектов конфигурации (Наименование/Код/Ссылка/…),
но не для собственных и общих реквизитов, заведённых разработчиком.

- MemberDescriptor: добавлен признак `standardLibrary` (по умолчанию true);
  собственные/общие реквизиты и табличные части помечаются false.
- ConfigurationTypesProvider: дискриминатор `instanceof StandardAttribute`
  в buildAttributeMembers; общие реквизиты и ТЧ — false.
- PlatformMemberPropertyAccessSemanticTokensSupplier: модификатор
  defaultLibrary вешается только при standardLibrary; иначе просто Property.
- PlatformMemberMethodCallSemanticTokensSupplier: modifiers() симметрично
  учитывает флаг (для текущих платформенных методов поведение не меняется).

Closes #3996

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: ca2e636a-f180-4b29-9b88-b3fbc0feeeab

📥 Commits

Reviewing files that changed from the base of the PR and between 5a282ae and 8765059.

📒 Files selected for processing (5)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinTypesJsonLoader.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptorFactoryTest.java
💤 Files with no reviewable changes (1)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplier.java

📝 Walkthrough

Walkthrough

Adds a standardLibrary boolean record component to MemberDescriptor (defaulting to true), propagates it through all copy/transform methods, migrates withLocalizedNames calls to withBilingualName, marks tabular-section, common-attribute, and non-StandardAttribute descriptors as false in ConfigurationTypesProvider, and gates the DefaultLibrary semantic token modifier on this flag in PlatformMemberPropertyAccessSemanticTokensSupplier.

Changes

StandardLibrary flag propagation and conditional DefaultLibrary semantic token modifier

Layer / File(s) Summary
MemberDescriptor standardLibrary component and copy API
src/main/java/.../types/model/MemberDescriptor.java
Adds boolean standardLibrary as a record component with Javadoc, defaults it to true in both compatibility constructors, propagates it through all copy/transform methods, and adds withStandardLibrary(boolean) with an unchanged-value fast path.
withLocalizedNameswithBilingualName migration
src/main/java/.../registry/BuiltinTypesJsonLoader.java, src/main/java/.../registry/GlobalScopeProvider.java, src/test/.../types/model/MemberDescriptorFactoryTest.java
Replaces withLocalizedNames(...) calls with withBilingualName(BilingualString.of(...)) in both JSON loaders and updates the corresponding factory test to validate the new API.
ConfigurationTypesProvider marks config-defined members as non-standard
src/main/java/.../registry/ConfigurationTypesProvider.java
Calls withStandardLibrary(false) on tabular-section collection descriptors, common-attribute descriptors, and attribute descriptors that are not StandardAttribute instances.
Semantic token supplier: conditional DefaultLibrary modifier + tests
src/main/java/.../semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplier.java, src/test/.../semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java
Refactors emit to resolve the full MemberDescriptor and apply DEFAULT_LIBRARY_MODIFIERS only when standardLibrary() is true. Two new tests verify that Наименование (standard attribute) receives Property + DefaultLibrary and Реквизит1 (developer-defined) receives plain Property.

Sequence Diagram(s)

sequenceDiagram
  participant Client as BSL Source
  participant Supplier as PlatformMemberPropertyAccessSemanticTokensSupplier
  participant TypeService as TypeService
  participant Descriptor as MemberDescriptor
  participant ConfigProvider as ConfigurationTypesProvider

  ConfigProvider->>Descriptor: withStandardLibrary(false) for non-StandardAttribute / tabular / common attrs
  Client->>Supplier: property access (e.g. Объект.Реквизит1)
  Supplier->>TypeService: memberAt(type, "Реквизит1")
  TypeService-->>Supplier: MemberDescriptor (standardLibrary=false)
  Supplier->>Descriptor: standardLibrary()
  Descriptor-->>Supplier: false
  Supplier-->>Client: emit SemanticToken Property (no DefaultLibrary)

  Client->>Supplier: property access (e.g. Объект.Наименование)
  Supplier->>TypeService: memberAt(type, "Наименование")
  TypeService-->>Supplier: MemberDescriptor (standardLibrary=true)
  Supplier->>Descriptor: standardLibrary()
  Descriptor-->>Supplier: true
  Supplier-->>Client: emit SemanticToken Property + DefaultLibrary
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • 1c-syntax/bsl-language-server#3989: The PR that originally introduced PlatformMemberPropertyAccessSemanticTokensSupplier, which this PR refines by making DefaultLibrary conditional on the new standardLibrary flag.
  • 1c-syntax/bsl-language-server#3952: Also modifies MemberDescriptor and semantic token modifier emission, adding the async flag to conditionally emit modifiers for method/global tokens.
  • 1c-syntax/bsl-language-server#3969: Modifies MemberDescriptor.specialize(...) and related copy methods, overlapping with this PR's propagation of standardLibrary through the same paths.

Suggested reviewers

  • theshadowco

Poem

🐇 A rabbit hopped through the codebase with glee,
Spotting platform members dressed all fancy-free!
"Not every field's a library gem,"
Said the bunny, adding flags to them.
Now DefaultLibrary shines only when true —
Configuration fields get plain Property too! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The pull request title clearly describes the main change: preventing custom configuration attributes from being colored with the defaultLibrary modifier in semantic tokens.
Linked Issues check ✅ Passed All coding requirements from #3996 are implemented: standardLibrary flag added to MemberDescriptor, ConfigurationTypesProvider marks standard vs custom attributes correctly, semantic token suppliers apply defaultLibrary only for standard library members, and comprehensive tests validate the behavior.
Out of Scope Changes check ✅ Passed All changes are directly aligned with the objectives in #3996: adding standardLibrary discrimination, updating MemberDescriptor, ConfigurationTypesProvider, semantic token suppliers, and related test cases. No unrelated changes detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch worktree-issue-3996-semantic-tokens-defaultlibrary

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java (1)

193-237: ⚡ Quick win

Add regression tests for the remaining standardLibrary=false paths.

These new tests cover standard vs custom object attributes, but the PR also changes common attributes and tabular-section members in ConfigurationTypesProvider. Please add explicit token assertions for those two paths to prevent regressions in the same contract.

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/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java`
around lines 193 - 237, The PR modifies ConfigurationTypesProvider to handle
standardLibrary=false for multiple attribute types, but the current tests only
cover standard and custom object attributes. Add two additional regression test
methods following the same pattern as
testStandardAttributeColoredAsDefaultLibrary and
testOwnAttributeColoredAsPlainProperty: one test method for common attributes
(which should verify that common attributes are colored appropriately based on
whether they are standard library members) and another test method for
tabular-section members (which should verify tabular-section member coloring).
Each new test should initialize the server context, define a test BSL snippet
with the relevant attribute type, call helper.getDecodedTokens with the
supplier, and assert the expected token types and modifiers using
helper.assertContainsTokens.

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/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java`:
- Around line 193-237: The PR modifies ConfigurationTypesProvider to handle
standardLibrary=false for multiple attribute types, but the current tests only
cover standard and custom object attributes. Add two additional regression test
methods following the same pattern as
testStandardAttributeColoredAsDefaultLibrary and
testOwnAttributeColoredAsPlainProperty: one test method for common attributes
(which should verify that common attributes are colored appropriately based on
whether they are standard library members) and another test method for
tabular-section members (which should verify tabular-section member coloring).
Each new test should initialize the server context, define a test BSL snippet
with the relevant attribute type, call helper.getDecodedTokens with the
supplier, and assert the expected token types and modifiers using
helper.assertContainsTokens.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 39d6783d-70d0-41b9-9cc4-2301f303760b

📥 Commits

Reviewing files that changed from the base of the PR and between 3dd3a91 and 5a282ae.

📒 Files selected for processing (5)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Test Results

 3 468 files  ± 0   3 468 suites  ±0   1h 38m 42s ⏱️ + 9m 39s
 3 478 tests + 2   3 460 ✅ + 2   18 💤 ±0  0 ❌ ±0 
20 868 runs  +12  20 756 ✅ +12  112 💤 ±0  0 ❌ ±0 

Results for commit 5a282ae. ± Comparison against base commit 3dd3a91.

♻️ This comment has been updated with latest results.

…ndardLibrary

- emit property-сапплаера переписан без лямбды и без code-подобного
  комментария (S2211, S125);
- удалён избыточный шорткат MemberDescriptor.withLocalizedNames, чтобы
  вернуть число методов записи под лимит (S1448); три вызова заменены на
  withBilingualName(BilingualString.of(...));
- откат спекулятивной правки modifiers() в method-сапплаере: снятие
  defaultLibrary с методов общих модулей — отдельный вопрос (п.3 issue),
  не вносим без поведения и покрытия.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant