fix(types): типизация свойств и возвращаемых значений методов oscript#4007
Merged
Conversation
Свойства объектов в дампе синтакс-помощника OneScript не несли тип (returnType), из-за чего вывод типов давал Unknown и пропадали подсказки по членам — например, HTTPЗапрос.Заголовки (Соответствие) не предлагал методы Соответствия (#4006). Типы свойств и возвращаемые значения методов восстановлены напрямую из исходников движка OneScript (атрибуты [ContextProperty]/[ContextMethod] и C#-типы геттеров/возврата), сопоставлены с зарегистрированными типами реестра. Курируемые конкретные returnType методов сохранены, "Произвольный" заменён на конкретный тип только там, где движок возвращает его явно. platform-types (builtin-oscript): - свойствам объектов проставлен returnType: 1188 из 1825 (остальные — значения перечислений, у которых тип члена не задаётся, и единичные edge-case'ы: рефлексируемые .NET-члены, обёртки enum, модель XML Schema) - методам дозаполнены/уточнены возвращаемые типы (ПолучитьТелоКакПоток → Поток, БуферДвоичныхДанных.Разделить → Массив и т.п.) - исправлены протёкшие из генератора «сырые» C#-имена типов (System.Object/Byte/Int16, XMLSchema.IXS* → Произвольный/Число) - ru/en-пары членов получают одинаковый тип, чтобы склейка двуязычных членов при загрузке не расходилась Добавлены регрессионные тесты на типы свойств и методов объектов oscript.
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:
📝 WalkthroughWalkthroughAdds LSP deprecation-tag detection and locale-aware completion filtering/marking, introduces BuiltinTypesJsonLoader for centralized JSON parsing, updates providers to use it (removing bilingual-pair collapse), and adds unit tests including OneScript HTTPЗапрос regressions. ChangesCompletion deprecation handling and tests
Builtin types JSON loader & OneScript regression tests
Sequence Diagram(s)sequenceDiagram
participant Client
participant CompletionProvider
participant TypeService
participant CompletionItemBuilder
Client->>CompletionProvider: initialize(capabilities with CompletionItem tag support)
CompletionProvider->>CompletionProvider: cache deprecatedTagSupport
Client->>CompletionProvider: completion request (dot, document, locale)
CompletionProvider->>TypeService: getMembers(typeRef, fileType, Language)
TypeService->>CompletionProvider: filtered members (locale-applicable)
CompletionProvider->>CompletionItemBuilder: build items and apply deprecation marking
CompletionItemBuilder->>CompletionProvider: return items
CompletionProvider->>Client: respond with completion items
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
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 |
Member
Author
|
/buildJar |
Contributor
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 7419932008 Файлы внутри:
|
Подтверждает, что после простановки returnType ru/en-пара свойства склеивается в один двуязычный член: в ru-локали показывается только Заголовки, в en — Headers (а не оба написания).
Contributor
Test Results 3 072 files +12 3 072 suites +12 1h 22m 20s ⏱️ - 4m 31s Results for commit 2ac7bb5. ± Comparison against base commit d837e31. This pull request removes 7 and adds 11 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
CompletionProvider при dot-completion: - помечает устаревшие свойства/методы CompletionItemTag.Deprecated (при отсутствии tagSupport у клиента — legacy-флагом setDeprecated), клиент рисует их зачёркнутыми; - не показывает устаревший [DeprecatedName]-алиас «чужой» локали: в ru-локали HTTPЗапрос больше не предлагает англоязычные GetBodyAsBinary/SetBodyFromBinary (есть актуальное русское имя), симметрично для en-локали. Нейтральные неустаревшие имена (значения перечислений ANSI/MD5) фильтром не затрагиваются. Записи устаревших алиасов остаются в json — DeprecatedMethodCall по-прежнему ловит легаси-вызовы (тесты диагностики зелёные). Тесты: CompletionDeprecatedMemberTest (скрытие алиаса + пометка устаревшего русского члена).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java (1)
617-626: 💤 Low valueConsider future-proofing tag assignment.
Currently,
setTags(List.of(CompletionItemTag.Deprecated))replaces any existing tags. If multiple tags are needed in the future (e.g.,Experimental), this will require refactoring to append. For now, since only one tag type is used, this is acceptable.♻️ Optional future-proof approach
if (deprecatedTagSupport) { - item.setTags(List.of(CompletionItemTag.Deprecated)); + var tags = new ArrayList<CompletionItemTag>(); + if (item.getTags() != null) { + tags.addAll(item.getTags()); + } + tags.add(CompletionItemTag.Deprecated); + item.setTags(tags); } else {🤖 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/providers/CompletionProvider.java` around lines 617 - 626, The markDeprecated method currently overwrites any existing tags by calling item.setTags(List.of(CompletionItemTag.Deprecated)); change it to preserve and append the Deprecated tag instead: when deprecatedTagSupport is true, retrieve existing tags from the CompletionItem (getTags), create a new modifiable list if needed, add CompletionItemTag.Deprecated only if not already present, then call setTags with the updated list (handle null/empty existing tags and avoid duplicates); keep the else branch using item.setDeprecated(Boolean.TRUE) as-is.
🤖 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/providers/CompletionProvider.java`:
- Around line 617-626: The markDeprecated method currently overwrites any
existing tags by calling item.setTags(List.of(CompletionItemTag.Deprecated));
change it to preserve and append the Deprecated tag instead: when
deprecatedTagSupport is true, retrieve existing tags from the CompletionItem
(getTags), create a new modifiable list if needed, add
CompletionItemTag.Deprecated only if not already present, then call setTags with
the updated list (handle null/empty existing tags and avoid duplicates); keep
the else branch using item.setDeprecated(Boolean.TRUE) as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 8345d6fb-0cdc-4d6e-a379-92f861af9435
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionDeprecatedMemberTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProviderTest.java
…ated в completion Двуязычие членов (вместо склейки по порядку) -------------------------------------------- Члены платформенных типов oscript теперь хранятся одной записью с явными nameRu/nameEn — как в builtin-platform-types.json (BSL), а не двумя соседними моноязычными записями, склеиваемыми на загрузке. Убрана хрупкая зависимость от порядка: 1425 ru/en-пар «запечены» в двуязычные записи, loader-эвристика mergeBilingualPairs (и её хелперы/фикстура/тесты) удалена. Loader по-прежнему читает nameRu/nameEn (как для BSL). Deprecated [DeprecatedName]-алиасы (HTTPЗапрос.GetBodyAsBinary/ SetBodyFromBinary, Файл.Exist) помечены как одноязычные (только nameEn) — русского написания у них нет. Локаль-скоуп в слое типов (а не в CompletionProvider) ---------------------------------------------------- Добавлен TypeService.getCompletionMembers(ref, fileType, language): отдаёт члены для автодополнения, отсекая устаревшие члены без написания в нужной локали. В ru-локали HTTPЗапрос больше не предлагает англоязычные GetBodyAsBinary/SetBodyFromBinary. getMembers (резолв имён, вывод типов, диагностики) остаётся двуязычным — устаревшие имена по-прежнему резолвятся, DeprecatedMethodCall их ловит. Прежний фильтр в CompletionProvider убран. Тег deprecated в автокомплите для всех членов --------------------------------------------- CompletionItemTag.Deprecated (или legacy-флаг при отсутствии tagSupport) ставится на любые устаревшие пункты: методы/свойства платформы, глобальные функции, члены конфигурации и пользовательские методы oscript. Тесты: CompletionDeprecatedMemberTest (скрытие алиаса в ru, пометка устаревшего русского платформенного члена и пользовательского метода), oscriptObjectPropertiesAreBilingualSingleMember и oscriptBilingualMemberIsSingleBilingualMember.
Парсинг встроенных JSON-паков платформенных типов вынесен из провайдеров в общий BuiltinTypesJsonLoader. И BSL-, и OneScript-провайдеры грузят одну и ту же структуру ресурса — теперь через один загрузчик, а не через static-метод BSL-провайдера, в который лез OScript-провайдер. BuiltinPlatformTypesProvider и BuiltinOScriptPlatformTypesProvider оставляют только провайдерную логику (путь ресурса, кэш, языковой скоуп, gating по bsl-context). Логика разбора (load/readMembers/readSignatures/readMetadata/ readTypeSet/...) перенесена без изменений поведения. Ссылки в тестах обновлены на BuiltinTypesJsonLoader.load.
nixel2007
commented
Jun 4, 2026
nixel2007
commented
Jun 4, 2026
nixel2007
commented
Jun 4, 2026
nixel2007
commented
Jun 4, 2026
По ревью: - TypeService.getCompletionMembers → перегрузка getMembers(ref, fileType, language). Отбор членов для автодополнения идёт чисто по применимости к языку, без проверок устаревания. - Модель: MemberDescriptor.appliesTo(Language) (через BilingualString .hasLanguage) — член применим к локали, если у него есть написание в ней. Двуязычные и нейтральные члены применимы к обеим локалям, одноязычные (англоязычные [DeprecatedName]-алиасы) — лишь к своей. Признак зависит только от имени члена. - Загрузчик: член без явной локализации имени (только `name`) считается нейтральным — оба слота имени заполняются, чтобы он был применим к обеим локалям (значения перечислений ANSI/MD5 и т.п.). - CompletionProvider: детект tagSupport через map + filter + isPresent. - Тесты приведены к given/when/then; устранён java:S5841 (проверка непустоты перед doesNotContain). Тесты загрузчика вынесены в отдельный BuiltinTypesJsonLoaderTest; в BuiltinPlatformTypesProviderTest остались только тесты провайдера.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionDeprecatedMemberTest.java (3)
56-69: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winStructure test methods with clear given/when/then blocks.
The test mixes setup and action on lines 58-61. Separate the "given" (setup), "when" (action), and "then" (assertion) sections with blank lines and consistent comments as suggested in the past review.
♻️ Proposed structure
`@Test` void httpRequestRuCompletionHidesEnglishDeprecatedAliasesAndDuplicateLatinNames() { - // given — обращение к члену HTTPЗапрос в ru-локали + // given var content = "Запрос = Новый HTTPЗапрос(\"/api\");\nЗапрос."; // when var labels = labelsAt(content, content.length()); - // then — канонические члены в русском написании есть; английская сторона - // двуязычного члена не дублируется, устаревшие [DeprecatedName]-алиасы скрыты + // then assertThat(labels) .contains("Заголовки", "ПолучитьТелоКакДвоичныеДанные") .doesNotContain("Headers", "GetBodyAsBinaryData") .doesNotContain("GetBodyAsBinary", "SetBodyFromBinary"); }Based on learnings, comprehensive unit tests should follow existing test patterns in the codebase.
🤖 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/CompletionDeprecatedMemberTest.java` around lines 56 - 69, The test method httpRequestRuCompletionHidesEnglishDeprecatedAliasesAndDuplicateLatinNames in CompletionDeprecatedMemberTest mixes setup and action; refactor it to clearly separate given/when/then by adding blank lines and consistent comments: keep the existing "given — ..." comment and the setup (content = ...), insert a blank line then the "when" comment and call to labelsAt(content, content.length()), another blank line then the "then" comment and the assertThat(...) block; ensure the labelsAt call remains the action and assertions remain unchanged.
95-113: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winStructure test methods with clear given/when/then blocks.
The test mixes setup comments with the code on lines 96-100. Separate the "given" (setup), "when" (action), and "then" (assertion) sections with blank lines and consistent comments.
♻️ Proposed structure
`@Test` void deprecatedLocalMethodIsMarkedInCompletion() { - // given — `#4006-follow-up`: устаревший пользовательский метод (помечен - // doc-комментарием) тоже должен получать тег deprecated в автокомплите + // given var content = "// Устарела. Используйте Актуальную.\n" + "Процедура Старая() Экспорт\nКонецПроцедуры\n\n" + "Процедура Актуальная() Экспорт\n\tСтар\nКонецПроцедуры"; // when var items = itemsAt(content, content.indexOf("\tСтар") + 5); // then var old = items.stream() .filter(it -> "Старая".equals(it.getLabel())) .findFirst() .orElseThrow(); assertThat(isMarkedDeprecated(old)) .as("устаревший пользовательский метод помечается deprecated") .isTrue(); }Based on learnings, comprehensive unit tests should follow existing test patterns in the codebase.
🤖 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/CompletionDeprecatedMemberTest.java` around lines 95 - 113, The test method deprecatedLocalMethodIsMarkedInCompletion mixes setup, action, and assertion without clear separation; refactor it to follow given/when/then blocks by grouping the setup into a "given" section (build content variable and comment), leaving a blank line, putting the action call to itemsAt in a "when" section, leaving another blank line, and then the assertions (finding old and assertThat(isMarkedDeprecated(old))) in a "then" section; keep the same variable names (content, items, old) and helper/isMarkedDeprecated and itemsAt usages, but add blank lines and consistent "// given", "// when", "// then" comments to match existing test patterns.
72-92: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winStructure test methods with clear given/when/then blocks.
The test mixes setup and action on lines 74-79. Separate the "given" (setup), "when" (action), and "then" (assertion) sections with blank lines and consistent comments.
♻️ Proposed structure
`@Test` void deprecatedRussianMemberIsMarkedAndEnglishCounterpartHidden() { - // given — обращение к члену СистемнаяИнформация в ru-локали + // given var content = "СИ = Новый СистемнаяИнформация;\nСИ."; // when var items = itemsAt(content, content.length()); var labels = items.stream().map(CompletionItem::getLabel).toList(); - // then — русское устаревшее имя видно и помечено deprecated + // then var envVars = items.stream() .filter(it -> "ПеременныеСреды".equals(it.getLabel())) .findFirst() .orElseThrow(); assertThat(isMarkedDeprecated(envVars)) .as("устаревший член помечается deprecated-тегом/флагом") .isTrue(); - // ...а англоязычное написание в ru-локали скрыто assertThat(labels) .contains("ПеременныеСреды") .doesNotContain("EnvironmentVariables"); }Based on learnings, comprehensive unit tests should follow existing test patterns in the codebase.
🤖 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/CompletionDeprecatedMemberTest.java` around lines 72 - 92, In deprecatedRussianMemberIsMarkedAndEnglishCounterpartHidden move setup, action and assertions into distinct given/when/then blocks: keep creating content and any preconditions in the "given" section (var content = ...), add a blank line and a "// when" comment before the call to itemsAt(...) and mapping to labels (var items = itemsAt(...); var labels = ...), then add a blank line and a "// then" comment before the assertions that locate envVars and call isMarkedDeprecated(...) and the labels assertions; reference the method CompletionDeprecatedMemberTest.deprecatedRussianMemberIsMarkedAndEnglishCounterpartHidden and the symbols itemsAt, labels, envVars, and isMarkedDeprecated when making the edits.
🤖 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/CompletionDeprecatedMemberTest.java`:
- Around line 56-69: The test method
httpRequestRuCompletionHidesEnglishDeprecatedAliasesAndDuplicateLatinNames in
CompletionDeprecatedMemberTest mixes setup and action; refactor it to clearly
separate given/when/then by adding blank lines and consistent comments: keep the
existing "given — ..." comment and the setup (content = ...), insert a blank
line then the "when" comment and call to labelsAt(content, content.length()),
another blank line then the "then" comment and the assertThat(...) block; ensure
the labelsAt call remains the action and assertions remain unchanged.
- Around line 95-113: The test method deprecatedLocalMethodIsMarkedInCompletion
mixes setup, action, and assertion without clear separation; refactor it to
follow given/when/then blocks by grouping the setup into a "given" section
(build content variable and comment), leaving a blank line, putting the action
call to itemsAt in a "when" section, leaving another blank line, and then the
assertions (finding old and assertThat(isMarkedDeprecated(old))) in a "then"
section; keep the same variable names (content, items, old) and
helper/isMarkedDeprecated and itemsAt usages, but add blank lines and consistent
"// given", "// when", "// then" comments to match existing test patterns.
- Around line 72-92: In
deprecatedRussianMemberIsMarkedAndEnglishCounterpartHidden move setup, action
and assertions into distinct given/when/then blocks: keep creating content and
any preconditions in the "given" section (var content = ...), add a blank line
and a "// when" comment before the call to itemsAt(...) and mapping to labels
(var items = itemsAt(...); var labels = ...), then add a blank line and a "//
then" comment before the assertions that locate envVars and call
isMarkedDeprecated(...) and the labels assertions; reference the method
CompletionDeprecatedMemberTest.deprecatedRussianMemberIsMarkedAndEnglishCounterpartHidden
and the symbols itemsAt, labels, envVars, and isMarkedDeprecated when making the
edits.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3b60ebe3-4d45-45dc-962e-dbb99fb1cf14
📒 Files selected for processing (8)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/TypeService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/BilingualString.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/MemberDescriptor.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinTypesJsonLoader.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionDeprecatedMemberTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinTypesJsonLoaderTest.java
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinTypesJsonLoader.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/BuiltinPlatformTypesProviderTest.java
|
sfaqer
reviewed
Jun 5, 2026
…#4007) По ревью sfaqer: пометка устаревания в автодополнении теперь сверяется с целевой версией платформы (targetVersion из конфига либо режим совместимости конфигурации) — как в DeprecatedMethodCall. Член помечается deprecated, только если PlatformMemberCalls.firesDeprecated(deprecatedSinceVersion, target); для oscript sentinel "*" срабатывает всегда. Source-члены (doc-комментарий) — по-прежнему без версии. Заодно: члены, недоступные в целевой версии (sinceVersion новее target, PlatformMemberCalls.firesUnavailable) больше не предлагаются в автодополнении — консистентно с UnavailableMemberCall, который помечает их вызов.
…rmMemberVersions Провайдер не должен зависеть от пакета diagnostics. Версионная логика (targetCompatibilityMode / firesDeprecated / firesUnavailable / sentinel DEPRECATED_ALWAYS) вынесена из diagnostics.platform.PlatformMemberCalls в новый types.PlatformMemberVersions — ближе к системе типов. Ею пользуются и CompletionProvider, и диагностики (DeprecatedMethodCall/UnavailableMemberCall). PlatformMemberCalls остаётся в diagnostics и отвечает только за резолв платформенных членов в сайтах вызовов модуля (collect/hasDeletedPrefix) — это диагностический разбор AST. Тесты версионной логики вынесены в PlatformMemberVersionsTest; в PlatformMemberCallsTest остались проверки префикса «Удалить»/«Delete».
Contributor
|
/buildJar |
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
4 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.
Свойства объектов в дампе синтакс-помощника OneScript не несли тип (returnType), из-за чего вывод типов давал Unknown и пропадали подсказки по членам — например, HTTPЗапрос.Заголовки (Соответствие) не предлагал методы Соответствия (#4006).
Типы свойств и возвращаемые значения методов восстановлены напрямую из исходников движка OneScript (атрибуты [ContextProperty]/[ContextMethod] и C#-типы геттеров/возврата), сопоставлены с зарегистрированными типами реестра. Курируемые конкретные returnType методов сохранены, "Произвольный" заменён на конкретный тип только там, где движок возвращает его явно.
platform-types (builtin-oscript):
Добавлены регрессионные тесты на типы свойств и методов объектов oscript.
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit