Skip to content

fix(types): не подмешивать «Произвольный» к объявленному типу элемента коллекции#4185

Merged
nixel2007 merged 6 commits into
developfrom
claude/issue-4179
Jun 22, 2026
Merged

fix(types): не подмешивать «Произвольный» к объявленному типу элемента коллекции#4185
nixel2007 merged 6 commits into
developfrom
claude/issue-4179

Conversation

@nixel2007

@nixel2007 nixel2007 commented Jun 22, 2026

Copy link
Copy Markdown
Member

#4179: переменной с результатом функции, возвращающей Массив из Число,
присваивался тип Массив из Число, Произвольный — платформенный дефолтный тип
элемента коллекции (Произвольный) добавлялся поверх объявленного через JsDoc.

attachDefaultElementTypes теперь, если у ссылки уже есть конкретный объявленный
тип элемента, отбрасывает из дефолтов универсальный Произвольный/ANY (он не
уточняет известный тип). Осмысленные дефолты обёрточных коллекций
(ЭлементСпискаЗначений, КлючИЗначение) сохраняются, поэтому вывод типа в
Для Каждого не ломается. Согласуется с уже существующим срезанием
«Массив из Произвольный» → «Массив» в ConfigurationModuleMembersProvider.

Co-Authored-By: Claude Opus 4.8 (1M context) [email protected]
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved collection type inference so explicitly declared element types are no longer overridden by generic “arbitrary” defaults.
    • Enhanced handling of the universal arbitrary type so it’s consistently recognized and intersected as the canonical “Any” type.
    • Updated platform array type metadata so Массив and ФиксированныйМассив correctly report an arbitrary element type.
  • Tests

    • Added/updated coverage for collection return element type inference and canonical “Any” behavior.

claude added 2 commits June 22, 2026 05:43
…а коллекции

#4179: переменной с результатом функции, возвращающей `Массив из Число`,
присваивался тип `Массив из Число, Произвольный` — платформенный дефолтный тип
элемента коллекции (`Произвольный`) добавлялся поверх объявленного через JsDoc.

attachDefaultElementTypes теперь, если у ссылки уже есть конкретный объявленный
тип элемента, отбрасывает из дефолтов универсальный `Произвольный`/ANY (он не
уточняет известный тип). Осмысленные дефолты обёрточных коллекций
(`ЭлементСпискаЗначений`, `КлючИЗначение`) сохраняются, поэтому вывод типа в
`Для Каждого` не ломается. Согласуется с уже существующим срезанием
«Массив из Произвольный» → «Массив» в ConfigurationModuleMembersProvider.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
…роверки BSL/OS

- builtin-platform-types.json / builtin-oscript-platform-types.json: у `Массив`
  (и `ФиксированныйМассив` в OneScript) добавлен дефолтный тип элемента
  `Произвольный` — это платформенная правда и одновременно триггер #4179, без
  которого баг не воспроизводился в тестовом контексте.
- CollectionReturnElementTypeInferenceTest: тест `Массив из Число` → `[Число]`
  теперь реально проверяет фильтрацию `Произвольный`; добавлены проверка
  сохранения дефолта обёрточной коллекции (СписокЗначений → ЭлементСпискаЗначений)
  и симметрии дефолта `Массив` для BSL и OneScript.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
@coderabbitai

coderabbitai Bot commented Jun 22, 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: 62f6432c-ab68-4ffa-a13e-3e668309cb09

📥 Commits

Reviewing files that changed from the base of the PR and between 6a99c3e and 2ffe4e2.

📒 Files selected for processing (3)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/model/TypeRef.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/util/SignatureSelection.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/ChainedAccessorInferenceTest.java

📝 Walkthrough

Walkthrough

TypeRef introduces a compact constructor to canonicalize "Произвольный" and "Arbitrary" to TypeKind.ANY with qualifiedName "Any". TypeRegistry.bootstrap() registers "произвольный" and "arbitrary" aliases to TypeRef.ANY with bilingual naming. SignatureSelection.typesIntersect treats only the canonical TypeRef.ANY as universally compatible. ExpressionTypeInferencer.attachDefaultElementTypes gains a new isOnlyAny helper and loop logic to skip injecting the universal any default when a TypeRef already declares element types. Built-in JSON registries declare elementTypes for array types. Tests validate the combined behavior.

Changes

Collection Element Type Inference and Arbitrary Type Canonicalization

Layer / File(s) Summary
TypeRef arbitrary type canonicalization
src/main/java/.../types/model/TypeRef.java
Compact constructor normalizes "Произвольный" and "Arbitrary" (case-insensitive) to TypeKind.ANY with qualifiedName "Any", ensuring these platform names behave identically to TypeRef.ANY at construction time.
TypeRegistry aliases and array elementTypes declarations
src/main/java/.../types/registry/TypeRegistry.java, src/main/resources/.../types/registry/builtin-platform-types.json, src/main/resources/.../types/registry/builtin-oscript-platform-types.json
TypeRegistry.bootstrap() maps lowercase "произвольный" and "arbitrary" to TypeRef.ANY in aliasIndex with bilingual display name. Массив in the platform registry and both Массив and ФиксированныйМассив in the oscript registry gain elementTypes: ["Произвольный"].
SignatureSelection universal type handling
src/main/java/.../types/util/SignatureSelection.java
Javadoc reflects that arbitrary-name variants are canonicalized into TypeRef.ANY at TypeRef construction. typesIntersect treats intersection as universally compatible only when either side equals the canonical TypeRef.ANY, removing previous universal-name detection via dedicated helper.
attachDefaultElementTypes filtering and isOnlyAny helper
src/main/java/.../types/inferencer/ExpressionTypeInferencer.java
Loop now skips refs with empty registry defaults and skips calling withElement when base already has element types and defaults consist solely of the canonical TypeRef.ANY. New private isOnlyAny(TypeSet) helper implements singleton check. Javadoc documents the new behavior.
Collection element type inference tests
src/test/java/.../types/CollectionReturnElementTypeInferenceTest.java, src/test/java/.../types/ChainedAccessorInferenceTest.java
New test class verifies declared array element types are not polluted by platform default, wrapper collection defaults are preserved, and Массив default canonicalizes to TypeRef.ANY in both BSL and OneScript contexts. Existing test updated to expect canonical "Any" qualifiedName.

Sequence Diagram(s)

No sequence diagram is applicable for this change. The PR comprises type canonicalization logic, registry configuration, and type intersection filtering without multi-component interaction flows to visualize.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐇 A bunny sorted through the type array heap,
Found "arbitrary" sneaking past where it should sleep.
"Canonicalize at source!" the rabbit declared,
And made "Произвольный" and "Arbitrary" properly paired.
Now Массив and its kin are correctly arrayed —
No phantom defaults where real types are made! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 35.71% 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 title accurately describes the main fix: preventing the platform's arbitrary/universal type from polluting explicitly declared collection element types.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 claude/issue-4179

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/types/CollectionReturnElementTypeInferenceTest.java (1)

79-91: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Add a ФиксированныйМассив default-element assertion to match the registry changes.

This PR updates OneScript defaults for both Массив and ФиксированныйМассив, but this test currently validates only Массив. Adding one assertion here closes that gap with minimal effort.

Suggested test addition
   `@Test`
   void arrayHasArbitraryDefaultElementTypeInBothLanguages() {
@@
     for (var fileType : new FileType[]{FileType.BSL, FileType.OS}) {
       var arrayRef = typeRegistry.resolve("Массив", fileType).orElseThrow();
       assertThat(typeRegistry.getDefaultElementTypes(arrayRef).refs())
         .as("Массив (%s) имеет дефолтный тип элемента Произвольный", fileType)
         .extracting(r -> r.qualifiedName())
         .containsExactly("Произвольный");
     }
+
+    var fixedArrayRef = typeRegistry.resolve("ФиксированныйМассив", FileType.OS).orElseThrow();
+    assertThat(typeRegistry.getDefaultElementTypes(fixedArrayRef).refs())
+      .as("ФиксированныйМассив (OS) имеет дефолтный тип элемента Произвольный")
+      .extracting(r -> r.qualifiedName())
+      .containsExactly("Произвольный");
   }

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/CollectionReturnElementTypeInferenceTest.java`
around lines 79 - 91, The test method
arrayHasArbitraryDefaultElementTypeInBothLanguages currently only validates the
default element type for Массив across both file types, but the PR updates
defaults for both Массив and ФиксированныйМассив. Add an additional assertion
after the existing loop that validates ФиксированныйМассив also has the default
element type Произвольный for both FileType.BSL and FileType.OS, following the
same pattern used for Массив: resolve the type using typeRegistry.resolve,
retrieve default element types using getDefaultElementTypes, and assert the
qualified name is Произвольный.

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/types/CollectionReturnElementTypeInferenceTest.java`:
- Around line 79-91: The test method
arrayHasArbitraryDefaultElementTypeInBothLanguages currently only validates the
default element type for Массив across both file types, but the PR updates
defaults for both Массив and ФиксированныйМассив. Add an additional assertion
after the existing loop that validates ФиксированныйМассив also has the default
element type Произвольный for both FileType.BSL and FileType.OS, following the
same pattern used for Массив: resolve the type using typeRegistry.resolve,
retrieve default element types using getDefaultElementTypes, and assert the
qualified name is Произвольный.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 317cea8d-ec3b-45a6-a857-5e06a91f0cd9

📥 Commits

Reviewing files that changed from the base of the PR and between 4dbf671 and 674e53d.

⛔ Files ignored due to path filters (1)
  • src/test/resources/types/CollectionReturnElementType.bsl is excluded by !src/test/resources/**
📒 Files selected for processing (4)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.java
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-oscript-platform-types.json
  • src/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.json
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionReturnElementTypeInferenceTest.java

claude added 3 commits June 22, 2026 06:43
…ypeRef.isAny

По ревью: вместо пересборки набора дефолтных типов элементов без any проверяем,
что дефолт состоит из единственного универсального типа, и тогда не подмешиваем
его к уже объявленному конкретному типу элемента.

Распознавание универсального типа (канонический TypeRef.ANY либо платформенное
имя Произвольный/Arbitrary — в метаданных «Произвольный» не резолвится в ANY)
вынесено в TypeRef.isAny(); дублирующая приватная копия в SignatureSelection
теперь делегирует туда.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
…isAny

По ревью: универсальный тип не должен распознаваться сверкой имени. «Произвольный»/
«Arbitrary» теперь регистрируются в TypeRegistry.bootstrap() как алиасы TypeRef.ANY
(resolve("Произвольный") → ANY), а ANY получает отображаемое имя «Произвольный».

В результате getDefaultElementTypes(Массив) отдаёт {ANY}, а проверка в
attachDefaultElementTypes — это сравнение equals(TypeRef.ANY) без проверки имени.
TypeRef.isAny() удалён; SignatureSelection вернул свою прежнюю проверку
(синтакс-помощник присылает именованный «Произвольный» мимо resolve).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
…eRef

Цель: универсальный тип всегда и везде сводится к TypeRef.ANY с самого низа
системы типов. Добавлен компактный конструктор TypeRef: любой реф, построенный
из имени «Произвольный»/«Arbitrary» (провайдеры платформы, JSON-загрузчик,
интернинг, JsDoc-fallback), сразу становится TypeRef.ANY.

Благодаря этому проверка имени больше не нужна выше: SignatureSelection.isAny
теперь просто equals(TypeRef.ANY) (типы параметров сигнатур строятся сырым
new TypeRef и тоже канонизируются). Вместе с алиасом resolve("Произвольный")→ANY
и отображаемым именем ANY→«Произвольный» это даёт единое представление.

ChainedAccessorInferenceTest обновлён: Массив.Получить(0) → ANY ("Any").

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
@nixel2007
nixel2007 force-pushed the claude/issue-4179 branch from e6603c4 to 3057bd1 Compare June 22, 2026 07:38
Проверка свелась к equals(TypeRef.ANY) (имя «Произвольный» канонизируется в
TypeRef.ANY при создании TypeRef), поэтому отдельный метод-обёртка не нужен.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01E3nicquvAcTyP4X6Ymxq4F
@sonarqubecloud

Copy link
Copy Markdown

@github-actions

Copy link
Copy Markdown
Contributor

Test Results

 3 528 files  + 6   3 528 suites  +6   1h 32m 41s ⏱️ - 1m 48s
 3 461 tests + 3   3 443 ✅ + 3   18 💤 ±0  0 ❌ ±0 
20 766 runs  +18  20 654 ✅ +18  112 💤 ±0  0 ❌ ±0 

Results for commit 2ffe4e2. ± Comparison against base commit 4dbf671.

@nixel2007
nixel2007 merged commit 8b63770 into develop Jun 22, 2026
36 of 37 checks passed
@nixel2007
nixel2007 deleted the claude/issue-4179 branch June 22, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants