feat(providers): иерархия типов для OneScript-классов библиотеки extends#4014
Merged
Conversation
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 OneScript extends/interface annotation discovery, a TypeRelationIndex for supertype/subtype/interface relations (cycle-guarded), LSP providers for implementations and type hierarchy, inference updates for parent fields, inherited-member registration, docs updates, and extensive tests. ChangesOneScript Type Hierarchy and Inheritance
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~75 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.java (1)
201-201: ⚡ Quick winPrefer
toLowerCase(Locale.ROOT)for case-insensitive comparison.For consistency with interface name matching (lines 91, 134) and to avoid locale-dependent behavior, replace
equalsIgnoreCasewith explicittoLowerCase(Locale.ROOT)comparison.♻️ Proposed refactor
for (var candidate : serverContext.getDocuments().values()) { if (candidate.getFileType() == FileType.OS - && FilenameUtils.getBaseName(candidate.getUri().getPath()).equalsIgnoreCase(name)) { + && FilenameUtils.getBaseName(candidate.getUri().getPath()).toLowerCase(Locale.ROOT).equals(name.toLowerCase(Locale.ROOT))) { return Optional.of(candidate); } }🤖 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/ImplementationProvider.java` at line 201, The comparison using FilenameUtils.getBaseName(candidate.getUri().getPath()).equalsIgnoreCase(name) in ImplementationProvider should be made locale-safe and consistent with other matches: replace the equalsIgnoreCase call with explicit toLowerCase(Locale.ROOT) on both sides (e.g., FilenameUtils.getBaseName(candidate.getUri().getPath()).toLowerCase(Locale.ROOT).equals(name.toLowerCase(Locale.ROOT))) and add the java.util.Locale import if missing; this change should be applied where the base name is compared to name in the ImplementationProvider class to match the approach used at lines 91 and 134.src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.java (1)
118-132: ⚡ Quick winReturn immutable collection for consistency.
The method returns
List.of()(immutable) at line 121 but returns the mutableArrayListat line 131. This inconsistency means callers cannot rely on a consistent mutability contract. Utility methods should return immutable collections to prevent accidental modification and ensure defensive programming.🛡️ Proposed fix
- return result; + return List.copyOf(result);🤖 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/types/oscript/OScriptExtends.java` around lines 118 - 132, The method implementedInterfaceNames in OScriptExtends returns an immutable List.of() in the early-exit branch but a mutable ArrayList in the normal path; change the normal path to return an immutable copy (e.g., wrap the result with List.copyOf(result) or return Collections.unmodifiableList(result)) before returning so callers always get an immutable collection; update the return of result in implementedInterfaceNames to use that immutable wrapper while keeping the rest of the logic (DocumentContext, metaResolver, and result accumulation) unchanged.
🤖 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/ImplementationProvider.java`:
- Line 201: The comparison using
FilenameUtils.getBaseName(candidate.getUri().getPath()).equalsIgnoreCase(name)
in ImplementationProvider should be made locale-safe and consistent with other
matches: replace the equalsIgnoreCase call with explicit
toLowerCase(Locale.ROOT) on both sides (e.g.,
FilenameUtils.getBaseName(candidate.getUri().getPath()).toLowerCase(Locale.ROOT).equals(name.toLowerCase(Locale.ROOT)))
and add the java.util.Locale import if missing; this change should be applied
where the base name is compared to name in the ImplementationProvider class to
match the approach used at lines 91 and 134.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.java`:
- Around line 118-132: The method implementedInterfaceNames in OScriptExtends
returns an immutable List.of() in the early-exit branch but a mutable ArrayList
in the normal path; change the normal path to return an immutable copy (e.g.,
wrap the result with List.copyOf(result) or return
Collections.unmodifiableList(result)) before returning so callers always get an
immutable collection; update the return of result in implementedInterfaceNames
to use that immutable wrapper while keeping the rest of the logic
(DocumentContext, metaResolver, and result accumulation) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 0bd027fb-03a6-4726-af49-e9c8ceba21b9
⛔ Files ignored due to path filters (9)
src/test/resources/oscript-libraries/interface-abstract-lib/lib.configis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-abstract-lib/src/АбстрактноеХранилище.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-abstract-lib/src/ИнтерфейсХранилища.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-abstract-lib/src/КонкретноеХранилище.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-lib/lib.configis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-lib/src/МойИнтерфейс.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-lib/src/НеРеализация.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-lib/src/Реализация1.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/interface-lib/src/Реализация2.osis excluded by!src/test/resources/**
📒 Files selected for processing (9)
docs/en/index.mddocs/index.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationAbstractClassTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProviderTest.java
✅ Files skipped from review due to trivial changes (2)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationAbstractClassTest.java
- docs/index.md
🚧 Files skipped from review as they are similar to previous changes (3)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
Contributor
Test Results 3 192 files + 66 3 192 suites +66 1h 37m 52s ⏱️ + 4m 24s Results for commit 66a58ea. ± Comparison against base commit 1fc87d0. This pull request removes 117 and adds 191 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Member
Author
|
/buildJar |
Contributor
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 7464512744 Файлы внутри:
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.java (1)
646-648: ⚡ Quick winStrengthen routing assertions to validate non-empty success paths.
At Line 646 and Line 647, and again at Line 670–672, assertions only check non-null /
isLeft(). These tests can pass even if hierarchy/implementation results are empty. Add minimal content checks so routing regressions are caught.Suggested assertion hardening
- assertThat(supertypes).isNotNull(); - assertThat(subtypes).isNotNull(); + assertThat(supertypes).isNotNull().isNotEmpty(); + assertThat(subtypes).isNotNull().isNotEmpty(); ... - assertThat(result).isNotNull(); - assertThat(result.isLeft()).isTrue(); + assertThat(result).isNotNull(); + assertThat(result.isLeft()).isTrue(); + assertThat(result.getLeft()).isNotEmpty();Also applies to: 670-672
🤖 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/BSLTextDocumentServiceTest.java` around lines 646 - 648, The current assertions in BSLTextDocumentServiceTest only verify that supertypes and subtypes are not null (and similar isLeft() checks later) which allows empty results to pass; update the assertions around variables supertypes and subtypes (and the corresponding isLeft() checks at the other block) to also assert they are non-empty (e.g., size > 0 or hasSizeGreaterThan(0)) and/or contain an expected sample element to ensure routing returns at least one success path; locate the checks in the test methods referencing supertypes, subtypes and the isLeft() result and strengthen them to validate minimal content rather than just non-null.
🤖 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.
Inline comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptClassResolver.java`:
- Around line 97-103: The current loop in OScriptClassResolver that iterates
serverContext.getDocuments().values() to find an .os by basename is
non-deterministic when multiple files share the same base name; change the logic
in the method containing that loop to collect all matches (where
candidate.getFileType() == FileType.OS and
FilenameUtils.getBaseName(candidate.getUri().getPath()).equalsIgnoreCase(name)),
sort the matches deterministically (for example by candidate.getUri().getPath()
or candidate.getUri().toString()), and then return Optional.of(firstSortedMatch)
or Optional.empty() if none — this ensures OScriptClassResolver picks a stable
file when duplicate basenames exist.
---
Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.java`:
- Around line 646-648: The current assertions in BSLTextDocumentServiceTest only
verify that supertypes and subtypes are not null (and similar isLeft() checks
later) which allows empty results to pass; update the assertions around
variables supertypes and subtypes (and the corresponding isLeft() checks at the
other block) to also assert they are non-empty (e.g., size > 0 or
hasSizeGreaterThan(0)) and/or contain an expected sample element to ensure
routing returns at least one success path; locate the checks in the test methods
referencing supertypes, subtypes and the isLeft() result and strengthen them to
validate minimal content rather than just non-null.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b94d957f-8dba-46de-966a-3effbafab892
⛔ Files ignored due to path filters (1)
src/test/resources/standalone-class.osis excluded by!src/test/resources/**
📒 Files selected for processing (11)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptClassResolver.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServerTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionParentFieldTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptClassResolverTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtendsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptInheritanceMembersTest.java
✅ Files skipped from review due to trivial changes (1)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServerTest.java
🚧 Files skipped from review as they are similar to previous changes (4)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptInheritanceMembersTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionParentFieldTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.java
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionParentFieldTest.java (1)
134-134: ⚡ Quick winAdd import for
java.util.List.The method signature uses the fully qualified name
java.util.List<String>instead of importing the type. As per coding guidelines, imports should be optimized before committing. Adding the import improves readability and follows standard Java style.📦 Proposed fix
Add the import at the top of the file:
import java.nio.file.Path; +import java.util.List; import static org.assertj.core.api.Assertions.assertThat;Then update the method signature:
- private java.util.List<String> labelsAfterDot(String content, String receiver) { + private List<String> labelsAfterDot(String content, String receiver) {🤖 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/CompletionParentFieldTest.java` at line 134, The method labelsAfterDot currently uses the fully-qualified type java.util.List<String>; add an import statement for java.util.List at the top of the file and change the method signature to use List<String> (i.e., private List<String> labelsAfterDot(String content, String receiver)); run your IDE's optimize/imports to ensure imports are formatted and unused imports removed; make sure the change is applied to the method declaration named labelsAfterDot.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/providers/CompletionParentFieldTest.java`:
- Line 134: The method labelsAfterDot currently uses the fully-qualified type
java.util.List<String>; add an import statement for java.util.List at the top of
the file and change the method signature to use List<String> (i.e., private
List<String> labelsAfterDot(String content, String receiver)); run your IDE's
optimize/imports to ensure imports are formatted and unused imports removed;
make sure the change is applied to the method declaration named labelsAfterDot.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 235421c8-4ac4-46c0-9d31-6a81637b3f9e
📒 Files selected for processing (11)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptClassResolver.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProvider.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionInheritedMembersTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionParentFieldTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProviderTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtendsTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptInheritanceMembersTest.java
💤 Files with no reviewable changes (1)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProviderTest.java
🚧 Files skipped from review as they are similar to previous changes (9)
- src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/CompletionInheritedMembersTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptClassResolver.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtendsTest.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentServiceTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProvider.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.java
- src/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptInheritanceMembersTest.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptExtends.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProvider.java
Member
Author
|
/buildJar |
Contributor
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 7466203611 Файлы внутри:
|
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptLibraryIndex.java (1)
254-254: 💤 Low valueConsider defensive handling of URI path extraction.
While
.osdocuments typically havefile:URIs with non-null paths,uri.getPath()can returnnullfor opaque URIs.FilenameUtils.getBaseName(null)returns an empty string, which would produceList.of("")— a non-empty list containing a blank class name. This could confuse downstream consumers expecting valid identifiers.Consider either:
- Adding a null-safe fallback (e.g.,
Optional.ofNullable(uri.getPath()).map(FilenameUtils::getBaseName).filter(s -> !s.isBlank()).map(List::of).orElse(List.of())), or- Documenting the assumption that
.osURIs are always hierarchical file URIs with non-null paths.🤖 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/types/oscript/OScriptLibraryIndex.java` at line 254, The current return uses FilenameUtils.getBaseName(uri.getPath()) and can produce List.of("") when uri.getPath() is null/blank; in OScriptLibraryIndex replace that direct call with a null/blank-safe check (e.g., check uri.getPath() != null && !uri.getPath().isBlank() or use Optional.ofNullable(uri.getPath())...) and only return List.of(FilenameUtils.getBaseName(...)) when the result is a non-blank name; otherwise return an empty list so downstream consumers never receive a blank identifier. Ensure you reference uri.getPath(), FilenameUtils.getBaseName, and the method in OScriptLibraryIndex where this return occurs.
🤖 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/types/oscript/OScriptLibraryIndex.java`:
- Line 254: The current return uses FilenameUtils.getBaseName(uri.getPath()) and
can produce List.of("") when uri.getPath() is null/blank; in OScriptLibraryIndex
replace that direct call with a null/blank-safe check (e.g., check uri.getPath()
!= null && !uri.getPath().isBlank() or use
Optional.ofNullable(uri.getPath())...) and only return
List.of(FilenameUtils.getBaseName(...)) when the result is a non-blank name;
otherwise return an empty list so downstream consumers never receive a blank
identifier. Ensure you reference uri.getPath(), FilenameUtils.getBaseName, and
the method in OScriptLibraryIndex where this return occurs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c26e7ae3-25fc-4b9e-9ea7-f4981031810d
⛔ Files ignored due to path filters (6)
src/test/resources/oscript-libraries/cycle-lib/lib.configis excluded by!src/test/resources/**src/test/resources/oscript-libraries/cycle-lib/src/ИнтерфейсЦиклА.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/cycle-lib/src/ИнтерфейсЦиклБ.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/cycle-lib/src/РеализаторЦикла.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/cycle-lib/src/ЦиклА.osis excluded by!src/test/resources/**src/test/resources/oscript-libraries/cycle-lib/src/ЦиклБ.osis excluded by!src/test/resources/**
📒 Files selected for processing (7)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/ImplementationProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/providers/TypeHierarchyProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptLibraryIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptModuleMembersProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/TypeRelationIndex.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/OScriptLibraryIndexTest.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/types/oscript/TypeRelationIndexTest.java
Member
Author
|
@claude review |
|
Claude finished @nixel2007's task in 3m 17s —— View job Code ReviewБольшой и хорошо структурированный PR. Архитектурное решение централизовать всё наследование в Ниже — замечания, сгруппированные по приоритету. Потенциальный NPE / NoSuchElementException
var primaryName = names.isEmpty() ? "" : names.iterator().next();
var item = new TypeHierarchyItem(primaryName, SymbolKind.Class, ...);
var range = module.getSelectionRange();
return range != null ? range : Ranges.create(0, 0, 0, 0);
Производительность (дизайн-трейдофф, упомянутый в PR)
PR явно выбирает «живое» чтение для hot-reload — это приемлемо для малых проектов. Для будущей оптимизации можно рассмотреть обратный индекс
Нарушение контракта иммутабельности
return List.copyOf(result);(Это уже отмечал CodeRabbit, приводим для полноты.) Минорные замечания
Что сделано хорошо
|
Member
Author
Code review (at
|
nixel2007
added a commit
that referenced
this pull request
Jun 8, 2026
- TypeHierarchyProvider.toItem: guard на пустой classNames перед
names.iterator().next() (NoSuchElementException на опаковом URI).
- ImplementationProvider.classSelectionRange: fallback на Ranges.create(0,0,0,0),
если module.getSelectionRange() == null — согласованно с
TypeHierarchyProvider.selectionRange.
- OScriptLibraryIndex.classNames: guard от опакового URI (getPath()==null),
чтобы не вернуть List.of("").
- TypeHierarchyProvider.ITEM_COMPARATOR: сделан private static final (не зависит
от состояния бина).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
added a commit
that referenced
this pull request
Jun 8, 2026
…уктора По ревью PR #4014: аннотации наследования библиотеки extends объявляются на конструкторе ПриСозданииОбъекта. Раньше parentClassName/isInterface/ implementedInterfaceNames сканировали ВСЕ методы — вспомогательный метод с &Расширяет("X") (например, из другой библиотеки) давал ложное наследование. Теперь читаем аннотации только с метода-конструктора. Конструктор ищется по имени через Methods.isOscriptClassConstructorName (ПриСозданииОбъекта/ OnObjectCreate), а НЕ через SymbolTree.getConstructor(): ConstructorSymbol создаётся лишь для классифицированных OScript-классов, а наследование работает и для обычных .os-файлов (basename-резолв) — иначе ломается иерархия plain-.os. Заодно implementedInterfaceNames возвращает List.copyOf (иммутабельно, как и ранний выход List.of()). Тесты: аннотация на вспомогательном методе не считается наследованием/ реализацией. Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
added a commit
that referenced
this pull request
Jun 10, 2026
- TypeHierarchyProvider.toItem: guard на пустой classNames перед
names.iterator().next() (NoSuchElementException на опаковом URI).
- ImplementationProvider.classSelectionRange: fallback на Ranges.create(0,0,0,0),
если module.getSelectionRange() == null — согласованно с
TypeHierarchyProvider.selectionRange.
- OScriptLibraryIndex.classNames: guard от опакового URI (getPath()==null),
чтобы не вернуть List.of("").
- TypeHierarchyProvider.ITEM_COMPARATOR: сделан private static final (не зависит
от состояния бина).
Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
added a commit
that referenced
this pull request
Jun 10, 2026
…уктора По ревью PR #4014: аннотации наследования библиотеки extends объявляются на конструкторе ПриСозданииОбъекта. Раньше parentClassName/isInterface/ implementedInterfaceNames сканировали ВСЕ методы — вспомогательный метод с &Расширяет("X") (например, из другой библиотеки) давал ложное наследование. Теперь читаем аннотации только с метода-конструктора. Конструктор ищется по имени через Methods.isOscriptClassConstructorName (ПриСозданииОбъекта/ OnObjectCreate), а НЕ через SymbolTree.getConstructor(): ConstructorSymbol создаётся лишь для классифицированных OScript-классов, а наследование работает и для обычных .os-файлов (basename-резолв) — иначе ломается иерархия plain-.os. Заодно implementedInterfaceNames возвращает List.copyOf (иммутабельно, как и ранний выход List.of()). Тесты: аннотация на вспомогательном методе не считается наследованием/ реализацией. Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
force-pushed
the
claude/os-files-type-hierarchy-HzJ5n
branch
from
June 10, 2026 14:52
2781c78 to
d9dfb95
Compare
Добавлена поддержка LSP-запросов textDocument/prepareTypeHierarchy,
typeHierarchy/supertypes и typeHierarchy/subtypes для .os-файлов,
использующих библиотеку наследования nixel2007/extends.
Наследование определяется по аннотации &Расширяет("Родитель")
(и английскому псевдониму &Extends) над конструктором
ПриСозданииОбъекта. Имя родителя резолвится так же, как в
Новый Родитель: через каталог library-классов (OScriptLibraryIndex)
либо по basename файла.
https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
Класс-наследник (&Расширяет/&Extends) теперь получает экспортные члены родителя во всей системе типов: автодополнение после точки, hover, signature help, переход к определению, семантические токены и вывод типов. Реализовано через ленивый MemberSource на типе наследника, который тянет члены родителя из TypeRegistry.getMembers — тот же приём, что и специализация generic-типов. Это даёт «бесплатно»: - транзитивное наследование (родитель тянет своего родителя); - переопределение (собственные члены выигрывают дедупликацию по имени); - hot-reload смены &Расширяет (источник ленивый); - корректный go-to-definition (унаследованный член хранит sourceSymbol метода родителя). Добавлена защита от циклов наследования (ThreadLocal-гард). Извлечение родителя вынесено в общий util OScriptExtends (DRY с TypeHierarchyProvider). https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…ностей)
Распознавание родителя переключено с прямого имени &Расширяет на роль
"Расширяет" через AutumnMetaAnnotationResolver. Это покрывает мета-аннотации
фреймворка «ОСень»: класс, помеченный пользовательской аннотацией, чьё
определение несёт &Расширяет (как &ХранилищеСущностей в autumn-data),
наследует методы супер-класса (ПолучитьОдно, Получить, Сохранить и т.п.).
Прямое &Расширяет("X") по-прежнему работает (роль распознаётся и без
класса-определения), английский &Extends — через явный fallback.
Добавлен fixture по образцу autumn-data и тест AutumnDataInheritanceTest.
https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…тель) Поле, хранящее экземпляр родителя, типизируется родительским классом, что даёт автодополнение/hover по членам супер-класса при обращении к родителю: - явный держатель — поле модуля с аннотацией &Родитель (имя произвольное); - неявный — поле _ОбъектРодитель, которое библиотека extends создаёт в собранном объекте (в исходниках наследника не объявлено) — типизируется фолбэком в inferIdentifier. Тип выводится ТОЛЬКО если класс объявляет наследование (&Расширяет напрямую или через мета-аннотацию): источник типа — OScriptExtends.parentClassName, который пуст без наследования (покрыто негативным тестом). https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…ует)
Реализован textDocument/implementation (ранее возвращал пусто) для библиотеки
extends: интерфейс — класс с &Интерфейс, класс реализует его через &Реализует.
- курсор на экспортном методе интерфейса → одноимённые методы реализующих классов;
- курсор в файле-интерфейсе → сами реализующие классы.
Поддержан сложный случай из документации extends (комбинирование наследования
и интерфейсов): абстрактный родитель объявляет &Реализует("Интерфейс"), а
реализация — в наследнике. Реализация интерфейса определяется транзитивно по
цепочке &Расширяет, поэтому наследники абстрактного класса тоже считаются
реализациями.
Capability setImplementationProvider зарегистрирован. Детектирование интерфейса
и &Реализует учитывает мета-аннотации «ОСени» (через AutumnMetaAnnotationResolver).
https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
- fix(javadoc): экранировать & в примере autumn-data (doclint html) - refactor: общий OScriptClassResolver (classNames/resolveClassDocument/ isLibraryClass) вместо дублирования в TypeHierarchyProvider и ImplementationProvider (устранение дублей для Sonar) - test: OScriptExtendsTest, OScriptClassResolverTest, маршрутизация type hierarchy/implementation в BSLTextDocumentServiceTest, проверки capability в BSLLanguageServerTest, non-OS prepare https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…entationProvider - ImplementationProvider: сортировка локаций по URI (он уникален в результатах), убран мёртвый guard на пустые имена, упрощён methodNameAt (конструктор отсекается фильтром экспортности) — меньше мёртвого кода для Sonar. - Тесты: ветки non-OS/без-наследования/без-аргумента, basename-резолв, плоское поле (не родитель), null-prepare, базовый класс без наследуемых членов; маршрутизация в сервисе и capability в сервере. https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…solver
TypeHierarchyProvider больше не импортирует OScriptLibraryIndex (перешёл на
OScriptClassResolver), из-за чего {@link OScriptLibraryIndex} в javadoc
ломал сборку build-javadoc. Ссылка обновлена на OScriptClassResolver.
https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
…ng-тестов По ревью CodeRabbit: - OScriptClassResolver.resolveClassDocument: при совпадении basename у нескольких .os выбираем минимальный по URI (порядок getDocuments() не гарантирован) — детерминированный результат. - BSLTextDocumentServiceTest: routing-тесты type hierarchy/implementation теперь открывают связанные документы и проверяют непустой результат (super/subtypes и список реализаций), а не только non-null. https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
- ImplementationProvider.methodNameAt помечен @nullable: пакет @NullMarked, метод может вернуть null. Чинит S2637 (возврат null) и S2583 (проверка methodName != null больше не «всегда true»). - OScriptModuleMembersProvider: ThreadLocal INHERITANCE_IN_PROGRESS очищается через remove(), когда набор пуст — не держим пустой Set на пуловых потоках (S5164). - OScriptInheritanceMembersTest: assertion doesNotContain дополнен contains, чтобы не проходить вхолостую на пустом списке (S5841). https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
- OScriptExtends.parentClassName: вынесен parentFromAnnotations, логика на стримах — снижены cognitive complexity (S3776 17→<15) и вложенность (S134). - ImplementationProvider: цикл сведён к одному continue (S135). - OScriptModuleMembersProvider: static-поле INHERITANCE_IN_PROGRESS поднято к началу класса (S1213); java.util.Set → Set (S1942). - Тесты: многострочные фикстуры переведены на text blocks (S6126), убран неиспользуемый импорт java.util.List (S1128). https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
added a commit
that referenced
this pull request
Jun 11, 2026
- ссылка на библиотеку extends ведёт на github.com/nixel2007/extends (4 javadoc); - classNames: убран фолбэк для нерезолвящихся URI — .os-документы всегда иерархические file:-URI, basename обязан разрешаться (нерезолв — ошибка в коде); - тест переименован под фактическое поведение, убраны FQN в пользу импорта. Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
nixel2007
commented
Jun 11, 2026
…ана документов По ревью архитектуры: subtypes() и поиск реализаторов сканировали все документы workspace на каждый запрос. Теперь прямые отношения хранятся в TypeRelationIndex — ленивом обратном индексе «имя родителя → URI наследников» и «имя интерфейса → URI реализаторов» (только сырые имена, lowercase; разрешение имён и транзитивность — по-прежнему на момент запроса). Инкрементальная инвалидация: правка/добавление/ удаление .os-документа меняет вклад точечно, правка класса-определения аннотации, переиндексация библиотек и перепопуляция контекста сбрасывают индекс на ленивую пересборку. Прежний live-класс переименован в TypeRelations (он не хранилище): supertype/ inheritedMembers без изменений, subtypes() — лукап по индексу, implementsAny заменён на implementors(интерфейс) — поиск «вниз» (замыкание производных интерфейсов + поддеревья наследников реализаторов) без перебора кандидатов; ImplementationProvider больше не сканирует документы. Общий classSelectionRange вынесен в TypeRelations (дубль в двух провайдерах), мёртвый guard и прокси-метод в TypeHierarchyProvider убраны. Co-Authored-By: Claude Opus 4.8 <[email protected]>
- S1905: убран лишний каст — у ServerContextDocumentAddedEvent типизированный getSource; - S2211: явные типы параметров лямбд в TypeRelationIndex; - S3776/S1541: implementors разложен на interfaceClosureNames и expandImplementors; - S1213: статический компаратор выше инстанс-полей в TypeHierarchyProvider; - S2325: constructorAnnotations в OScriptExtends стал static; - S5976: четыре однотипных parentClassName-теста сведены в параметризованный; - S125×2: переформулированы русские комментарии, похожие на код; убран провенанс-комментарий про ревью; - S1128: удалён неиспользуемый импорт в TypeRelationsTest. Co-Authored-By: Claude Opus 4.8 <[email protected]>
… по ревью По ревью PR #4014: - ссылки в javadoc указывают на github.com/nixel2007/extends (канонический репозиторий) вместо oscript-library/extends — в OScriptExtends, ImplementationProvider, TypeHierarchyProvider, TypeRelations; - «опакового» → «непрозрачного» (несуществующее слово) в комментарии OScriptLibraryIndex.classNames. https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
- ссылка на библиотеку extends ведёт на github.com/nixel2007/extends (4 javadoc); - classNames: убран фолбэк для нерезолвящихся URI — .os-документы всегда иерархические file:-URI, basename обязан разрешаться (нерезолв — ошибка в коде); - тест переименован под фактическое поведение, убраны FQN в пользу импорта. Co-Authored-By: Claude Opus 4.8 <[email protected]>
Co-Authored-By: Claude Opus 4.8 <[email protected]>
nixel2007
force-pushed
the
claude/os-files-type-hierarchy-HzJ5n
branch
from
June 11, 2026 07:00
1e0218e to
d803fdc
Compare
nixel2007
commented
Jun 11, 2026
OScriptMetaAnnotationResolver не зависит от экосистемы ОСени: это движок пользовательских аннотаций autumn-library/annotations. Константы движка (&Аннотация, &ПсевдонимДля, Значение) выделены в класс-константник OScriptAnnotations, методы чтения аннотаций (find, stringParameter, isAnnotationDefinition) — методы компонента-резолвера. AutumnAnnotations остался константником имён аннотаций ОСени. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…tends Проверка «класс — определение аннотации» — ответственность резолвера мета-аннотаций; потребители обращаются к нему напрямую. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…actOScriptLazyIndex Барьер первичной сборки, инвалидация по OScriptLibraryIndexedEvent, точечная переиндексация на правку и сброс при изменении класса-определения аннотации — общие для autumn-индексов и TypeRelationIndex; теперь они наследуют одну базу. AbstractAutumnLibraryIndex оставил себе лишь источник пересборки из записей-классов библиотек. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…nds_ OScriptExtends, TypeRelations и TypeRelationIndex — поддержка библиотеки наследования extends; выделены в подпакет types.oscript.extends_ (extends — зарезервированное слово Java). Co-Authored-By: Claude Opus 4.8 <[email protected]>
Класс-константник по образцу AutumnAnnotations (имена аннотаций ОСени) и OScriptAnnotations (служебные имена движка аннотаций): &Расширяет, &Реализует, &Интерфейс, &Родитель и неявное поле _ОбъектРодитель — в ExtendsAnnotations. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…индекса Basename-фолбэк снесён: имя класса берётся из qualifiedName записей OScriptLibraryIndex, для незарегистрированного файла список пуст. TypeHierarchyProvider не строит элементы для незарегистрированных документов; фикстура type-hierarchy оформлена библиотекой с lib.config. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…bilities Вместо setImplementationProvider(Boolean.TRUE) — ImplementationRegistrationOptions по образцу соседних провайдеров. Co-Authored-By: Claude Opus 4.8 <[email protected]>
types.inferencer.annotations → types.oscript.annotations, types.inferencer.autumn → types.oscript.autumn: всё oscript-зависимое — в пакете oscript, библиотеко-специфичное — в его подпакетах (annotations, autumn, extends_); зависимости между пакетами прослеживаются по импортам. У каждого пакета package-info.java с javadoc и @NullMarked. Co-Authored-By: Claude Opus 4.8 <[email protected]>
…ionResolver Явные типы лямбд (S2211), константа вместо магического числа (S109), переформулированы комментарии, похожие на код (S125), убран импорт класса своего пакета (S1128). Co-Authored-By: Claude Opus 4.8 <[email protected]>
|
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
2 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 nixel2007/extends (autumn-library/extends) — как в системе типов, так и в LSP-фичах.
Возможности
textDocument/prepareTypeHierarchy,typeHierarchy/supertypes,typeHierarchy/subtypes) для.os-классов: супер- и подтипы по&Расширяет. Работает и для иерархии интерфейсов (интерфейс&Расширяетинтерфейс).prepareTypeHierarchyвозвращает пусто для файлов, не участвующих в иерархии.textDocument/implementation) для интерфейсов (&Интерфейс): на экспортном методе → одноимённые методы реализующих классов; в теле файла-интерфейса → сами классы. Транзитивно по двум измерениям:&Реализует, наследник&Расширяет);TypeRegistry.getMembers.&Родитель) и неявного (_ОбъектРодитель) — типом становится супер-класс (только если класс объявляет&Расширяет).&ХранилищеСущностейиз autumn-data), а не только прямой&Расширяет.Модель extends (аннотации)
&Расширяет("Имя")/&Extends("Имя")— наследование класса от класса либо интерфейса от интерфейса.&Реализует("Имя")— класс реализует интерфейс (повторяемая).&Интерфейс— маркер интерфейса.&Родитель/_ОбъектРодитель— держатель экземпляра родителя.Имя резолвится так же, как в
Новый Имя:qualifiedNameизlib.config(OScriptLibraryIndex) либо basename файла.Архитектура
Понятие наследования вынесено в отдельную сущность системы типов —
TypeRelationIndex: единая точка истины об отношениях&Расширяет/&Реализуети единственное место транзитивных обходов и защиты от циклов. Раньше обходы и cycle-guard были бы продублированы в трёх местах (унаследованные члены, иерархия типов, переход к реализациям) — теперь все три делегируют в индекс.TypeRegistryне изменён: унаследованные члены подмешиваются ленивымMemberSource, делегирующим индексу (хук-функция); override-дедуп — штатный first-wins вgetMembers.TypeRelationIndexзависит только отOScriptExtends(разбор аннотаций); разрешение имён в документы/типы передаётся функциями — граф бинов остаётся ацикличным.OScriptExtends(аннотации) иOScriptClassResolver(имена ↔ документы) — низкоуровневые адаптеры.Тесты
Покрыты: иерархия типов (корень/лист/супер-/подтипы), переход к реализациям (интерфейс, абстрактный класс, иерархия интерфейсов), наследование членов (транзитивно, override, отсечение не-экспортных), типизация явного и неявного поля-родителя, autumn-data, юнит-тесты
TypeRelationIndex(в т.ч. обрыв циклов в обоих измерениях), маршрутизация вBSLTextDocumentServiceи регистрация capabilities вBSLLanguageServer.https://claude.ai/code/session_0181QDW1r8NYKkWybSgcjyPv
Summary by CodeRabbit
New Features
Documentation
Tests
Summary by CodeRabbit
New Features
Documentation