-
Notifications
You must be signed in to change notification settings - Fork 121
Add support for textDocument/semanticTokens/range #3705
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the 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. Comment |
Implement the textDocument/semanticTokens/range endpoint which allows clients to request semantic tokens for a specific range of a document. This is useful for large documents where only the visible portion needs semantic tokens. Changes: - Add getSemanticTokensRange method to SemanticTokensProvider - Add semanticTokensRange method to BSLTextDocumentService - Enable range support in BSLLanguageServer by setting setRange(TRUE) - Add filterTokensByRange helper to filter tokens within a range - Add comprehensive tests for the new functionality Co-authored-by: nixel2007 <[email protected]>
Clarify that LSP ranges are end-exclusive in test comments. Co-authored-by: nixel2007 <[email protected]>
| * @param range Диапазон для фильтрации | ||
| * @return Отфильтрованный список токенов | ||
| */ | ||
| private static List<SemanticTokenEntry> filterTokensByRange(List<SemanticTokenEntry> entries, Range range) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot здесь могут быть миллионы токенов. Есть идеи по оптимизации?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added optimizations in f6f8d01:
- Parallel stream for large collections (>1000 tokens)
- Two-phase filtering: quick line-based pre-filter first (simple integer comparison), then detailed character checks only for boundary lines
This reduces complexity for typical viewport requests since most tokens can be eliminated by the fast line check alone.
- Use parallel stream for collections with >1000 tokens - Add quick line-based pre-filter (simple integer comparison) - Perform detailed character position checks only for boundary lines - Rename isTokenInRange to isTokenInRangeDetailed for clarity Co-authored-by: nixel2007 <[email protected]>
Replace magic number 1000 with PARALLEL_PROCESSING_THRESHOLD constant for better maintainability. Co-authored-by: nixel2007 <[email protected]>
Test Results 2 781 files ± 0 2 781 suites ±0 46m 24s ⏱️ - 2m 42s Results for commit bc85d81. ± Comparison against base commit 116269d. This pull request removes 8 and adds 11 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
|
@copilot поправь по новому внутреннему api после мержа девелопа в текущий бранч |
Update method to use toDeltaEncodedArray and toList helper methods introduced in the develop merge. Co-authored-by: nixel2007 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements support for the textDocument/semanticTokens/range LSP endpoint, allowing clients to request semantic tokens for specific document ranges rather than the entire document. This optimization is particularly beneficial for large files where only a visible portion needs syntax highlighting.
Key Changes
- Added
getSemanticTokensRange()method with optimized filtering for tokens within specified ranges - Implemented parallel processing for large token collections (>1000 tokens) to improve performance
- Added comprehensive test coverage with 7 test cases covering various range scenarios
- Enabled range support in server capabilities by changing
setRange(Boolean.FALSE)tosetRange(Boolean.TRUE)
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| SemanticTokensProvider.java | Added getSemanticTokensRange() method with optimized filterTokensByRange() helper that uses parallel streams for large collections and performs efficient boundary checking |
| BSLTextDocumentService.java | Added semanticTokensRange() endpoint handler following the established pattern with null-safety checks and document context refresh |
| BSLLanguageServer.java | Enabled range support in server capabilities registration |
| SemanticTokensProviderTest.java | Added 7 comprehensive tests covering multi-line ranges, single-line ranges, partial-line ranges, empty ranges, full document ranges, resultId absence, and SDBL query token filtering |
| var decoded = decode(tokens.getData()); | ||
|
|
||
| // Should contain tokens that overlap with the range (A; and Перем Б) | ||
| assertThat(decoded).isNotEmpty(); | ||
| assertThat(decoded).allMatch(t -> t.line == 1, | ||
| "All tokens should be from line 1"); | ||
| } |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test for partial line range filtering only verifies that non-empty tokens are returned and all are from the expected line, but it doesn't verify which specific tokens are included or excluded. Consider adding assertions similar to the single line test (lines 1489-1495) to verify that the expected tokens such as "А", "Перем", and "Б" are present within the range, and that tokens outside the range like the leading "Перем" at the start of the line are correctly excluded.
| // region Range tokens tests | ||
|
|
||
| @Test | ||
| void rangeTokens_returnsOnlyTokensInRange() { | ||
| // given - multi-line document | ||
| String bsl = """ | ||
| Перем А; | ||
| Перем Б; | ||
| Перем В; | ||
| Перем Г; | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range covering lines 1-2 (0-based, end-exclusive: from line 1 to line 3 means lines 1 and 2) | ||
| Range range = new Range(new Position(1, 0), new Position(3, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then | ||
| var decoded = decode(tokens.getData()); | ||
|
|
||
| // Should contain tokens only from lines 1-2 (the range is end-exclusive, so line 3 is excluded) | ||
| assertThat(decoded).allMatch(t -> t.line >= 1 && t.line <= 2, | ||
| "All tokens should be within the requested range (lines 1-2)"); | ||
|
|
||
| // Should not contain tokens from line 0 (before range) or line 3 (at or after range end) | ||
| assertThat(decoded).noneMatch(t -> t.line == 0 || t.line == 3, | ||
| "No tokens should be from lines outside the range"); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_singleLine() { | ||
| // given | ||
| String bsl = """ | ||
| Перем А; | ||
| Перем Б; | ||
| Перем В; | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range covering only line 1 | ||
| Range range = new Range(new Position(1, 0), new Position(2, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then | ||
| var decoded = decode(tokens.getData()); | ||
|
|
||
| // Should contain tokens only from line 1 | ||
| assertThat(decoded).allMatch(t -> t.line == 1, | ||
| "All tokens should be from line 1"); | ||
|
|
||
| var expected = List.of( | ||
| new ExpectedToken(1, 0, 5, SemanticTokenTypes.Keyword, "Перем"), | ||
| new ExpectedToken(1, 6, 1, SemanticTokenTypes.Variable, SemanticTokenModifiers.Definition, "Б"), | ||
| new ExpectedToken(1, 7, 1, SemanticTokenTypes.Operator, ";") | ||
| ); | ||
|
|
||
| assertContainsTokens(decoded, expected); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_partialLine() { | ||
| // given | ||
| String bsl = """ | ||
| Процедура Тест() | ||
| Перем А; Перем Б; | ||
| КонецПроцедуры | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range covering part of line 1 (middle section) | ||
| Range range = new Range(new Position(1, 8), new Position(1, 18)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then | ||
| var decoded = decode(tokens.getData()); | ||
|
|
||
| // Should contain tokens that overlap with the range (A; and Перем Б) | ||
| assertThat(decoded).isNotEmpty(); | ||
| assertThat(decoded).allMatch(t -> t.line == 1, | ||
| "All tokens should be from line 1"); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_emptyRangeReturnsNoTokens() { | ||
| // given | ||
| String bsl = """ | ||
| Перем А; | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range at end of document where no tokens exist | ||
| Range range = new Range(new Position(5, 0), new Position(10, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then | ||
| assertThat(tokens.getData()).isEmpty(); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_fullDocumentRange() { | ||
| // given | ||
| String bsl = """ | ||
| Перем А; | ||
| Перем Б; | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range covering entire document | ||
| Range range = new Range(new Position(0, 0), new Position(100, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens rangeTokens = provider.getSemanticTokensRange(documentContext, params); | ||
| SemanticTokens fullTokens = provider.getSemanticTokensFull( | ||
| documentContext, | ||
| new SemanticTokensParams(textDocumentIdentifier) | ||
| ); | ||
|
|
||
| // then - range tokens should contain same tokens as full request | ||
| assertThat(rangeTokens.getData()).isEqualTo(fullTokens.getData()); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_doesNotHaveResultId() { | ||
| // given - per LSP spec, range requests don't use resultId | ||
| String bsl = """ | ||
| Перем А; | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| Range range = new Range(new Position(0, 0), new Position(1, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then - resultId should be null for range requests | ||
| assertThat(tokens.getResultId()).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void rangeTokens_withSdblQuery() { | ||
| // given - test that SDBL tokens are also filtered correctly | ||
| String bsl = """ | ||
| Процедура Тест() | ||
| А = 1; | ||
| Запрос = "Выбрать * из Справочник.Контрагенты"; | ||
| Б = 2; | ||
| КонецПроцедуры | ||
| """; | ||
|
|
||
| DocumentContext documentContext = TestUtils.getDocumentContext(bsl); | ||
| referenceIndexFiller.fill(documentContext); | ||
| TextDocumentIdentifier textDocumentIdentifier = TestUtils.getTextDocumentIdentifier(documentContext.getUri()); | ||
|
|
||
| // Request range covering only line 2 (the query line) | ||
| Range range = new Range(new Position(2, 0), new Position(3, 0)); | ||
| var params = new SemanticTokensRangeParams(textDocumentIdentifier, range); | ||
|
|
||
| // when | ||
| SemanticTokens tokens = provider.getSemanticTokensRange(documentContext, params); | ||
|
|
||
| // then | ||
| var decoded = decode(tokens.getData()); | ||
|
|
||
| // Should contain tokens only from line 2 | ||
| assertThat(decoded).isNotEmpty(); | ||
| assertThat(decoded).allMatch(t -> t.line == 2, | ||
| "All tokens should be from line 2"); | ||
|
|
||
| // Should contain SDBL keyword tokens (Выбрать, из) as well as Namespace and Class | ||
| int keywordTypeIdx = legend.getTokenTypes().indexOf(SemanticTokenTypes.Keyword); | ||
| int namespaceTypeIdx = legend.getTokenTypes().indexOf(SemanticTokenTypes.Namespace); | ||
| int classTypeIdx = legend.getTokenTypes().indexOf(SemanticTokenTypes.Class); | ||
|
|
||
| assertThat(decoded).anyMatch(t -> t.type == keywordTypeIdx, "Should contain keyword tokens"); | ||
| assertThat(decoded).anyMatch(t -> t.type == namespaceTypeIdx, "Should contain namespace token for Справочник"); | ||
| assertThat(decoded).anyMatch(t -> t.type == classTypeIdx, "Should contain class token for Контрагенты"); | ||
| } | ||
|
|
||
| // endregion |
Copilot
AI
Dec 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider testing the parallel processing path explicitly. The current tests don't verify behavior when the number of tokens exceeds PARALLEL_PROCESSING_THRESHOLD (1000). While the parallel stream should produce the same results as a sequential stream for filtering operations, it would be valuable to have a test that generates a large document with more than 1000 tokens to ensure the parallel path works correctly and produces deterministic results.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot вместо проверок на contains сделай тест на точное соответствие decoded token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in bc85d81. All range tests now use assertTokensMatch for exact token verification instead of assertContainsTokens and partial assertions.
|
/buildJar |
|
✅ Собраны JAR-файлы для этого PR по команде Артефакт: 4979284676 Файлы внутри:
|
Replace assertContainsTokens and partial assertions with assertTokensMatch for exact token verification in all range token tests. Co-authored-by: nixel2007 <[email protected]>
getSemanticTokensRangemethod toSemanticTokensProvidersemanticTokensRangemethod toBSLTextDocumentServiceBSLLanguageServer.getSemanticTokensProvider()semanticTokensRangeSummary
This PR implements support for the
textDocument/semanticTokens/rangeLSP endpoint. This feature allows clients to request semantic tokens for a specific range of a document, which is particularly useful for large documents where only the visible portion needs highlighting.Changes
SemanticTokensProvider.java:
getSemanticTokensRange()method that filters tokens by the specified rangefilterTokensByRange()for large collections:PARALLEL_PROCESSING_THRESHOLD(1000)toDeltaEncodedArrayAPI after develop mergeBSLTextDocumentService.java:
semanticTokensRange()method to handle the LSP requestBSLLanguageServer.java:
setRange(Boolean.FALSE)tosetRange(Boolean.TRUE)to enable range supportSemanticTokensProviderTest.java:
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.