Code Review and Static Analysis

Definition: Code Review is peer inspection of proposed pull requests; Static Analysis uses automated tools (Linters/SAST) to detect bugs and security flaws without executing code.

How It Works

Static Analysis parses source code into an Abstract Syntax Tree (AST) to flag unused variables, type mismatches, formatting errors, and OWASP-category security flaws, without ever running the program, common tools include ESLint (JavaScript), Pylint (Python), and SonarQube (multi-language).
Code Review is human evaluation of domain logic correctness, readability, architecture alignment, and test coverage, things automated tools generally can’t judge, like “does this actually solve the right problem” or “is this the right abstraction for this domain.”
SAST (Static Application Security Testing) specifically targets security vulnerabilities: SQL injection patterns, hardcoded secrets, insecure deserialization, and known-vulnerable dependency versions, flagged before code ever reaches production.
Linters enforce style and catch likely-bug patterns (unreachable code, unused imports, comparing a value to NaN with ==), formatters (Prettier, Black, gofmt) go further and rewrite code into a single consistent style automatically, removing style debates entirely.
A typical PR pipeline runs static analysis automatically as a CI gate before a human reviewer is even assigned, so reviewers spend their time on logic and design, not catching a missing semicolon or an unused variable.
Review comments are commonly categorized by severity: blocking (“must fix before merge”), non-blocking (“nice to have, author’s call”), and questions (“help me understand this”), a shared taxonomy keeps reviews efficient and unambiguous.
Pair programming and mob programming are sometimes used as a form of continuous, real-time code review, catching issues as code is written rather than after a pull request is opened.
Type checkers (TypeScript, mypy) sit in between linting and full static analysis, they don’t just flag style, they prove certain classes of bugs (passing a string where a number is expected) are impossible without ever running the code.
Static analysis findings are typically triaged by severity, a critical SQL injection finding blocks a merge automatically, while a minor style suggestion might just be a warning that doesn’t block anything.

Under the Hood

The typical PR pipeline sequence:

open PR -> static analysis / lint -> pass? -> human review -> address feedback -> approve -> merge

Given: a pull request with a missing await on an async database call, a real bug that would cause a race condition in production. Step: run it through both static analysis and human review. Answer: ESLint’s require-await-adjacent rules can catch certain missing-await patterns automatically before a human ever looks at the code, but subtler cases (an await that’s technically present but on the wrong promise) usually require a human reviewer who understands the actual data flow, illustrating why both layers matter, together they catch more than either alone.

Given: a PR that passes all static analysis checks cleanly, then a human reviewer notices the new caching logic will silently serve stale data for logged-out users under a specific edge case. Step: classify this finding. Answer: this is a “blocking” domain-logic issue, exactly the class of problem static analysis structurally cannot catch, since it requires understanding what the feature is supposed to do, not just whether the code is syntactically or stylistically correct.

Given: a 1,200-line pull request touching 30 files, submitted for review all at once. Step: compare expected review quality against the same change split into 5 focused PRs of roughly 200-250 lines each, submitted incrementally. Answer: research on code review effectiveness consistently shows defect-detection rate drops sharply past a few hundred lines per review, reviewers skim rather than read carefully once a diff gets too large, splitting the change catches meaningfully more issues even though total code reviewed is identical.

Given: a codebase with a SonarQube Quality Gate requiring at least 80% test coverage on new code, and a PR that adds a feature with only 60% coverage on the new lines. Step: apply the quality gate rule. Answer: the pipeline blocks the merge automatically, before a human reviewer even needs to raise the coverage gap manually, turning what used to be a recurring, awkward reviewer complaint into an objective, consistently enforced automated gate.

Why It Matters

Ensures consistent codebase standards, shares domain knowledge across engineers (a reviewer often learns the change alongside approving it), and catches bugs early, before they reach production where they’re dramatically more expensive to fix.
Static analysis scales in a way human review can’t: it runs on every single commit, instantly, and never gets tired or distracted, freeing human reviewer attention for the judgment calls only a person can make.
Code review is one of the highest-leverage practices for spreading tacit knowledge across a team, junior engineers learn conventions and senior engineers stay aware of what’s changing across a codebase they don’t touch daily.
SAST tools catching a hardcoded API key or a SQL injection pattern before merge is dramatically cheaper than discovering the same issue after a breach, security-focused static analysis is now standard in most mature CI pipelines.
Review comment threads become a lightweight, searchable record of design decisions and their reasoning, useful months later when someone asks “why was this written this way.”

Common Pitfalls

Bickering over code formatting in human reviews instead of enforcing automated linters and formatters (Prettier, Black), wasting reviewer time and creating friction over something a tool should have already resolved before a human ever saw the diff.
Rubber-stamp reviews: approving a PR without actually reading the diff carefully, defeating the entire purpose of the review while creating a false sense of safety.
Reviewing PRs so large (1000+ lines) that a reviewer can’t reasonably hold the whole change in their head, large PRs get worse review quality on average than a series of small, focused ones.
Treating static analysis warnings as optional noise and letting them accumulate unaddressed, a build with 500 ignored warnings makes it easy to miss the one warning that actually matters.
Making review feedback personal or harsh instead of focused on the code, this discourages people from submitting work for review early and honestly, exactly the opposite of what a healthy review culture needs.
Configuring static analysis rules so strictly, or so loosely, that they either block legitimate work constantly or catch nothing meaningful, rule sets need periodic tuning to stay useful rather than being “set and forget.”
Treating every static analysis finding as equally urgent, without severity triage, teams either drown in low-value warnings or start ignoring the tool output entirely, both outcomes defeat the purpose.
Assigning review to whoever’s available rather than someone with relevant context, a reviewer unfamiliar with the module tends to focus on surface-level nits instead of the logic that actually matters.

Comparison

	Static Analysis / Linting	Human Code Review	Dynamic Testing
Runs code	No	No (reads the diff)	Yes
Catches	Style, known bug patterns, some security flaws	Logic errors, design issues, domain correctness	Runtime bugs, actual behavior under real inputs
Speed	Seconds, fully automated	Minutes to hours, depends on reviewer availability	Seconds to minutes, depends on suite size
Judgment required	None, rule-based	High, contextual and domain-specific	None, assertion-based
Scales with team size	Yes, effortlessly	Limited by reviewer bandwidth	Yes, effortlessly

Static Analysis Tool Reference

Tool	Language / scope	Focus
ESLint	JavaScript / TypeScript	Linting, code quality rules
Pylint / Ruff	Python	Linting, style, some bug patterns
SonarQube	Multi-language	Code quality, security, technical debt tracking
Prettier / Black	JavaScript / Python	Automated formatting, no judgment calls
Semgrep / CodeQL	Multi-language	Security-focused static analysis (SAST)

Review Comment Severity Reference

Category	Meaning	Blocks merge?
Blocking	Must be fixed, a bug, security issue, or design flaw	Yes
Non-blocking	A suggestion, author decides whether to apply it	No
Question	Reviewer needs clarification before deciding	Usually, until answered
Nitpick	Minor style or wording preference	No, often labeled explicitly as such

Common OWASP Categories SAST Tools Flag

Category	Example flaw
Injection	Unsanitized input concatenated into a SQL query
Broken authentication	Hardcoded credentials or weak session handling
Sensitive data exposure	Secrets committed in plaintext to the repository
Security misconfiguration	Debug mode or verbose errors left enabled in production

Example

ESLint flagging a missing await statement, or ESLint’s no-unused-vars rule catching a leftover debug import, runs automatically before a PR is even assigned to a team member, giving instant feedback that a human reviewer never needs to manually point out.

GitHub’s built-in review workflow (requested reviewers, inline comments, required approvals before merge) is the most widely used code review tooling today, often paired with GitHub Actions running ESLint, SonarQube, or CodeQL as required status checks before the merge button unlocks.

SonarQube’s “Quality Gate” feature blocks a merge automatically if new code introduces too many code smells, too much duplication, or drops test coverage below a configured threshold, turning code quality standards into an enforced CI gate rather than a matter of reviewer diligence alone.

GitHub’s CodeQL, used internally to scan major open-source projects, performs semantic analysis rather than plain pattern matching, it can trace how untrusted user input flows through a codebase and flag it if it reaches a dangerous function without being sanitized, a technique called taint analysis.

Referenced by

Dig deeper

Spot an error in this note?