Fix IllegalArgumentException in CFG builder on dangling top-level preprocessor before a loop

[Copilot]

A module starting with a dangling preprocessor directive followed by a loop whose body contains an unterminated #Если crashed CFG construction with IllegalArgumentException: no such vertex in graph: <empty block> (thrown from buildLoopSubgraph).

Root cause

buildGraph set hasTopLevelPreprocessor = true for any leading module preprocessor directive that the grammar consumes. Only visitPreproc_if clears that flag, so a dangling #Иначе/#ИначеЕсли/#КонецЕсли left it set. The stale flag then leaked into a later nested #Если (inside the loop body) that had no matching #КонецЕсли, forcing an unbalanced preprocessor subgraph. This corrupted the block stack and removed the loop-break vertex, so the subsequent FALSE_BRANCH edge referenced a vertex no longer in the graph.

Minimal reproducer:

#Иначе
Пока А Цикл
#Если Сервер Тогда
КонецЦикла;

Changes

• CfgBuildingParseTreeVisitor.buildGraph: set hasTopLevelPreprocessor only when the leading preprocessor is an actual #Если (preproc_if() != null); otherwise leave it false. Other directives are still visited (harmless no-ops).
• ControlFlowGraphBuilderTest: added test_danglingTopLevelPreprocessorBeforeLoopShouldNotCrash regression test.

if (probablyPreprocessor instanceof BSLParser.PreprocessorContext preprocessor) {
  hasTopLevelPreprocessor = preprocessor.preproc_if() != null;
  preprocessor.accept(this);
}

[nixel2007]

@claude review pls

[claude]

Claude finished @nixel2007's task in 1m 34s —— View job

---

Code Review

• Gather context and read changed files
• Review the fix in CfgBuildingParseTreeVisitor.java
• Review the test in ControlFlowGraphBuilderTest.java
• Final summary

---

CfgBuildingParseTreeVisitor.java — the fix

The root cause analysis is accurate. The old guard:

if (probablyPreprocessor != fileBlock) {
  hasTopLevelPreprocessor = true;
  probablyPreprocessor.accept(this);
}

blindly set hasTopLevelPreprocessor = true for any node returned by Trees.getPreviousNode, including dangling #Иначе/#ИначеЕсли/#КонецЕсли. The stale flag then reached the next visitPreproc_if and made it take the "top-level" branch (lines 459-462), which adds currentBlock.begin() to the graph and clears the flag, leaving the block stack inconsistent for the inner #Если that had no matching #КонецЕсли. That ultimately caused the loop-break vertex to be removed from the graph before buildLoopSubgraph referenced it in FALSE_BRANCH.

The new code is correct. Setting hasTopLevelPreprocessor = preprocessor.preproc_if() != null elegantly handles both paths with one assignment: true only for a genuine leading #Если, false for everything else.

The instanceof BSLParser.PreprocessorContext preprocessor guard is also cleaner than the original != fileBlock identity check — it explicitly asserts the expected type rather than relying on the semantic contract of Trees.getPreviousNode.

One small observation (not a bug, just worth knowing): hasTopLevelPreprocessor is never reset to false at the top of buildGraph. If someone reuses the same visitor instance across multiple buildGraph calls (unlikely given current callers, but possible), and the first call sets it to true via visitPreproc_if without clearing (e.g. early return at line 453), the next call might see stale state. The new code mitigates this partially — if a preprocessor node exists before the file block, it is now explicitly set to true or false. But if no preprocessor exists before the file block (the instanceof check fails), the field keeps whatever value was left from a previous call. This was already the case before this PR; it's just worth noting. A hasTopLevelPreprocessor = false; at the start of buildGraph (around line 65 with the other resets) would make it fully safe.

---

ControlFlowGraphBuilderTest.java — the test

The test is a good, minimal regression: it uses the exact reproducer from the bug report and targets the right configuration (producePreprocessorConditions(true) is required to trigger the preprocessor-condition code path). The assertions are intentionally light — the goal is "must not throw", which is appropriate for a crash-regression test.

Optional enhancement: the fix logically covers #ИначеЕсли and #КонецЕсли as dangling directives too (all three were broken before). Adding two quick extra var code snippets for those cases would make the regression coverage complete, but this is not blocking.

---

Summary

The fix is correct, minimal, and well-explained. The comment in Russian matches the project style and clearly states the invariant. No functional issues. Approving with the optional note about resetting hasTopLevelPreprocessor at the top of buildGraph for defensive completeness.

[Copilot]

Pull request overview

This PR fixes a CFG builder crash caused by treating dangling top-level preprocessor directives as module-opening #Если directives, which could corrupt the block stack when later malformed nested preprocessors appeared inside loops.

Changes:

• Restricts hasTopLevelPreprocessor activation to leading preproc_if() only.
• Adds a regression test for dangling top-level #Иначе before a loop with an unterminated nested #Если.

Reviewed changes

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

File	Description
src/main/java/com/github/_1c_syntax/bsl/languageserver/cfg/CfgBuildingParseTreeVisitor.java	Updates top-level preprocessor handling to avoid stale flag leakage from dangling non-#Если directives.
src/test/java/com/github/_1c_syntax/bsl/languageserver/cfg/ControlFlowGraphBuilderTest.java	Adds regression coverage ensuring the reported malformed preprocessor/loop case no longer crashes CFG construction.

[github-actions]

Test Results

 2 455 files   -   491   2 455 suites   - 491   48m 33s ⏱️ - 36m 2s
 2 678 tests +    1   2 678 ✅ +    1  0 💤 ±0  0 ❌ ±0 
13 390 runs   - 2 672  13 390 ✅  - 2 672  0 💤 ±0  0 ❌ ±0 

Results for commit d7e57a1. ± Comparison against base commit 6515d30.