Static Analysis Is Not a Weaker Kind of Code Review

Before anyone opens a pull request in one of my open source repositories, 306 static analysis rules have already returned a verdict on the change. Ninety-four of them are security rules. They ran at compile time, on every target framework the project builds, and a rule that fires is a build error rather than a comment somebody might read later.

The whole configuration is four lines of MSBuild:

<AnalysisLevel>10.0-all</AnalysisLevel>
<CodeAnalysisTreatWarningsAsErrors>true</CodeAnalysisTreatWarningsAsErrors>
<EnableNETAnalyzers>true</EnableNETAnalyzers>
<EnforceCodeStyleInBuild>true</EnforceCodeStyleInBuild>

That combination selects a generated configuration file that ships inside the .NET SDK, and the file is not abstract. It lists 307 analyzer rules by identifier, 306 of them set to error, one (CA1516) set to none. Split by category:

Sixty-four repositories share this configuration through a custom SDK, so the number is the same in all of them. Nobody has to remember to switch it on, and no reviewer has to hold 306 rules in their head.

I am writing this because of a habit I keep running into, which is a team pasting a diff into a language model and asking whether it looks correct, or secure, in a repository where none of the above is turned on.

What the Model Is Being Asked to Redo

The substitution is not a model standing in for a human reviewer. It is a decidable question being re-asked in a form that cannot be decided.

Does this code dispose the stream on every path is a question with a right answer that a compiler already computes from the syntax tree. Does this comparison use a culture-sensitive overload where an ordinal one is required has an answer, and CA1310 is that answer, written down once by people who had the bug. Asking a model instead does not make the question harder to answer. It makes the answer unreliable in ways that are difficult to detect, because the answer arrives fluent either way.

The rule set has properties a prompt does not. It returns the same verdict on the same input every time, which means a passing build is evidence rather than an impression. Its scope is enumerable, so I can read all 306 rules and know exactly which classes of defect are covered and which are not. And it costs nothing per invocation, which is why it runs on every build instead of when somebody remembers to ask.

None of that is an argument that models are bad at reading code. My opinion is that they are good at it, and that the goodness is beside the point. A probabilistic reviewer that agrees with the deterministic one 95 percent of the time is not 95 percent of a linter, because the 5 percent is unmarked.

Review Is the Principle That Breaks

I wrote about the boundaries I hold for my own AI use, and the one this violates is review: the output has to be checkable before it is accepted, and the operator has to be competent to check it. Reading output is not reviewing it.

Applied here, that gets uncomfortable. Evaluating "the model found no security issues" requires knowing which security issues were possible, which is most of the knowledge the 94 rules encode. An operator who could audit the model's recall would have configured the analyzers, because they would know the analyzers exist. So the substitution is most attractive exactly where it is least safe, and the failure is silent by construction: a clean report and an unrun check are the same experience from the outside.

That is a claim about conditions, not about people. Nobody is born knowing that CA2100 exists. The conditions are what make the difference between a team that finds out and a team that ships a fluent report.

Arguing With a Rule

The other property a rule set has is that disagreement with it is recorded.

Six rules are suppressed in my SDK, each with a written reason sitting next to it. One of them:

<NoWarn>CA1724;CA1034;CA1000;CA2260;CA1515;IDE0370;</NoWarn>
<!-- IDE0370: Unnecessary suppression operator. Conflicts with multi-targeting: a null-forgiving '!'
     (e.g. on object.ToString()) is required on modern TFMs where the API is nullable-annotated but is
     flagged redundant on netstandard2.0/2.1 where the same API is oblivious. No single source edit
     satisfies all target frameworks, so the rule cannot be enforced on multi-targeted libraries. -->

That is a position. It is versioned, it names the rule, it explains the conflict, and anyone can disagree with it in a pull request. The standing convention in these repositories is that suppressions are never global, and never wider than the smallest scope that works, and always carry a justification.

The analysis level is pinned for a related reason:

<!-- AnalysisLevel is pinned rather than set to 'latest-all'. With TreatWarningsAsErrors and
     CodeAnalysisTreatWarningsAsErrors both on below, 'latest' means every new rule that ships
     in a .NET SDK update becomes a build-breaking error in every consuming repository at once,
     on the day the runner image updates - with no change on their side and nothing they can do
     but pin the SDK. Pinning moves that to a deliberate, reviewable bump here. -->

A model consulted about a diff leaves none of this behind. There is no artifact to disagree with, no record of what was checked, and nothing that a future change can be measured against. The verdict evaporates the moment the tab closes.

The Gate That Blocks the Release

The second half of the same idea is that a check nobody is required to pass is a suggestion.

Test runs in these repositories collect coverage and a test report, both of which are handed to SonarCloud along with the build. The scanner starts with sonar.qualitygate.wait=true, which makes the step fail when the project fails its gate, and the release step is gated on that step succeeding. 47 of the 60 repositories with a CI workflow run it. A package cannot be published past a gate the project did not pass, and no human decides that in the moment.

One detail in that setup says more than the gate itself. The scanner is passed an explicit version:

/v:"${{ steps.analysis_version.outputs.version }}"

with a comment recording why. SonarCloud's new-code period anchors to recorded version boundaries. Without the version, the scanner reports it as "not provided", the period widens to the entire history, and the new-code coverage condition quietly starts measuring the whole codebase instead of the change. The gate keeps passing. It just stops meaning what it claims.

Somebody had to go and check whether a green result still corresponded to the thing it was supposed to measure. That is the faculty being skipped, and it is not a faculty a model can supply on request, because the question is whether the measurement is wired up correctly at all.

The Same Mistake, One Scale Up

Those 306 rules are a compressed record of other people's incidents. Each one exists because something went wrong somewhere, repeatedly, and somebody encoded the lesson in a form a compiler can apply for free forever. CA5350 is there because weak cryptography shipped. CA2016 is there because cancellation tokens were dropped. A rule set is decades of failures anyone can inherit by setting a property.

The same thing is true one level up, of libraries. The reason not to write a new date-time library, or a new crypto primitive, or a new expression evaluator, is rarely the writing. It is that the mature one has been beaten on by people who had the failures the author has not had yet, and the value of it lives in exactly the edge cases nobody thinks to write down.

Generation made producing the fifth version cheap. It did not make the fifth version encode anything. Some of what a model writes when asked for a parser or a retry policy or a permissions check is a plausible reconstruction of the shallow parts of a solved problem, with the accumulated corrections absent, and it arrives looking finished. This is the same error as replacing the linter, at a larger scale: declining an artifact that already encodes the knowledge, in favor of one generated fresh that only resembles it.

I said in an earlier post that the deeper change between two of my own engines was that I stopped writing parsers for things that already had parsers. That was a lesson about knowing the landscape. Generation makes not knowing the landscape cheaper than it has ever been.

Where the Model Actually Belongs

The inversion is the useful part, and I want to be specific about it rather than end on a prohibition.

My SDK ships seven analyzers of my own, KTSU0001 through KTSU0007, all at error severity, six of them with automatic code fixes. They enforce conventions no shipped rule covers: a missing required package reference, a missing InternalsVisibleTo for the test project, ArgumentNullException.ThrowIfNull used where Ensure.NotNull is required for framework compatibility, a hand-written null check that should be that same call, an orphaned PackageVersion entry, a transitive package used directly without a reference, and a build-time package reference missing PrivateAssets.

Every one of those started as a comment somebody made in review, more than once. Writing a Roslyn analyzer is a fiddly, well-documented, mechanical task with a test harness and a release-tracking file, which is a precise description of the work a language model is genuinely good at and that most teams never get around to, because the payoff is diffuse and the API is tedious.

So the model belongs on the side of making the question decidable, not on the side of answering an undecidable version of it in perpetuity. Asked to review a diff, it produces one opinion that helps once. Asked to help write the eighth analyzer, it produces a rule that runs on every build in 64 repositories, forever, for free, with the same answer every time, and with a title and a description that say what it checks.

What Transfers

Turn the analyzers up before adding anything to the review process. In .NET that is four properties and an afternoon of fixing what they find. Whatever the ecosystem, the equivalent exists and is almost certainly off.

Check that the checker ran. A configuration that silently covers nothing and a configuration that covers everything produce the same green tick, and the difference is only visible to somebody who went and looked.

When a review comment repeats, write the rule. The second time is a coincidence and the third time is a missing analyzer.

And keep the record. A suppression with a reason beside it is a decision the team can revisit. A verdict from a chat window is not, and six months later nobody can reconstruct what was actually checked.

The question I would ask of any repository before pointing a model at its code: what does the build already know, and is it switched on?