Static Analysis Findings as a Review Signal

A review is a manual execution of a checking procedure by someone whose attention is finite and whose context is partial. A static analyser executes a checking procedure by a program that has complete context and no attention budget, at the cost of not knowing what the code is for. The two are complementary in a specific way: the analyser is reliable where the property is local and syntactic, and unreliable where the property requires intent. Placing its output into the review flow is therefore not a matter of adding signal but of routing the right findings to the right reader at the right moment, and most of the design difficulty lives in the ranking.

Sound and complete are both unavailable

An analyser that reports every possible defect is undecidable in general. Implementations choose a position on the spectrum, and the choice determines the character of their output.

A sound analyser reports no false negatives: if it says nothing, the property holds. This is achieved by being conservative, which produces false positives — warnings about paths that cannot actually occur. Soundness is essential for properties where a missed defect is catastrophic and a false alarm is cheap, which is why soundness is the norm in safety-critical tooling and rare in developer-facing linters.

A complete analyser reports no false positives: everything it reports is a real defect. This is achieved by only reporting cases where the path is fully established, which produces false negatives — defects it does not see. Developer-facing tools aim here, because a warning that is wrong teaches the developer to ignore warnings, and the cost of that lesson compounds.

Neither position is a defect of engineering. It is the consequence of asking a program to decide a property that depends on values only available at run time, and any statement about an analyser’s accuracy is a statement about which side of this trade it was tuned to.

Path sensitivity is what makes a finding credible

The credibility of a finding depends on how much of the program the analyser had to reason about, and the mechanism for that is path sensitivity: tracking the conditions under which a statement executes and the constraints on values along each path.

A null dereference report is weak if the analyser merely observed that a pointer was assigned without a check. It is strong if the analyser explored the branch structure, determined that the only assignment on the reported path was a call that can return null, and that no dominating check exists. The second report is credible because the analyser can explain the path, and explanation is what makes a finding actionable — a reviewer can verify a stated path in a way they cannot verify a bare claim.

The cost of path sensitivity is combinatorial. The number of paths through a function grows exponentially with the number of branches, so every practical analyser bounds the exploration: it merges states at join points, widens loops to a fixed number of iterations, and gives up on functions beyond a size threshold. Each bound is a source of both false positives and false negatives, and the bounds are why analyser output on a large function is systematically less reliable than on a small one — the same property that makes human review less reliable there.

Interprocedural analysis extends the reasoning across call boundaries, which is necessary for the data-flow properties that review cannot reach, and expensive for the same reason: the call graph is large, recursive, and in dynamic languages not statically known. Analysers that stop at function boundaries find local defects; analysers that cross them find flow defects and pay in time and in false positives from imprecise summaries of what a callee does to its arguments.

Ranking is the whole problem

A diff-level analyser that reports everything it finds is useless, not because the findings are wrong but because they are numerous and mostly irrelevant to the change under review. The relevant subset has a specific shape: findings introduced by the diff, in code the diff touched, of a class the diff’s intent could plausibly have caused.

Three filters implement this, and they compose.

Novelty. Compare the findings on the base against the findings on the change and report only the difference. This is the single most effective filter, because a repository with ten thousand existing findings produces a change with two new ones, and the reviewer sees two. The implementation requires stable finding identity: a finding must be matched across versions by its location and its kind, not by its message text, because messages change when the analyser is upgraded and an unstable identity turns every upgrade into a flood of “new” findings.

Reachability from the diff. Restrict to findings whose reported location is inside a changed region or reachable from one. A new function that calls an old function with a tainted argument produces a finding at the old function’s sink, which is outside the diff; the finding is caused by the change and must be attributed to it. Implementing this correctly requires the call graph the analyser already built, and getting it wrong in the permissive direction reintroduces the flood.

Severity calibrated to the change. A finding about an unused variable is noise in a change that rewrites a parser and signal in a change that is meant to be a cleanup. The analyser does not know which, so severity must be a property of the rule plus a property of the change’s stated intent, and the second part has to come from the change description.

Where findings belong in the review flow

There are three placements, and they differ in who pays the cost of triage.

In the editor, before the change exists. The developer sees the finding while writing and fixes it or suppresses it deliberately. This is the cheapest placement per finding and the most effective, because the context is fully loaded. Its weakness is coverage: it only works for developers with the analyser configured, which means it does not apply uniformly across a team.

As a required check on the change. The analyser runs in continuous integration and a failure blocks the merge. This is uniform and unavoidable, and it converts the analyser into a gate, with the throughput consequences that any serialisation point in the merge path carries. Gates work for properties with near-zero false positives and fail for properties with any meaningful rate, because a developer blocked by a false positive will find the escape hatch, and the escape hatch is usually a blanket suppression for the rule.

As advisory annotations on the diff. Findings appear as comments, the reviewer decides whether to raise them. This preserves the reviewer’s judgement and gives them information they did not have to gather. Its weakness is that it makes the reviewer responsible for triage, and triage is exactly the work that should be automated.

The placement that scales is a combination: gates for rules with a demonstrated low false-positive rate, annotations for the rest, and a separate mechanism for suppressing individual findings with a recorded reason. What does not scale is a single gate over every rule, because the false-positive rate of the union is the sum of the rates, and the sum is what drives developers to disable the tool.

Suppression is the load-bearing mechanism

Every analyser needs a way to say “this finding is understood and accepted”, and the design of that mechanism determines whether the tool survives contact with a codebase.

An inline suppression comment is the common form. It is local, visible at the site, and travels with the code. Its failure mode is accumulation: a suppression written for a genuine reason becomes wrong when the code around it changes, and nothing re-examines it. A suppression without a reason is indistinguishable from a mistake, and a suppression with a reason becomes stale silently.

A baseline file records the set of accepted findings at a point in time and reports only findings outside it. This is the mechanism that makes adoption possible on a codebase with existing findings, because it lets a team start from the current state and prevent regressions without fixing history. Its failure mode is that the baseline becomes the specification of acceptable code: a defect that was in the baseline stays acceptable forever, and the baseline grows because every new finding that a developer does not want to fix gets added to it rather than suppressed individually, which removes the per-finding scrutiny that a comment at least invites.

The third form is a rule-level exclusion for a path or a pattern, such as excluding generated code. This is coarse, necessary, and dangerous in one specific way: the exclusion pattern is a string, and strings drift. A directory rename silently un-excludes generated code, or silently excludes hand-written code, and neither event produces any signal.

The design principle that ties these together is that a suppression should be a claim with an owner and a reason, and the claim should be re-evaluated when the code changes. Tools that support per-finding expiry or review of suppression comments enforce this; tools that do not leave the judgement to whoever reads the file next.

What analysis cannot contribute

The classes an analyser is bad at are the classes human review is also bad at, and the overlap is not a coincidence: both are limited by the absence of a model of intent. Reachability of an error handler, adequacy of a retry policy, whether a cache invalidation covers every write — each requires knowing what the system is supposed to do, and no amount of path exploration supplies that.

The complementary statement is more useful. An analyser’s strength is exactly the set of properties that can be decided from the code alone: definite assignment, resource leaks along a path, type mismatches across a boundary, unreachable branches, and the data-flow reachability that review cannot establish by reading. Delegating those to a tool is not a reduction in review rigour; it is a reallocation of the reviewer’s attention toward the questions only a reader can answer.