fix(LC0089): keep compilation-scoped settings out of analyzer instance fields - #501
Merged
Merged
Conversation
…instance fields CognitiveComplexity stored the complexity threshold and the LC0089i enablement flag in mutable instance fields written at CompilationStart. Analyzer instances are shared across analysis passes and projects, so an overlapping pass with a different alcops.json or ruleset could overwrite them mid-analysis, applying the wrong threshold or increment enablement. Capture both as locals in the CompilationStart closure (like the existing recursion graph service) and thread them as parameters. No functional change under single-pass analysis; the race is not reproducible in unit tests, so no new fixture. Found while verifying issue #254. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Verification of issue #254 against the decompiled NAV SDK showed the documented declaration-cache skipping (AnalysisState.declarationAnalysisDataMap) is dead code: every shipping driver path passes a null AnalysisState, so no per-declaration action is ever cache-skipped, and there is no SyntaxNodeAction-vs-CodeBlockAction asymmetry. The real callback filters are the per-file AnalysisScope and the module-only partial pass, both uniform across action kinds - which is also the actual mechanism behind the #243 AC0032 false positives. The two-phase accumulator anti-pattern verdict stands, re-justified. Also records the CognitiveComplexity design decisions in a new rule doc. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
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.
Problem
Issue #254 audited the
RegisterCodeBlockActionanalyzers for incremental-compilation staleness. Verification against the decompiled NAV SDK source (18.0.x) refuted the audit's core premise — the declaration-cache skipping machinery is dead code (nullAnalysisStateon every shipping driver path;CompilationWithAnalyzersnever instantiated), there is no SyntaxNodeAction-vs-CodeBlockAction asymmetry, and the "LC0059 settings staleness" recommendation is moot becauseALCopsSettingsProvidernever invalidates its cache anyway. Full analysis: issue comment.The verification did surface one real defect: CognitiveComplexity (LC0089/LC0089i/LC0090) stores per-compilation state in mutable instance fields, written at
CompilationStart. Analyzer instances are materialized once per project and shared across analysis passes, so an overlapping pass or sibling project with a differentalcops.json/ ruleset can overwritecomplexityThresholdand the LC0089i enablement flag mid-analysis — the same bug class as the LC0054 fix, and a violation of the repo's stateless-analyzer rule.Solution
CompilationStartclosure (like the existing recursion graph service) and thread them as parameters; the affected methods becamestatic. No functional change under single-pass analysis..claude/rules/sdk-analyzer-infrastructure.mdto the verified model: no per-declaration cache skipping exists; the real callback filters are the per-fileAnalysisScopeand the module-only partial pass, uniform across all action kinds — which is also the actual mechanism behind the [Bug]: AC0032 - false positive #243 AC0032 false positives (the accumulator was scope-starved, not cache-skipped; the fix(AC0032): rewrite to atomic per-object analysis with variable-map optimization #253 fix remains correct, with corrected rationale). The two-phase accumulator anti-pattern verdict stands..claude/rules/diagnostics/lc0089-cognitive-complexity.mdrecording the design decisions.Testing
ContinuousIntegrationBuild=true): clean.dotnet format --verify-no-changes: no findings.Closes #254
🤖 Generated with Claude Code