feat(semantictokens): не красить собственные реквизиты конфигурации как defaultLibrary#4132
Conversation
…ак 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]>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a ChangesStandardLibrary flag propagation and conditional DefaultLibrary semantic token modifier
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
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java (1)
193-237: ⚡ Quick winAdd regression tests for the remaining
standardLibrary=falsepaths.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
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberMethodCallSemanticTokensSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplier.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationTypesProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PlatformMemberPropertyAccessSemanticTokensSupplierTest.java
…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]>
|



Closes #3996
Проблема
PlatformMemberPropertyAccessSemanticTokensSupplier(PR #3989) вешал модификатор LSPdefaultLibraryна все разрезолвленные свойства. НоdefaultLibraryозначает «символ из стандартной библиотеки/платформы». Это верно для свойств платформенных типов и стандартных реквизитов (Наименование,Код,Ссылка, …), но не для собственных реквизитов конфигурации (Контрагент.ИНН), которые завёл разработчик.Что сделано
MemberDescriptor— добавлен признакstandardLibrary(по умолчаниюtrue, чтобы все платформенные источники остались как есть). Все compat-конструкторы иwith…-методы проносят флаг. Заодно убран избыточный шорткатwithLocalizedNames(был сахаром надwithBilingualName(BilingualString.of(...))) — это удержало число методов записи под лимитом Sonar S1448.ConfigurationTypesProvider— дискриминаторattribute instanceof StandardAttributeвbuildAttributeMembers(тот же, что на:978): стандартные реквизиты →true, собственные →false. Общие реквизиты (buildCommonAttributeMembers) и имена/колонки табличных частей →false.PlatformMemberPropertyAccessSemanticTokensSupplier—defaultLibraryвешается только приstandardLibrary, иначе простоProperty.Тесты
PlatformMemberPropertyAccessSemanticTokensSupplierTest:testStandardAttributeColoredAsDefaultLibrary—Объект.Наименование→Property + DefaultLibrary;testOwnAttributeColoredAsPlainProperty—Объект.Реквизит1→Propertyбез модификаторов.Прогон: 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
Tests