commit 55c8a0957794545579c72f1b791a5abee71eb041
parent bb1c0452c45545c3c8d4d5bf6ef4089405b2e458
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Wed, 23 Sep 2026 20:16:43 -0300
odin: run the compilers and analysers over the change
analyser runs what the repository's languages already have — go build,
vet with review-vet where it is installed, staticcheck, odin check with
the ols.json collections, tsc, ruff, mypy, cargo with clippy where it is,
and semgrep — on the tree the change arrives at, each bounded by a
timeout, and keeps a fault wherever it lands and an opinion only on a
line the change added. The parsers are tested on canned output; go vet
is run live. On commit 8070974 the Odin reading reports the same 17
findings as the Go tool, id for id.
Both tools skipped go vet on this repository because the vet sidecar is
a module of its own under sidecar/govet: a package under a nested go.mod
is left out of the list now, in both.
Diffstat:
9 files changed, 1550 insertions(+), 4 deletions(-)
diff --git a/analysers.go b/analysers.go
@@ -211,6 +211,11 @@ func goPackages(tree string, files []string) []string {
if _, err := os.Stat(filepath.Join(tree, dir)); err != nil {
continue
}
+ // A directory under a go.mod of its own is another module's, and
+ // the go command run at the root cannot name it.
+ if nestedModule(tree, dir) {
+ continue
+ }
dirs[dir] = true
}
var out []string
@@ -221,6 +226,17 @@ func goPackages(tree string, files []string) []string {
return out
}
+// nestedModule is whether a directory sits under a go.mod below the
+// tree's root, which makes it another module's package.
+func nestedModule(tree, dir string) bool {
+ for d := dir; d != "." && d != "" && d != "/"; d = filepath.Dir(d) {
+ if _, err := os.Stat(filepath.Join(tree, d, "go.mod")); err == nil {
+ return true
+ }
+ }
+ return false
+}
+
// position matches the file:line:column: message the Go tools print.
var position = regexp.MustCompile(`^(.+?):(\d+)(?::(\d+))?: (.*)$`)
diff --git a/analysers_test.go b/analysers_test.go
@@ -298,3 +298,19 @@ func TestOdinCollectionsReadOlsJSON(t *testing.T) {
t.Errorf("got %v, want %v", got, want)
}
}
+
+func TestGoPackagesSkipsNestedModules(t *testing.T) {
+ dir := t.TempDir()
+ for _, path := range []string{"go.mod", "a/a.go", "sidecar/govet/go.mod", "sidecar/govet/main.go", "sidecar/gofront/main.go"} {
+ if err := os.MkdirAll(filepath.Join(dir, filepath.Dir(path)), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(filepath.Join(dir, path), []byte("x"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ }
+ got := goPackages(dir, []string{"a/a.go", "sidecar/govet/main.go", "sidecar/gofront/main.go", "vendor/x/x.go"})
+ if want := []string{"./a", "./sidecar/gofront"}; !slices.Equal(got, want) {
+ t.Errorf("got %v, want %v", got, want)
+ }
+}
diff --git a/justfile b/justfile
@@ -41,7 +41,7 @@ odin-test:
#!/usr/bin/env bash
set -euo pipefail
mkdir -p build
- for p in frontend git tree change finding report check; do
+ for p in frontend git tree change finding report check analyser; do
{{odin}} test odin/$p {{odin_flags}} -out:build/${p}_test
done
diff --git a/odin/analyser/analyser.odin b/odin/analyser/analyser.odin
@@ -0,0 +1,342 @@
+/*
+Package analyser runs the compilers and analysers a repository's languages
+already have, which are the deterministic checks with the most to say.
+What review decides is which of their findings belong to the change: an
+error anywhere in a unit the change touched is the change's to answer,
+because the tree does not compile until it is; a warning is the change's
+only where it lands on a line the change added. Each is run with its
+strictest settings and asked for JSON, on the tree the change arrives at.
+*/
+package analyser
+
+import "core:fmt"
+import "core:os"
+import "core:path/filepath"
+import "core:strconv"
+import "core:strings"
+import "core:text/regex"
+import "jfm:sh"
+
+import "../change"
+import "../finding"
+import "../tree"
+
+// timeout_seconds bounds one analyser's run. A cold first analysis of a
+// large module can take minutes; past this the analyser says nothing
+// rather than holding the review.
+timeout_seconds :: 300
+
+// Diagnostic is one thing an analyser said, located in the tree. A fault
+// is a compile error — the tree does not build — rather than an
+// analyser's opinion about code that does.
+Diagnostic :: struct {
+ file: string,
+ line: int,
+ code: string,
+ message: string,
+ severity: finding.Severity,
+ fault: bool,
+}
+
+// Analyser is one compiler or analyser, run over the units the change's
+// files belong to. name prefixes its rule ids: name/code. covers is
+// whether a path is one of its language. ready is whether it can run in
+// this tree, with the reason it cannot, told once. run analyses the units
+// the files belong to and returns what it found with paths relative to
+// the tree, or why it could not. in_place is whether it needs the working
+// tree's surroundings — installed packages, a node_modules — and so cannot
+// read a range's materialised tree.
+Analyser :: struct {
+ name: string,
+ covers: proc(path: string) -> bool,
+ ready: proc(tree: string) -> (ok: bool, why: string),
+ run: proc(tree, root: string, files: []string) -> (found: []Diagnostic, err: string),
+ in_place: bool,
+}
+
+// all is the compilers and analysers review knows how to run, in the
+// order their findings are worth having.
+all :: proc(allocator := context.temp_allocator) -> []Analyser {
+ list := make([]Analyser, 9, allocator)
+ list[0], list[1], list[2] = go_build, go_vet, staticcheck
+ list[3], list[4], list[5] = odin_check, tsc, ruff
+ list[6], list[7], list[8] = mypy, cargo, semgrep
+ return list
+}
+
+// check runs every analyser that covers a changed file and can run, and
+// keeps the findings that belong to the change. What could not run is
+// said on stderr.
+check :: proc(
+ c: ^change.Change,
+ t: tree.Tree,
+ ranged: bool,
+ analysers := []Analyser{},
+ allocator := context.allocator,
+) -> []finding.Finding {
+ context.allocator = allocator
+ out := make([dynamic]finding.Finding)
+ changed := make(map[string]bool, context.temp_allocator)
+ for f in c.files {
+ changed[f] = true
+ }
+ which := analysers if len(analysers) > 0 else all()
+ for a in which {
+ files := make([dynamic]string, context.temp_allocator)
+ for f in c.files {
+ if a.covers(f) && tree.exists(t, f) {
+ append(&files, f)
+ }
+ }
+ if len(files) == 0 {
+ continue
+ }
+ if ok, why := a.ready(t.dir); !ok {
+ if why != "" {
+ fmt.eprintfln("skipping %s: %s", a.name, why)
+ }
+ continue
+ }
+ if a.in_place && ranged {
+ fmt.eprintfln(
+ "skipping %s: it reads the working tree, and the change is a range",
+ a.name,
+ )
+ continue
+ }
+ found, err := a.run(t.dir, t.root, files[:])
+ if err != "" {
+ fmt.eprintfln("skipping %s: %s", a.name, err)
+ continue
+ }
+ seen := make(map[string]bool, context.temp_allocator)
+ for d in found {
+ if d.file == "" || d.line == 0 {
+ continue
+ }
+ // A fault is the change's wherever it lands: the unit it
+ // touched no longer compiles. An opinion is the change's only
+ // on a line it added.
+ if !d.fault && (!changed[d.file] || !added_line(c, d.file, d.line)) {
+ continue
+ }
+ key := fmt.tprintf("%s:%d:%s:%s", d.file, d.line, d.code, d.message)
+ if seen[key] {
+ continue
+ }
+ seen[strings.clone(key, context.temp_allocator)] = true
+ rule := a.name if d.code == "" else fmt.aprintf("%s/%s", a.name, d.code)
+ append(
+ &out,
+ finding.Finding {
+ job = "static",
+ rule = rule,
+ severity = d.severity,
+ file = strings.clone(d.file),
+ line = d.line,
+ message = strings.clone(d.message),
+ verified = true,
+ },
+ )
+ }
+ }
+ return out[:]
+}
+
+// added_line is whether the diff added the line of the file.
+added_line :: proc(c: ^change.Change, file: string, line: int) -> bool {
+ lines := c.added[file]
+ for l in lines {
+ if l.line == line {
+ return true
+ }
+ }
+ return false
+}
+
+// execute works one command in the tree and returns its output. A command
+// that reports findings by exiting non-zero is not a failed run: its
+// stdout is the answer, and only an empty stdout with a non-zero exit is a
+// fault. A run past the timeout is a fault of its own.
+execute :: proc(
+ dir: string,
+ name: string,
+ args: []string,
+ allocator := context.allocator,
+) -> (
+ stdout: string,
+ err: string,
+) {
+ out, errs, e := execute_both(dir, name, args, allocator)
+ if e != "" && len(out) == 0 {
+ return "", e if errs == "" else e
+ }
+ return out, ""
+}
+
+// execute_both works one command and returns both streams: the Odin
+// compiler writes its JSON to stderr, and a tool that fails before it
+// starts says why there too.
+execute_both :: proc(
+ dir: string,
+ name: string,
+ args: []string,
+ allocator := context.allocator,
+) -> (
+ stdout, stderr: string,
+ err: string,
+) {
+ argv := make([dynamic]string, context.temp_allocator)
+ if _, found := sh.which("timeout", context.temp_allocator); found {
+ append(&argv, "timeout", fmt.tprintf("%d", timeout_seconds))
+ }
+ append(&argv, name)
+ append(&argv, ..args)
+ r := sh.exec(argv[:], {dir = dir}, allocator)
+ if r.err != nil {
+ return "", "", fmt.aprintf("%s: %s", name, os.error_string(r.err), allocator = allocator)
+ }
+ if r.code == 124 && len(argv) > 2 && argv[0] == "timeout" {
+ return r.stdout, r.stderr, fmt.aprintf("a run past %d seconds is not waited for", timeout_seconds, allocator = allocator)
+ }
+ if !r.ok {
+ detail := strings.trim_space(r.stderr)
+ if detail == "" {
+ detail = fmt.tprintf("exit %d", r.code)
+ }
+ return r.stdout, r.stderr, fmt.aprintf("%s: %s", name, tail(detail, 200), allocator = allocator)
+ }
+ return r.stdout, r.stderr, ""
+}
+
+// on_path is a ready that needs only the binary.
+on_path :: proc(binary: string) -> bool {
+ _, found := sh.which(binary, context.temp_allocator)
+ return found
+}
+
+// relative is a path relative to the tree, where it lies inside it.
+relative :: proc(tree_dir, path: string) -> string {
+ if strings.has_prefix(path, tree_dir) &&
+ len(path) > len(tree_dir) &&
+ path[len(tree_dir)] == '/' {
+ return path[len(tree_dir) + 1:]
+ }
+ if strings.has_prefix(path, "./") {
+ return path[2:]
+ }
+ return path
+}
+
+// join is a path under a directory.
+join :: proc(dir, name: string, allocator := context.temp_allocator) -> string {
+ return filepath.join({dir, name}, allocator) or_else name
+}
+
+// dir_of is the directory a path sits in, "." at the top.
+dir_of :: proc(path: string) -> string {
+ if i := strings.last_index_byte(path, '/'); i >= 0 {
+ return path[:i]
+ }
+ return "."
+}
+
+// nearest is the path, relative to the tree, of the first file called
+// name in dir or a directory above it, or empty.
+nearest :: proc(tree_dir, dir, name: string) -> string {
+ d := dir
+ for {
+ candidate :=
+ name if d == "." || d == "" else strings.concatenate({d, "/", name}, context.temp_allocator)
+ if os.is_file(join(tree_dir, candidate)) {
+ return candidate
+ }
+ if d == "." || d == "" || d == "/" {
+ return ""
+ }
+ d = dir_of(d)
+ }
+}
+
+// tail is the last few bytes of a longer text, for an error message.
+tail :: proc(s: string, n: int) -> string {
+ trimmed := strings.trim_space(s)
+ if len(trimmed) <= n {
+ return trimmed
+ }
+ return strings.concatenate({"…", trimmed[len(trimmed) - n:]}, context.temp_allocator)
+}
+
+// position reads file and line out of file:line:col: message, as the Go
+// tools print it.
+position :: proc(text: string) -> (file: string, line: int, message: string, ok: bool) {
+ re, err := regex.create(
+ `^(.+?):(\d+)(?::(\d+))?: (.*)$`,
+ {},
+ context.temp_allocator,
+ context.temp_allocator,
+ )
+ if err != nil {
+ return
+ }
+ cap, matched := regex.match(re, text, context.temp_allocator)
+ if !matched {
+ return
+ }
+ line, _ = strconv.parse_int(cap.groups[2])
+ return cap.groups[1], line, cap.groups[4], true
+}
+
+// split_json_objects cuts a stream of top-level JSON objects, with
+// anything between them — vet's # comment lines, prose about a failed
+// package — left out. An object opens only at the start of a line, as vet
+// writes them, so a brace in the prose opens nothing.
+split_json_objects :: proc(out: string, allocator := context.allocator) -> []string {
+ chunks := make([dynamic]string, allocator)
+ depth, start := 0, -1
+ in_string, line_start := false, true
+ i := 0
+ for i < len(out) {
+ c := out[i]
+ switch {
+ case in_string:
+ if c == '\\' {
+ i += 1
+ } else if c == '"' {
+ in_string = false
+ }
+ case depth > 0 && c == '"':
+ in_string = true
+ case c == '{' && (depth > 0 || line_start):
+ if depth == 0 {
+ start = i
+ }
+ depth += 1
+ case c == '}' && depth > 0:
+ depth -= 1
+ if depth == 0 {
+ append(&chunks, out[start:i + 1])
+ start = -1
+ }
+ }
+ line_start = c == '\n'
+ i += 1
+ }
+ return chunks[:]
+}
+
+// object is the outermost JSON object in a text that has prose around it.
+object :: proc(text: string) -> (string, bool) {
+ start := strings.index_byte(text, '{')
+ end := strings.last_index_byte(text, '}')
+ if start < 0 || end < start {
+ return "", false
+ }
+ return text[start:end + 1], true
+}
+
+// lines_of is a text's lines, for the tools that write one JSON object
+// per line.
+lines_of :: proc(text: string, allocator := context.temp_allocator) -> []string {
+ return strings.split_lines(text, allocator)
+}
diff --git a/odin/analyser/analyser_test.odin b/odin/analyser/analyser_test.odin
@@ -0,0 +1,276 @@
+package analyser
+
+import "core:fmt"
+import "core:os"
+import "core:path/filepath"
+import "core:testing"
+import "jfm:sh"
+
+import "../change"
+import "../finding"
+import "../tree"
+
+@(test)
+parse_go_build_reads_the_errors :: proc(t: ^testing.T) {
+ out := "{\"ImportPath\":\"probe\",\"Action\":\"build-output\",\"Output\":\"# probe\\n\"}\n{\"ImportPath\":\"probe\",\"Action\":\"build-output\",\"Output\":\"./a.go:3:15: undefined: helper\\n\"}\n{\"ImportPath\":\"probe\",\"Action\":\"build-fail\"}\n"
+ got := parse_go_build("/tree", out, context.temp_allocator)
+ testing.expect_value(t, len(got), 1)
+ if len(got) == 1 {
+ testing.expect_value(t, got[0].file, "a.go")
+ testing.expect_value(t, got[0].line, 3)
+ testing.expect(t, got[0].fault)
+ testing.expect_value(t, got[0].message, "undefined: helper")
+ }
+}
+
+@(test)
+parse_go_vet_reads_each_analyser :: proc(t: ^testing.T) {
+ out := "# probe\n{\n\t\"probe\": {\n\t\t\"printf\": [{\"posn\": \"/tree/a.go:6:14\", \"message\": \"wrong type\"}],\n\t\t\"shadow\": [{\"posn\": \"/tree/b.go:9:2\", \"message\": \"declaration of err shadows\"}]\n\t}\n}\n# other\nvet: other/x.go:3: undefined: y\n"
+ got := parse_go_vet("/tree", out, context.temp_allocator)
+ testing.expect_value(t, len(got), 2)
+ by := make(map[string]Diagnostic, context.temp_allocator)
+ for d in got {
+ by[d.code] = d
+ }
+ testing.expect_value(t, by["printf"].file, "a.go")
+ testing.expect_value(t, by["printf"].line, 6)
+ testing.expect_value(t, by["printf"].severity, finding.Severity.Must_Fix)
+ testing.expect_value(t, by["shadow"].severity, finding.Severity.Consider)
+ Case :: struct {
+ name: string,
+ want: finding.Severity,
+ }
+ for c in ([]Case{{"printf", .Must_Fix}, {"nilness", .Must_Fix}, {"shadow", .Consider}, {"unusedwrite", .Consider}, {"rangeint", .Note}, {"stringscut", .Note}}) {
+ testing.expectf(t, vet_severity(c.name) == c.want, "%s: %v", c.name, vet_severity(c.name))
+ }
+ chunks := split_json_objects(
+ "# a\n{\"x\": {\"y\": \"}\"}}\nprose {not\n{\"z\": 1}\n",
+ context.temp_allocator,
+ )
+ testing.expect_value(t, len(chunks), 2)
+ if len(chunks) == 2 {
+ testing.expect_value(t, chunks[0], `{"x": {"y": "}"}}`)
+ testing.expect_value(t, chunks[1], `{"z": 1}`)
+ }
+ file, line := split_position("/tree/a.go:6:14")
+ testing.expect_value(t, file, "/tree/a.go")
+ testing.expect_value(t, line, 6)
+ file, line = split_position("a.go:9")
+ testing.expect_value(t, file, "a.go")
+ testing.expect_value(t, line, 9)
+}
+
+@(test)
+parse_odin_tells_vet_from_type_errors :: proc(t: ^testing.T) {
+ out := `{"error_count": 3, "errors": [
+ {"type": "error", "pos": {"file": "/tree/lib/a.odin", "line": 6}, "msgs": ["'unused' declared but not used"]},
+ {"type": "warning", "pos": {"file": "/tree/lib/a.odin", "line": 4}, "msgs": ["Syntax Error: With '-strict-style' the attached brace style (1TBS) is enforced"]},
+ {"type": "error", "pos": {"file": "/tree/lib/a.odin", "line": 9}, "msgs": ["Undeclared name: foo"]}
+ ]}`
+ got := parse_odin("/tree", out, context.temp_allocator)
+ testing.expect_value(t, len(got), 3)
+ if len(got) == 3 {
+ testing.expect_value(t, got[0].code, "vet")
+ testing.expect_value(t, got[0].severity, finding.Severity.Consider)
+ testing.expect(t, !got[0].fault)
+ testing.expect_value(t, got[1].code, "style")
+ testing.expect_value(t, got[1].severity, finding.Severity.Note)
+ testing.expect_value(t, got[2].code, "")
+ testing.expect(t, got[2].fault)
+ testing.expect_value(t, got[2].file, "lib/a.odin")
+ }
+}
+
+@(test)
+parse_tsc_ruff_mypy_cargo_semgrep :: proc(t: ^testing.T) {
+ ts := parse_tsc(
+ "/tree",
+ "/tree/web",
+ "src/a.ts(12,5): error TS2322: Type 'string' is not assignable to type 'number'.\nnoise\n",
+ context.temp_allocator,
+ )
+ testing.expect_value(t, len(ts), 1)
+ if len(ts) == 1 {
+ testing.expect_value(t, ts[0].file, "web/src/a.ts")
+ testing.expect_value(t, ts[0].line, 12)
+ testing.expect_value(t, ts[0].code, "TS2322")
+ testing.expect(t, ts[0].fault)
+ }
+ rf, rerr := parse_ruff(
+ "/tree",
+ `[{"code":"F401","message":"os imported but unused","filename":"/tree/a.py","location":{"row":1,"column":8}},
+ {"code":"E501","message":"Line too long","filename":"/tree/a.py","location":{"row":9,"column":89}}]`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, rerr, "")
+ testing.expect_value(t, len(rf), 2)
+ if len(rf) == 2 {
+ testing.expect_value(t, rf[0].severity, finding.Severity.Consider)
+ testing.expect_value(t, rf[0].file, "a.py")
+ testing.expect_value(t, rf[1].severity, finding.Severity.Note)
+ }
+ _, prose := parse_ruff("/tree", "not json", context.temp_allocator)
+ testing.expect(t, prose != "", "prose was read as a report")
+ my := parse_mypy(
+ "/tree",
+ `{"file": "a.py", "line": 4, "column": 4, "message": "Incompatible return value type", "hint": null, "code": "return-value", "severity": "error"}
+{"file": "a.py", "line": 4, "column": 4, "message": "See the docs", "hint": null, "code": "return-value", "severity": "note"}
+`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, len(my), 2)
+ if len(my) == 2 {
+ testing.expect_value(t, my[0].severity, finding.Severity.Must_Fix)
+ testing.expect_value(t, my[0].code, "return-value")
+ testing.expect_value(t, my[1].severity, finding.Severity.Note)
+ }
+ cg := parse_cargo(
+ "/tree",
+ "/tree/crate",
+ `{"reason":"compiler-artifact","target":{}}
+{"reason":"compiler-message","message":{"level":"error","message":"cannot find value x","code":{"code":"E0425"},"spans":[{"file_name":"src/main.rs","line_start":3,"is_primary":false},{"file_name":"src/main.rs","line_start":4,"is_primary":true}]}}
+{"reason":"compiler-message","message":{"level":"warning","message":"unused variable","code":{"code":"unused_variables"},"spans":[{"file_name":"src/lib.rs","line_start":7,"is_primary":true}]}}
+{"reason":"compiler-message","message":{"level":"note","message":"aborting","code":null,"spans":[]}}
+`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, len(cg), 2)
+ if len(cg) == 2 {
+ testing.expect_value(t, cg[0].file, "crate/src/main.rs")
+ testing.expect_value(t, cg[0].line, 4)
+ testing.expect_value(t, cg[0].code, "E0425")
+ testing.expect(t, cg[0].fault)
+ testing.expect_value(t, cg[1].severity, finding.Severity.Consider)
+ testing.expect(t, !cg[1].fault)
+ }
+ sg, serr := parse_semgrep(
+ "/tree",
+ `{"results":[
+ {"check_id":"go.lang.security.audit.crypto.math_random","path":"a.go","start":{"line":12},"extra":{"message":"math/rand is not secure\n","severity":"WARNING"}},
+ {"check_id":"python.lang.best-practice.open-never-closed","path":"/tree/b.py","start":{"line":3},"extra":{"message":"file never closed","severity":"ERROR"}},
+ {"check_id":"generic.note","path":"c.js","start":{"line":1},"extra":{"message":"fyi","severity":"INFO"}}
+ ],"errors":[]}`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, serr, "")
+ testing.expect_value(t, len(sg), 3)
+ if len(sg) == 3 {
+ testing.expect_value(t, sg[0].severity, finding.Severity.Consider)
+ testing.expect_value(t, sg[0].message, "math/rand is not secure")
+ testing.expect_value(t, sg[1].severity, finding.Severity.Must_Fix)
+ testing.expect_value(t, sg[1].file, "b.py")
+ testing.expect_value(t, sg[2].severity, finding.Severity.Note)
+ }
+ sc := parse_staticcheck(
+ "/tree",
+ `{"code":"SA4006","location":{"file":"/tree/a.go","line":5},"message":"never used"}
+{"code":"compile","location":{"file":"/tree/a.go","line":1},"message":"broken"}
+{"code":"ST1000","location":{"file":"/tree/a.go","line":1},"message":"style"}
+`,
+ context.temp_allocator,
+ )
+ testing.expect_value(t, len(sc), 2)
+ if len(sc) == 2 {
+ testing.expect_value(t, sc[0].severity, finding.Severity.Must_Fix)
+ testing.expect_value(t, sc[1].severity, finding.Severity.Note)
+ }
+ testing.expect_value(t, staticcheck_severity("S1002"), finding.Severity.Consider)
+ testing.expect_value(t, staticcheck_severity("U1000"), finding.Severity.Consider)
+}
+
+@(test)
+nearest_walks_up :: proc(t: ^testing.T) {
+ temp := os.temp_directory(context.temp_allocator) or_else ""
+ root, err := os.make_directory_temp(temp, "review-analyser-*", context.temp_allocator)
+ testing.expect(t, err == nil)
+ defer os.remove_all(root)
+ testing.expect(t, os.make_directory_all(join(root, "web/src/deep")) == nil)
+ testing.expect(
+ t,
+ os.write_entire_file(join(root, "web/tsconfig.json"), transmute([]byte)string("{}")) ==
+ nil,
+ )
+ testing.expect_value(t, nearest(root, "web/src/deep", "tsconfig.json"), "web/tsconfig.json")
+ testing.expect_value(t, nearest(root, "web/src/deep", "Cargo.toml"), "")
+ testing.expect_value(t, relative("/tree", "/tree/a/b.go"), "a/b.go")
+ testing.expect_value(t, relative("/tree", "./a.go"), "a.go")
+ testing.expect_value(t, relative("/tree", "/elsewhere/a.go"), "/elsewhere/a.go")
+ testing.expect_value(t, dir_of("a.go"), ".")
+ testing.expect_value(t, dir_of("x/y/a.go"), "x/y")
+
+ for path in ([]string{"a", "sidecar/govet", "sidecar/gofront"}) {
+ testing.expect(t, os.make_directory_all(join(root, path)) == nil)
+ }
+ for path in ([]string{"go.mod", "sidecar/govet/go.mod"}) {
+ testing.expect(t, os.write_entire_file(join(root, path), transmute([]byte)string("module x")) == nil)
+ }
+ pkgs := go_packages(root, {"a/a.go", "sidecar/govet/main.go", "sidecar/gofront/main.go", "vendor/x/x.go"})
+ testing.expect_value(t, fmt.tprint(pkgs), `["./a", "./sidecar/gofront"]`)
+}
+
+// go_fixture is a module with one package: a printf fault on line 6 that
+// vet sees, and nothing the compiler minds.
+go_fixture :: proc(t: ^testing.T) -> (root: string, ok: bool) {
+ temp := os.temp_directory(context.temp_allocator) or_else ""
+ scratch, err := os.make_directory_temp(temp, "review-govet-*", context.temp_allocator)
+ if err != nil {
+ testing.fail_now(t, "no scratch directory")
+ }
+ root = scratch
+ git := proc(root: string, args: ..string) -> bool {
+ argv := make([dynamic]string, context.temp_allocator)
+ append(
+ &argv,
+ "git",
+ "-c",
+ "user.email=t@t",
+ "-c",
+ "user.name=t",
+ "-c",
+ "commit.gpgsign=false",
+ )
+ append(&argv, ..args)
+ return sh.exec(argv[:], {dir = root}, context.temp_allocator).ok
+ }
+ ok = git(root, "init", "-q")
+ ok &&=
+ os.write_entire_file(
+ join(root, "go.mod"),
+ transmute([]byte)string("module probe\n\ngo 1.27.0\n"),
+ ) ==
+ nil
+ ok &&=
+ os.write_entire_file(
+ join(root, "a.go"),
+ transmute([]byte)string(
+ "package probe\n\nimport \"fmt\"\n\nfunc F() {\n\tfmt.Printf(\"%d\", \"s\")\n}\n",
+ ),
+ ) ==
+ nil
+ ok &&= git(root, "add", "go.mod", "a.go")
+ return root, ok
+}
+
+@(test)
+go_vet_reads_the_change :: proc(t: ^testing.T) {
+ if !on_path("go") {
+ testing.fail_now(t, "go is not on the path")
+ }
+ root, made := go_fixture(t)
+ testing.expect(t, made)
+ defer os.remove_all(root)
+ c, gathered := change.gather("", root, context.temp_allocator)
+ testing.expect(t, gathered)
+ tr, at_ok := tree.at(root, "", context.temp_allocator)
+ testing.expect(t, at_ok)
+ findings := check(&c, tr, false, {go_build, go_vet}, context.temp_allocator)
+ testing.expect_value(t, len(findings), 1)
+ if len(findings) == 1 {
+ testing.expect_value(t, findings[0].rule, "go-vet/printf")
+ testing.expect_value(t, findings[0].file, "a.go")
+ testing.expect_value(t, findings[0].line, 6)
+ testing.expect_value(t, findings[0].severity, finding.Severity.Must_Fix)
+ testing.expect(t, findings[0].verified)
+ }
+ _ = fmt.tprint(filepath.SEPARATOR)
+}
diff --git a/odin/analyser/go.odin b/odin/analyser/go.odin
@@ -0,0 +1,354 @@
+package analyser
+
+import "core:encoding/json"
+import "core:os"
+import "core:slice"
+import "core:strings"
+import "jfm:sh"
+
+import "../finding"
+
+is_go :: proc(path: string) -> bool {
+ return strings.has_suffix(path, ".go")
+}
+
+// go_ready is whether a Go tool can run: the binary, and a module at the
+// tree's root.
+go_ready :: proc(binary: string, tree_dir: string) -> (bool, string) {
+ if !on_path(binary) {
+ return false, ""
+ }
+ if !os.is_file(join(tree_dir, "go.mod")) {
+ return false, "no go.mod at the repository root"
+ }
+ return true, ""
+}
+
+// go_packages names the packages of the files, as patterns the go command
+// takes. The leading ./ is load-bearing: without it a directory reads as a
+// module path and matches nothing.
+go_packages :: proc(tree_dir: string, files: []string) -> []string {
+ dirs := make(map[string]bool, context.temp_allocator)
+ for f in files {
+ dir := dir_of(f)
+ if dir == "vendor" || strings.has_prefix(dir, "vendor/") {
+ continue
+ }
+ if !os.is_dir(join(tree_dir, dir)) {
+ continue
+ }
+ // A directory under a go.mod of its own is another module's, and
+ // the go command run at the root cannot name it.
+ if nested_module(tree_dir, dir) {
+ continue
+ }
+ dirs[dir] = true
+ }
+ out := make([dynamic]string, context.temp_allocator)
+ for dir in dirs {
+ append(&out, strings.concatenate({"./", dir}, context.temp_allocator))
+ }
+ slice.sort(out[:])
+ return out[:]
+}
+
+// nested_module is whether a directory sits under a go.mod below the
+// tree's root, which makes it another module's package.
+nested_module :: proc(tree_dir, dir: string) -> bool {
+ d := dir
+ for d != "." && d != "" && d != "/" {
+ if os.is_file(join(tree_dir, join(d, "go.mod"))) {
+ return true
+ }
+ d = dir_of(d)
+ }
+ return false
+}
+
+// go_build compiles the packages the change touched. What does not
+// compile is the change's wherever the error lands.
+go_build := Analyser {
+ name = "go-build",
+ covers = is_go,
+ ready = proc(tree_dir: string) -> (bool, string) {return go_ready("go", tree_dir)},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ pkgs := go_packages(tree_dir, files)
+ if len(pkgs) == 0 {
+ return nil, ""
+ }
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "build", "-json", "-o", "/dev/null")
+ append(&args, ..pkgs)
+ out, err := execute(tree_dir, "go", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_go_build(tree_dir, out), ""
+ },
+}
+
+// parse_go_build reads the compiler's errors out of go build -json: build
+// events whose output lines are file:line:col: message, relative to the
+// tree.
+parse_go_build :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ Event :: struct {
+ action: string `json:"Action"`,
+ output: string `json:"Output"`,
+ }
+ found := make([dynamic]Diagnostic, allocator)
+ for line in lines_of(out) {
+ event: Event
+ if json.unmarshal_string(line, &event, allocator = context.temp_allocator) != nil ||
+ event.action != "build-output" {
+ continue
+ }
+ for text in lines_of(event.output) {
+ if strings.has_prefix(text, "#") {
+ continue
+ }
+ file, number, message, ok := position(strings.trim_space(text))
+ if !ok {
+ continue
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, file), allocator),
+ line = number,
+ message = strings.clone(message, allocator),
+ severity = .Must_Fix,
+ fault = true,
+ },
+ )
+ }
+ }
+ return found[:]
+}
+
+// vet_tool is the multichecker built from sidecar/govet: vet's own
+// analysers and the ones from golang.org/x/tools it leaves out. When it
+// is on the path, vet runs it instead of its default set.
+vet_tool :: "review-vet"
+
+// modernizers are the names of the modernize suite's analysers, which
+// report an older idiom where a newer one exists.
+modernizers :: `any atomictypes embedlit errorsastype forvar importcomment mapsloop minmax newexpr
+ omitzero plusbuild rangeint reflecttypeassert reflecttypefor slicesbackward slicesclip slicescontains slicessort
+ stditerators stringscut stringscutprefix stringsseq stringsbuilder testingcontext unsafefuncs waitgroup`
+
+// vet_severity is how seriously to take one of vet's analysers. Vet's
+// default set and the bug-finding extras are faults the analyser argues
+// for; shadow and unusedwrite are judgement; modernize is taste.
+vet_severity :: proc(analyzer: string) -> finding.Severity {
+ switch analyzer {
+ case "shadow", "unusedwrite":
+ return .Consider
+ }
+ if strings.has_prefix(analyzer, "modernize") {
+ return .Note
+ }
+ for name in strings.fields(modernizers, context.temp_allocator) {
+ if name == analyzer {
+ return .Note
+ }
+ }
+ return .Must_Fix
+}
+
+// go_vet runs vet over the packages the change touched — with review-vet
+// where it is installed — and reads its JSON.
+go_vet := Analyser {
+ name = "go-vet",
+ covers = is_go,
+ ready = proc(tree_dir: string) -> (bool, string) {return go_ready("go", tree_dir)},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ pkgs := go_packages(tree_dir, files)
+ if len(pkgs) == 0 {
+ return nil, ""
+ }
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "vet", "-json")
+ if tool, found := sh.which(vet_tool, context.temp_allocator); found {
+ append(&args, strings.concatenate({"-vettool=", tool}, context.temp_allocator))
+ }
+ append(&args, ..pkgs)
+ out, err := execute(tree_dir, "go", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_go_vet(tree_dir, out), ""
+ },
+}
+
+// Vet_Finding is one thing one of vet's analysers said.
+Vet_Finding :: struct {
+ posn: string `json:"posn"`,
+ message: string `json:"message"`,
+}
+
+// parse_go_vet reads vet's JSON: one object per package, one list per
+// analyser. The JSON is preceded by a comment line naming the package, and
+// a package that fails to type-check is reported in prose rather than
+// JSON; both are skipped, since the build has already said what does not
+// compile.
+parse_go_vet :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ for chunk in split_json_objects(out, context.temp_allocator) {
+ report: map[string]map[string][]Vet_Finding
+ if json.unmarshal_string(chunk, &report, allocator = context.temp_allocator) != nil {
+ continue
+ }
+ for _, analysers in report {
+ for analyzer in sorted(analysers) {
+ for f in analysers[analyzer] {
+ file, line := split_position(f.posn)
+ if file == "" {
+ continue
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, file), allocator),
+ line = line,
+ code = strings.clone(analyzer, allocator),
+ message = strings.clone(f.message, allocator),
+ severity = vet_severity(analyzer),
+ },
+ )
+ }
+ }
+ }
+ }
+ return found[:]
+}
+
+// sorted is a map's keys in order.
+sorted :: proc(m: map[string]$V) -> []string {
+ keys, _ := slice.map_keys(m, context.temp_allocator)
+ slice.sort(keys)
+ return keys
+}
+
+// split_position reads file and line out of file:line:col.
+split_position :: proc(posn: string) -> (file: string, line: int) {
+ rest := posn
+ // The column, then the line, are the last two colon-separated fields.
+ last := strings.last_index_byte(rest, ':')
+ if last < 0 {
+ return "", 0
+ }
+ if _, is_number := parse_number(rest[last + 1:]); is_number {
+ prev := strings.last_index_byte(rest[:last], ':')
+ if prev >= 0 {
+ if n, ok := parse_number(rest[prev + 1:last]); ok {
+ return rest[:prev], n
+ }
+ }
+ n, _ := parse_number(rest[last + 1:])
+ return rest[:last], n
+ }
+ return "", 0
+}
+
+parse_number :: proc(s: string) -> (int, bool) {
+ if len(s) == 0 {
+ return 0, false
+ }
+ n := 0
+ for i in 0 ..< len(s) {
+ if s[i] < '0' || s[i] > '9' {
+ return 0, false
+ }
+ n = n * 10 + int(s[i] - '0')
+ }
+ return n, true
+}
+
+// staticcheck is the Go analyser beyond vet, run with JSON output over
+// the packages the change touched. Only a finding code — SA4006, S1002 —
+// is a finding about the code; a compile error the build has already
+// reported.
+staticcheck := Analyser {
+ name = "staticcheck",
+ covers = is_go,
+ ready = proc(tree_dir: string) -> (bool, string) {return go_ready("staticcheck", tree_dir)},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ pkgs := go_packages(tree_dir, files)
+ if len(pkgs) == 0 {
+ return nil, ""
+ }
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "-f", "json")
+ append(&args, ..pkgs)
+ out, err := execute(tree_dir, "staticcheck", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_staticcheck(tree_dir, out), ""
+ },
+}
+
+// Problem is one finding in staticcheck's -f json output, one object per
+// line.
+Problem :: struct {
+ code: string `json:"code"`,
+ location: struct {
+ file: string `json:"file"`,
+ line: int `json:"line"`,
+ } `json:"location"`,
+ message: string `json:"message"`,
+}
+
+parse_staticcheck :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ for line in lines_of(out) {
+ p: Problem
+ if json.unmarshal_string(line, &p, allocator = context.temp_allocator) != nil ||
+ !is_check_code(p.code) {
+ continue
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, p.location.file), allocator),
+ line = p.location.line,
+ code = strings.clone(p.code, allocator),
+ message = strings.clone(p.message, allocator),
+ severity = staticcheck_severity(p.code),
+ },
+ )
+ }
+ return found[:]
+}
+
+// is_check_code is whether a code is a staticcheck finding code: capitals
+// then digits.
+is_check_code :: proc(code: string) -> bool {
+ i := 0
+ for i < len(code) && code[i] >= 'A' && code[i] <= 'Z' {
+ i += 1
+ }
+ if i == 0 || i == len(code) {
+ return false
+ }
+ for j in i ..< len(code) {
+ if code[j] < '0' || code[j] > '9' {
+ return false
+ }
+ }
+ return true
+}
+
+// staticcheck_severity maps a staticcheck category onto the report's
+// severities. SA is a fault the analyser argues for; U is code that serves
+// nobody, and S a simplification; ST and QF are style.
+staticcheck_severity :: proc(code: string) -> finding.Severity {
+ switch {
+ case strings.has_prefix(code, "SA"):
+ return .Must_Fix
+ case (strings.has_prefix(code, "S") && !strings.has_prefix(code, "ST")) ||
+ strings.has_prefix(code, "U"):
+ return .Consider
+ }
+ return .Note
+}
diff --git a/odin/analyser/odin.odin b/odin/analyser/odin.odin
@@ -0,0 +1,117 @@
+package analyser
+
+import "core:encoding/json"
+import "core:os"
+import "core:strings"
+
+// odin_check type-checks each package the change touched with every vet
+// switch and the compiler's own style, and reads its JSON errors. The
+// compiler writes its JSON to stderr and exits non-zero when it has
+// errors, which is the answer, not a failure.
+odin_check := Analyser {
+ name = "odin-check",
+ covers = proc(path: string) -> bool {return strings.has_suffix(path, ".odin")},
+ ready = proc(tree_dir: string) -> (bool, string) {return on_path("odin"), ""},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ dirs := make(map[string]bool, context.temp_allocator)
+ for f in files {
+ dirs[dir_of(f)] = true
+ }
+ found := make([dynamic]Diagnostic)
+ for dir in sorted(dirs) {
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "check", dir, "-vet", "-strict-style", "-json-errors", "-no-entry-point")
+ append(&args, ..collections(tree_dir))
+ stdout, stderr, err := execute_both(tree_dir, "odin", args[:], context.temp_allocator)
+ out := strings.concatenate({stdout, stderr}, context.temp_allocator)
+ if err != "" && !strings.contains(out, "error_count") {
+ return found[:], strings.concatenate({"odin: ", tail(out, 200)})
+ }
+ append(&found, ..parse_odin(tree_dir, out, context.temp_allocator))
+ }
+ return found[:], ""
+ },
+}
+
+// collections are the -collection flags the repository's ols.json
+// declares, which is where an Odin project names the collections its
+// imports resolve through; a relative path is relative to the repository.
+collections :: proc(tree_dir: string, allocator := context.temp_allocator) -> []string {
+ Config :: struct {
+ collections: []struct {
+ name: string `json:"name"`,
+ path: string `json:"path"`,
+ } `json:"collections"`,
+ }
+ data, err := os.read_entire_file_from_path(join(tree_dir, "ols.json"), context.temp_allocator)
+ if err != nil {
+ return nil
+ }
+ config: Config
+ if json.unmarshal(data, &config, allocator = context.temp_allocator) != nil {
+ return nil
+ }
+ flags := make([dynamic]string, allocator)
+ for c in config.collections {
+ if c.name == "" || c.path == "" {
+ continue
+ }
+ path := c.path
+ if !strings.has_prefix(path, "/") {
+ path = join(tree_dir, path, allocator)
+ }
+ append(&flags, strings.concatenate({"-collection:", c.name, "=", path}, allocator))
+ }
+ return flags[:]
+}
+
+// Odin_Report is the compiler's -json-errors.
+Odin_Report :: struct {
+ errors: []struct {
+ type: string `json:"type"`,
+ pos: struct {
+ file: string `json:"file"`,
+ line: int `json:"line"`,
+ } `json:"pos"`,
+ msgs: []string `json:"msgs"`,
+ } `json:"errors"`,
+}
+
+// parse_odin reads the compiler's -json-errors: a type error is a fault,
+// a vet failure the vet's opinion, a style failure a note. The compiler
+// prints its own prose before the JSON when it cannot even start; the
+// object is taken from wherever it sits.
+parse_odin :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ raw, ok := object(out)
+ if !ok {
+ return found[:]
+ }
+ report: Odin_Report
+ if json.unmarshal_string(raw, &report, allocator = context.temp_allocator) != nil {
+ return found[:]
+ }
+ for e in report.errors {
+ message := strings.join(e.msgs, "; ", allocator)
+ d := Diagnostic {
+ file = strings.clone(relative(tree_dir, e.pos.file), allocator),
+ line = e.pos.line,
+ message = message,
+ severity = .Must_Fix,
+ fault = true,
+ }
+ switch {
+ case strings.contains(message, "-strict-style"):
+ d.code, d.severity, d.fault = "style", .Note, false
+ case e.type == "warning":
+ d.severity, d.fault = .Consider, false
+ case strings.contains(message, "declared but not used"),
+ strings.contains(message, "shadow"):
+ // A vet failure fails the build under -vet, but it is the
+ // vet's opinion, not a type error.
+ d.code, d.severity, d.fault = "vet", .Consider, false
+ }
+ append(&found, d)
+ }
+ return found[:]
+}
diff --git a/odin/analyser/others.odin b/odin/analyser/others.odin
@@ -0,0 +1,421 @@
+package analyser
+
+import "core:encoding/json"
+import "core:os"
+import "core:strconv"
+import "core:strings"
+import "core:text/regex"
+import "jfm:sh"
+
+import "../finding"
+
+// grammar_of is whether a path is TypeScript or JavaScript.
+is_script :: proc(path: string) -> bool {
+ for ext in ([]string{".ts", ".tsx", ".js", ".jsx", ".mjs", ".cjs"}) {
+ if strings.has_suffix(path, ext) {
+ return true
+ }
+ }
+ return false
+}
+
+// tsc type-checks each project a changed TypeScript file belongs to,
+// under the project's own tsconfig — strictness is the project's to set —
+// and emits nothing. It needs the project's node_modules, so it reads the
+// working tree only.
+tsc := Analyser {
+ name = "tsc",
+ covers = is_script,
+ in_place = true,
+ ready = proc(tree_dir: string) -> (bool, string) {
+ return on_path("tsc") || os.is_file(join(tree_dir, "node_modules/.bin/tsc")), ""
+ },
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ binary := join(tree_dir, "node_modules/.bin/tsc")
+ if !os.is_file(binary) {
+ binary = "tsc"
+ }
+ projects := make(map[string]bool, context.temp_allocator)
+ for f in files {
+ if p := nearest(tree_dir, dir_of(f), "tsconfig.json"); p != "" {
+ projects[p] = true
+ }
+ }
+ if len(projects) == 0 {
+ return nil, "no tsconfig.json above the changed files"
+ }
+ found := make([dynamic]Diagnostic)
+ for project in sorted(projects) {
+ dir := join(tree_dir, dir_of(project))
+ out, err := execute(
+ dir,
+ binary,
+ {"--noEmit", "--pretty", "false", "-p", "tsconfig.json"},
+ context.temp_allocator,
+ )
+ if err != "" {
+ return found[:], err
+ }
+ append(&found, ..parse_tsc(tree_dir, dir, out, context.temp_allocator))
+ }
+ return found[:], ""
+ },
+}
+
+// parse_tsc reads the compiler's plain output — file(line,col): error
+// TSnnnn: message — with paths relative to the project directory it ran
+// in.
+parse_tsc :: proc(tree_dir, dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ re, err := regex.create(
+ `^(.+?)\((\d+),(\d+)\): error (TS\d+): (.*)$`,
+ {},
+ context.temp_allocator,
+ context.temp_allocator,
+ )
+ if err != nil {
+ return found[:]
+ }
+ for line in lines_of(out) {
+ cap, ok := regex.match(re, strings.trim_space(line), context.temp_allocator)
+ if !ok {
+ continue
+ }
+ number, _ := strconv.parse_int(cap.groups[2])
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, join(dir, cap.groups[1])), allocator),
+ line = number,
+ code = strings.clone(cap.groups[4], allocator),
+ message = strings.clone(cap.groups[5], allocator),
+ severity = .Must_Fix,
+ fault = true,
+ },
+ )
+ }
+ return found[:]
+}
+
+is_python :: proc(path: string) -> bool {
+ return strings.has_suffix(path, ".py")
+}
+
+// ruff lints the changed Python files with the project's own
+// configuration and reads its JSON. Its style codes are notes; what
+// pyflakes would have said is worth considering.
+ruff := Analyser {
+ name = "ruff",
+ covers = is_python,
+ ready = proc(tree_dir: string) -> (bool, string) {return on_path("ruff"), ""},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "check", "--output-format", "json", "--exit-zero")
+ append(&args, ..files)
+ out, err := execute(tree_dir, "ruff", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_ruff(tree_dir, out)
+ },
+}
+
+// Ruff_Finding is one entry of ruff's JSON array.
+Ruff_Finding :: struct {
+ code: string `json:"code"`,
+ message: string `json:"message"`,
+ filename: string `json:"filename"`,
+ location: struct {
+ row: int `json:"row"`,
+ } `json:"location"`,
+}
+
+// parse_ruff reads ruff's JSON array. Style codes — E, W, import order,
+// docstrings — are notes; the rest is worth considering.
+parse_ruff :: proc(
+ tree_dir, out: string,
+ allocator := context.allocator,
+) -> (
+ []Diagnostic,
+ string,
+) {
+ report: []Ruff_Finding
+ if json.unmarshal_string(out, &report, allocator = context.temp_allocator) != nil {
+ return nil, "ruff: its answer is not JSON"
+ }
+ found := make([dynamic]Diagnostic, allocator)
+ for r in report {
+ severity := finding.Severity.Consider
+ if strings.has_prefix(r.code, "E") ||
+ strings.has_prefix(r.code, "W") ||
+ strings.has_prefix(r.code, "I") ||
+ strings.has_prefix(r.code, "D") {
+ severity = .Note
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, r.filename), allocator),
+ line = r.location.row,
+ code = strings.clone(r.code, allocator),
+ message = strings.clone(r.message, allocator),
+ severity = severity,
+ },
+ )
+ }
+ return found[:], ""
+}
+
+// mypy type-checks the changed Python files and reads its JSON, one
+// object per line. Imports it cannot find are the environment's business,
+// not the change's, and are not reported.
+mypy := Analyser {
+ name = "mypy",
+ covers = is_python,
+ in_place = true,
+ ready = proc(tree_dir: string) -> (bool, string) {return on_path("mypy"), ""},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ args := make([dynamic]string, context.temp_allocator)
+ append(&args, "--output", "json", "--no-error-summary", "--ignore-missing-imports")
+ append(&args, ..files)
+ out, err := execute(tree_dir, "mypy", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_mypy(tree_dir, out), ""
+ },
+}
+
+// Mypy_Finding is one line of mypy's JSON.
+Mypy_Finding :: struct {
+ file: string `json:"file"`,
+ line: int `json:"line"`,
+ message: string `json:"message"`,
+ code: string `json:"code"`,
+ severity: string `json:"severity"`,
+}
+
+// parse_mypy reads mypy's JSON, one object per line: an error is a type
+// error and must-fix, anything else a note.
+parse_mypy :: proc(tree_dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ for line in lines_of(out) {
+ r: Mypy_Finding
+ if json.unmarshal_string(line, &r, allocator = context.temp_allocator) != nil ||
+ r.file == "" {
+ continue
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, r.file), allocator),
+ line = r.line,
+ code = strings.clone(r.code, allocator),
+ message = strings.clone(r.message, allocator),
+ severity = .Must_Fix if r.severity == "error" else .Note,
+ },
+ )
+ }
+ return found[:]
+}
+
+// cargo type-checks each crate a changed Rust file belongs to — with
+// clippy where it is installed, which checks and lints in one run — and
+// reads the compiler's JSON messages. The target directory is the
+// repository's own, so a range's materialised tree reuses the build
+// cache.
+cargo := Analyser {
+ name = "cargo",
+ covers = proc(path: string) -> bool {return strings.has_suffix(path, ".rs")},
+ ready = proc(tree_dir: string) -> (bool, string) {return on_path("cargo"), ""},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ crates := make(map[string]bool, context.temp_allocator)
+ for f in files {
+ if c := nearest(tree_dir, dir_of(f), "Cargo.toml"); c != "" {
+ crates[c] = true
+ }
+ }
+ if len(crates) == 0 {
+ return nil, "no Cargo.toml above the changed files"
+ }
+ verb :=
+ "clippy" if sh.exec({"cargo", "clippy", "--version"}, allocator = context.temp_allocator).ok else "check"
+ found := make([dynamic]Diagnostic)
+ for crate in sorted(crates) {
+ dir := join(tree_dir, dir_of(crate))
+ target := join(root, join(dir_of(crate), "target"))
+ os.set_env("CARGO_TARGET_DIR", target)
+ out, err := execute(
+ dir,
+ "cargo",
+ {verb, "--message-format", "json", "--quiet"},
+ context.temp_allocator,
+ )
+ os.unset_env("CARGO_TARGET_DIR")
+ if err != "" {
+ return found[:], err
+ }
+ append(&found, ..parse_cargo(tree_dir, dir, out, context.temp_allocator))
+ }
+ return found[:], ""
+ },
+}
+
+// Cargo_Event is one line of cargo's JSON stream.
+Cargo_Event :: struct {
+ reason: string `json:"reason"`,
+ message: struct {
+ level: string `json:"level"`,
+ message: string `json:"message"`,
+ code: Maybe(struct {
+ code: string `json:"code"`,
+ }) `json:"code"`,
+ spans: []struct {
+ file: string `json:"file_name"`,
+ line: int `json:"line_start"`,
+ primary: bool `json:"is_primary"`,
+ } `json:"spans"`,
+ } `json:"message"`,
+}
+
+// parse_cargo reads the compiler messages out of cargo's JSON stream, one
+// per primary span, with paths relative to the crate directory it ran in.
+parse_cargo :: proc(tree_dir, dir, out: string, allocator := context.allocator) -> []Diagnostic {
+ found := make([dynamic]Diagnostic, allocator)
+ for line in lines_of(out) {
+ event: Cargo_Event
+ if json.unmarshal_string(line, &event, allocator = context.temp_allocator) != nil ||
+ event.reason != "compiler-message" {
+ continue
+ }
+ for span in event.message.spans {
+ if !span.primary {
+ continue
+ }
+ d := Diagnostic {
+ file = strings.clone(relative(tree_dir, join(dir, span.file)), allocator),
+ line = span.line,
+ message = strings.clone(event.message.message, allocator),
+ }
+ if code, given := event.message.code.?; given {
+ d.code = strings.clone(code.code, allocator)
+ }
+ switch event.message.level {
+ case "error":
+ d.severity, d.fault = .Must_Fix, true
+ case "warning":
+ d.severity = .Consider
+ case:
+ continue
+ }
+ append(&found, d)
+ break
+ }
+ }
+ return found[:]
+}
+
+// semgrep_languages are the extensions semgrep parses: its rules are
+// about patterns, not types, and say things a compiler does not.
+semgrep_languages :: `.go .ts .tsx .js .jsx .mjs .cjs .py .rs .java .kt .kts .rb .php .c .h .cc .cpp .hpp .cs
+ .swift .scala .lua .ex .exs .dart .sh .bash .tf .yaml .yml .json .html .sol`
+
+// semgrep_config is the rule set semgrep is pointed at: the repository's
+// own configuration where it has one — that is the repository's word on
+// what matters — and the registry's default pack otherwise, fetched over
+// the network. Not auto: semgrep refuses to build that selection with
+// metrics off, and review never sends metrics.
+semgrep_config :: proc(tree_dir: string) -> string {
+ for name in ([]string{".semgrep.yml", ".semgrep.yaml", ".semgrep"}) {
+ if os.exists(join(tree_dir, name)) {
+ return name
+ }
+ }
+ return "p/default"
+}
+
+// semgrep runs the pattern analyser over the changed files it can parse,
+// and reads its JSON. A rule's severity is the rule author's word on it,
+// kept as reported: ERROR must-fix, WARNING consider, INFO note.
+semgrep := Analyser {
+ name = "semgrep",
+ covers = proc(path: string) -> bool {
+ for ext in strings.fields(semgrep_languages, context.temp_allocator) {
+ if strings.has_suffix(path, ext) {
+ return true
+ }
+ }
+ return false
+ },
+ ready = proc(tree_dir: string) -> (bool, string) {return on_path("semgrep"), ""},
+ run = proc(tree_dir, root: string, files: []string) -> ([]Diagnostic, string) {
+ args := make([dynamic]string, context.temp_allocator)
+ append(
+ &args,
+ "scan",
+ "--json",
+ "--quiet",
+ "--config",
+ semgrep_config(tree_dir),
+ "--metrics",
+ "off",
+ )
+ append(&args, ..files)
+ out, err := execute(tree_dir, "semgrep", args[:], context.temp_allocator)
+ if err != "" {
+ return nil, err
+ }
+ return parse_semgrep(tree_dir, out)
+ },
+}
+
+// Semgrep_Report is semgrep's JSON report: one result per match.
+Semgrep_Report :: struct {
+ results: []struct {
+ check_id: string `json:"check_id"`,
+ path: string `json:"path"`,
+ start: struct {
+ line: int `json:"line"`,
+ } `json:"start"`,
+ extra: struct {
+ message: string `json:"message"`,
+ severity: string `json:"severity"`,
+ } `json:"extra"`,
+ } `json:"results"`,
+}
+
+// parse_semgrep reads semgrep's JSON report, located by each match's
+// start line, with the rule's id and severity.
+parse_semgrep :: proc(
+ tree_dir, out: string,
+ allocator := context.allocator,
+) -> (
+ []Diagnostic,
+ string,
+) {
+ report: Semgrep_Report
+ if json.unmarshal_string(out, &report, allocator = context.temp_allocator) != nil {
+ return nil, "semgrep: its answer is not JSON"
+ }
+ found := make([dynamic]Diagnostic, allocator)
+ for r in report.results {
+ severity := finding.Severity.Note
+ switch strings.to_upper(r.extra.severity, context.temp_allocator) {
+ case "ERROR", "HIGH", "CRITICAL":
+ severity = .Must_Fix
+ case "WARNING", "MEDIUM":
+ severity = .Consider
+ }
+ append(
+ &found,
+ Diagnostic {
+ file = strings.clone(relative(tree_dir, r.path), allocator),
+ line = r.start.line,
+ code = strings.clone(r.check_id, allocator),
+ message = strings.clone(strings.trim_space(r.extra.message), allocator),
+ severity = severity,
+ },
+ )
+ }
+ return found[:], ""
+}
diff --git a/odin/review/main.odin b/odin/review/main.odin
@@ -9,6 +9,7 @@ package main
import "core:fmt"
import "core:os"
+import "../analyser"
import "../change"
import "../check"
import "../finding"
@@ -76,9 +77,12 @@ main :: proc() {
}
// The deterministic checks ask nothing of a provider, so they run
// before one is built and survive a model that cannot answer.
- findings := check.run(check.scope_of(&c, t))
- finding.sort(findings)
- kept, dismissed := report.filter(t.dir, findings)
+ findings := make([dynamic]finding.Finding)
+ append(&findings, ..check.run(check.scope_of(&c, t)))
+ _, ranged := tree.ends(rev)
+ append(&findings, ..analyser.check(&c, t, ranged))
+ finding.sort(findings[:])
+ kept, dismissed := report.filter(t.dir, findings[:])
env := report.Contract {
status = report.status_of(c, 0),
findings = kept,