Skip to content

Conversation

@theshadowco
Copy link
Member

@theshadowco theshadowco commented Jan 16, 2026

Описание

Связанные задачи

Closes

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Summary by CodeRabbit

  • Bug Fixes

    • Fixed token processing for multiline descriptions to correctly align with file line positions.
  • Refactor

    • Improved code structure for preprocessor directive handling.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 16, 2026

📝 Walkthrough

Walkthrough

Three semantic tokens providers receive refinements: parameter formatting is adjusted, a line indexing bug is corrected in multiline description processing to use actual file line numbers, and preprocessor token handling is extracted into a dedicated method while preserving existing behavior.

Changes

Cohort / File(s) Summary
Formatting
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
Method signature parameters reformatted across multiple lines; no semantic or logic changes.
Bug Fix
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
Line indexing corrected: elementsByLine lookups now use fileLine instead of lineIdx for accurate multiline token positioning.
Refactoring
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
Preprocessor token processing logic extracted into dedicated addOtherPreprocs(List, PreprocessorContext) method; behavior preserved.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • nixel2007

Poem

🐰 A hop, a fix, a format too,
Line numbers align as they should do,
Methods extracted, clean and bright,
Semantic tokens shine with light! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Исправления замечаний сонара' (Sonar findings fixes) is specific and directly related to code quality improvements, aligning with the refactoring and formatting changes in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings


📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbf9bc9 and 47117b5.

📒 Files selected for processing (3)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/main/java/**/*.java: Follow the Style Guide for code formatting and conventions
Use Lombok annotations to reduce boilerplate code and enable annotation processing in IDE
Write JavaDoc for public APIs and include comments for complex logic
Use meaningful, descriptive names for classes and methods following Java naming conventions
Optimize imports before committing; DO NOT optimize imports across the entire project unless specifically working on that task

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
src/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Java 17 for language and follow Java naming conventions

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.
📚 Learning: 2025-12-28T11:15:32.357Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.

Applied to files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
⏰ 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). (1)
  • GitHub Check: check-pr-exists
🔇 Additional comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (1)

395-399: LGTM!

Clean formatting improvement for readability. Multi-line parameter lists are easier to scan when there are many parameters.

src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java (2)

105-111: LGTM!

Clean delegation to the extracted helper method. The added parentheses on line 105 improve readability of the compound condition.


114-141: Good refactoring - method extraction improves modularity.

The extracted addOtherPreprocs(entries, preprocessor) method cleanly encapsulates per-preprocessor token handling. The logic for combining HASH with the first PREPROC_ keyword into a single Macro token is preserved, and the method is well-documented with inline comments explaining the different cases.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@theshadowco theshadowco enabled auto-merge (squash) January 16, 2026 13:27
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java (1)

161-172: Bug: fileLine becomes stale in the inner loop.

Line 161 correctly uses fileLine, but line 167 also uses fileLine inside an inner while loop that increments lineIdx. Since fileLine is computed once at line 157 (fileStartLine + lineIdx), it doesn't update as lineIdx advances, causing the lookup to repeatedly check the wrong line.

🐛 Proposed fix
       while (lineIdx < lines.length) {
-        var nextLineElements = elementsByLine.getOrDefault(fileLine, List.of());
+        var nextLineElements = elementsByLine.getOrDefault(fileStartLine + lineIdx, List.of());
         if (!nextLineElements.isEmpty()) {
           break;
         }
         lineIdx++;
       }
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cbf9bc9 and 47117b5.

📒 Files selected for processing (3)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
🧰 Additional context used
📓 Path-based instructions (2)
src/main/java/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

src/main/java/**/*.java: Follow the Style Guide for code formatting and conventions
Use Lombok annotations to reduce boilerplate code and enable annotation processing in IDE
Write JavaDoc for public APIs and include comments for complex logic
Use meaningful, descriptive names for classes and methods following Java naming conventions
Optimize imports before committing; DO NOT optimize imports across the entire project unless specifically working on that task

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
src/**/*.java

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use Java 17 for language and follow Java naming conventions

Files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.
📚 Learning: 2025-12-28T11:15:32.357Z
Learnt from: nixel2007
Repo: 1c-syntax/bsl-language-server PR: 0
File: :0-0
Timestamp: 2025-12-28T11:15:32.357Z
Learning: In PR `#3706` about semantic token optimization, I incorrectly stated that the algorithm in findSuffixMatchWithOffset correctly handles inline text insertions (text insertion without line breaks). The actual bug is that when lineOffset=0, tokens after the insertion point have the same deltaLine but different deltaStart, and the old algorithm failed to match them because it checked deltaStart equality. PR `#3707` fixes this by allowing deltaStart to differ for the boundary token when lineOffset==0.

Applied to files:

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/BslDocSemanticTokensSupplier.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java
⏰ 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). (1)
  • GitHub Check: check-pr-exists
🔇 Additional comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SemanticTokensProvider.java (1)

395-399: LGTM!

Clean formatting improvement for readability. Multi-line parameter lists are easier to scan when there are many parameters.

src/main/java/com/github/_1c_syntax/bsl/languageserver/semantictokens/PreprocessorSemanticTokensSupplier.java (2)

105-111: LGTM!

Clean delegation to the extracted helper method. The added parentheses on line 105 improve readability of the compound condition.


114-141: Good refactoring - method extraction improves modularity.

The extracted addOtherPreprocs(entries, preprocessor) method cleanly encapsulates per-preprocessor token handling. The logic for combining HASH with the first PREPROC_ keyword into a single Macro token is preserved, and the method is well-documented with inline comments explaining the different cases.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.

@nixel2007 nixel2007 disabled auto-merge January 16, 2026 13:51
@nixel2007 nixel2007 merged commit 65f3d0a into develop Jan 16, 2026
36 checks passed
@nixel2007 nixel2007 deleted the feature/fixes260116 branch January 16, 2026 13:52
@sonarqubecloud
Copy link

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