commit 027eec57b4a25b7a8a2d4399ddb898094d93b609
parent d88d25b5e123353506e14fbbc651d4c78e02af9b
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Wed, 23 Sep 2026 19:44:40 -0300
review: read Go through a sidecar and begin the Odin reading
The Go frontend ships as review-go, a stdlib-only sidecar printing the
same JSON the Odin sidecar does, so a reviewer in another language reads
Go the way this one does. Under odin/, on the jfm collection: frontend
drives both sidecars, git asks the repository, tree reads it at the end
of the change with a range unpacked once from git archive, change
gathers the diff and reads symbols, tests, comments and imports on the
added lines plus the repository's index, and review is the driver.
The odin-check analyser reads its collections from ols.json, which the
repository now carries, so the jfm imports type-check under review.
Diffstat:
17 files changed, 1520 insertions(+), 5 deletions(-)
diff --git a/.gitignore b/.gitignore
@@ -1,2 +1,4 @@
/review
/review-vet
+/review-go
+build/
diff --git a/analysers.go b/analysers.go
@@ -424,7 +424,8 @@ var odinCheck = Analyser{
}) {
// The compiler writes its JSON to stderr and exits non-zero
// when it has errors, which is the answer, not a failure.
- stdout, stderr, err := executeBoth(ctx, tree, nil, "odin", "check", dir, "-vet", "-strict-style", "-json-errors", "-no-entry-point")
+ args := append([]string{"check", dir, "-vet", "-strict-style", "-json-errors", "-no-entry-point"}, odinCollections(tree)...)
+ stdout, stderr, err := executeBoth(ctx, tree, nil, "odin", args...)
out := append(stdout, stderr...)
if err != nil && !bytes.Contains(out, []byte("error_count")) {
return diagnostics, fmt.Errorf("odin: %s", tail(strings.TrimSpace(string(out)), 200))
@@ -435,6 +436,38 @@ var odinCheck = Analyser{
},
}
+// odinCollections 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.
+// Without them the compiler cannot see past `import "name:pkg"`.
+func odinCollections(tree string) []string {
+ data, err := os.ReadFile(filepath.Join(tree, "ols.json"))
+ if err != nil {
+ return nil
+ }
+ var config struct {
+ Collections []struct {
+ Name string `json:"name"`
+ Path string `json:"path"`
+ } `json:"collections"`
+ }
+ if err := json.Unmarshal(data, &config); err != nil {
+ return nil
+ }
+ var flags []string
+ for _, c := range config.Collections {
+ if c.Name == "" || c.Path == "" {
+ continue
+ }
+ path := c.Path
+ if !filepath.IsAbs(path) {
+ path = filepath.Join(tree, path)
+ }
+ flags = append(flags, "-collection:"+c.Name+"="+path)
+ }
+ return flags
+}
+
// parseOdin reads the compiler's -json-errors: a type error is a fault, a
// vet failure the vet's opinion, a style failure a note.
func parseOdin(tree string, out []byte) []Diagnostic {
diff --git a/analysers_test.go b/analysers_test.go
@@ -6,7 +6,10 @@ package main
// tool's output is read into the same shape.
import (
+ "os"
"os/exec"
+ "path/filepath"
+ "slices"
"strings"
"testing"
)
@@ -280,3 +283,18 @@ func TestSemgrepConfig(t *testing.T) {
t.Errorf("got %q", got)
}
}
+
+func TestOdinCollectionsReadOlsJSON(t *testing.T) {
+ dir := t.TempDir()
+ if got := odinCollections(dir); got != nil {
+ t.Errorf("no ols.json: %v", got)
+ }
+ if err := os.WriteFile(filepath.Join(dir, "ols.json"), []byte(`{"collections":[{"name":"jfm","path":"../odin"},{"name":"abs","path":"/opt/abs"},{"name":""}]}`), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ got := odinCollections(dir)
+ want := []string{"-collection:jfm=" + filepath.Join(dir, "../odin"), "-collection:abs=/opt/abs"}
+ if !slices.Equal(got, want) {
+ t.Errorf("got %v, want %v", got, want)
+ }
+}
diff --git a/justfile b/justfile
@@ -1,8 +1,13 @@
# Standard recipes: build (debug), release, clean, test, install.
# install also builds review-vet, the go vet sidecar that `go-vet/<analyzer>`
-# findings use when it sits beside review on PATH.
+# findings use when it sits beside review on PATH, and review-go, the Go
+# parser sidecar the Odin reading under odin/ drives.
+# odin-test and odin-build cover the Odin reading; they need the jfm
+# collection at ~/Source/Personal/odin and review-go on PATH.
bin := "review"
+odin := env("ODIN", "odin")
+odin_flags := "-vet -strict-style -collection:jfm=" + home_directory() / "Source" / "Personal" / "odin"
# `just` alone lists the recipes.
default:
@@ -12,11 +17,13 @@ default:
build:
go build -o {{bin}} .
go build -C sidecar/govet -o ../../{{bin}}-vet .
+ go build -o {{bin}}-go ./sidecar/gofront
# Optimised build: stripped, reproducible paths.
release:
go build -trimpath -ldflags='-s -w' -o {{bin}} .
go build -C sidecar/govet -trimpath -ldflags='-s -w' -o ../../{{bin}}-vet .
+ go build -trimpath -ldflags='-s -w' -o {{bin}}-go ./sidecar/gofront
# Vet and run the tests.
test:
@@ -25,9 +32,24 @@ test:
# Remove build output and the Go build cache for this module.
clean:
- rm -f {{bin}} {{bin}}-vet
+ rm -f {{bin}} {{bin}}-vet {{bin}}-go
+ rm -rf build
go clean
+# Test the Odin reading, package by package.
+odin-test:
+ #!/usr/bin/env bash
+ set -euo pipefail
+ mkdir -p build
+ for p in frontend git tree change; do
+ {{odin}} test odin/$p {{odin_flags}} -out:build/${p}_test
+ done
+
+# Debug build of the Odin reading -> build/review-odin
+odin-build:
+ mkdir -p build
+ {{odin}} build odin/review {{odin_flags}} -debug -out:build/review-odin
+
# Copy the release binary to a directory on PATH. An installed copy is replaced in
# place; otherwise ~/.local/bin, ~/bin, GOBIN, GOPATH/bin, /usr/local/bin, else the
# first writable PATH entry.
@@ -49,4 +71,5 @@ install: release
[[ -n $dest ]] || { echo "install: no writable directory on PATH" >&2; exit 1; }
install -m 755 {{bin}} "$dest/{{bin}}"
install -m 755 {{bin}}-vet "$dest/{{bin}}-vet"
- echo "installed $dest/{{bin}} and $dest/{{bin}}-vet"
+ install -m 755 {{bin}}-go "$dest/{{bin}}-go"
+ echo "installed $dest/{{bin}}, $dest/{{bin}}-vet and $dest/{{bin}}-go"
diff --git a/odin/change/change.odin b/odin/change/change.odin
@@ -0,0 +1,405 @@
+/*
+Package change gathers the change under review — the staged change, or a
+revision range — and reads what it adds: the declarations, tests and
+comments on the lines the diff introduces, through the language sidecars,
+and the repository's own declarations at the end of the change, which is
+what new work is judged against.
+*/
+package change
+
+import "core:strconv"
+import "core:strings"
+
+import "../frontend"
+import "../git"
+import "../tree"
+
+// Symbol is a declaration the change adds.
+Symbol :: struct {
+ name: string,
+ kind: string,
+ doc: string,
+ file: string,
+ line: int,
+ exported: bool,
+ signature: string,
+ body: string,
+ pkg: string,
+}
+
+// Function is a test the change adds or touches, whole.
+Function :: struct {
+ name: string,
+ file: string,
+ line: int,
+ body: string,
+}
+
+// Located is a comment the change adds.
+Located :: struct {
+ text: string,
+ file: string,
+ line: int,
+}
+
+// Declared is one declaration somewhere in the repository, which a new
+// name might turn out to duplicate.
+Declared :: struct {
+ name: string,
+ kind: string,
+ file: string,
+ line: int,
+ text: string,
+ body: string,
+}
+
+// Diff_Line is one added line and the number it lands on.
+Diff_Line :: struct {
+ line: int,
+ text: string,
+}
+
+// Change is what is under review.
+Change :: struct {
+ // diff is the change itself, capped at max_diff.
+ diff: string,
+ truncated: bool,
+ // files are the paths it touches.
+ files: []string,
+ // message is the commit message: empty for a staged change.
+ message: string,
+ stat: string,
+ // added and removed are the diff's two sides, per file.
+ added: map[string][dynamic]Diff_Line,
+ removed: map[string][dynamic]string,
+ // What the sidecars read on the added lines.
+ symbols: [dynamic]Symbol,
+ tests: [dynamic]Function,
+ comments: [dynamic]Located,
+ imports: map[string][]string,
+ // uncovered are the code files no sidecar reads.
+ uncovered: [dynamic]string,
+}
+
+max_diff :: 60000
+
+// gather collects the change at a revision range, or the staged change
+// when the range is empty. Everything is allocated from allocator.
+gather :: proc(rev, root: string, allocator := context.allocator) -> (c: Change, ok: bool) {
+ context.allocator = allocator
+ diff_args, name_args, stat_args: []string
+ if rev == "" {
+ diff_args = {"diff", "--cached", "-U3", "--src-prefix=a/", "--dst-prefix=b/"}
+ name_args = {"diff", "--cached", "--name-only"}
+ stat_args = {"diff", "--cached", "--stat"}
+ } else {
+ diff_args = {"diff", rev, "-U3", "--src-prefix=a/", "--dst-prefix=b/"}
+ name_args = {"diff", rev, "--name-only"}
+ stat_args = {"diff", rev, "--stat"}
+ }
+ c.diff = git.run(root, diff_args) or_return
+ if len(c.diff) > max_diff {
+ c.diff = strings.concatenate({c.diff[:max_diff], "\n… diff truncated\n"})
+ c.truncated = true
+ }
+ c.files = git.lines(root, name_args) or_return
+ c.stat = git.run(root, stat_args) or_return
+ if rev != "" {
+ c.message, _ = git.run(root, {"log", "-1", "--format=%B", strings.trim_suffix(rev, "^")})
+ }
+ c.added, c.removed = diff_sides(c.diff)
+ c.imports = make(map[string][]string)
+ return c, true
+}
+
+// diff_sides splits a diff into the added and removed lines of each file.
+// The header lines are read wherever they sit, as they sit between hunks;
+// content is read only inside a hunk, so that a removed line beginning
+// "+++ " is not mistaken for the header it resembles.
+diff_sides :: proc(
+ diff: string,
+ allocator := context.allocator,
+) -> (
+ added: map[string][dynamic]Diff_Line,
+ removed: map[string][dynamic]string,
+) {
+ context.allocator = allocator
+ added = make(map[string][dynamic]Diff_Line)
+ removed = make(map[string][dynamic]string)
+ added_file, removed_file: string
+ line: int
+ in_hunk: bool
+ for text in strings.split_lines(diff, context.temp_allocator) {
+ switch {
+ case strings.has_prefix(text, "+++ "):
+ added_file = side(text[4:], "b/")
+ in_hunk = false
+ case strings.has_prefix(text, "--- "):
+ removed_file = side(text[4:], "a/")
+ in_hunk = false
+ case strings.has_prefix(text, "\\ "):
+ // The no-newline marker annotates the line above it.
+ case strings.has_prefix(text, "@@ "):
+ if start, found := hunk_start(text); found {
+ in_hunk = true
+ line = start
+ }
+ case strings.has_prefix(text, "+") && in_hunk:
+ if added_file != "" {
+ lines := added[added_file]
+ append(&lines, Diff_Line{line, strings.clone(text[1:])})
+ added[added_file] = lines
+ }
+ line += 1
+ case strings.has_prefix(text, "-") && in_hunk:
+ if removed_file != "" {
+ lines := removed[removed_file]
+ append(&lines, strings.clone(text[1:]))
+ removed[removed_file] = lines
+ }
+ case:
+ if in_hunk {
+ line += 1
+ }
+ }
+ }
+ return
+}
+
+// hunk_start reads the line a hunk's added side begins on, out of its
+// "@@ -a,b +c,d @@" header.
+hunk_start :: proc(text: string) -> (line: int, ok: bool) {
+ i := strings.index(text, " +")
+ if i < 0 {
+ return 0, false
+ }
+ rest := text[i + 2:]
+ end := 0
+ for end < len(rest) && rest[end] >= '0' && rest[end] <= '9' {
+ end += 1
+ }
+ if end == 0 {
+ return 0, false
+ }
+ return strconv.parse_int(rest[:end])
+}
+
+// side strips a diff's prefix from a header path, and a deletion's
+// /dev/null with it.
+side :: proc(path, prefix: string) -> string {
+ stripped := strings.trim_prefix(path, prefix)
+ if stripped == "/dev/null" {
+ return ""
+ }
+ return strings.clone(stripped)
+}
+
+// read fills the change's symbols, tests, comments and imports from the
+// sidecars, over the files at the end of the change. Only what the diff
+// added is reported, so a job sees new work rather than the file it
+// landed in: a declaration is new when its line is, a test when any line
+// of it is, a comment when it sits on an added line.
+read :: proc(c: ^Change, t: tree.Tree, allocator := context.allocator) -> bool {
+ context.allocator = allocator
+ ok := true
+ for sidecar in frontend.Sidecar {
+ names := make([dynamic]string, context.temp_allocator)
+ for name in c.files {
+ if s, covered := frontend.sidecar_for(name);
+ covered && s == sidecar && tree.exists(t, name) {
+ append(&names, name)
+ }
+ }
+ if len(names) == 0 {
+ continue
+ }
+ answers, err := scan(sidecar, t, names[:], context.temp_allocator)
+ if err != .None {
+ append(&c.uncovered, ..names[:])
+ ok = false
+ continue
+ }
+ for name in names {
+ file, answered := answers[name]
+ if !answered || file.error != "" {
+ append(&c.uncovered, name)
+ continue
+ }
+ read_file(c, t, name, file)
+ }
+ }
+ return ok
+}
+
+// read_file keeps what one file's answer says about the added lines.
+read_file :: proc(c: ^Change, t: tree.Tree, name: string, file: frontend.File) {
+ source, readable := tree.read(t, name, context.temp_allocator)
+ if !readable {
+ return
+ }
+ lines := strings.split_lines(string(source), context.temp_allocator)
+ touched := make(map[int]bool, context.temp_allocator)
+ // The lookup is bound before the loop: ranging over a map index with
+ // a missing key reads through a nil entry.
+ added := c.added[name]
+ for l in added {
+ touched[l.line] = true
+ }
+ c.imports[name] = clone_all(file.imports)
+ for decl in file.decls {
+ if decl.kind == "field" {
+ continue
+ }
+ if decl.test {
+ if touched_between(touched, decl.line, decl.end_line) {
+ append(
+ &c.tests,
+ Function {
+ name = strings.clone(decl.name),
+ file = name,
+ line = decl.line,
+ body = text(lines, decl.line, decl.end_line),
+ },
+ )
+ }
+ continue
+ }
+ if !touched[decl.line] {
+ continue
+ }
+ symbol := Symbol {
+ name = strings.clone(decl.name),
+ kind = strings.clone(decl.kind),
+ doc = strings.clone(decl.doc),
+ file = name,
+ line = decl.line,
+ exported = decl.exported,
+ signature = strings.clone(decl.text),
+ pkg = strings.clone(file.pkg),
+ }
+ if decl.kind == "func" {
+ symbol.body = text(lines, decl.line, decl.end_line)
+ }
+ append(&c.symbols, symbol)
+ }
+ for comment in file.comments {
+ if touched[comment.line] {
+ append(
+ &c.comments,
+ Located{text = strings.clone(comment.text), file = name, line = comment.line},
+ )
+ }
+ }
+}
+
+// index is every declaration in the repository at the end of the change,
+// tests and locals left out: a test is not a fact with two owners, and a
+// local is nobody else's to duplicate.
+index :: proc(t: tree.Tree, allocator := context.allocator) -> (out: []Declared, ok: bool) {
+ context.allocator = allocator
+ tracked := tree.files(t, context.temp_allocator) or_return
+ declared := make([dynamic]Declared)
+ ok = true
+ for sidecar in frontend.Sidecar {
+ names := make([dynamic]string, context.temp_allocator)
+ for name in tracked {
+ if s, covered := frontend.sidecar_for(name);
+ covered && s == sidecar && !is_test_file(name) {
+ append(&names, name)
+ }
+ }
+ if len(names) == 0 {
+ continue
+ }
+ answers, err := scan(sidecar, t, names[:], context.temp_allocator)
+ if err != .None {
+ ok = false
+ continue
+ }
+ for name in names {
+ file, answered := answers[name]
+ if !answered || file.error != "" {
+ continue
+ }
+ lines: []string
+ if source, readable := tree.read(t, name, context.temp_allocator); readable {
+ lines = strings.split_lines(string(source), context.temp_allocator)
+ }
+ for decl in file.decls {
+ if decl.test || decl.local {
+ continue
+ }
+ entry := Declared {
+ name = strings.clone(decl.name),
+ kind = strings.clone(decl.kind),
+ file = name,
+ line = decl.line,
+ text = strings.clone(decl.text),
+ }
+ if decl.kind == "func" {
+ entry.body = text(lines, decl.line, decl.end_line)
+ }
+ append(&declared, entry)
+ }
+ }
+ }
+ return declared[:], ok
+}
+
+// scan asks a sidecar about tracked files, handing it their paths in the
+// tree, and returns each answer under the tracked name it was asked for.
+scan :: proc(
+ sidecar: frontend.Sidecar,
+ t: tree.Tree,
+ names: []string,
+ allocator := context.allocator,
+) -> (
+ answers: map[string]frontend.File,
+ err: frontend.Scan_Error,
+) {
+ paths := make([]string, len(names), allocator)
+ by_path := make(map[string]string, allocator)
+ for name, i in names {
+ paths[i] = tree.path(t, name, allocator)
+ by_path[paths[i]] = name
+ }
+ out := frontend.scan(sidecar, paths, allocator) or_return
+ answers = make(map[string]frontend.File, allocator)
+ for file in out.files {
+ if name, known := by_path[file.name]; known {
+ answers[name] = file
+ }
+ }
+ return answers, .None
+}
+
+// is_test_file reports a file the test runner reads, in Go's habit.
+is_test_file :: proc(name: string) -> bool {
+ return strings.has_suffix(name, "_test.go")
+}
+
+touched_between :: proc(touched: map[int]bool, from, to: int) -> bool {
+ for line in from ..= to {
+ if touched[line] {
+ return true
+ }
+ }
+ return false
+}
+
+// text is the lines from one number to another, inclusive, or nothing
+// when the range falls outside the file.
+text :: proc(lines: []string, from, to: int, allocator := context.allocator) -> string {
+ if from < 1 || to > len(lines) || to < from {
+ return ""
+ }
+ return strings.join(lines[from - 1:to], "\n", allocator)
+}
+
+clone_all :: proc(items: []string, allocator := context.allocator) -> []string {
+ out := make([]string, len(items), allocator)
+ for item, i in items {
+ out[i] = strings.clone(item, allocator)
+ }
+ return out
+}
diff --git a/odin/change/change_test.odin b/odin/change/change_test.odin
@@ -0,0 +1,152 @@
+package change
+
+import "core:fmt"
+import "core:os"
+import "core:path/filepath"
+import "core:testing"
+import "jfm:sh"
+
+import "../frontend"
+import "../tree"
+
+canned :: `diff --git a/a.go b/a.go
+index 1..2 100644
+--- a/a.go
++++ b/a.go
+@@ -1,3 +1,5 @@
+ package a
++
++// added is new.
++func added() {}
+ func old() {}
+-func gone() {}
+diff --git a/b.txt b/b.txt
+deleted file mode 100644
+--- a/b.txt
++++ /dev/null
+@@ -1,2 +0,0 @@
+-first
+-+++ not a header
+`
+
+@(test)
+diff_sides_reads_both_sides :: proc(t: ^testing.T) {
+ added, removed := diff_sides(canned, context.temp_allocator)
+ testing.expect_value(t, len(added), 1)
+ a := added["a.go"]
+ testing.expect_value(t, len(a), 3)
+ testing.expect_value(t, a[0].line, 2)
+ testing.expect_value(t, a[1].line, 3)
+ testing.expect_value(t, a[1].text, "// added is new.")
+ testing.expect_value(t, a[2].line, 4)
+ testing.expect_value(t, len(removed["a.go"]), 1)
+ testing.expect_value(t, removed["a.go"][0], "func gone() {}")
+ testing.expect_value(t, len(removed["b.txt"]), 2)
+ testing.expect_value(t, removed["b.txt"][1], "+++ not a header")
+ testing.expect_value(t, hunk_start("@@ -10,7 +12 @@ func x") or_else -1, 12)
+}
+
+// new_repo makes a repository with two commits: a.go first, then a
+// function, a test and a comment added to it.
+new_repo :: 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-change-*", 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
+ }
+ write := proc(root, name, src: string) -> bool {
+ path := filepath.join({root, name}, context.temp_allocator) or_else name
+ return os.write_entire_file(path, transmute([]byte)src) == nil
+ }
+ ok = git(root, "init", "-q")
+ ok &&= write(root, "a.go", "package a\n\nfunc old() {}\n")
+ ok &&= git(root, "add", "a.go")
+ ok &&= git(root, "commit", "-q", "-m", "a: begin")
+ ok &&= write(
+ root,
+ "a.go",
+ "package a\n\nimport \"fmt\"\n\nfunc old() {}\n\n// added is new.\nfunc added() {}\n",
+ )
+ ok &&= write(
+ root,
+ "a_test.go",
+ "package a\n\nimport \"testing\"\n\nfunc TestAdded(t *testing.T) {\n\tadded()\n}\n",
+ )
+ ok &&= git(root, "add", "a.go", "a_test.go")
+ ok &&= git(root, "commit", "-q", "-m", "a: add added")
+ return root, ok
+}
+
+@(test)
+gather_reads_a_range_through_the_sidecar :: proc(t: ^testing.T) {
+ if !frontend.installed(.Go) {
+ testing.fail_now(t, "review-go is not on the path")
+ }
+ root, made := new_repo(t)
+ testing.expect(t, made, "the fixture repository")
+ defer os.remove_all(root)
+
+ c, ok := gather("HEAD^..HEAD", root, context.temp_allocator)
+ testing.expect(t, ok, "gather")
+ testing.expect_value(t, len(c.files), 2)
+ testing.expect_value(t, c.message, "a: add added")
+ testing.expect_value(t, c.truncated, false)
+
+ tr, at_ok := tree.at(root, "HEAD^..HEAD", context.temp_allocator)
+ testing.expect(t, at_ok, "materialise")
+ defer tree.close(tr)
+ testing.expect(t, tr.dir != root, "a range is read from scratch")
+ testing.expect(t, tree.exists(tr, "a_test.go"))
+
+ testing.expect(t, read(&c, tr, context.temp_allocator), "read")
+ testing.expect_value(t, len(c.uncovered), 0)
+ testing.expect_value(t, len(c.symbols), 1)
+ testing.expect_value(t, c.symbols[0].name, "added")
+ testing.expect_value(t, c.symbols[0].kind, "func")
+ testing.expect_value(t, c.symbols[0].doc, "added is new.")
+ testing.expect_value(t, c.symbols[0].body, "func added() {}")
+ testing.expect_value(t, c.symbols[0].pkg, "a")
+ testing.expect_value(t, len(c.tests), 1)
+ testing.expect_value(t, c.tests[0].name, "TestAdded")
+ testing.expect_value(t, c.tests[0].body, "func TestAdded(t *testing.T) {\n\tadded()\n}")
+ testing.expect_value(t, len(c.comments), 1)
+ testing.expect_value(t, c.comments[0].text, "added is new.")
+ testing.expect_value(t, len(c.imports["a.go"]), 1)
+ testing.expect_value(t, c.imports["a.go"][0], "fmt")
+
+ declared, indexed := index(tr, context.temp_allocator)
+ testing.expect(t, indexed, "index")
+ names := make([dynamic]string, context.temp_allocator)
+ for d in declared {
+ append(&names, d.name)
+ }
+ testing.expect_value(t, fmt.tprint(names[:]), `["old", "added"]`)
+}
+
+@(test)
+gather_of_nothing_staged_is_empty :: proc(t: ^testing.T) {
+ root, made := new_repo(t)
+ testing.expect(t, made, "the fixture repository")
+ defer os.remove_all(root)
+ c, ok := gather("", root, context.temp_allocator)
+ testing.expect(t, ok, "gather")
+ testing.expect_value(t, len(c.files), 0)
+ testing.expect_value(t, c.diff, "")
+ testing.expect_value(t, c.message, "")
+}
diff --git a/odin/frontend/frontend.odin b/odin/frontend/frontend.odin
@@ -0,0 +1,132 @@
+/*
+Package frontend reads source files through the language sidecars this
+repository ships: review-go for Go and odin-review-extract for Odin. Each
+sidecar is that language's own parser printing one JSON answer, so the
+reviewer here does not own a grammar. The answer's shape is shared; a field
+one language lacks stays zero.
+*/
+package frontend
+
+import "core:encoding/json"
+import "core:strings"
+import "jfm:sh"
+
+// Decl is one declaration as a sidecar reports it. Kinds are func, type,
+// field, const, var and value. A test is what the language's test runner
+// runs. A local declaration sits inside a function body.
+Decl :: struct {
+ name: string,
+ kind: string,
+ line: int,
+ end_line: int,
+ exported: bool,
+ test: bool,
+ local: bool,
+ text: string,
+ doc: string,
+}
+
+// Comment is one comment, located by its first line, marker stripped.
+Comment :: struct {
+ line: int,
+ text: string,
+}
+
+// File is what one source file declares. An error is a file the sidecar
+// could not parse; its lists are then empty rather than missing.
+File :: struct {
+ name: string,
+ pkg: string `json:"package"`,
+ imports: []string,
+ decls: []Decl,
+ comments: []Comment,
+ error: string,
+}
+
+Output :: struct {
+ files: []File,
+}
+
+// Sidecar names the parser a language is read through.
+Sidecar :: enum {
+ Go,
+ Odin,
+}
+
+// Scan_Error says why a scan returned nothing: the sidecar is not on the
+// path, it exited without an answer, or its answer was not the JSON expected.
+Scan_Error :: enum {
+ None,
+ Not_Installed,
+ Failed,
+ Unreadable,
+}
+
+// binary is the executable a sidecar is found on the path as.
+binary :: proc(sidecar: Sidecar) -> string {
+ switch sidecar {
+ case .Go:
+ return "review-go"
+ case .Odin:
+ return "odin-review-extract"
+ }
+ return ""
+}
+
+// sidecar_for is the sidecar that reads a path, by its extension.
+sidecar_for :: proc(path: string) -> (sidecar: Sidecar, covered: bool) {
+ switch {
+ case strings.has_suffix(path, ".go"):
+ return .Go, true
+ case strings.has_suffix(path, ".odin"):
+ return .Odin, true
+ }
+ return .Go, false
+}
+
+// installed reports whether a sidecar can be run.
+installed :: proc(sidecar: Sidecar) -> bool {
+ _, found := sh.which(binary(sidecar), context.temp_allocator)
+ return found
+}
+
+// scan asks a sidecar for what the files declare. The files are paths the
+// sidecar can read; the answer names each by the path given. Everything
+// the answer holds is allocated from allocator.
+scan :: proc(
+ sidecar: Sidecar,
+ files: []string,
+ allocator := context.allocator,
+) -> (
+ out: Output,
+ err: Scan_Error,
+) {
+ if len(files) == 0 {
+ return
+ }
+ if !installed(sidecar) {
+ return out, .Not_Installed
+ }
+ argv := make([]string, len(files) + 1, context.temp_allocator)
+ argv[0] = binary(sidecar)
+ copy(argv[1:], files)
+ r := sh.exec(argv, allocator = allocator)
+ if !r.ok && len(r.stdout) == 0 {
+ return out, .Failed
+ }
+ if e := json.unmarshal_string(r.stdout, &out, allocator = allocator); e != nil {
+ return out, .Unreadable
+ }
+ return out, .None
+}
+
+// tests are the declarations a file's test runner would run.
+tests :: proc(file: File, allocator := context.allocator) -> []Decl {
+ found := make([dynamic]Decl, allocator)
+ for decl in file.decls {
+ if decl.test {
+ append(&found, decl)
+ }
+ }
+ return found[:]
+}
diff --git a/odin/frontend/frontend_test.odin b/odin/frontend/frontend_test.odin
@@ -0,0 +1,94 @@
+package frontend
+
+import "core:os"
+import "core:path/filepath"
+import "core:testing"
+
+write_temp :: proc(t: ^testing.T, name, src: string) -> string {
+ dir, err := os.temp_directory(context.temp_allocator)
+ testing.expect(t, err == nil, "temp directory")
+ path, join_err := filepath.join({dir, name}, context.temp_allocator)
+ testing.expect(t, join_err == nil, "join")
+ testing.expect(t, os.write_entire_file(path, transmute([]byte)src) == nil, "write fixture")
+ return path
+}
+
+@(test)
+go_sidecar_lists_declarations :: proc(t: ^testing.T) {
+ if !installed(.Go) {
+ testing.fail_now(
+ t,
+ "review-go is not on the path: go build -o ~/go/bin/review-go ./sidecar/gofront",
+ )
+ }
+ path := write_temp(
+ t,
+ "review_frontend_fixture.go",
+ `package fixture
+
+import "fmt"
+
+// Limit bounds the work.
+const Limit = 3
+
+type Point struct{ X int }
+
+func helper() { fmt.Println(Limit) }
+
+func TestHelper(t *testing.T) { helper() }
+`,
+ )
+ defer os.remove(path)
+
+ out, err := scan(.Go, {path}, context.temp_allocator)
+ testing.expect_value(t, err, Scan_Error.None)
+ testing.expect_value(t, len(out.files), 1)
+ file := out.files[0]
+ testing.expect_value(t, file.name, path)
+ testing.expect_value(t, file.pkg, "fixture")
+ testing.expect_value(t, file.error, "")
+ testing.expect_value(t, len(file.imports), 1)
+ testing.expect_value(t, file.imports[0], "fmt")
+
+ by_name := make(map[string]Decl, context.temp_allocator)
+ for decl in file.decls {
+ by_name[decl.name] = decl
+ }
+ testing.expect_value(t, len(by_name), 5)
+ testing.expect_value(t, by_name["Limit"].kind, "const")
+ testing.expect_value(t, by_name["Limit"].line, 6)
+ testing.expect_value(t, by_name["Limit"].doc, "Limit bounds the work.")
+ testing.expect_value(t, by_name["Limit"].exported, true)
+ testing.expect_value(t, by_name["Point"].kind, "type")
+ testing.expect_value(t, by_name["X"].kind, "field")
+ testing.expect_value(t, by_name["helper"].exported, false)
+ testing.expect_value(t, by_name["helper"].text, "func helper() { fmt.Println(Limit) }")
+ testing.expect_value(t, by_name["TestHelper"].test, true)
+
+ found := tests(file, context.temp_allocator)
+ testing.expect_value(t, len(found), 1)
+ testing.expect_value(t, found[0].name, "TestHelper")
+ testing.expect_value(t, len(file.comments), 1)
+ testing.expect_value(t, file.comments[0].line, 5)
+}
+
+@(test)
+go_sidecar_reports_a_file_it_cannot_parse :: proc(t: ^testing.T) {
+ if !installed(.Go) {
+ testing.fail_now(t, "review-go is not on the path")
+ }
+ path := write_temp(t, "review_frontend_broken.go", "package x\nfunc {\n")
+ defer os.remove(path)
+ out, err := scan(.Go, {path}, context.temp_allocator)
+ testing.expect_value(t, err, Scan_Error.None)
+ testing.expect_value(t, len(out.files), 1)
+ testing.expect(t, len(out.files[0].error) > 0, "a parse error is reported")
+ testing.expect_value(t, len(out.files[0].decls), 0)
+}
+
+@(test)
+scan_of_nothing_asks_nothing :: proc(t: ^testing.T) {
+ out, err := scan(.Go, {}, context.temp_allocator)
+ testing.expect_value(t, err, Scan_Error.None)
+ testing.expect_value(t, len(out.files), 0)
+}
diff --git a/odin/git/git.odin b/odin/git/git.odin
@@ -0,0 +1,55 @@
+/*
+Package git asks the repository questions through the git program, which is
+the one reader of a repository worth trusting. Every answer is a string;
+a failed ask is reported as not ok, with git's own words on stderr kept for
+the caller that wants them.
+*/
+package git
+
+import "core:strings"
+import "jfm:sh"
+
+// run asks git, in root, and returns what it printed with trailing
+// whitespace removed.
+run :: proc(
+ root: string,
+ args: []string,
+ allocator := context.allocator,
+) -> (
+ out: string,
+ ok: bool,
+) {
+ argv := make([]string, len(args) + 1, context.temp_allocator)
+ argv[0] = "git"
+ copy(argv[1:], args)
+ r := sh.exec(argv, {dir = root}, allocator)
+ if !r.ok {
+ return r.stderr, false
+ }
+ return strings.trim_right_space(r.stdout), true
+}
+
+// lines is run, split into lines, empty ones dropped.
+lines :: proc(
+ root: string,
+ args: []string,
+ allocator := context.allocator,
+) -> (
+ out: []string,
+ ok: bool,
+) {
+ text := run(root, args, context.temp_allocator) or_return
+ kept := make([dynamic]string, allocator)
+ for line in strings.split_lines(text, context.temp_allocator) {
+ trimmed := strings.trim_space(line)
+ if len(trimmed) > 0 {
+ append(&kept, strings.clone(trimmed, allocator))
+ }
+ }
+ return kept[:], true
+}
+
+// toplevel is the repository a directory sits in.
+toplevel :: proc(dir: string, allocator := context.allocator) -> (root: string, ok: bool) {
+ return run(dir, {"rev-parse", "--show-toplevel"}, allocator)
+}
diff --git a/odin/odinfmt.json b/odin/odinfmt.json
@@ -0,0 +1,8 @@
+{
+ "$schema": "https://raw.githubusercontent.com/DanielGavin/ols/master/misc/odinfmt.schema.json",
+ "character_width": 100,
+ "tabs": true,
+ "tabs_width": 4,
+ "newline_style": "LF",
+ "align_constant_definitions": true
+}
diff --git a/odin/review/main.odin b/odin/review/main.odin
@@ -0,0 +1,73 @@
+// review, the Odin reading: what a change adds, read through the language
+// sidecars, and the repository's declarations it is judged against. The
+// change is the staged one, or the revision range given.
+//
+// review # the staged change
+// review HEAD^..HEAD
+package main
+
+import "core:fmt"
+import "core:os"
+
+import "../change"
+import "../git"
+import "../tree"
+
+main :: proc() {
+ rev := os.args[1] if len(os.args) > 1 else ""
+ cwd, cwd_err := os.get_working_directory(context.temp_allocator)
+ if cwd_err != nil {
+ fmt.eprintln("review: no working directory")
+ os.exit(2)
+ }
+ root, in_repo := git.toplevel(cwd)
+ if !in_repo {
+ fmt.eprintln("review: not in a git repository")
+ os.exit(2)
+ }
+ c, gathered := change.gather(rev, root)
+ if !gathered {
+ fmt.eprintfln("review: git could not describe %q", rev)
+ os.exit(1)
+ }
+ if len(c.files) == 0 {
+ fmt.println("nothing to review")
+ return
+ }
+ t, at_ok := tree.at(root, rev)
+ if !at_ok {
+ fmt.eprintfln("review: could not materialise %q", rev)
+ os.exit(1)
+ }
+ defer tree.close(t)
+
+ read_ok := change.read(&c, t)
+ fmt.printfln("=== change %s: %d files, %d added lines ===", rev, len(c.files), added(c))
+ for s in c.symbols {
+ mark := " exported" if s.exported else ""
+ fmt.printfln("symbol %s:%d %s %s%s", s.file, s.line, s.kind, s.name, mark)
+ }
+ for f in c.tests {
+ fmt.printfln("test %s:%d %s", f.file, f.line, f.name)
+ }
+ for l in c.comments {
+ fmt.printfln("comment %s:%d %s", l.file, l.line, l.text)
+ }
+ for name in c.uncovered {
+ fmt.printfln("unread %s", name)
+ }
+ declared, indexed := change.index(t)
+ fmt.printfln("=== index: %d declarations ===", len(declared))
+ if !read_ok || !indexed {
+ fmt.eprintln("review: a sidecar did not answer")
+ os.exit(1)
+ }
+}
+
+added :: proc(c: change.Change) -> int {
+ n := 0
+ for _, lines in c.added {
+ n += len(lines)
+ }
+ return n
+}
diff --git a/odin/tree/tree.odin b/odin/tree/tree.odin
@@ -0,0 +1,109 @@
+/*
+Package tree reads the repository as it stands at the end of a change. For
+the staged change and a bare revision that is the working tree, read in
+place; for a range it is a tree git holds and the filesystem does not, so
+the range's end is written once to a scratch directory and read from there.
+Every reader after that reads files, which is what a sidecar can be handed.
+*/
+package tree
+
+import "core:fmt"
+import "core:os"
+import "core:path/filepath"
+import "core:strings"
+import "jfm:sh"
+
+import "../git"
+
+// Tree is the repository's files at the end of the change.
+Tree :: struct {
+ // dir is where the files are read from: the repository root, or a
+ // scratch copy of the revision.
+ dir: string,
+ // root is the repository, which is what git is asked about.
+ root: string,
+ // rev is the revision materialised, or empty for the working tree.
+ rev: string,
+}
+
+// at is the tree the change arrives at. A range's end is materialised;
+// the working tree is read in place.
+at :: proc(root, rev: string, allocator := context.allocator) -> (t: Tree, ok: bool) {
+ after, ranged := ends(rev)
+ if !ranged {
+ return Tree{dir = root, root = root}, true
+ }
+ dir := materialise(root, after, allocator) or_return
+ return Tree{dir = dir, root = root, rev = after}, true
+}
+
+// close removes what materialising left behind.
+close :: proc(t: Tree) {
+ if t.rev != "" && t.dir != t.root {
+ os.remove_all(t.dir)
+ }
+}
+
+// materialise writes the revision's files to a scratch directory, out of
+// git's own archive of it: one ask, however many files. The archive is
+// unpacked by the tar on the path, which every platform now ships.
+materialise :: proc(root, rev: string, allocator := context.allocator) -> (dir: string, ok: bool) {
+ temp := os.temp_directory(context.temp_allocator) or_else ""
+ scratch, err := os.make_directory_temp(temp, "review-tree-*", allocator)
+ if err != nil {
+ return "", false
+ }
+ cmd := fmt.tprintf(
+ "git archive --format=tar %s | tar -x -C %s",
+ sh.quote(rev, context.temp_allocator),
+ sh.quote(scratch, context.temp_allocator),
+ )
+ r := sh.capture(cmd, {dir = root}, context.temp_allocator)
+ if !r.ok {
+ os.remove_all(scratch)
+ return "", false
+ }
+ return scratch, true
+}
+
+// files lists the tracked paths at the end of the change. The staged
+// change is tracked by the index, which is what ls-files reads.
+files :: proc(t: Tree, allocator := context.allocator) -> (out: []string, ok: bool) {
+ if t.rev == "" {
+ return git.lines(t.root, {"ls-files"}, allocator)
+ }
+ return git.lines(t.root, {"ls-tree", "-r", "--name-only", t.rev}, allocator)
+}
+
+// path is where a tracked file can be read by a program that reads files.
+path :: proc(t: Tree, name: string, allocator := context.allocator) -> string {
+ joined, err := filepath.join({t.dir, name}, allocator)
+ if err != nil {
+ return name
+ }
+ return joined
+}
+
+// read is a tracked file's contents at the end of the change.
+read :: proc(t: Tree, name: string, allocator := context.allocator) -> (data: []byte, ok: bool) {
+ contents, err := os.read_entire_file_from_path(
+ path(t, name, context.temp_allocator),
+ allocator,
+ )
+ return contents, err == nil
+}
+
+// exists reports whether the tree holds the path as a file.
+exists :: proc(t: Tree, name: string) -> bool {
+ return os.is_file(path(t, name, context.temp_allocator))
+}
+
+// ends reports the revision a range arrives at, and whether it is a range
+// at all. Both A..B and A...B are reviewed as B.
+ends :: proc(rev: string) -> (after: string, ranged: bool) {
+ i := strings.index(rev, "..")
+ if i < 0 {
+ return rev, false
+ }
+ return strings.trim_prefix(rev[i + 2:], "."), true
+}
diff --git a/odin/tree/tree_test.odin b/odin/tree/tree_test.odin
@@ -0,0 +1,25 @@
+package tree
+
+import "core:testing"
+
+@(test)
+ends_reads_a_range :: proc(t: ^testing.T) {
+ after, ranged := ends("abc")
+ testing.expect_value(t, after, "abc")
+ testing.expect_value(t, ranged, false)
+ after, ranged = ends("a..b")
+ testing.expect_value(t, after, "b")
+ testing.expect_value(t, ranged, true)
+ after, ranged = ends("a...b")
+ testing.expect_value(t, after, "b")
+ testing.expect_value(t, ranged, true)
+}
+
+@(test)
+working_tree_reads_in_place :: proc(t: ^testing.T) {
+ tr, ok := at("/tmp", "")
+ testing.expect(t, ok)
+ testing.expect_value(t, tr.dir, "/tmp")
+ testing.expect_value(t, path(tr, "a/b.go", context.temp_allocator), "/tmp/a/b.go")
+ close(tr)
+}
diff --git a/ols.json b/ols.json
@@ -0,0 +1,8 @@
+{
+ "$schema": "https://raw.githubusercontent.com/DanielGavin/ols/master/misc/ols.schema.json",
+ "collections": [
+ { "name": "jfm", "path": "../odin" }
+ ],
+ "enable_semantic_tokens": true,
+ "enable_snippets": true
+}
diff --git a/readme.md b/readme.md
@@ -434,7 +434,9 @@ in `criteria/` that reads like an exception was written to stop one.
## Languages
-Go is read with the standard library's parser. TypeScript and JavaScript — `.ts`,
+Go is read with the standard library's parser; the same reading ships as the `review-go`
+sidecar, built from `sidecar/gofront`, for a reviewer written in another language — the
+Odin reading under `odin/` drives it and `odin-review-extract` through one JSON shape. TypeScript and JavaScript — `.ts`,
`.tsx`, `.js`, `.jsx`, `.mjs`, `.cjs` — are read through ast-grep when it is on the
path, by pattern; Python and Rust through the same ast-grep, by node kind — a function
is whatever the grammar calls one, and its name is read out of the match. Odin is read
diff --git a/sidecar/gofront/main.go b/sidecar/gofront/main.go
@@ -0,0 +1,239 @@
+// review-go prints the declarations, imports and comments of the Go files it
+// is given, as JSON, using Go's own parser. It is the Go frontend as a
+// sidecar, so a reviewer written in another language reads Go the way this
+// one does. One line per invocation:
+//
+// review-go file.go ... > decls.json
+//
+// where the JSON is {"files":[{"name":"a.go","package":"main",
+// "imports":["fmt"],"decls":[{"name":"main","kind":"func","line":7,
+// "end_line":9,"exported":false,"test":false,"local":false,
+// "text":"func main() {","doc":"..."}],"comments":[{"line":5,"text":"..."}]}]}
+//
+// Kinds are func, type, field, const and var. A method is a func under its
+// own name. A test is a func the testing package would run. A local
+// declaration sits inside a function body. A file that does not parse is
+// reported with its error and nothing else, so the caller can tell a file
+// with no declarations from one it could not read. Build it and put it on
+// the path:
+//
+// go build -o ~/go/bin/review-go ./sidecar/gofront
+package main
+
+import (
+ "encoding/json"
+ "fmt"
+ "go/ast"
+ "go/parser"
+ "go/token"
+ "os"
+ "strings"
+)
+
+// Decl is one declaration as the sidecar reports it. The shape is the Odin
+// sidecar's, with what Go adds: a kind that tells const from var, and
+// whether the declaration is local to a function.
+type Decl struct {
+ Name string `json:"name"`
+ Kind string `json:"kind"`
+ Line int `json:"line"`
+ EndLine int `json:"end_line"`
+ Exported bool `json:"exported"`
+ Test bool `json:"test"`
+ Local bool `json:"local"`
+ Text string `json:"text"`
+ Doc string `json:"doc"`
+}
+
+// Comment is one comment, located by its first line, with the marker
+// stripped and the text trimmed.
+type Comment struct {
+ Line int `json:"line"`
+ Text string `json:"text"`
+}
+
+// File is what one Go file declares.
+type File struct {
+ Name string `json:"name"`
+ Package string `json:"package,omitempty"`
+ Imports []string `json:"imports"`
+ Decls []Decl `json:"decls"`
+ Comments []Comment `json:"comments"`
+ Error string `json:"error,omitempty"`
+}
+
+type Output struct {
+ Files []File `json:"files"`
+}
+
+func main() {
+ out := Output{Files: []File{}}
+ for _, arg := range os.Args[1:] {
+ source, err := os.ReadFile(arg)
+ if err != nil {
+ out.Files = append(out.Files, File{Name: arg, Imports: []string{}, Decls: []Decl{}, Comments: []Comment{}, Error: err.Error()})
+ continue
+ }
+ out.Files = append(out.Files, readFile(arg, source))
+ }
+ data, err := json.MarshalIndent(out, "", " ")
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "review-go: %v\n", err)
+ os.Exit(1)
+ }
+ os.Stdout.Write(data)
+}
+
+// readFile parses one file and lists everything in it a reviewer asks
+// about. Every declaration is reported, tests and locals included; what to
+// keep is the caller's decision.
+func readFile(name string, source []byte) File {
+ result := File{Name: name, Imports: []string{}, Decls: []Decl{}, Comments: []Comment{}}
+ fset := token.NewFileSet()
+ file, err := parser.ParseFile(fset, name, source, parser.ParseComments)
+ if err != nil {
+ result.Error = err.Error()
+ return result
+ }
+ result.Package = file.Name.Name
+ lines := strings.Split(string(source), "\n")
+ line := func(pos token.Pos) int { return fset.Position(pos).Line }
+ textAt := func(pos token.Pos) string {
+ n := line(pos)
+ if n < 1 || n > len(lines) {
+ return ""
+ }
+ return strings.TrimSpace(lines[n-1])
+ }
+ for _, imported := range file.Imports {
+ result.Imports = append(result.Imports, importName(imported))
+ }
+
+ // A declaration is local when it sits inside a function body. Inspect
+ // gives no leaving event, so the bodies still open are kept by their
+ // end, and closed as soon as a node starts past it.
+ var open []token.Pos
+ ast.Inspect(file, func(n ast.Node) bool {
+ if n == nil {
+ return false
+ }
+ for len(open) > 0 && n.Pos() >= open[len(open)-1] {
+ open = open[:len(open)-1]
+ }
+ switch d := n.(type) {
+ case *ast.FuncDecl:
+ name := d.Name.Name
+ result.Decls = append(result.Decls, Decl{
+ Name: name, Kind: "func", Line: line(d.Pos()), EndLine: line(d.End()),
+ Exported: ast.IsExported(name), Test: isTest(name),
+ Text: textAt(d.Pos()), Doc: doc(d.Doc),
+ })
+ if d.Body != nil {
+ open = append(open, d.Body.End())
+ }
+ case *ast.FuncLit:
+ open = append(open, d.Body.End())
+ case *ast.GenDecl:
+ result.Decls = append(result.Decls, specs(d, len(open) > 0, line, textAt)...)
+ }
+ return true
+ })
+
+ for _, group := range file.Comments {
+ for _, comment := range group.List {
+ result.Comments = append(result.Comments, Comment{
+ Line: line(comment.Pos()),
+ Text: strings.TrimSpace(strings.TrimPrefix(comment.Text, "//")),
+ })
+ }
+ }
+ return result
+}
+
+// specs lists what a const, var or type declaration declares. A struct's
+// fields are declarations too, and a field is where a restated fact often
+// sits.
+func specs(d *ast.GenDecl, local bool, line func(token.Pos) int, textAt func(token.Pos) string) []Decl {
+ var decls []Decl
+ for _, spec := range d.Specs {
+ switch s := spec.(type) {
+ case *ast.TypeSpec:
+ decls = append(decls, Decl{
+ Name: s.Name.Name, Kind: "type", Line: line(s.Pos()), EndLine: line(s.End()),
+ Exported: ast.IsExported(s.Name.Name), Local: local,
+ Text: textAt(s.Pos()), Doc: docOf(s.Doc, d),
+ })
+ structure, ok := s.Type.(*ast.StructType)
+ if !ok {
+ continue
+ }
+ for _, field := range structure.Fields.List {
+ for _, ident := range field.Names {
+ decls = append(decls, Decl{
+ Name: ident.Name, Kind: "field", Line: line(ident.Pos()), EndLine: line(field.End()),
+ Exported: ast.IsExported(ident.Name), Local: local,
+ Text: textAt(ident.Pos()), Doc: doc(field.Doc),
+ })
+ }
+ }
+ case *ast.ValueSpec:
+ for _, ident := range s.Names {
+ decls = append(decls, Decl{
+ Name: ident.Name, Kind: kindOf(d.Tok), Line: line(ident.Pos()), EndLine: line(s.End()),
+ Exported: ast.IsExported(ident.Name), Local: local,
+ Text: textAt(ident.Pos()), Doc: docOf(s.Doc, d),
+ })
+ }
+ }
+ }
+ return decls
+}
+
+// importName is the name an import binds in the file: its alias where it
+// has one, else the last element of its path.
+//
+//review:ignore duplicate-body the sidecar is one self-contained package main, and the Go tool keeps its own reading until it reads Go through this one
+func importName(spec *ast.ImportSpec) string {
+ if spec.Name != nil {
+ return spec.Name.Name
+ }
+ path := strings.Trim(spec.Path.Value, `"`)
+ if i := strings.LastIndex(path, "/"); i >= 0 {
+ path = path[i+1:]
+ }
+ return path
+}
+
+// isTest reports whether a function is one the testing package runs.
+func isTest(name string) bool {
+ return strings.HasPrefix(name, "Test") || strings.HasPrefix(name, "Fuzz") || strings.HasPrefix(name, "Benchmark")
+}
+
+func kindOf(tok token.Token) string {
+ switch tok {
+ case token.CONST:
+ return "const"
+ case token.VAR:
+ return "var"
+ }
+ return "value"
+}
+
+func doc(group *ast.CommentGroup) string {
+ if group == nil {
+ return ""
+ }
+ return strings.TrimSpace(group.Text())
+}
+
+// docOf is a spec's own doc, or its group's when the group declares only
+// it: `// Doc` above `type X struct` documents X, not a parenthesis.
+func docOf(own *ast.CommentGroup, group *ast.GenDecl) string {
+ if text := doc(own); text != "" {
+ return text
+ }
+ if len(group.Specs) == 1 {
+ return doc(group.Doc)
+ }
+ return ""
+}
diff --git a/sidecar/gofront/main_test.go b/sidecar/gofront/main_test.go
@@ -0,0 +1,137 @@
+package main
+
+import (
+ "encoding/json"
+ "strings"
+ "testing"
+)
+
+const fixture = `// Package fixture is read by the test.
+package fixture
+
+import (
+ "fmt"
+ yaml "go.yaml.in/yaml/v4"
+ "path/filepath"
+)
+
+// Limit bounds the work.
+const Limit = 3
+
+var (
+ // count is kept between calls.
+ count int
+ name = "x"
+)
+
+// Point is a place.
+type Point struct {
+ // X is across.
+ X, Y int
+ label string
+}
+
+// Move shifts the point.
+func (p *Point) Move(dx int) { p.X += dx }
+
+func helper() {
+ const inner = 1
+ type pair struct{ a, b int }
+ f := func() {
+ var deep = 2
+ _ = deep
+ }
+ f()
+ _ = inner
+ fmt.Println(yaml.Marshal, filepath.Join)
+}
+
+func TestHelper(t *testing.T) { helper() }
+
+/* block
+comment */
+func BenchmarkHelper(b *testing.B) {}
+`
+
+func TestReadFileListsEverything(t *testing.T) {
+ file := readFile("fixture.go", []byte(fixture))
+ if file.Error != "" {
+ t.Fatal(file.Error)
+ }
+ if file.Package != "fixture" {
+ t.Errorf("package %q", file.Package)
+ }
+ if got := strings.Join(file.Imports, ","); got != "fmt,yaml,filepath" {
+ t.Errorf("imports %q", got)
+ }
+ want := map[string]Decl{
+ "Limit": {Kind: "const", Line: 11, Exported: true, Doc: "Limit bounds the work.", Text: "const Limit = 3"},
+ "count": {Kind: "var", Line: 15, Doc: "count is kept between calls.", Text: "count int"},
+ "name": {Kind: "var", Line: 16, Text: `name = "x"`},
+ "Point": {Kind: "type", Line: 20, Exported: true, Doc: "Point is a place.", Text: "type Point struct {"},
+ "X": {Kind: "field", Line: 22, Exported: true, Doc: "X is across.", Text: "X, Y int"},
+ "Y": {Kind: "field", Line: 22, Exported: true, Doc: "X is across.", Text: "X, Y int"},
+ "label": {Kind: "field", Line: 23, Text: "label string"},
+ "Move": {Kind: "func", Line: 27, Exported: true, Doc: "Move shifts the point.", Text: "func (p *Point) Move(dx int) { p.X += dx }"},
+ "helper": {Kind: "func", Line: 29, Text: "func helper() {"},
+ "inner": {Kind: "const", Line: 30, Local: true, Text: "const inner = 1"},
+ "pair": {Kind: "type", Line: 31, Local: true, Text: "type pair struct{ a, b int }"},
+ "a": {Kind: "field", Line: 31, Local: true, Text: "type pair struct{ a, b int }"},
+ "b": {Kind: "field", Line: 31, Local: true, Text: "type pair struct{ a, b int }"},
+ "deep": {Kind: "var", Line: 33, Local: true, Text: "var deep = 2"},
+ "TestHelper": {Kind: "func", Line: 41, Exported: true, Test: true, Text: "func TestHelper(t *testing.T) { helper() }"},
+ "BenchmarkHelper": {Kind: "func", Line: 45, Exported: true, Test: true, Doc: "block\ncomment", Text: "func BenchmarkHelper(b *testing.B) {}"},
+ }
+ if len(file.Decls) != len(want) {
+ names := []string{}
+ for _, d := range file.Decls {
+ names = append(names, d.Name)
+ }
+ t.Errorf("%d declarations %v, want %d", len(file.Decls), names, len(want))
+ }
+ for _, got := range file.Decls {
+ w, ok := want[got.Name]
+ if !ok {
+ t.Errorf("unexpected %+v", got)
+ continue
+ }
+ w.Name = got.Name
+ w.EndLine = got.EndLine
+ if got != w {
+ t.Errorf("%s:\n got %+v\nwant %+v", got.Name, got, w)
+ }
+ if got.EndLine < got.Line {
+ t.Errorf("%s ends on %d before it starts on %d", got.Name, got.EndLine, got.Line)
+ }
+ }
+ for _, d := range file.Decls {
+ if d.Name == "helper" && d.EndLine != 39 {
+ t.Errorf("helper ends on %d, want 39", d.EndLine)
+ }
+ }
+ comments := map[int]string{}
+ for _, c := range file.Comments {
+ comments[c.Line] = c.Text
+ }
+ for line, text := range map[int]string{1: "Package fixture is read by the test.", 14: "count is kept between calls.", 43: "/* block\ncomment */"} {
+ if comments[line] != text {
+ t.Errorf("comment on %d: %q, want %q", line, comments[line], text)
+ }
+ }
+}
+
+func TestReadFileReportsAParseError(t *testing.T) {
+ file := readFile("broken.go", []byte("package x\nfunc {"))
+ if file.Error == "" || len(file.Decls) != 0 {
+ t.Errorf("%+v", file)
+ }
+ data, err := json.Marshal(Output{Files: []File{file}})
+ if err != nil {
+ t.Fatal(err)
+ }
+ for _, key := range []string{`"imports":[]`, `"decls":[]`, `"comments":[]`, `"error":`} {
+ if !strings.Contains(string(data), key) {
+ t.Errorf("%s missing from %s", key, data)
+ }
+ }
+}