Skip to content

Mark parameter usages with SemanticTokenTypes.Parameter#3674

Merged
nixel2007 merged 13 commits intodevelopfrom
copilot/add-semantic-token-type-parameter
Dec 17, 2025
Merged

Mark parameter usages with SemanticTokenTypes.Parameter#3674
nixel2007 merged 13 commits intodevelopfrom
copilot/add-semantic-token-type-parameter

Conversation

Copy link
Contributor

Copilot AI commented Dec 17, 2025

Implementation

The implementation now marks parameters with SemanticTokenTypes.Parameter instead of SemanticTokenTypes.Variable using a combined approach adapted from the develop branch pattern:

Variable definitions (from SymbolTree):

  • Processed from symbolTree.getVariables() (excluding parameters)
  • Use referenceResolver.findReference() to determine if definition or usage
  • Marked with SemanticTokenTypes.Variable and Definition modifier when appropriate

Parameter declarations in method signatures:

  • Handled by addMethodSymbols method
  • Marked with SemanticTokenTypes.Parameter and SemanticTokenModifiers.Definition
  • Ensures parameter declarations are always highlighted

All variable and parameter usages (from ReferenceIndex):

  • Handled by addVariableSymbols via referenceIndex.getReferencesFrom()
  • Check VariableKind.PARAMETER to emit correct token type
  • Marked with SemanticTokenTypes.Parameter or SemanticTokenTypes.Variable
  • Include Definition modifier when OccurrenceType.DEFINITION

Testing

All tests in SemanticTokensProviderTest now pass:

  • Removed @MockitoBean annotation and all Mockito dependencies
  • Added @CleanupContextBeforeClassAndAfterEachTestMethod annotation for proper test isolation
  • All tests call referenceIndexFiller.fill(documentContext) after creating DocumentContext
  • Tests verify parameter and variable token types with real ReferenceIndex data
  • parameterAndVariableUsages test comprehensively verifies both parameter and variable declarations and usages in expressions

The combined approach (SymbolTree for variable definitions + ReferenceIndex for all usages) ensures complete coverage without duplication or conflicts. Parameters are filtered out from SymbolTree processing to avoid conflicts with parameter handling in addMethodSymbols.

Original prompt

Места использования переменной с типом Parameter тоже должны иметь тип SematicTokenType.Parameter


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 17, 2025

Important

Review skipped

Bot user detected.

To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Note

Other AI code review bot(s) detected

CodeRabbit 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 @coderabbitai help to get the list of available commands and usage tips.

Copilot AI and others added 2 commits December 17, 2025 08:09
- Modified addVariableSymbols to use SemanticTokenTypes.Parameter for variables with VariableKind.PARAMETER
- Added addParameterReferenceTokens to handle parameter references from ReferenceIndex
- Removed duplicate parameter token creation from addMethodSymbols
- Added test for parameter definition semantic token type

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
- Use pattern matching for instanceof in addParameterReferenceTokens
- Extract getSemanticTokenType helper method to reduce duplication

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copilot AI changed the title [WIP] Add semantic token type for parameter usage Mark parameter usages with SemanticTokenTypes.Parameter Dec 17, 2025
Copilot AI requested a review from nixel2007 December 17, 2025 08:19
@github-actions
Copy link
Contributor

github-actions bot commented Dec 17, 2025

Test Results

0 files   - 2 700  0 suites   - 2 700   0s ⏱️ - 48m 7s
0 tests  - 1 011  0 ✅  - 1 011  0 💤 ±0  0 ❌ ±0 
0 runs   - 9 099  0 ✅  - 9 099  0 💤 ±0  0 ❌ ±0 

Results for commit 0d987a7. ± Comparison against base commit 321bdf9.

♻️ This comment has been updated with latest results.

Replace weak test with comprehensive test that verifies:
- Parameter declarations in method signature
- Local variable declarations (explicit with Перем)
- Implicit variable declarations (first assignment)
- For loop variable declarations

The test now properly checks that parameters get Parameter token type
and variables get Variable token type for all declaration types.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copilot AI and others added 2 commits December 17, 2025 09:08
- Restore comment "// 3.1) Method call occurrences as Method tokens"
- Remove single-use helper function getSemanticTokenType() and inline it
- Use stream API in addVariableSymbols
- Unify variable and parameter token handling via reference index
- Keep fallback to symbol tree when reference index is empty (for tests)

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Extended test to verify:
- Parameter declarations (Парам1, Парам2) get Parameter token type
- Explicit variable declarations (Перем) get Variable token type
- Implicit variable declarations (first assignment) get Variable token type
- For loop variables get Variable token type
- Total count verification

Note: In test environment with mocked ReferenceIndex, only declarations
are verified. Actual usage tracking requires real ReferenceIndex which
works in production but not in this test setup.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Created SemanticTokensProviderIntegrationTest that uses real ReferenceIndex
(not mocked) to properly test both parameter and variable declarations AND usages.

The test verifies:
- Parameter declarations in method signature get Parameter token type
- Parameter usages in expressions get Parameter token type
- Variable declarations get Variable token type
- Variable usages get Variable token type
- Loop variables get Variable token type

Updated addVariableSymbols to always process both ReferenceIndex (for usages)
and VariableSymbol tree (for declarations), ensuring complete coverage.
TokenEntry deduplication via HashSet handles any overlaps.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
…fier

- Restored parameter token generation in addMethodSymbols
- Parameters in method signatures are now marked with SemanticTokenTypes.Parameter
  and SemanticTokenModifiers.Definition
- This ensures parameter declarations are highlighted even when ReferenceIndex
  doesn't include them

Addresses review feedback to restore deleted parameter processing from addMethodSymbols.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
@nixel2007
Copy link
Member

@copilot А теперь почини тесты. ReferenceIndex не должен быть моком.

- Removed @MockitoBean annotation for ReferenceIndex
- Removed all Mockito imports
- Added @CleanupContextBeforeClassAndAfterEachTestMethod annotation
- Added referenceIndexFiller.fill() to all test methods that create DocumentContext
- Simplified sameFileMethodCall test to work with real ReferenceIndex
- Now all tests use real ReferenceIndexFiller instead of mocks

Note: 3 tests still failing - investigating why variable declarations aren't
being found in ReferenceIndex for some test cases.

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot А теперь почини тесты. ReferenceIndex не должен быть моком.

Убрал мок ReferenceIndex. Все тесты теперь используют реальный ReferenceIndexFiller. Добавил referenceIndexFiller.fill(documentContext) во все тесты. Commit ea0f63c

Остались 3 падающих теста - они ожидают токены для объявлений переменных, которые не находятся в ReferenceIndex. Возможно, ReferenceIndexFiller не индексирует все объявления переменных для тестовых случаев без полной структуры метода.

@nixel2007
Copy link
Member

@copilot Посмотри как reference index используется в других тестах проекта и какие в них есть процедуры инициализации

Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot Посмотри как reference index используется в других тестах проекта и какие в них есть процедуры инициализации

Изучил использование ReferenceIndex в других тестах проекта. Обнаружил:

  1. ReferenceIndexFillerTest и ReferenceIndexTest используют тот же паттерн: @SpringBootTest + @CleanupContextBeforeClassAndAfterEachTestMethod + referenceIndexFiller.fill(documentContext)

  2. Мой код уже следует этому паттерну - я добавил @CleanupContextBeforeClassAndAfterEachTestMethod, убрал mock и добавил referenceIndexFiller.fill() во все тесты

  3. ReferenceIndexTest дополнительно использует @PostConstruct для вызова serverContext.populateContext(), но это нужно только когда тест использует файлы из metadata (конфигурация проекта)

  4. Тесты падают потому, что ожидают токены для объявлений переменных, но эти объявления не находятся через ReferenceIndex. Это может быть ограничением ReferenceIndexFiller - возможно, он не индексирует модульные переменные или переменные в определённых контекстах.

Мой код теперь соответствует паттернам других тестов в проекте. Возможно, падающие тесты выявляют реальное ограничение ReferenceIndexFiller, и их нужно либо адаптировать под это ограничение, либо исправить ReferenceIndexFiller.

@nixel2007
Copy link
Member

@copilot А в documentContext().getSymbolTree().getVariables() есть нужные тебе символы переменных? Если да, то давай тогда делать комбинированное решение, но без копипаста и конфликта определений параметров.

…eIndex for all usages

Adapted implementation to match develop branch pattern:
- Use symbolTree.getVariables() for variable definitions with referenceResolver check
- Filter out parameters from symbolTree processing (handled in addMethodSymbols)
- Use ReferenceIndex for all variable and parameter usages
- Check VariableKind to emit Parameter vs Variable token type
- Parameters in method signatures get Definition modifier (in addMethodSymbols)
- No code duplication, no conflicts between parameter sources

This combined approach ensures:
- Variables from SymbolTree are properly highlighted (fixes failing tests)
- Parameter usages from ReferenceIndex get Parameter token type
- Variable usages from ReferenceIndex get Variable token type
- All tokens have correct Definition modifier when appropriate

Co-authored-by: nixel2007 <1132840+nixel2007@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Dec 17, 2025

@copilot А в documentContext().getSymbolTree().getVariables() есть нужные тебе символы переменных? Если да, то давай тогда делать комбин�...

Да, в documentContext.getSymbolTree().getVariables() есть нужные символы переменных. Сделал комбинированное решение по паттерну из develop ветки:

  1. SymbolTree для определений переменных (кроме параметров) с проверкой через referenceResolver
  2. ReferenceIndex для всех использований переменных и параметров с проверкой VariableKind
  3. addMethodSymbols для определений параметров с модификатором Definition

Нет дублирования и конфликтов - параметры отфильтровываются из обработки SymbolTree. Все тесты проходят. Commit d0f3704

@nixel2007
Copy link
Member

/buildJar

@github-actions
Copy link
Contributor

✅ Собраны JAR-файлы для этого PR по команде /buildJar.

Артефакт: 4898936778

Файлы внутри:

  • bsl-language-server-copilot-add-semantic-token-type-parameter-d0f3704-exec.jar

@nixel2007 nixel2007 marked this pull request as ready for review December 17, 2025 13:49
Copilot AI review requested due to automatic review settings December 17, 2025 13:49
Copy link
Contributor

Copilot AI left a 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 pull request implements semantic token differentiation between parameters and variables in the BSL Language Server. Parameters are now correctly marked with SemanticTokenTypes.Parameter instead of SemanticTokenTypes.Variable, improving syntax highlighting accuracy for parameters in both declarations and usages.

Key changes:

  • Modified SemanticTokensProvider to distinguish parameters from variables using VariableKind.PARAMETER
  • Refactored test suite to remove Mockito dependencies and use real ReferenceIndex data
  • Added comprehensive test coverage for parameter and variable token type verification

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java Implements dual-path token processing: filters parameters from SymbolTree processing, adds ReferenceIndex-based processing for all variable/parameter usages with correct token types, and adds Definition modifier to parameter declarations
src/test/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProviderTest.java Removes Mockito mocking in favor of real ReferenceIndexFiller, adds referenceIndexFiller.fill() calls to all tests, and introduces two comprehensive test methods validating parameter vs variable token types
src/test/resources/providers/SemanticTokensProviderParameterTest.bsl New test fixture with BSL code containing parameters and variables in various contexts (declarations, usages in expressions, conditions, and loops)

…rs/SemanticTokensProviderTest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@nixel2007 nixel2007 enabled auto-merge (squash) December 17, 2025 14:00
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@sonarqubecloud
Copy link

@nixel2007 nixel2007 merged commit 8ceec7b into develop Dec 17, 2025
41 checks passed
@nixel2007 nixel2007 deleted the copilot/add-semantic-token-type-parameter branch December 17, 2025 14:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants