feat: new diagnostic InsertionWithoutChangeAndValidate - #4397
Open
pravets wants to merge 1 commit into
Open
Conversation
Detects #Insert / #EndInsert directives used inside a method without &ChangeAndValidate annotation, which causes compilation error in 1C:Enterprise configuration extensions. - Diagnostic type: ERROR, severity: CRITICAL, minutesToFix: 1 - Checks all PREPROC_INSERT / PREPROC_ENDINSERT tokens within method bodies - Skips methods with CHANGEANDVALIDATE annotation - RU/EN messages, docs, and test fixture included
Contributor
📝 WalkthroughWalkthroughThe change adds a critical diagnostic for ChangesInsertion directive diagnostic
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.java (2)
54-55: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep the diagnostic lifecycle hook protected.
The base diagnostic lifecycle initializes
documentContext, clears diagnostics, and then callscheck(). This override widens the hook topublic, so callers can invoke it without that lifecycle and trigger a null context or stale diagnostics. Useprotected void check()to match the base contract. The repository base class currently declares this hook as protected. (raw.githubusercontent.com)Proposed fix
`@Override` - public void check() { + protected void check() {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.java` around lines 54 - 55, Change the check() override in InsertionWithoutChangeAndValidateDiagnostic from public to protected so it matches AbstractDiagnostic’s lifecycle hook visibility and cannot be called outside the initialized diagnostic flow.Source: MCP tools
56-69: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAvoid rescanning all tokens for every method.
The inner stream scans the complete token list for each unannotated method. This can make the diagnostic scale as
O(methods × tokens). Filter insertion positions once and return early when the document has no insertion directives.Proposed refactor
var tokens = documentContext.getTokens(); + var insertionPositions = tokens.stream() + .filter(token -> token.getType() == BSLLexer.PREPROC_INSERT + || token.getType() == BSLLexer.PREPROC_ENDINSERT) + .map(token -> new Position(token.getLine() - 1, 0)) + .toList(); + if (insertionPositions.isEmpty()) { + return; + } documentContext.getSymbolTree().getMethods() ... - .filter(method -> tokens.stream() - .filter(token -> token.getType() == BSLLexer.PREPROC_INSERT - || token.getType() == BSLLexer.PREPROC_ENDINSERT) - .anyMatch(token -> { - var tokenPosition = new Position(token.getLine() - 1, 0); - return Ranges.containsPosition(method.getRange(), tokenPosition); - })) + .filter(method -> insertionPositions.stream() + .anyMatch(position -> Ranges.containsPosition(method.getRange(), position)))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.java` around lines 56 - 69, Update the diagnostic flow around the symbol-tree method stream to precompute insertion and end-insertion token positions once, return immediately when none exist, and have each method check only those filtered positions instead of rescanning the complete tokens list. Preserve the existing CHANGEANDVALIDATE annotation exclusion and range-matching behavior.src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnosticTest.java (1)
38-47: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the
#КонецВставкиdirective form.
INSERTION_WITHOUT_CHANGE_AND_VALIDATEcurrently asserts only one aggregate diagnostic and one#Вставкаrange. Since the diagnostic accepts bothPREPROC_INSERTandPREPROC_ENDINSERT, add a separate annotated procedure/function that contains only#КонецВставкиand assert its diagnostic/range instead of relying on the existing aggregate case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnosticTest.java` around lines 38 - 47, Extend the test fixture and test method test() to include a separate annotated procedure or function containing only `#КонецВставки`, then assert its diagnostic and exact range independently. Keep the existing `#Вставка` coverage and aggregate diagnostic assertions, ensuring both PREPROC_INSERT and PREPROC_ENDINSERT forms are validated.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.java`:
- Around line 54-55: Change the check() override in
InsertionWithoutChangeAndValidateDiagnostic from public to protected so it
matches AbstractDiagnostic’s lifecycle hook visibility and cannot be called
outside the initialized diagnostic flow.
- Around line 56-69: Update the diagnostic flow around the symbol-tree method
stream to precompute insertion and end-insertion token positions once, return
immediately when none exist, and have each method check only those filtered
positions instead of rescanning the complete tokens list. Preserve the existing
CHANGEANDVALIDATE annotation exclusion and range-matching behavior.
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnosticTest.java`:
- Around line 38-47: Extend the test fixture and test method test() to include a
separate annotated procedure or function containing only `#КонецВставки`, then
assert its diagnostic and exact range independently. Keep the existing `#Вставка`
coverage and aggregate diagnostic assertions, ensuring both PREPROC_INSERT and
PREPROC_ENDINSERT forms are validated.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a632a1b-5457-43f9-bf76-856b74935730
⛔ Files ignored due to path filters (1)
src/test/resources/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.bslis excluded by!src/test/resources/**
📒 Files selected for processing (6)
docs/diagnostics/InsertionWithoutChangeAndValidate.mddocs/en/diagnostics/InsertionWithoutChangeAndValidate.mdsrc/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic.javasrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic_en.propertiessrc/main/resources/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnostic_ru.propertiessrc/test/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/InsertionWithoutChangeAndValidateDiagnosticTest.java
2 tasks
nixel2007
reviewed
Aug 23, 2026
| .filter(method -> method.getAnnotations().stream() | ||
| .map(Annotation::getKind) | ||
| .noneMatch(kind -> kind == AnnotationKind.CHANGEANDVALIDATE)) | ||
| .filter(method -> tokens.stream() |
Member
There was a problem hiding this comment.
Вот тут точно не прокатит. Это n*m поиск (токены в документе, количество методов).
Кажется, дешевле будет подписаться на узлы preproc insert/end и проверить что метод не имеет нужной директивы
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Описание
Новая диагностика InsertionWithoutChangeAndValidate — находит использование директив
#Вставка/#КонецВставкивнутри методов без аннотации&ИзменениеИКонтроль.Без этой аннотации платформа 1С не может скомпилировать модуль — вставка кода расширения разрешена только в методах, явно объявленных как изменяющие.
Closes #3815
Метаданные
Как работает
&ИзменениеИКонтрольPREPROC_INSERT/PREPROC_ENDINSERTв теле методаСостав PR
_ru.properties/_en.propertiesЧеклист
Summary by CodeRabbit
New Features
Documentation
Tests