Suspicious places¶

The option --suspicious adds a second kind of output. A finding states a defect. A suspicious place states that the code needs a reader. Visao keeps the two apart. A suspicious place never changes the exit status, and Visao prints it with the lines of code around it.

The mode serves three jobs:

Review triage. It states where to look first in a change, or in a file that nobody on the team wrote.
Onboarding. It states what is unusual in a codebase, measured against the rest of that codebase.
Human judgement. It lists the shapes that are correct in one codebase and a mistake in another.

Each rule reads a language-agnostic layer. A rule reads the universal AST in UniVisitor_Suspect.cpp, or the VM commands in CommandAnalyzer_*. Each rule therefore applies to all languages that Visao parses, and a new language receives the complete set.

Classification¶

Each suspicious place carries a category, a confidence and a rule id. The category states the kind of observation. The confidence states how probable it is that a reader wants to see it. There is no severity, because severity belongs to findings.

Category	Contents
`copy-paste`	Code that repeats: two identical arms of a branch, two identical bodies in a switch, a condition that a chain tests two times.
`inconsistency`	Code that differs from the habit of its own file. The file checks the result of a call at each other call site. The file calls a function with a different order of arguments.
`complexity`	The nesting depth, the number of statements, the number of parameters, the length of a boolean expression and the number of exits.
`naming`	Two names in one scope that differ by one character. A name that states one type while the declaration states another.
`magic-value`	The same unexplained literal in several places. A hardcoded path, host, port or timeout.
`error-handling`	A handler that never reads the value that it caught. A returned error value that the caller drops. A handler that catches each type.
`boundary`	The shapes of an off-by-one: `<=` against a length, `+1` and `-1` around an index, a loop that starts at one over a container that counts from zero.
`dead-ish`	A value that the code computes and drops. A write that the next write replaces.
`concurrency`	A sleep that orders two operations. Shared mutable state with no lock near it. A lock that one path takes and another path releases.
`api-shape`	A result that this call site drops and each other call site reads. A parameter that receives the same literal at each call site.
`type-mixing`	A comparison between two kinds of value. Arithmetic on a value with a string type.
`todo`	The markers `TODO`, `FIXME`, `HACK` and `XXX`, and code in comments.
`security`	The code that a reviewer always reads: a check of a password or a secret, an access decision, a sign-in, a cryptographic primitive, a token, and a change of privilege.

Confidence¶

high: The shape is unusual, and a reviewer asks about it. Visao prints it by default.
medium: The shape is worth a look in unfamiliar code. Visao prints it by default.
low: The shape is an inventory item. Visao counts it always and prints it with --suspicious-all.

The security category holds a different question from the other categories. The other rules state that the code looks unusual. The security rules state that the code decides something, and that a reader must confirm the decision. A password check with no defect in it is the place where a mistake stays invisible, so Visao lists it each time.

Relation to findings¶

A rule that states a defect belongs with the defect rules. The suspicious places layer does not repeat such a rule. It holds no entry for a null dereference, for an index after the end of a container or for an unreleased handle. Visao reports these as findings.

Rules¶

The catalogue holds the taxonomy. The table below states the rules and the layer of each one. The layer decides the reach.

Rule	Category	Layer and reach
`identical-branch-arms`	copy-paste	Universal AST. Compares the two arms as the reader sees them. Reports where the front-end supplies the extent of a construct: the tree-sitter languages, C and C++.
`identical-case-bodies`	copy-paste	Universal AST, with the same reach.
`repeated-condition`	copy-paste	Universal AST, with the same reach.
`long-boolean-chain`	complexity	Universal AST, over the lowered condition. All languages.
`deep-nesting`, `long-function`, `many-exits`	complexity	Universal AST, over the body of the function. All languages.
`many-parameters`	complexity	Universal AST. A language that groups parameters in its tree, such as Go with `a, b int`, counts one group as one parameter.
`less-equal-against-length`	boundary	Universal AST, over the lowered condition of the loop. Asks the container vocabulary for the names of a length. All languages.
`loop-from-one`	boundary	Universal AST, for the loops that hold a counter. Lua counts from one, so the rule reports nothing there.
`todo-marker`, `commented-out-code`	todo	The text of the input, one time for each file. All languages.
`hardcoded-endpoint`, `repeated-literal`	magic-value	The text of the input. All languages.
`password-check`, `authorization-decision`, `authentication-flow`, `crypto-primitive`, `token-handling`, `privilege-change`	security	Two layers and two dictionaries. Visao lists a function at its declaration when the name of the function states what it does, through `Utils::SecuritySurface` on the universal AST. Visao lists a call at the call site when the resource vocabulary holds the API. All languages.

Output¶

Console¶

Each suspicious place prints four things. It prints its classification. It prints its position. It prints the code around it. It prints two sentences: what looks unusual, and what to check.

SUSPICIOUS  copy-paste  high   both arms of the branch are identical
  src/parser.c:116-120
    114 |     int n = read_len(s);
    115 |
  > 116 |     if (flags & F_UPPER)
  > 117 |         out = to_upper(in);
  > 118 |     else
  > 119 |         out = to_upper(in);
    120 |
  odd:   the two arms differ in no token, so the condition decides nothing
  check: was one arm meant to differ?

A run ends with a count for each category. A large codebase reads from the top down.

Machine formats¶

--suspicious-output <file> writes JSON. The file holds one array with one object for each suspicious place. Each object holds category, confidence, rule, ruleId, title, odd, check, file, line, endLine, column and snippet. The value of snippet is an array of {line, text, marked}.

SARIF output holds the same information. Each result carries kind: "review" and level: "note". The rule id is visao/suspicious/<category>/<rule>, and the tags carry the category and the confidence. A code review interface therefore shows the block, and a gate that reads findings does not count it.

Over the protocol¶

In server mode the analysis result holds a section of its own. It sits beside the findings, the SBOM and the authorship sections:

{
  "report":    [ ... SARIF ... ],
  "suspicion": [
    {
      "category":   "security",
      "confidence": "high",
      "rule":       "password-check",
      "ruleId":     "visao/suspicious/security/password-check",
      "title":      "a password or secret is handled here (check_password)",
      "odd":        "...",
      "check":      "...",
      "file": "auth.py", "line": 6, "endLine": 9, "column": 1,
      "snippet": [ { "line": 5, "text": "...", "marked": false } ]
    }
  ]
}

Visao emits each suspicious place over the protocol. The client filters by category and by confidence, because the protocol is not a user interface. An analysis requests the mode with the names of the flags, with an underscore: suspicious, suspicious_only, suspicious_all, suspicious_categories, suspicious_output and suspicious_context. See Server mode.

Flags¶

--suspicious prints the suspicious places with the findings.
--suspicious-only prints the suspicious places and no findings.
--suspicious-all includes the low confidence.
--suspicious-categories <a,b,...> restricts the output to these categories.
--suspicious-output <file> writes the JSON file.
--suspicious-context <N> sets the number of code lines around each place. The default is 2.

A suspicious place never changes the result of --fail-on. A build does not break because code needs a reader.