semanticTokens#3510
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds full-document semantic tokens: new SemanticTokensProvider and legend bean, registers semantic tokens capability in the server, exposes a TextDocument semanticTokensFull endpoint, extensive unit tests, documentation updates, and a small utility genericization and test removal. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server as BSLLanguageServer
participant TextDoc as BSLTextDocumentService
participant Provider as SemanticTokensProvider
participant Index as ReferenceIndex
Note over Server: Initialization
Client->>Server: initialize(params)
Server->>Server: build capabilities (includes SemanticTokensProvider registration)
Server-->>Client: InitializeResult
Note over Client,TextDoc: Token request flow
Client->>TextDoc: semanticTokensFull(SemanticTokensParams)
TextDoc->>Provider: getSemanticTokensFull(docContext, params)
Provider->>Index: resolve references (definitions/usages)
Provider->>Provider: collect symbols, comments, AST, lexical tokens
Provider->>Provider: merge multiline docs (if supported)
Provider->>Provider: sort, dedupe, delta-encode
Provider-->>TextDoc: SemanticTokens (delta-encoded)
TextDoc-->>Client: CompletableFuture<SemanticTokens>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Pull Request Overview
This PR implements semantic tokens support for BSL (1C BSL Language Server), providing enhanced syntax highlighting capabilities. The feature enables IDEs to display different token types with appropriate visual styling based on their semantic meaning.
- Adds comprehensive semantic tokens provider with support for keywords, strings, numbers, comments, functions, methods, variables, parameters, macros, decorators, operators, and namespaces
- Implements semantic tokens legend configuration defining the supported token types
- Integrates semantic tokens functionality into the BSL language server capabilities and text document service
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| SemanticTokensProviderTest.java | Comprehensive test suite validating semantic token emission for various BSL language constructs |
| SemanticTokensLegendConfiguration.java | Spring configuration defining the semantic tokens legend with supported token types |
| SemanticTokensProvider.java | Core provider implementation that analyzes BSL code and generates semantic tokens |
| BSLTextDocumentService.java | Integration of semantic tokens provider into the text document service |
| BSLLanguageServer.java | Registration of semantic tokens capability in server initialization |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
# Conflicts: # src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java # src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java # src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (3)
630-644: Off-by-one error in decode() loop condition.The loop condition
i + 4 < data.size()will skip the last valid token whendata.size()is exactly divisible by 5. Each token consists of 5 integers, and you need to access indicesithroughi + 4, so the condition should ensurei + 4is a valid index.Apply this diff to fix the loop condition:
- for (int i = 0; i + 4 < data.size(); i += 5) { + for (int i = 0; i + 5 <= data.size(); i += 5) {
647-653: Off-by-one error causes potential IndexOutOfBoundsException.The loop condition
i + 3 < data.size()is insufficient since you're accessingdata.get(i + 3)for the token type. When the loop is intended to process 5-element groups, this condition can allow an iteration wherei + 4(for modifiers) would be out of bounds, although currently onlyi + 3is accessed. For consistency with the semantic token format and to match the step size, usei + 5 <= data.size().- for (int i = 0; i + 3 < data.size(); i += 5) { + for (int i = 0; i + 5 <= data.size(); i += 5) {
656-661: Same off-by-one issue in countOfType().- for (int i = 0; i + 3 < data.size(); i += 5) { + for (int i = 0; i + 5 <= data.size(); i += 5) {
🧹 Nitpick comments (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java (1)
74-76: Generic return type looks good; ensure underlying ANTLR API matchesThe change to a generic return type
Collection<T>is a nice improvement for callers that know a more specificParseTreesubtype; it stays binary‑compatible and aligns with how this utility is used elsewhere.One thing to double‑check: this wrapper now assumes
org.antlr.v4.runtime.tree.Trees.findAllRuleNodesitself has a matching generic signature (<T extends ParseTree> Collection<T> ...). If your custom ANTLR fork still returnsCollection<ParseTree>, this method will no longer compile and would need either a cast or to keep the non‑generic return type. Based on learnings, this likely matches your forked ANTLR, but it’s worth verifying.Optionally, for API consistency, you might later consider whether the varargs/collection overloads of
findAllRuleNodesshould also be made generic in a similar fashion, though that’s not required for this PR.src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java (1)
329-343: Implementation follows the established pattern.The
semanticTokensFullmethod correctly follows the existing pattern used by other providers (null-check,withFreshDocumentContextwrapper).Minor: There are three consecutive blank lines (341-343) that could be reduced to one for consistency with the rest of the file.
return withFreshDocumentContext( documentContext, () -> semanticTokensProvider.getSemanticTokensFull(documentContext, params) ); } - - - @Override public CompletableFuture<List<CallHierarchyIncomingCall>> callHierarchyIncomingCalls(src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (2)
284-303: Consider extracting duplicate range-containment logic.The
Ranges.containsRange(r, commentRange)check is performed for every comment against every description range when multiline support is enabled. For documents with many comments and descriptions, this O(n×m) complexity could become a performance concern.Consider using a more efficient data structure (e.g., interval tree or pre-sorting ranges) if performance becomes an issue with large files. For now, the implementation is functionally correct.
492-501: Unused reference check result.The method checks
reference.isSourceDefinedSymbolReference()and then callsreference.getSourceDefinedSymbol(), but the result ofgetSourceDefinedSymbol()is only used to add a token - the actual symbol value is ignored. This works correctly but the pattern could be simplified.private void addMethodCallTokens(List<TokenEntry> entries, URI uri) { for (var reference : referenceIndex.getReferencesFrom(uri, SymbolKind.Method)) { - if (!reference.isSourceDefinedSymbolReference()) { - continue; - } - - reference.getSourceDefinedSymbol() - .ifPresent(symbol -> addRange(entries, reference.getSelectionRange(), SemanticTokenTypes.Method)); + if (reference.isSourceDefinedSymbolReference() && reference.getSourceDefinedSymbol().isPresent()) { + addRange(entries, reference.getSelectionRange(), SemanticTokenTypes.Method); + } } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
docs/en/index.md(1 hunks)docs/index.md(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java(7 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java(9 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SemanticTokensLegendConfiguration.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/package-info.java(1 hunks)src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.java(1 hunks)src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.md
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Keep documentation up to date with code changes
Files:
docs/en/index.mddocs/index.md
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SemanticTokensLegendConfiguration.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/Trees.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java
**/test/java/**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use appropriate test frameworks (JUnit, AssertJ, Mockito) for testing
Files:
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧠 Learnings (4)
📓 Common learnings
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
📚 Learning: 2025-11-19T09:02:03.154Z
Learnt from: theshadowco
Repo: 1c-syntax/bsl-language-server PR: 3610
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/folding/QueryPackageFoldingRangeSupplier.java:26-26
Timestamp: 2025-11-19T09:02:03.154Z
Learning: В проекте bsl-language-server используется кастомная версия ANTLR4 от 1c-syntax (io.github.1c-syntax:antlr4), которая включает базовый класс org.antlr.v4.runtime.Tokenizer с методами getAst() и getTokens(). SDBLTokenizer и BSLTokenizer наследуются от этого базового класса.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/package-info.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java
📚 Learning: 2025-02-10T17:12:56.150Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3408
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ProtectedModuleDiagnostic.java:63-66
Timestamp: 2025-02-10T17:12:56.150Z
Learning: In BSL Language Server, `documentContext.getServerContext().getConfiguration()` is guaranteed to return a non-null value, making null checks unnecessary.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
📚 Learning: 2025-01-19T20:47:40.061Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 3388
File: src/main/java/com/github/_1c_syntax/bsl/languageserver/configuration/codelens/TestRunnerAdapterOptions.java:46-46
Timestamp: 2025-01-19T20:47:40.061Z
Learning: Configuration classes in the BSL Language Server project use mutable collections (HashMap, ArrayList) and Data annotation from Lombok, allowing for modification of configuration properties after initialization.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java
🧬 Code graph analysis (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/events/LanguageServerInitializeRequestReceivedEvent.java (1)
LanguageServerInitializeRequestReceivedEvent(36-56)src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java (1)
Component(132-753)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, windows-latest)
🔇 Additional comments (19)
docs/en/index.md (1)
113-115: LGTM!Documentation accurately reflects the new semantic tokens capability. The table entries are consistent with existing format and correctly indicate that
semanticTokens/fullis supported withmultilineTokenSupport = true, while delta and range variants are not yet implemented.docs/index.md (1)
113-115: LGTM!Russian documentation correctly mirrors the English version and accurately documents the semantic tokens capabilities.
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/package-info.java (1)
1-33: LGTM!The package-info file follows project conventions with proper license header, descriptive Javadoc, and
@NullMarkedannotation for null-safety consistency.src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLTextDocumentService.java (1)
155-155: LGTM!The
semanticTokensProviderfield follows the existing dependency injection pattern used for other providers in this class.src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/SemanticTokensLegendConfiguration.java (1)
46-70: Configuration is well-structured and complete.All 12 semantic token types and both token modifiers declared in the legend are actively used throughout the
SemanticTokensProvider:
- Token types: Keyword, String, Number, Comment, Function, Method, Variable, Parameter, Macro, Decorator, Operator, Namespace — all present
- Token modifiers: Documentation and Definition — both utilized
The immutable lists via
List.of()are appropriate for configuration beans, and the token set comprehensively covers BSL language constructs.src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (3)
59-75: LGTM: Test class setup is well-structured.The test class properly uses Spring Boot testing with
@SpringBootTest,@DirtiesContext, and@MockitoBeanfor theReferenceIndex. The@BeforeEachinitialization ensures consistent test state by disabling multiline token support.
77-113: LGTM: Comprehensive token type emission test.The test covers a good range of token types including Decorator, Macro, Method, Parameter, Keyword, String, Number, Comment, and Operator using realistic BSL code snippets.
581-625: LGTM: Method call highlighting test with proper mocking.The test properly sets up mock references for same-file method calls and validates that Method tokens are emitted at call sites. The use of
RETURNS_DEEP_STUBSand proper mock configuration is appropriate.src/main/java/com/github/_1c_syntax/bsl/languageserver/BSLLanguageServer.java (4)
100-100: LGTM: SemanticTokensLegend injection follows existing patterns.The legend field is properly declared as a final dependency, following the same pattern as other injected components in this class. With
@RequiredArgsConstructor, Spring will inject theSemanticTokensLegendbean.
370-376: LGTM: Semantic tokens provider configuration is correct.The provider is configured with:
fullset toTRUEfor full document token supportrangeset toFALSEas range-based tokens are not implementedThis aligns with the
getSemanticTokensFullmethod inSemanticTokensProvider.
340-347: LGTM: Clean refactoring of rename prepare support check.The extraction of
hasRenamePrepareSupportinto a separate method improves readability while maintaining the same Optional-based null-safe navigation.
222-224: No null-safety issue—configuration chain is guaranteed safe.The method chain
configuration.getCapabilities().getTextDocumentSync().getChange()is null-safe by design. Each intermediate object is initialized with a default value and cannot be null:
CapabilitiesOptions.capabilitiesis initialized withnew CapabilitiesOptions()and has no setterTextDocumentSyncCapabilityOptions.textDocumentSyncis initialized withnew TextDocumentSyncCapabilityOptions()TextDocumentSyncKind.changeis initialized withDEFAULT_CHANGE(TextDocumentSyncKind.Incremental)src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (7)
85-87: LGTM: Provider class declaration is well-structured.The class is properly annotated with
@Componentand@RequiredArgsConstructor, following the project's conventions for Spring-managed providers.
156-165: LGTM: Client capability detection for multiline tokens.The event listener correctly extracts the
multilineTokenSupportcapability from client initialization parameters using null-safe Optional chaining.
174-210: LGTM: Main tokenization orchestration is well-organized.The
getSemanticTokensFullmethod follows a clear sequence:
- Symbol processing (methods, variables, parameters)
- Documentation handling
- Comment processing
- AST-based tokens (annotations, preprocessor)
- Method call references
- Lexical tokens
- Delta encoding
The flow is logical and maintains separation of concerns.
461-490: LGTM: Delta encoding implementation is correct.The encoding algorithm properly:
- Deduplicates entries using a HashSet
- Sorts by line then start position
- Calculates delta values correctly (resetting
prevCharto 0 on new lines)- Handles the first token specially
The use of
recordforTokenEntryensures correct equals/hashCode for deduplication.
311-340: LGTM: Annotation and compiler directive processing is correct.The AST traversal correctly:
- Merges
&with directive/annotation name into a single Decorator token- Handles annotation parameters as Parameter tokens
- Falls back gracefully when optional components are missing
342-396: LGTM: Preprocessor handling with correct token type differentiation.The implementation correctly:
- Treats regions (
#Область/#КонецОбласти) as Namespace tokens- Marks region names as Variable tokens
- Handles
#Использовать(use directives) as Namespace with library name as Variable- Processes other preprocessor directives as Macro tokens
- Avoids duplicate token emission by skipping region-containing preprocessors
503-555: LGTM: Lexical token processing with appropriate exclusions.The implementation correctly:
- Categorizes strings, numbers, operators, and keywords
- Skips
&and annotation tokens to avoid duplicates with AST-based processing- Uses symbolic name suffix matching for keyword detection
- Excludes
PREPROC_*keywords since they're handled via AST
| private void addRange(List<TokenEntry> entries, Range range, String type, String... modifiers) { | ||
| int explicitLength = Math.max(0, range.getEnd().getCharacter() - range.getStart().getCharacter()); | ||
| addRange(entries, range, explicitLength, type, modifiers); | ||
| } |
There was a problem hiding this comment.
Potential bug: Length calculation assumes single-line tokens.
The addRange method calculates length as range.getEnd().getCharacter() - range.getStart().getCharacter(), which is only correct for single-line tokens. For multi-line tokens, this calculation will yield incorrect or even negative lengths.
The overloaded method with explicitLength (line 438) handles multi-line correctly, but this convenience method is used in many places and could silently produce incorrect token lengths for any range spanning multiple lines.
Consider adding a guard or using the document text length for accurate multi-line calculation:
private void addRange(List<TokenEntry> entries, Range range, String type, String... modifiers) {
- int explicitLength = Math.max(0, range.getEnd().getCharacter() - range.getStart().getCharacter());
+ int startLine = range.getStart().getLine();
+ int endLine = range.getEnd().getLine();
+ int explicitLength;
+ if (startLine == endLine) {
+ explicitLength = Math.max(0, range.getEnd().getCharacter() - range.getStart().getCharacter());
+ } else {
+ // Multi-line tokens need explicit length from caller or should use document text
+ // For now, fall back to first line only - caller should use explicit length overload
+ explicitLength = 0;
+ }
addRange(entries, range, explicitLength, type, modifiers);
}Alternatively, verify that all callers of this method only pass single-line ranges.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private void addRange(List<TokenEntry> entries, Range range, String type, String... modifiers) { | |
| int explicitLength = Math.max(0, range.getEnd().getCharacter() - range.getStart().getCharacter()); | |
| addRange(entries, range, explicitLength, type, modifiers); | |
| } | |
| private void addRange(List<TokenEntry> entries, Range range, String type, String... modifiers) { | |
| int startLine = range.getStart().getLine(); | |
| int endLine = range.getEnd().getLine(); | |
| int explicitLength; | |
| if (startLine == endLine) { | |
| explicitLength = Math.max(0, range.getEnd().getCharacter() - range.getStart().getCharacter()); | |
| } else { | |
| // Multi-line tokens need explicit length from caller or should use document text | |
| // For now, fall back to first line only - caller should use explicit length overload | |
| explicitLength = 0; | |
| } | |
| addRange(entries, range, explicitLength, type, modifiers); | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
around lines 432-435, the convenience addRange method computes length as
range.getEnd().getCharacter() - range.getStart().getCharacter(), which is only
valid for single-line ranges; update it to detect multi-line ranges
(range.getStart().getLine() != range.getEnd().getLine()) and in that case
compute the accurate length by retrieving the document text for the range (e.g.
document.getText(range)) and using its length when calling the overloaded
addRange(entries, range, explicitLength, ...); if the document is not available
in this context, throw an IllegalArgumentException or require callers to pass
explicitLength for any multi-line ranges so incorrect negative/short lengths
cannot be produced.
| import java.util.concurrent.CompletableFuture; | ||
| import java.util.concurrent.CountDownLatch; | ||
| import java.util.concurrent.Executors; | ||
| import java.util.concurrent.TimeUnit; |
There was a problem hiding this comment.
Unused import java.util.concurrent.Executors should be removed. It's no longer needed after the test removal.
…rs/SemanticTokensProviderTest.java Co-authored-by: Copilot <[email protected]>
…rs/SemanticTokensProviderTest.java Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (3)
581-625: LGTM!The test correctly validates that method call sites are highlighted as Method tokens when resolved via the reference index. The mock setup appropriately simulates same-file method resolution.
As a minor optimization, line 595 could avoid splitting the entire BSL string:
- int callStart = bsl.split("\n")[callLine].indexOf("CallMe"); + String[] lines = bsl.split("\n"); + int callStart = lines[callLine].indexOf("CallMe");However, this is purely a micro-optimization and not necessary for test code.
647-662: Use consistent loop condition for semantic token tuples.While the current conditions (
i + 3 < data.size()) are technically safe since these methods only accessdata.get(i + 3), they differ from the convention established indecode()at line 634. For consistency and to ensure complete 5-element tuples are validated, usei + 4 < data.size()in both loops.Apply this diff:
private Set<Integer> indexesOfTypes(List<Integer> data) { // data: [deltaLine, deltaStart, length, tokenType, tokenModifiers] per token Set<Integer> res = new HashSet<>(); - for (int i = 0; i + 3 < data.size(); i += 5) { + for (int i = 0; i + 4 < data.size(); i += 5) { res.add(data.get(i + 3)); } return res; } private long countOfType(List<Integer> data, int typeIdx) { long cnt = 0; - for (int i = 0; i + 3 < data.size(); i += 5) { + for (int i = 0; i + 4 < data.size(); i += 5) { if (data.get(i + 3) == typeIdx) cnt++; } return cnt; }
628-668: Consider making helper methods static.The helper methods (
decode,indexesOfTypes,countOfType,assertPresent) and theDecodedTokenrecord don't access instance state (exceptlegendinassertPresent). For clarity and to signal they're pure utility functions, consider making them static where possible, or extract them to a separate test utility class.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.java: Follow the Style Guide provided in docs/en/contributing/StyleGuide.md
Use Lombok annotations to reduce boilerplate code and enable annotation processing in your IDE
Optimize imports before committing but do NOT optimize imports across the entire project unless specifically working on that task
Follow Java naming conventions with meaningful, descriptive names; keep class and method names concise but clear
Write JavaDoc for public APIs and include comments for complex logic
Use Target Java 17 as the language version
Files:
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
**/test/java/**/*.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use appropriate test frameworks (JUnit, AssertJ, Mockito) for testing
Files:
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java
🧬 Code graph analysis (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/util/TestUtils.java (1)
TestUtils(37-80)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: Analyse
🔇 Additional comments (10)
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java (10)
1-57: LGTM!The license header, package declaration, and imports are well-organized and appropriate for a Spring Boot integration test with Mockito mocking.
59-75: LGTM!The test class setup properly uses Spring Boot test context with appropriate isolation via
@DirtiesContext, and the@BeforeEachhook correctly resets the multiline token support state before each test.
77-113: LGTM!The test correctly verifies emission of expected token types across common BSL constructs. The Cyrillic text in the test data (lines 81, 86) is properly encoded—past review comments about corrupted characters do not apply to the current code.
115-176: LGTM!The test comprehensively validates preprocessor token handling, including the special-case logic for region directives. The calculation accounting for region directive overhead (lines 146-163) correctly distinguishes between macro tokens and namespace tokens.
178-233: LGTM!The test properly validates the 1:1 mapping between lexer operator/punctuator tokens and semantic Operator tokens. The comprehensive set of operator types (lines 200-222) ensures thorough coverage.
235-339: LGTM!The three annotation tests comprehensively cover different annotation patterns: parameter-less decorators, decorators with string parameters, and custom annotations with named parameters. The verification logic correctly validates token types and counts for each scenario.
341-402: LGTM!Both tests properly validate their respective token scenarios. The
useDirective_isNamespacetest confirms namespace token emission for import directives, while the datetime/literal test correctly verifies that special literals (UNDEFINED, TRUE, FALSE) are highlighted as keywords.
404-515: LGTM!The documentation modifier tests thoroughly validate that documentation comments (leading, trailing, and multiline) are correctly marked with the documentation modifier, while non-documentation comments are not. The multiline merging test correctly toggles the feature flag and verifies single-token emission for consecutive documentation lines.
517-579: LGTM!Both tests correctly validate their respective scenarios: region names are properly highlighted as variables within namespace contexts, and variable declarations correctly receive the definition modifier.
634-634: Loop condition is correct.The condition
i + 4 < data.size()correctly ensures all 5 elements of each semantic token tuple (deltaLine, deltaStart, length, tokenType, tokenModifiers) are accessible before processing. The past review comment suggesting this would skip the last token is incorrect.
|
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
New Features
Documentation
Tests
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.