Features/bcs metaborders#3830
Conversation
📝 WalkthroughWalkthroughIntroduces a new MetadataBorders diagnostic that validates metadata access against configured border patterns. Includes diagnostic implementation with JSON configuration parsing, schema updates, localization files in English and Russian, comprehensive documentation, and test cases with multiple validation scenarios. Changes
Sequence DiagramsequenceDiagram
participant Client as Language Server
participant Visitor as MetadataBorders<br/>Diagnostic
participant Config as Configuration
participant Checker as Pattern<br/>Matcher
Client->>Visitor: initialize with config
Visitor->>Config: load metadataBordersParameters
Config-->>Visitor: return pattern map (JSON)
Client->>Visitor: visitFile(filePath)
Visitor->>Visitor: compute statementPatterns<br/>(filter patterns by file path)
Visitor-->>Visitor: store applicable patterns
Client->>Visitor: visitStatement(statement)
Visitor->>Checker: check each pattern<br/>against statement text
Checker-->>Visitor: pattern match result
alt pattern matches
Visitor->>Client: recordDiagnostic(statement)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/diagnostics/MetadataBorders.md`:
- Around line 28-33: The fenced code block showing the query example lacks a
language identifier and triggers MD040; add a language tag (e.g., "text" or
"sql") to the opening fence so the block begins with ```text (or ```sql) before
the query string ("ВЫБРАТЬ ... РегистрСведений.КонтактнаяИнформация КАК КИ") to
satisfy the linter and improve readability.
In `@docs/en/diagnostics/MetadataBorders.md`:
- Around line 14-19: The English "Description" and "Examples" sections in
MetadataBorders.md are placeholder text; replace them with a clear English
description of the diagnostic (what it detects and how it works) under the "##
Description" heading and add concrete Examples under "## Examples" that include
a problematic code snippet that triggers the diagnostic and a corrected snippet
showing the fix; ensure the content mirrors the Russian doc's intent and follows
project guidelines by providing both the failing sample and the corrected sample
for the diagnostic.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.java`:
- Around line 44-55: Add a class-level JavaDoc to the public
MetadataBordersDiagnostic class that succinctly describes the diagnostic's
purpose and behavior, documents its severity/activation (referencing the
`@DiagnosticMetadata` annotation values), explains what code pattern or metadata
border it detects, and notes any important usage/configuration details (e.g.,
activatedByDefault=false, DiagnosticTag.DESIGN) and expected fix guidance; place
the JavaDoc immediately above the class declaration (public class
MetadataBordersDiagnostic extends AbstractVisitorDiagnostic) and keep it concise
but informative for public API consumers.
- Around line 100-104: The current lambda over statementPatterns runs
diagnosticStorage.addDiagnostic(ctx) for every regex hit, creating duplicates;
change the logic in MetadataBordersDiagnostic so each statement only yields one
diagnostic by short‑circuiting after the first match (e.g., replace the forEach
with a loop or use a flag so that when matcher.find() triggers
diagnosticStorage.addDiagnostic(ctx) you break out of the pattern loop/stop
further matching for that ctx). Refer to statementPatterns, Matcher matcher, and
diagnosticStorage.addDiagnostic(ctx) to locate and modify the code.
- Around line 66-76: The mapFromJSON method currently only catches
JsonProcessingException but may throw PatternSyntaxException when
CaseInsensitivePattern.compile(entry.getKey()) or compile(entry.getValue())
receives invalid regex from metadataBordersParameters; update mapFromJSON to
handle invalid patterns the same way as SymbolProvider.java by wrapping the
compilation in a try-catch (catch PatternSyntaxException) or using a helper that
returns Optional, then filter out or skip entries with invalid regex and log a
warning (do not let the exception propagate) so configuration loading remains
resilient.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
src/test/resources/diagnostics/MetadataBordersDiagnostic.bslis excluded by!src/test/resources/**
📒 Files selected for processing (8)
docs/diagnostics/MetadataBorders.mddocs/en/diagnostics/MetadataBorders.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/parameters-schema.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/configuration/schema.jsonsrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic_en.propertiessrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic_ru.propertiessrc/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnosticTest.java
| ``` | ||
| "ВЫБРАТЬ | ||
| ... | ||
| ИЗ | ||
| РегистрСведений.КонтактнаяИнформация КАК КИ" | ||
| ``` |
There was a problem hiding this comment.
Add a language identifier to the fenced query example.
The fenced block is missing a language tag, which triggers MD040 and reduces readability.
🛠️ Suggested fix
-```
+```text
"ВЫБРАТЬ
...
ИЗ
РегистрСведений.КонтактнаяИнформация КАК КИ"
-```
+```📝 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.
| ``` | |
| "ВЫБРАТЬ | |
| ... | |
| ИЗ | |
| РегистрСведений.КонтактнаяИнформация КАК КИ" | |
| ``` |
🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 28-28: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/diagnostics/MetadataBorders.md` around lines 28 - 33, The fenced code
block showing the query example lacks a language identifier and triggers MD040;
add a language tag (e.g., "text" or "sql") to the opening fence so the block
begins with ```text (or ```sql) before the query string ("ВЫБРАТЬ ...
РегистрСведений.КонтактнаяИнформация КАК КИ") to satisfy the linter and improve
readability.
| ## Description | ||
| <!-- Описание диагностики заполняется вручную. Необходимо понятным языком описать смысл и схему работу --> | ||
|
|
||
| ## Examples | ||
| <!-- В данном разделе приводятся примеры, на которые диагностика срабатывает, а также можно привести пример, как можно исправить ситуацию --> | ||
|
|
There was a problem hiding this comment.
Fill in the English Description/Examples sections (currently placeholders).
These sections must be real content (including a problematic snippet and fix) to match the RU doc and project guidelines.
As per coding guidelines: "Update diagnostic documentation in both Russian and English with examples of problematic code and fixes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/en/diagnostics/MetadataBorders.md` around lines 14 - 19, The English
"Description" and "Examples" sections in MetadataBorders.md are placeholder
text; replace them with a clear English description of the diagnostic (what it
detects and how it works) under the "## Description" heading and add concrete
Examples under "## Examples" that include a problematic code snippet that
triggers the diagnostic and a corrected snippet showing the fix; ensure the
content mirrors the Russian doc's intent and follows project guidelines by
providing both the failing sample and the corrected sample for the diagnostic.
| @DiagnosticMetadata( | ||
| type = DiagnosticType.CODE_SMELL, | ||
| severity = DiagnosticSeverity.INFO, | ||
| minutesToFix = 1, | ||
| activatedByDefault = false, | ||
| tags = { | ||
| DiagnosticTag.DESIGN | ||
| } | ||
|
|
||
| ) | ||
| public class MetadataBordersDiagnostic extends AbstractVisitorDiagnostic { | ||
|
|
There was a problem hiding this comment.
Add class-level JavaDoc for the public diagnostic.
As per coding guidelines: "Write JavaDoc for public APIs and include comments for complex logic."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.java`
around lines 44 - 55, Add a class-level JavaDoc to the public
MetadataBordersDiagnostic class that succinctly describes the diagnostic's
purpose and behavior, documents its severity/activation (referencing the
`@DiagnosticMetadata` annotation values), explains what code pattern or metadata
border it detects, and notes any important usage/configuration details (e.g.,
activatedByDefault=false, DiagnosticTag.DESIGN) and expected fix guidance; place
the JavaDoc immediately above the class declaration (public class
MetadataBordersDiagnostic extends AbstractVisitorDiagnostic) and keep it concise
but informative for public API consumers.
| private static Map<Pattern, Pattern> mapFromJSON(String userSettings) { | ||
| ObjectMapper mapper = new ObjectMapper(); | ||
| MapType mapType = mapper.getTypeFactory().constructMapType(HashMap.class, String.class, String.class); | ||
| try { | ||
| Map<String, String> stringMap = mapper.readValue(userSettings, mapType); | ||
| return stringMap.entrySet().stream() | ||
| .filter(entry -> !entry.getKey().isBlank() && !entry.getValue().isBlank()) | ||
| .collect(Collectors.toMap( | ||
| entry -> CaseInsensitivePattern.compile(entry.getKey()), | ||
| entry -> CaseInsensitivePattern.compile(entry.getValue()))); | ||
| } catch (JsonProcessingException e) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify how other diagnostics handle invalid regex patterns and whether PatternSyntaxException
# is already handled centrally.
rg -n "CaseInsensitivePattern\.compile|Pattern\.compile" src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics
rg -n "PatternSyntaxException" src/main/java/com/github/_1c_syntax/bsl/languageserverRepository: 1c-syntax/bsl-language-server
Length of output: 28434
🏁 Script executed:
# Find and examine the CaseInsensitivePattern class to understand its compile method
fd -t f "CaseInsensitivePattern.java" src/Repository: 1c-syntax/bsl-language-server
Length of output: 55
🏁 Script executed:
# Examine the MetadataBordersDiagnostic.java file to understand the full context
cat -n src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.java | head -100Repository: 1c-syntax/bsl-language-server
Length of output: 4685
🏁 Script executed:
fd -t f "CaseInsensitivePattern.java" src/Repository: 1c-syntax/bsl-language-server
Length of output: 55
🏁 Script executed:
# Also check if there are any tests for mapFromJSON or exception handling
fd -t f "MetadataBordersDiagnosticTest.java" .Repository: 1c-syntax/bsl-language-server
Length of output: 176
🏁 Script executed:
# Find CaseInsensitivePattern - it might be in a different location
rg -l "class CaseInsensitivePattern" src/Repository: 1c-syntax/bsl-language-server
Length of output: 55
🏁 Script executed:
# Examine the test file to see how mapFromJSON is tested
cat -n src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnosticTest.javaRepository: 1c-syntax/bsl-language-server
Length of output: 3697
🏁 Script executed:
# Search more broadly for CaseInsensitivePattern
rg "class CaseInsensitivePattern|new CaseInsensitivePattern" --max-count 5Repository: 1c-syntax/bsl-language-server
Length of output: 55
🏁 Script executed:
# Check if CaseInsensitivePattern is imported from an external library
rg "import.*CaseInsensitivePattern" src/Repository: 1c-syntax/bsl-language-server
Length of output: 14028
🏁 Script executed:
# Let's also check if Pattern.compile is ever wrapped or if PatternSyntaxException is ever caught
rg "catch.*PatternSyntaxException" src/Repository: 1c-syntax/bsl-language-server
Length of output: 198
🏁 Script executed:
# Check the broader context of error handling in diagnostics with user-supplied patterns
rg -A5 "\.compile\(" src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ | grep -A5 "catch"Repository: 1c-syntax/bsl-language-server
Length of output: 655
🏁 Script executed:
# Check SymbolProvider.java to see how it handles PatternSyntaxException
sed -n '70,85p' src/main/java/com/github/_1c_syntax/bsl/languageserver/providers/SymbolProvider.javaRepository: 1c-syntax/bsl-language-server
Length of output: 713
🏁 Script executed:
# Check if there are any other diagnostics that accept user configuration for patterns
rg "@DiagnosticParameter.*String" src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ -A5 | grep -B5 "\.compile"Repository: 1c-syntax/bsl-language-server
Length of output: 416
🏁 Script executed:
# Check ReservedParameterNamesDiagnostic to see if it handles PatternSyntaxException
sed -n '45,70p' src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ReservedParameterNamesDiagnostic.javaRepository: 1c-syntax/bsl-language-server
Length of output: 771
🏁 Script executed:
# Check BadWordsDiagnostic which also compiles user-supplied patterns
sed -n '50,75p' src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/BadWordsDiagnostic.javaRepository: 1c-syntax/bsl-language-server
Length of output: 879
🏁 Script executed:
# Look for other DiagnosticParameter with String type that compile patterns
rg "@DiagnosticParameter" src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/ -A3 | grep -B3 -A3 "String.class"Repository: 1c-syntax/bsl-language-server
Length of output: 17642
Add exception handling for invalid regex patterns in user-supplied metadataBordersParameters.
The mapFromJSON method currently only catches JsonProcessingException, leaving PatternSyntaxException from CaseInsensitivePattern.compile unhandled. Invalid user-supplied regex patterns will crash configuration loading. Add a try-catch around the pattern compilation or filter out invalid entries to keep the server resilient, following the pattern used in SymbolProvider.java.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.java`
around lines 66 - 76, The mapFromJSON method currently only catches
JsonProcessingException but may throw PatternSyntaxException when
CaseInsensitivePattern.compile(entry.getKey()) or compile(entry.getValue())
receives invalid regex from metadataBordersParameters; update mapFromJSON to
handle invalid patterns the same way as SymbolProvider.java by wrapping the
compilation in a try-catch (catch PatternSyntaxException) or using a helper that
returns Optional, then filter out or skip entries with invalid regex and log a
warning (do not let the exception propagate) so configuration loading remains
resilient.
| statementPatterns.forEach(pattern -> { | ||
| Matcher matcher = pattern.matcher(ctx.getText()); | ||
| while (matcher.find()) { | ||
| diagnosticStorage.addDiagnostic(ctx); | ||
| } |
There was a problem hiding this comment.
Avoid duplicate diagnostics per statement.
The loop adds a diagnostic for every regex hit in the same statement, which can produce duplicates. Consider short‑circuiting after the first match.
✅ Suggested fix
- statementPatterns.forEach(pattern -> {
- Matcher matcher = pattern.matcher(ctx.getText());
- while (matcher.find()) {
- diagnosticStorage.addDiagnostic(ctx);
- }
- });
+ for (Pattern pattern : statementPatterns) {
+ if (pattern.matcher(ctx.getText()).find()) {
+ diagnosticStorage.addDiagnostic(ctx);
+ break;
+ }
+ }📝 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.
| statementPatterns.forEach(pattern -> { | |
| Matcher matcher = pattern.matcher(ctx.getText()); | |
| while (matcher.find()) { | |
| diagnosticStorage.addDiagnostic(ctx); | |
| } | |
| for (Pattern pattern : statementPatterns) { | |
| if (pattern.matcher(ctx.getText()).find()) { | |
| diagnosticStorage.addDiagnostic(ctx); | |
| break; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/MetadataBordersDiagnostic.java`
around lines 100 - 104, The current lambda over statementPatterns runs
diagnosticStorage.addDiagnostic(ctx) for every regex hit, creating duplicates;
change the logic in MetadataBordersDiagnostic so each statement only yields one
diagnostic by short‑circuiting after the first match (e.g., replace the forEach
with a loop or use a flag so that when matcher.find() triggers
diagnosticStorage.addDiagnostic(ctx) you break out of the pattern loop/stop
further matching for that ctx). Refer to statementPatterns, Matcher matcher, and
diagnosticStorage.addDiagnostic(ctx) to locate and modify the code.
2 participants
Описание
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Для диагностик
Дополнительно
Summary by CodeRabbit
New Features
Documentation