В ховер добавлена информация о месте объявления переменной #3730
Conversation
📝 WalkthroughWalkthroughThe changes add localized variable type information to variable hover content. A new helper method in VariableSymbolMarkupContentBuilder maps variable kinds to resource strings, with corresponding English and Russian localization files. Tests are updated to verify the new variable info section appears in generated hover markup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~14 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (1)
57-226: Consider adding test coverage for GLOBAL, PARAMETER, and DYNAMIC variable kinds.The existing tests cover MODULE and LOCAL variables well. To ensure comprehensive coverage of the new
getVariableInfologic, consider adding tests for:
- GLOBAL variables
- METHOD parameters (PARAMETER kind)
- DYNAMIC variables at both module and method levels
Example test structure
@Test void testMethodParameterVariable() { // Test PARAMETER kind with "Параметр метода %s" format } @Test void testDynamicVariableOfModule() { // Test DYNAMIC kind at module scope } @Test void testDynamicVariableOfMethod() { // Test DYNAMIC kind at method scope with "Динамическая переменная метода %s" format }src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.properties (1)
8-8: Optional: Remove trailing blank line for consistency.The Russian localization file (_ru.properties) doesn't have this trailing blank line. Consider removing it for consistency between the two resource files.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.propertiessrc/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.propertiessrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java
🧰 Additional context used
📓 Path-based instructions (3)
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/hover/VariableSymbolMarkupContentBuilder.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/hover/VariableSymbolMarkupContentBuilder.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java
src/test/java/**/*Test.java
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Maintain or improve test coverage and use appropriate test frameworks (JUnit, AssertJ, Mockito)
Files:
src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T18:49:48.712Z
Learning: Applies to docs/{en/,}diagnostics/**/*.md : Update diagnostic documentation in both Russian and English with examples of problematic code and fixes
📚 Learning: 2025-12-18T18:49:48.712Z
Learnt from: CR
Repo: 1c-syntax/bsl-language-server PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-12-18T18:49:48.712Z
Learning: Applies to src/main/resources/**/diagnostics/**/*.properties : Resource bundles must have both Russian and English versions for diagnostics
Applied to files:
src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.properties
📚 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/hover/VariableSymbolMarkupContentBuilder.java
📚 Learning: 2025-01-19T21:34:39.797Z
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-19T21:34:39.797Z
Learning: In BSL Language Server configuration classes, immutable collections (Set.of, List.of) should be used for default values, while mutability is achieved through setters generated by Data annotation.
Applied to files:
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.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/hover/VariableSymbolMarkupContentBuilder.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). (14)
- GitHub Check: Benchmark
- GitHub Check: Analyse
- GitHub Check: CodeQL analysis (java)
- GitHub Check: Agent
- GitHub Check: build
- GitHub Check: build (25, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (25, windows-latest)
- GitHub Check: build (25, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (21, ubuntu-latest)
🔇 Additional comments (11)
src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.properties (1)
1-7: LGTM! Russian localization properly structured.The Russian localization keys are well-defined with appropriate format placeholders (%s) for method names. The corresponding English version is also present, satisfying the bilingual resource bundle requirement.
src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java (4)
27-27: LGTM! Resources dependency properly added.The
Resourcesfield is correctly added and will be auto-injected via the existing@RequiredArgsConstructorannotation.Also applies to: 45-45
60-62: LGTM! Variable info section cleanly integrated.The new variable information section is appropriately positioned between the signature and location, with proper formatting using the
addSectionIfNotEmptyhelper.
100-102: LGTM! Clean delegation to Resources utility.The helper method provides a clean abstraction for resource string retrieval.
88-98: No action needed. The switch expression is safe as written:symbol.getScope()is guaranteed to return a non-null value for LOCAL, PARAMETER, and DYNAMIC variable kinds based on how VariableSymbols are constructed inVariableSymbolComputer. The scope field is always set during builder construction, andgetVariableScope()has safeguards to prevent null returns.src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java (5)
71-86: LGTM! Test properly updated for new content structure.The test correctly accounts for the additional variable info block, with updated assertions for block count and content validation.
102-121: LGTM! Test assertions correctly adjusted.Block indices properly shifted to accommodate the new variable info section, with accurate regex patterns for file links.
138-159: LGTM! Local variable info correctly validated.The test properly verifies "Локальная переменная метода ИмяФункции" content for method-scoped variables.
176-196: LGTM! Consistent test updates maintained.Block structure and assertions properly aligned with the new hover content format.
214-226: LGTM! Object module variable test updated correctly.The test validates module-level variable info from metadata-based object modules.
src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.properties (1)
1-7: LGTM! English localization properly structured.The English translations are accurate and correctly mirror the Russian resource file structure with appropriate format placeholders for method names.
There was a problem hiding this comment.
Pull request overview
This PR enhances the hover information for variables by adding details about the variable's kind and scope (e.g., "Module-level variable", "Local variable of method MethodName"). The feature supports internationalization with both Russian and English translations.
Key Changes:
- Added localized variable kind descriptions to hover tooltips
- Implemented switch expression to categorize variables by their kind (GLOBAL, MODULE, LOCAL, PARAMETER, DYNAMIC)
- Updated all existing tests to accommodate the new information block in hover content
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/main/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder.java | Implements the getVariableInfo() method to generate localized variable kind descriptions using a switch expression on VariableKind enum |
| src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_ru.properties | Adds Russian translations for all variable kinds (global, module, local, parameter, dynamic) |
| src/main/resources/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilder_en.properties | Adds English translations for all variable kinds (global, module, local, parameter, dynamic) |
| src/test/java/com/github/_1c_syntax/bsl/languageserver/hover/VariableSymbolMarkupContentBuilderTest.java | Updates test assertions to expect 3-4 blocks instead of 2-3 blocks, adding assertions for the new variable info block between signature and location |
| methodParameter=Parameter of method %s | ||
| dynamicVariableOfModule=Dynamic variable of module | ||
| dynamicVariableOfMethod=Dynamic variable of method %s | ||
|
|
There was a problem hiding this comment.
The English properties file has an extra blank line at the end (line 8), while the Russian properties file ends at line 7. For consistency with other resource files in the project, both files should have the same format. Consider removing the extra blank line from the English file or adding it to the Russian file to maintain consistency.
|
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
New Features
Localization
Tests
✏️ Tip: You can customize this high-level summary in your review settings.