fix(types): не подмешивать «Произвольный» к объявленному типу элемента коллекции#4185
Conversation
…а коллекции #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
|
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 (3)
📝 WalkthroughWalkthrough
ChangesCollection Element Type Inference and Arbitrary Type Canonicalization
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
🚥 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/types/CollectionReturnElementTypeInferenceTest.java (1)
79-91: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winAdd 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
⛔ Files ignored due to path filters (1)
src/test/resources/types/CollectionReturnElementType.bslis excluded by!src/test/resources/**
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/inferencer/ExpressionTypeInferencer.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-oscript-platform-types.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/types/registry/builtin-platform-types.jsonsrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/CollectionReturnElementTypeInferenceTest.java
…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
e6603c4 to
3057bd1
Compare
Проверка свелась к 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
|



#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
МассивandФиксированныйМассивcorrectly report an arbitrary element type.Tests