Audit: Analyzers using RegisterCodeBlockAction (incremental compilation risk)

[Arthurvdv]

Background

The NAV SDK RegisterCodeBlockAction callback can be skipped during incremental compilation when the SDK determines a declaration has not changed (via AnalysisState.declarationAnalysisDataMap caching). This was the root cause of AC0032 false positives (fixed in #253).

While CodeBlockAction results are typically cached and replayed correctly for pure per-method analysis, analyzers that depend on external state (settings, compilation-wide indexes, other objects) risk producing stale diagnostics when that external state changes but the analyzed method does not.

Risky patterns

1. RegisterCodeBlockAction — can be skipped entirely for unchanged declarations during incremental compilation in VS Code
2. CompilationStart + accumulator + CompilationEnd — accumulator will be incomplete if inner callbacks are skipped (no instances found currently, but guard against future additions)

Safe patterns

• RegisterSyntaxNodeAction — always fires, never skipped
• RegisterOperationAction — always fires, never skipped
• RegisterSymbolAction — fires per-symbol, safe for object-level analysis
• CompilationStart → RegisterSymbolAction — safe (used for pre-building indexes)

Audit results

Analyzers using RegisterCodeBlockAction (5 total)

Analyzer	Cop	Diagnostic IDs	External state dependency	Risk
CognitiveComplexity	LinterCop	LC0059	Settings threshold (loaded in CompilationStart, stored as instance field)	Medium — if user changes alcops.json threshold without editing the method, cached result uses old threshold
CyclomaticComplexityAndMaintainabilityIndex	LinterCop	LC0060, LC0061	Settings threshold (loaded per-invocation inside CodeBlockAction)	Low — settings read each time, purely per-method
IdentifiersInEventSubscribers	LinterCop	LC0049	None	Low — purely local naming check
UseSequentialGuid	PlatformCop	PC0029	None	Low — purely local pattern detection
PartialRecordOperations	PlatformCop	PC0030	None (per-method analysis of reads/writes)	Low — purely local flow analysis

CompilationStart → safe inner callback (not at risk, listed for completeness)

These use CompilationStart only to build compilation-wide indexes or load configuration, then register safe inner callbacks (RegisterSymbolAction):

• AllowInCustomizationsForOmittedFields (AC0026)
• FieldGroupsRequired (AC0013)
• InterfaceObjectNameGuide (LC0049)
• TranslatableTextShouldBeTranslated (LC0091)
• NamingPattern (LC0092)

Recommendations

1. CognitiveComplexity (LC0059): Consider moving settings loading into the CodeBlockAction itself (like CyclomaticComplexity does), or switching to RegisterSyntaxNodeAction on MethodDeclaration nodes. The current pattern means a threshold change in alcops.json may not take effect until the method is edited.

2. All CodeBlockAction analyzers: If any future changes add cross-method or cross-object dependencies, migrate to RegisterSyntaxNodeAction to avoid incremental skipping.

3. Guard rails: Add a code review checklist item: "Does this analyzer use RegisterCodeBlockAction? If so, does it depend on any state outside the code block?"

No action required now

This is a low-priority backlog item for awareness and future-proofing. The current analyzers using CodeBlockAction are mostly safe because they perform purely per-method analysis. The only actionable item is the CognitiveComplexity settings threshold staleness (medium risk, edge case).

[Arthurvdv]

The audit's claims were verified against the decompiled NAV SDK source (18.0.x, net10.0 tree — older AL 12–15 trees not re-checked). The core premise does not hold up, but the verification surfaced one real defect. Summary:

Refuted / corrected claims

1. The diagnostic IDs don't exist. LC0059/LC0060/LC0061 are not in this repo. The analyzers meant are CognitiveComplexity = LC0089/LC0089i/LC0090 and CyclomaticComplexityAndMaintainabilityIndex = LC0007–LC0010.

2. The declaration-cache skipping machinery is dead code. CompilationWithAnalyzers — the only owner of AnalysisState and cached-diagnostic replay — is never instantiated anywhere in the SDK. Both shipping entry points (AnalyzerDriverBase.cs:371-394: the per-file keystroke pass and the whole-project pass) pass a null AnalysisState, and every skip gate reads analysisStateOpt?.TryStartAnalyzingDeclaration(...) ?? true (AnalyzerExecutor.cs:996-1001) — i.e. always execute. declarationAnalysisDataMap lives on CompilationData (not AnalysisState), stores only syntax-shape data (never diagnostics), and its cacheAnalysisData flag evaluates false on every real path.

3. There is no action-kind asymmetry. SyntaxNode, Operation, OperationBlock and CodeBlock actions all gate on the identical TryStartAnalyzingDeclaration call (AnalyzerExecutor.cs:983 / 1086 / 1110 / 1128). If per-declaration skipping were active, RegisterSyntaxNodeAction would be skipped in lockstep with RegisterCodeBlockAction. "Migrate to SyntaxNodeAction to avoid incremental skipping" therefore buys nothing.

4. What actually skips callbacks (and what actually caused #243): per-pass scoping, uniform across all per-declaration action kinds:

• The per-file keystroke pass filters by AnalysisScope.FilterTreeOpt — only the edited file's declarations are analyzed (AnalysisScope.cs:80-100), while compilation-level events still complete. This is the real mechanism behind the [Bug]: AC0032 - false positive #243 AC0032 false positives: the old accumulator saw only the edited file's usages when CompilationEnd fired (it was scope-starved, not "partially stale from caching"). The fix(AC0032): rewrite to atomic per-object analysis with variable-map optimization #253 fix remains fully correct — self-contained per-object analysis is immune to this — but for this reason, not because SyntaxNodeAction is unskippable.
• The module-only partial pass (GetPerModuleAnalyzerDiagnostics, AnalyzersHelper.cs:70-78) enqueues just a module symbol event, so no per-declaration action of any kind fires.

So the standing guidance holds with a corrected justification: two-phase accumulator patterns stay broken; "safe" means self-contained per declaration, not a particular registration API.

5. The LC0089 "settings staleness" recommendation is moot. ALCopsSettingsProvider caches statically by directory path with no invalidation, so an alcops.json edit doesn't take effect until restart no matter where settings are read. Additionally, CompilationStart re-runs on every per-file pass (AnalyzersHelper.cs:31 calls ClearAnalyzerState()), so "stale threshold until the method is edited" cannot occur.

6. Confirmed as stated: no CompilationStart + accumulator + CompilationEnd pattern exists in production code.

What verification found instead

CognitiveComplexity stores per-compilation state in mutable instance fields (complexityThreshold, IsIncrementDiagnosticsEnabled), written at CompilationStart. Analyzer instances are shared across passes and projects (Workspaces/ProjectInfo.cs:113 materializes DiagnosticAnalyzers once), so an overlapping pass or a sibling project with a different alcops.json / ruleset can overwrite them mid-analysis — wrong threshold or wrong LC0089i enablement. Same bug class as the LC0054 fix, and a violation of the repo's own stateless-analyzer rule. The linked PR converts them to CompilationStart closure locals and corrects .claude/rules/sdk-analyzer-infrastructure.md, which documented the refuted mechanism.

Everything else audited (LC0007–LC0010, LC0028, LC0095, LC0097, PC0029, PC0030, PC0035) is verified stateless and stays on RegisterCodeBlockAction — which also gets the SDK's pre-computed operation trees for free.

[Arthurvdv]

Resolved by #501 (squash-merged): the LC0089 instance-field race is fixed and .claude/rules/sdk-analyzer-infrastructure.md now documents the verified incremental-analysis model. The audit's original premise is refuted per the analysis above — no analyzer is being migrated off RegisterCodeBlockAction.