mirror of
https://github.com/esengine/DeepSeek-Reasonix.git
synced 2026-11-07 02:36:01 -10:00
chore(standards): machine-enforce comment, file-size, and layering rules (#7806)
* chore(standards): gate comments, file size, and layering in CI The comment and file-size rules lived only in an untracked CLAUDE.md written for a TypeScript project, so nothing ever checked them, and REASONIX.md told editors to match the surrounding comment density -- which is what reproduced the essays. tools/repolint makes the rules machine-checkable and ratchets them: recorded debt passes, anything new fails the required lint check. Weights are excess lines rather than counts, so trading a short violation for a long one still trips the gate. Layering encodes the contract REASONIX.md already claimed and the import graph already satisfies, with one recorded exception (capdiag reaching into boot). * chore(standards): point CLAUDE.md at REASONIX.md Claude Code loads CLAUDE.md and Reasonix loads REASONIX.md, so a repo with only the latter leaves Claude Code sessions with no standing instructions. A committed one-line pointer keeps the single source of truth without the two files drifting apart again; the negation un-ignores it for contributors whose global gitignore drops CLAUDE.md.
This commit is contained in:
16 files changed
+2661
-4
No files matched your search
@@ -3,3 +3,4 @@
|
||||
# Full tests run in CI; vet is the fast, cross-platform local check.
|
||||
set -e
|
||||
go vet ./...
|
||||
go run ./tools/repolint
|
||||
@@ -458,9 +458,11 @@ jobs:
|
||||
timeout-minutes: 20
|
||||
run: ../scripts/desktop-build.sh windows/amd64 v0.0.0-ci canary
|
||||
|
||||
# repolint scans desktop/ and sdk/ too, so this job also runs for diffs the
|
||||
# `code` filter would otherwise skip.
|
||||
lint:
|
||||
needs: changes
|
||||
if: always() && (github.event_name != 'pull_request' || needs.changes.outputs.code != 'false')
|
||||
if: always() && (github.event_name != 'pull_request' || needs.changes.outputs.code != 'false' || needs.changes.outputs.desktop != 'false' || needs.changes.outputs.sdk != 'false')
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v7
|
||||
@@ -470,6 +472,9 @@ jobs:
|
||||
go-version-file: go.mod
|
||||
cache: true
|
||||
|
||||
- name: repo standards
|
||||
run: go run ./tools/repolint
|
||||
|
||||
- name: golangci-lint
|
||||
uses: golangci/golangci-lint-action@v9
|
||||
with:
|
||||
|
||||
@@ -73,3 +73,6 @@ scripts/official-theme-art/out/
|
||||
scripts/official-theme-art/__pycache__/
|
||||
agent-complexity-report.txt
|
||||
site/.generated/
|
||||
|
||||
# Committed pointer at REASONIX.md; overrides a global CLAUDE.md ignore rule.
|
||||
!/CLAUDE.md
|
||||
@@ -0,0 +1,6 @@
|
||||
# Reasonix
|
||||
|
||||
The project's standing instructions live in `REASONIX.md` — one file, read by
|
||||
both Reasonix and Claude Code. Add rules there, never here.
|
||||
|
||||
@REASONIX.md
|
||||
@@ -7,7 +7,7 @@ LDFLAGS := -s -w \
|
||||
-X main.buildTimeUTC=$(BUILD_TIME_UTC)
|
||||
GOEXE := $(shell go env GOEXE)
|
||||
|
||||
.PHONY: build vet fmt test desktop-test desktop-test-short desktop-test-times sdk-test sdk-test-race hooks cross clean
|
||||
.PHONY: build vet fmt lint lint-update test desktop-test desktop-test-short desktop-test-times sdk-test sdk-test-race hooks cross clean
|
||||
|
||||
build:
|
||||
CGO_ENABLED=0 go build -ldflags "$(LDFLAGS)" -o bin/reasonix$(GOEXE) ./cmd/reasonix
|
||||
@@ -19,6 +19,12 @@ vet:
|
||||
fmt:
|
||||
gofmt -w .
|
||||
|
||||
lint:
|
||||
go run ./tools/repolint
|
||||
|
||||
lint-update:
|
||||
go run ./tools/repolint -update
|
||||
|
||||
test:
|
||||
go test ./...
|
||||
|
||||
|
||||
+25
-2
@@ -6,15 +6,37 @@ agent. It is the Reasonix analog of Claude Code's CLAUDE.md.
|
||||
|
||||
## Conventions
|
||||
|
||||
- Go kernel under `internal/`; each package owns one concern and documents it in a
|
||||
package comment. Match the surrounding comment density and idiom when editing.
|
||||
- Go kernel under `internal/`; each package owns one concern. A package's long
|
||||
explanation belongs in its `doc.go`, not spread across implementation files.
|
||||
- One transport-agnostic `control.Controller` sits behind every frontend (chat
|
||||
TUI, HTTP/SSE serve, Wails desktop). Add behavior to the controller, not a
|
||||
frontend, so all three inherit it.
|
||||
- Layering (enforced): utility packages import nothing under `reasonix/`; only
|
||||
the frontends `cli`, `serve`, `acp`, `bot`, `botruntime`, `boot` and the hosts
|
||||
`cmd/`, `desktop/` may import `control`; nothing below a frontend may import
|
||||
one. The declared sets live in `tools/repolint/layers.go`.
|
||||
- Cache-first: the system-prompt prefix (base prompt + tools + memory) must stay
|
||||
byte-stable across turns so DeepSeek's automatic prefix cache stays warm. Never
|
||||
mutate it mid-session — ride the turn tail instead (see `control.Compose`).
|
||||
|
||||
## Comments
|
||||
|
||||
Default is none — the code is the truth. Write one only when the **why** is
|
||||
non-obvious: a hidden constraint, a workaround anchored to something verifiable,
|
||||
an invariant the type system cannot express, or an external-protocol quirk.
|
||||
|
||||
- Declaration doc: ≤5 lines. Package comment: ≤8 lines, or ≤40 in a `doc.go`.
|
||||
- Every other comment: ≤3 lines. Struct-field and trailing `//`: 1 line.
|
||||
- Never: restatements of the code, phase/stage narrative, incident or
|
||||
conversation history, section banners, commented-out code, `@param` lists.
|
||||
- `TODO(#nnn):` and `HACK(#nnn):` need the issue anchor. `FIXME` is banned.
|
||||
- One responsibility per file; 800 lines is the ceiling.
|
||||
|
||||
`go run ./tools/repolint` enforces all of it against a ratchet baseline: recorded
|
||||
debt is tolerated, anything new fails CI. Never widen the baseline to land a
|
||||
change — fix the code. `-update` exists for carrying debt through a rename or an
|
||||
extraction, and that diff must be justified in the PR.
|
||||
|
||||
## Memory
|
||||
|
||||
- Standing instructions are hierarchical: committed/shared `REASONIX.md`,
|
||||
@@ -40,6 +62,7 @@ Run these **before every commit** to catch the fastest CI failures locally:
|
||||
```bash
|
||||
gofmt -w . # catches gofmt (saves ~13s CI)
|
||||
go vet ./... # catches vet warnings (saves ~52s CI/lint)
|
||||
go run ./tools/repolint # catches comment/size/layering regressions
|
||||
go test ./internal/tool/builtin/ ./internal/boot/ # catches tool/boot test breaks
|
||||
```
|
||||
|
||||
|
||||
@@ -0,0 +1,96 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"os"
|
||||
)
|
||||
|
||||
// Ratchet snapshot, in weighted units: no file may exceed its budget and no
|
||||
// rule may exceed its repo ceiling. Both are relaxed only by hand, in a diff a
|
||||
// reviewer sees.
|
||||
type Baseline struct {
|
||||
Limits map[string]int `json:"limits"`
|
||||
Files map[string]map[string]int `json:"files"`
|
||||
}
|
||||
|
||||
func loadBaseline(path string) (*Baseline, error) {
|
||||
data, err := os.ReadFile(path)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var b Baseline
|
||||
if err := json.Unmarshal(data, &b); err != nil {
|
||||
return nil, fmt.Errorf("%s: %w", path, err)
|
||||
}
|
||||
if b.Limits == nil {
|
||||
b.Limits = map[string]int{}
|
||||
}
|
||||
if b.Files == nil {
|
||||
b.Files = map[string]map[string]int{}
|
||||
}
|
||||
return &b, nil
|
||||
}
|
||||
|
||||
func baselineFrom(findings []Finding) *Baseline {
|
||||
b := &Baseline{Limits: map[string]int{}, Files: map[string]map[string]int{}}
|
||||
for _, rule := range allRules {
|
||||
b.Limits[rule] = 0
|
||||
}
|
||||
for _, f := range findings {
|
||||
b.Limits[f.Rule] += f.Weight
|
||||
if b.Files[f.File] == nil {
|
||||
b.Files[f.File] = map[string]int{}
|
||||
}
|
||||
b.Files[f.File][f.Rule] += f.Weight
|
||||
}
|
||||
return b
|
||||
}
|
||||
|
||||
func (b *Baseline) write(path string) error {
|
||||
data, err := json.MarshalIndent(b, "", " ")
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
return os.WriteFile(path, append(data, '\n'), 0o644)
|
||||
}
|
||||
|
||||
func (b *Baseline) exceeded(findings []Finding) ([]Finding, []string) {
|
||||
counts := map[string]map[string]int{}
|
||||
totals := map[string]int{}
|
||||
for _, f := range findings {
|
||||
if counts[f.File] == nil {
|
||||
counts[f.File] = map[string]int{}
|
||||
}
|
||||
counts[f.File][f.Rule] += f.Weight
|
||||
totals[f.Rule] += f.Weight
|
||||
}
|
||||
|
||||
var msgs []string
|
||||
over := map[string]map[string]bool{}
|
||||
for _, file := range sortedKeys(counts) {
|
||||
for _, rule := range sortedKeys(counts[file]) {
|
||||
allowed := b.Files[file][rule]
|
||||
if got := counts[file][rule]; got > allowed {
|
||||
msgs = append(msgs, fmt.Sprintf("%s: %s is %d over its %d budget", file, rule, got, allowed))
|
||||
if over[file] == nil {
|
||||
over[file] = map[string]bool{}
|
||||
}
|
||||
over[file][rule] = true
|
||||
}
|
||||
}
|
||||
}
|
||||
for _, rule := range sortedKeys(totals) {
|
||||
if totals[rule] > b.Limits[rule] {
|
||||
msgs = append(msgs, fmt.Sprintf("repo total for %s is %d, above the %d ceiling", rule, totals[rule], b.Limits[rule]))
|
||||
}
|
||||
}
|
||||
|
||||
var shown []Finding
|
||||
for _, f := range findings {
|
||||
if over[f.File][f.Rule] {
|
||||
shown = append(shown, f)
|
||||
}
|
||||
}
|
||||
return shown, msgs
|
||||
}
|
||||
File diff suppressed because it is too large.
Load diff
@@ -0,0 +1,59 @@
|
||||
package main
|
||||
|
||||
import "testing"
|
||||
|
||||
func budget(files map[string]map[string]int, limits map[string]int) *Baseline {
|
||||
return &Baseline{Limits: limits, Files: files}
|
||||
}
|
||||
|
||||
func TestBaselineAllowsWhatItRecorded(t *testing.T) {
|
||||
b := budget(map[string]map[string]int{"a.go": {ruleEssay: 4}}, map[string]int{ruleEssay: 4})
|
||||
_, msgs := b.exceeded([]Finding{{"a.go", 3, ruleEssay, "", 4}})
|
||||
if len(msgs) != 0 {
|
||||
t.Fatalf("recorded debt reported as new: %v", msgs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestSwappingAShortEssayForALongOneTripsTheRatchet(t *testing.T) {
|
||||
b := budget(map[string]map[string]int{"a.go": {ruleEssay: 1}}, map[string]int{ruleEssay: 1})
|
||||
_, msgs := b.exceeded([]Finding{{"a.go", 3, ruleEssay, "", 27}})
|
||||
if len(msgs) == 0 {
|
||||
t.Fatal("a 27-line-over block replacing a 1-line-over block must fail")
|
||||
}
|
||||
}
|
||||
|
||||
func TestUnbaselinedFileMustBeClean(t *testing.T) {
|
||||
b := budget(map[string]map[string]int{"a.go": {ruleEssay: 9}}, map[string]int{ruleEssay: 9})
|
||||
over, msgs := b.exceeded([]Finding{{"new.go", 1, ruleEssay, "", 1}})
|
||||
if len(msgs) != 1 || len(over) != 1 || over[0].File != "new.go" {
|
||||
t.Fatalf("new file not gated: over=%v msgs=%v", over, msgs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRepoCeilingCatchesDebtMovedBetweenFiles(t *testing.T) {
|
||||
b := budget(map[string]map[string]int{"a.go": {ruleBanner: 2}, "b.go": {ruleBanner: 2}}, map[string]int{ruleBanner: 2})
|
||||
_, msgs := b.exceeded([]Finding{
|
||||
{"a.go", 1, ruleBanner, "", 1}, {"a.go", 2, ruleBanner, "", 1},
|
||||
{"b.go", 1, ruleBanner, "", 1}, {"b.go", 2, ruleBanner, "", 1},
|
||||
})
|
||||
if len(msgs) != 1 {
|
||||
t.Fatalf("per-file budgets pass but the repo ceiling of 2 must fail: %v", msgs)
|
||||
}
|
||||
}
|
||||
|
||||
func TestBaselineRoundTripsWeights(t *testing.T) {
|
||||
b := baselineFrom([]Finding{
|
||||
{"a.go", 1, ruleEssay, "", 3},
|
||||
{"a.go", 9, ruleEssay, "", 2},
|
||||
{"a.go", 4, ruleBanner, "", 1},
|
||||
})
|
||||
if got := b.Files["a.go"][ruleEssay]; got != 5 {
|
||||
t.Fatalf("essay budget = %d, want 5", got)
|
||||
}
|
||||
if got := b.Limits[ruleEssay]; got != 5 {
|
||||
t.Fatalf("essay ceiling = %d, want 5", got)
|
||||
}
|
||||
if got := b.Limits[ruleBanner]; got != 1 {
|
||||
t.Fatalf("banner ceiling = %d, want 1", got)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,145 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"regexp"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const (
|
||||
capDocGoPackage = 40
|
||||
capPackageDoc = 8
|
||||
capDeclDoc = 5
|
||||
capFieldDoc = 3
|
||||
capFloating = 3
|
||||
)
|
||||
|
||||
var (
|
||||
bannerRe = regexp.MustCompile(`^//\s*[-=~*_+#/\x{2500}-\x{257F}]{6,}\s*$`)
|
||||
labelledRe = regexp.MustCompile(`^//\s*[-=~*_+#\x{2500}-\x{257F}]{3,}.*[-=~*_+#\x{2500}-\x{257F}]{3,}\s*$`)
|
||||
anchoredRe = regexp.MustCompile(`\b(TODO|HACK)\(#\d+\)`)
|
||||
bareMarkRe = regexp.MustCompile(`\b(TODO|HACK)\b`)
|
||||
fixmeRe = regexp.MustCompile(`\bFIXME\b`)
|
||||
narrativeRe = regexp.MustCompile(`(?i)\b(phase|stage)\s+\d+[a-z]?\d*\b`)
|
||||
directiveRe = regexp.MustCompile(`^//(go:|lint:|nolint|export |sys |line )`)
|
||||
docCodeRe = regexp.MustCompile(`^//(\t| {4,})`)
|
||||
)
|
||||
|
||||
func checkComments(s *sourceFile) []Finding {
|
||||
var out []Finding
|
||||
limits := s.commentLimits()
|
||||
preamble := s.cgoPreamble()
|
||||
|
||||
for _, cg := range s.file.Comments {
|
||||
if cg == preamble {
|
||||
continue
|
||||
}
|
||||
limit, attached := limits[cg]
|
||||
if !attached {
|
||||
limit = capFloating
|
||||
}
|
||||
start, end := s.line(cg.Pos()), s.line(cg.End())
|
||||
if n := end - start + 1; n > limit {
|
||||
out = append(out, Finding{s.rel, start, ruleEssay,
|
||||
fmt.Sprintf("%d-line comment block exceeds the %d-line limit for this position", n, limit), n - limit})
|
||||
}
|
||||
out = append(out, s.checkCommentText(cg, attached)...)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func (s *sourceFile) commentLimits() map[*ast.CommentGroup]int {
|
||||
limits := map[*ast.CommentGroup]int{}
|
||||
if s.file.Doc != nil {
|
||||
if s.rel == "doc.go" || strings.HasSuffix(s.rel, "/doc.go") {
|
||||
limits[s.file.Doc] = capDocGoPackage
|
||||
} else {
|
||||
limits[s.file.Doc] = capPackageDoc
|
||||
}
|
||||
}
|
||||
ast.Inspect(s.file, func(n ast.Node) bool {
|
||||
switch d := n.(type) {
|
||||
case *ast.FuncDecl:
|
||||
set(limits, d.Doc, capDeclDoc)
|
||||
case *ast.GenDecl:
|
||||
set(limits, d.Doc, capDeclDoc)
|
||||
case *ast.TypeSpec:
|
||||
set(limits, d.Doc, capDeclDoc)
|
||||
case *ast.ValueSpec:
|
||||
set(limits, d.Doc, capDeclDoc)
|
||||
case *ast.Field:
|
||||
set(limits, d.Doc, capFieldDoc)
|
||||
}
|
||||
return true
|
||||
})
|
||||
return limits
|
||||
}
|
||||
|
||||
// The block comment preceding `import "C"` is compiler input, not prose.
|
||||
func (s *sourceFile) cgoPreamble() *ast.CommentGroup {
|
||||
for _, decl := range s.file.Decls {
|
||||
gen, ok := decl.(*ast.GenDecl)
|
||||
if !ok || gen.Tok != token.IMPORT || gen.Doc == nil {
|
||||
continue
|
||||
}
|
||||
for _, spec := range gen.Specs {
|
||||
if imp, ok := spec.(*ast.ImportSpec); ok && imp.Path.Value == `"C"` {
|
||||
return gen.Doc
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func set(limits map[*ast.CommentGroup]int, cg *ast.CommentGroup, limit int) {
|
||||
if cg != nil {
|
||||
limits[cg] = limit
|
||||
}
|
||||
}
|
||||
|
||||
func (s *sourceFile) checkCommentText(cg *ast.CommentGroup, attached bool) []Finding {
|
||||
var out []Finding
|
||||
for _, c := range cg.List {
|
||||
base, ownsLine := s.line(c.Pos()), !s.trailing(c.Pos())
|
||||
// Commented-out code sits in a body or between declarations; a doc
|
||||
// comment describing a wire format is prose that happens to parse.
|
||||
deadCodeCandidate := ownsLine && !attached
|
||||
for i, text := range strings.Split(c.Text, "\n") {
|
||||
line, trimmed := base+i, strings.TrimSpace(text)
|
||||
if directiveRe.MatchString(trimmed) {
|
||||
continue
|
||||
}
|
||||
switch {
|
||||
case bannerRe.MatchString(trimmed), labelledRe.MatchString(trimmed):
|
||||
out = append(out, Finding{s.rel, line, ruleBanner, "section-banner separator", 1})
|
||||
case deadCodeCandidate && !docCodeRe.MatchString(text) && looksLikeCode(trimmed):
|
||||
out = append(out, Finding{s.rel, line, ruleDeadCode, "commented-out code", 1})
|
||||
}
|
||||
if fixmeRe.MatchString(trimmed) {
|
||||
out = append(out, Finding{s.rel, line, ruleMarker, "FIXME is banned: fix it or open an issue and use TODO(#nnn)", 1})
|
||||
} else if bareMarkRe.MatchString(trimmed) && !anchoredRe.MatchString(trimmed) {
|
||||
out = append(out, Finding{s.rel, line, ruleMarker, "TODO/HACK needs a (#nnn) issue anchor", 1})
|
||||
}
|
||||
if narrativeRe.MatchString(trimmed) {
|
||||
out = append(out, Finding{s.rel, line, ruleNarrative, "phase/stage narrative belongs in the commit message", 1})
|
||||
}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func looksLikeCode(text string) bool {
|
||||
body := strings.TrimSpace(strings.TrimLeft(text, "/*"))
|
||||
body = strings.TrimSuffix(body, "*/")
|
||||
if len(body) < 4 || len(body) > 160 {
|
||||
return false
|
||||
}
|
||||
if !strings.Contains(body, ":=") && !strings.ContainsAny(body, "(){};") {
|
||||
return false
|
||||
}
|
||||
_, err := parser.ParseFile(token.NewFileSet(), "", "package p\nfunc _() {\n"+body+"\n}\n", parser.SkipObjectResolution)
|
||||
return err == nil
|
||||
}
|
||||
@@ -0,0 +1,101 @@
|
||||
package main
|
||||
|
||||
import "testing"
|
||||
|
||||
func rules(t *testing.T, rel, src string) []string {
|
||||
t.Helper()
|
||||
s := parseBytes(rel, []byte(src))
|
||||
if s == nil {
|
||||
t.Fatalf("parse %s failed", rel)
|
||||
}
|
||||
var out []string
|
||||
for _, f := range checkComments(s) {
|
||||
out = append(out, f.Rule)
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func TestCgoPreambleIsCompilerInputNotProse(t *testing.T) {
|
||||
src := "package main\n\n/*\n#include <stdint.h>\n\nstatic void tick(void) {\n\tif (x != NULL) {\n\t\treturn;\n\t}\n\tstart(1);\n}\n*/\nimport \"C\"\n"
|
||||
if got := rules(t, "watchdog.go", src); len(got) != 0 {
|
||||
t.Fatalf("cgo preamble flagged: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestTrailingCommentIsNotDeadCode(t *testing.T) {
|
||||
src := "package p\n\ntype T struct {\n\tKind string // tool | plan | recovery; empty = tool\n\tSum string // ssh.FingerprintSHA256(key)\n}\n"
|
||||
if got := rules(t, "t.go", src); len(got) != 0 {
|
||||
t.Fatalf("trailing field comments flagged: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDocCommentCodeBlockIsNotDeadCode(t *testing.T) {
|
||||
src := "package p\n\n// Run does a thing:\n//\n//\tx := Run(ctx)\nfunc Run() {}\n"
|
||||
if got := rules(t, "t.go", src); len(got) != 0 {
|
||||
t.Fatalf("doc code block flagged: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCommentedOutCodeInBodyIsFlagged(t *testing.T) {
|
||||
src := "package p\n\nfunc Run() {\n\t// old := compute(1)\n\t_ = 0\n}\n"
|
||||
if got := rules(t, "t.go", src); len(got) != 1 || got[0] != ruleDeadCode {
|
||||
t.Fatalf("want one %s, got %v", ruleDeadCode, got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestEssayWeightIsTheExcessOverTheLimit(t *testing.T) {
|
||||
src := "package p\n\nfunc Run() {\n\t// one\n\t// two\n\t// three\n\t// four\n\t// five\n\t_ = 0\n}\n"
|
||||
s := parseBytes("t.go", []byte(src))
|
||||
found := checkComments(s)
|
||||
if len(found) != 1 || found[0].Rule != ruleEssay {
|
||||
t.Fatalf("want one %s, got %v", ruleEssay, found)
|
||||
}
|
||||
if found[0].Weight != 5-capFloating {
|
||||
t.Fatalf("weight = %d, want %d", found[0].Weight, 5-capFloating)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDocGoCarriesTheLongPackageExplanation(t *testing.T) {
|
||||
long := "package p\n"
|
||||
doc := "// Package p explains itself.\n"
|
||||
for range capPackageDoc + 2 {
|
||||
doc += "//\n// more prose\n"
|
||||
}
|
||||
if got := rules(t, "sub/doc.go", doc+long); len(got) != 0 {
|
||||
t.Fatalf("doc.go package comment flagged: %v", got)
|
||||
}
|
||||
if got := rules(t, "sub/other.go", doc+long); len(got) == 0 {
|
||||
t.Fatal("same package comment outside doc.go should be flagged")
|
||||
}
|
||||
}
|
||||
|
||||
func TestMarkersAndNarrative(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name, comment, want string
|
||||
}{
|
||||
{"bare todo", "// TODO: later", ruleMarker},
|
||||
{"anchored todo", "// TODO(#312): later", ""},
|
||||
{"fixme", "// FIXME later", ruleMarker},
|
||||
{"stage narrative", "// Stage 6b2 hands off the prompt", ruleNarrative},
|
||||
{"lowercase hack prose", "// this is a hack around a driver bug", ""},
|
||||
{"banner", "// ----------------", ruleBanner},
|
||||
{"labelled banner", "// ─── helpers ───", ruleBanner},
|
||||
{"ascii labelled banner", "// ==== setup ====", ruleBanner},
|
||||
{"prose with ellipsis", "// waits for the child... then reaps it", ""},
|
||||
{"prose with a dash", "// clamp to width-1 — Yoga miscounts wrap", ""},
|
||||
{"build directive", "//go:generate stringer -type=T", ""},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
got := rules(t, "t.go", "package p\n\n"+tc.comment+"\nfunc Run() {}\n")
|
||||
if tc.want == "" {
|
||||
if len(got) != 0 {
|
||||
t.Fatalf("want clean, got %v", got)
|
||||
}
|
||||
return
|
||||
}
|
||||
if len(got) != 1 || got[0] != tc.want {
|
||||
t.Fatalf("want %s, got %v", tc.want, got)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,89 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"path"
|
||||
"strings"
|
||||
)
|
||||
|
||||
const modulePrefix = "reasonix/"
|
||||
|
||||
// Transport-agnostic control.Controller sits behind every frontend; nothing
|
||||
// below it may reach up. See REASONIX.md.
|
||||
var frontends = []string{
|
||||
"internal/acp",
|
||||
"internal/boot",
|
||||
"internal/bot",
|
||||
"internal/botruntime",
|
||||
"internal/cli",
|
||||
"internal/serve",
|
||||
}
|
||||
|
||||
// Utility layer: these packages carry no knowledge of the kernel and must
|
||||
// stay importable from anywhere without dragging a dependency graph along.
|
||||
var leaves = []string{
|
||||
"internal/ablation",
|
||||
"internal/billing",
|
||||
"internal/diff",
|
||||
"internal/extension/rpcwire",
|
||||
"internal/filelock",
|
||||
"internal/fileref",
|
||||
"internal/fileutil",
|
||||
"internal/fileutil/encoding",
|
||||
"internal/frontmatter",
|
||||
"internal/i18n",
|
||||
"internal/mcpdiag",
|
||||
"internal/nilutil",
|
||||
"internal/planmode",
|
||||
"internal/proc",
|
||||
"internal/releaseasset",
|
||||
"internal/retrieval",
|
||||
"internal/shellparse",
|
||||
"internal/store",
|
||||
"internal/sysproxy",
|
||||
"internal/taskintent",
|
||||
"internal/textutil",
|
||||
"internal/workspacelease",
|
||||
}
|
||||
|
||||
func checkLayering(imports map[string][]importRef) []Finding {
|
||||
var out []Finding
|
||||
for _, rel := range sortedKeys(imports) {
|
||||
pkg := path.Dir(rel)
|
||||
for _, ref := range imports[rel] {
|
||||
dep, ok := strings.CutPrefix(ref.path, modulePrefix)
|
||||
if !ok {
|
||||
continue
|
||||
}
|
||||
if msg := violates(pkg, dep); msg != "" {
|
||||
out = append(out, Finding{rel, ref.line, ruleLayering, msg, 1})
|
||||
}
|
||||
}
|
||||
}
|
||||
return out
|
||||
}
|
||||
|
||||
func violates(pkg, dep string) string {
|
||||
switch {
|
||||
case matches(leaves, pkg):
|
||||
return fmt.Sprintf("%s is a utility-layer package and must not import %s", pkg, dep)
|
||||
case under(dep, "internal/control") && !matches(frontends, pkg) && !under(pkg, "internal/control") && !host(pkg):
|
||||
return fmt.Sprintf("%s may not import %s: the controller is reachable from frontends and entrypoints only", pkg, dep)
|
||||
case matches(frontends, dep) && !matches(frontends, pkg) && !host(pkg):
|
||||
return fmt.Sprintf("%s may not import the %s frontend: move shared behavior below the controller", pkg, dep)
|
||||
}
|
||||
return ""
|
||||
}
|
||||
|
||||
func host(pkg string) bool { return under(pkg, "cmd") || under(pkg, "desktop") }
|
||||
|
||||
func under(pkg, root string) bool { return pkg == root || strings.HasPrefix(pkg, root+"/") }
|
||||
|
||||
func matches(roots []string, pkg string) bool {
|
||||
for _, root := range roots {
|
||||
if under(pkg, root) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
@@ -0,0 +1,46 @@
|
||||
package main
|
||||
|
||||
import "testing"
|
||||
|
||||
func TestLayeringContract(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
name, pkg, dep string
|
||||
wantViolation bool
|
||||
}{
|
||||
{"utility package stays a leaf", "internal/fileutil", "internal/config", true},
|
||||
{"utility package may use the stdlib only", "internal/textutil", "internal/agent", true},
|
||||
{"kernel may not reach the controller", "internal/agent", "internal/control", true},
|
||||
{"kernel may not reach a frontend", "internal/agent", "internal/cli", true},
|
||||
{"diagnostics may not reach the composition root", "internal/capdiag", "internal/boot", true},
|
||||
{"frontend may use the controller", "internal/serve", "internal/control", false},
|
||||
{"frontend may use another frontend", "internal/cli", "internal/serve", false},
|
||||
{"frontend subpackage may use its parent", "internal/bot/qq", "internal/bot", false},
|
||||
{"entrypoint may use a frontend", "cmd/reasonix", "internal/cli", false},
|
||||
{"desktop host may use the controller", "desktop", "internal/control", false},
|
||||
{"controller may use the kernel", "internal/control", "internal/agent", false},
|
||||
{"kernel may use a utility package", "internal/agent", "internal/fileutil", false},
|
||||
} {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
if got := violates(tc.pkg, tc.dep) != ""; got != tc.wantViolation {
|
||||
t.Fatalf("violates(%q, %q) = %v, want %v", tc.pkg, tc.dep, got, tc.wantViolation)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestLayeringReadsImportsFromSource(t *testing.T) {
|
||||
src := "package agent\n\nimport (\n\t\"fmt\"\n\t\"reasonix/internal/cli\"\n)\n\nvar _ = fmt.Sprint\nvar _ = cli.Run\n"
|
||||
s := parseBytes("internal/agent/a.go", []byte(src))
|
||||
found := checkLayering(map[string][]importRef{s.rel: s.importRefs()})
|
||||
if len(found) != 1 || found[0].Rule != ruleLayering || found[0].Line != 5 {
|
||||
t.Fatalf("want one %s on line 5, got %+v", ruleLayering, found)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLayeringIgnoresTestFiles(t *testing.T) {
|
||||
src := "package agent\n\nimport \"reasonix/internal/cli\"\n\nvar _ = cli.Run\n"
|
||||
s := parseBytes("internal/agent/a_test.go", []byte(src))
|
||||
if refs := s.importRefs(); len(refs) != 0 {
|
||||
t.Fatalf("test file imports should not be layered: %v", refs)
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,140 @@
|
||||
// repolint enforces the repo standards that gofmt/vet/golangci cannot express.
|
||||
package main
|
||||
|
||||
import (
|
||||
"flag"
|
||||
"fmt"
|
||||
"maps"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"slices"
|
||||
"sort"
|
||||
)
|
||||
|
||||
type Finding struct {
|
||||
File string
|
||||
Line int
|
||||
Rule string
|
||||
Msg string
|
||||
// Excess over the rule's limit, so trading a short violation for a long
|
||||
// one still trips the ratchet. One for rules that are pass/fail.
|
||||
Weight int
|
||||
}
|
||||
|
||||
const (
|
||||
ruleEssay = "essay"
|
||||
ruleBanner = "banner"
|
||||
ruleMarker = "marker"
|
||||
ruleDeadCode = "commented-code"
|
||||
ruleNarrative = "narrative"
|
||||
ruleFileSize = "file-size"
|
||||
ruleTestSize = "test-file-size"
|
||||
ruleLayering = "layering"
|
||||
)
|
||||
|
||||
var allRules = []string{
|
||||
ruleEssay, ruleBanner, ruleMarker, ruleDeadCode,
|
||||
ruleNarrative, ruleFileSize, ruleTestSize, ruleLayering,
|
||||
}
|
||||
|
||||
func main() {
|
||||
root := flag.String("root", ".", "repository root to scan")
|
||||
baselinePath := flag.String("baseline", "", "baseline file (default <root>/tools/repolint/baseline.json)")
|
||||
update := flag.Bool("update", false, "rewrite the baseline from the current tree")
|
||||
strict := flag.Bool("strict", false, "report every finding, ignoring the baseline")
|
||||
flag.Parse()
|
||||
|
||||
if *baselinePath == "" {
|
||||
*baselinePath = filepath.Join(*root, "tools", "repolint", "baseline.json")
|
||||
}
|
||||
|
||||
findings, err := run(*root)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "repolint:", err)
|
||||
os.Exit(2)
|
||||
}
|
||||
sort.Slice(findings, func(i, j int) bool {
|
||||
if findings[i].File != findings[j].File {
|
||||
return findings[i].File < findings[j].File
|
||||
}
|
||||
return findings[i].Line < findings[j].Line
|
||||
})
|
||||
|
||||
if *update {
|
||||
if err := baselineFrom(findings).write(*baselinePath); err != nil {
|
||||
fmt.Fprintln(os.Stderr, "repolint:", err)
|
||||
os.Exit(2)
|
||||
}
|
||||
fmt.Printf("wrote %s (%d findings across %d files)\n", *baselinePath, len(findings), countFiles(findings))
|
||||
return
|
||||
}
|
||||
|
||||
if *strict {
|
||||
report(findings)
|
||||
fmt.Printf("\n%d findings across %d files\n", len(findings), countFiles(findings))
|
||||
if len(findings) > 0 {
|
||||
os.Exit(1)
|
||||
}
|
||||
return
|
||||
}
|
||||
|
||||
baseline, err := loadBaseline(*baselinePath)
|
||||
if err != nil {
|
||||
fmt.Fprintln(os.Stderr, "repolint:", err)
|
||||
os.Exit(2)
|
||||
}
|
||||
over, msgs := baseline.exceeded(findings)
|
||||
if len(msgs) == 0 {
|
||||
fmt.Printf("repolint: clean (%d baselined findings)\n", len(findings))
|
||||
return
|
||||
}
|
||||
report(over)
|
||||
fmt.Fprintln(os.Stderr)
|
||||
for _, m := range msgs {
|
||||
fmt.Fprintln(os.Stderr, "repolint:", m)
|
||||
}
|
||||
fmt.Fprintf(os.Stderr, "\nNew standards violations. Fix them, or if this is a deliberate\n"+
|
||||
"carry-forward (file rename, extraction), run:\n\n go run ./tools/repolint -update\n\n"+
|
||||
"and justify the baseline diff in the pull request.\n")
|
||||
os.Exit(1)
|
||||
}
|
||||
|
||||
func run(root string) ([]Finding, error) {
|
||||
paths, err := collect(root)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
var findings []Finding
|
||||
imports := map[string][]importRef{}
|
||||
for _, rel := range paths {
|
||||
src, err := parseSource(root, rel)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
if src == nil {
|
||||
continue
|
||||
}
|
||||
findings = append(findings, checkComments(src)...)
|
||||
findings = append(findings, checkSize(src)...)
|
||||
imports[rel] = src.importRefs()
|
||||
}
|
||||
return append(findings, checkLayering(imports)...), nil
|
||||
}
|
||||
|
||||
func report(findings []Finding) {
|
||||
for _, f := range findings {
|
||||
fmt.Fprintf(os.Stderr, "%s:%d: [%s] %s\n", f.File, f.Line, f.Rule, f.Msg)
|
||||
}
|
||||
}
|
||||
|
||||
func countFiles(findings []Finding) int {
|
||||
seen := map[string]bool{}
|
||||
for _, f := range findings {
|
||||
seen[f.File] = true
|
||||
}
|
||||
return len(seen)
|
||||
}
|
||||
|
||||
func sortedKeys[V any](m map[string]V) []string {
|
||||
return slices.Sorted(maps.Keys(m))
|
||||
}
|
||||
@@ -0,0 +1,17 @@
|
||||
package main
|
||||
|
||||
import "fmt"
|
||||
|
||||
const maxFileLines = 800
|
||||
|
||||
func checkSize(s *sourceFile) []Finding {
|
||||
if s.lines <= maxFileLines {
|
||||
return nil
|
||||
}
|
||||
rule := ruleFileSize
|
||||
if s.isTest() {
|
||||
rule = ruleTestSize
|
||||
}
|
||||
return []Finding{{s.rel, 1, rule,
|
||||
fmt.Sprintf("%d lines exceeds the %d-line ceiling", s.lines, maxFileLines), s.lines - maxFileLines}}
|
||||
}
|
||||
@@ -0,0 +1,138 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"go/ast"
|
||||
"go/parser"
|
||||
"go/token"
|
||||
"io/fs"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"sort"
|
||||
"strconv"
|
||||
"strings"
|
||||
)
|
||||
|
||||
var skipDirs = map[string]bool{
|
||||
"node_modules": true,
|
||||
"third_party": true,
|
||||
"vendor": true,
|
||||
"testdata": true,
|
||||
"dist": true,
|
||||
"bin": true,
|
||||
}
|
||||
|
||||
var generatedRe = regexp.MustCompile(`^// Code generated .* DO NOT EDIT\.$`)
|
||||
|
||||
type sourceFile struct {
|
||||
rel string
|
||||
fset *token.FileSet
|
||||
file *ast.File
|
||||
src []string
|
||||
lines int
|
||||
}
|
||||
|
||||
// A comment sharing its line with code annotates that code; only a comment
|
||||
// that owns its line can be commented-out code.
|
||||
func (s *sourceFile) trailing(pos token.Pos) bool {
|
||||
p := s.fset.Position(pos)
|
||||
if p.Line < 1 || p.Line > len(s.src) || p.Column < 2 {
|
||||
return false
|
||||
}
|
||||
line := s.src[p.Line-1]
|
||||
if p.Column-1 > len(line) {
|
||||
return false
|
||||
}
|
||||
return strings.TrimSpace(line[:p.Column-1]) != ""
|
||||
}
|
||||
|
||||
func collect(root string) ([]string, error) {
|
||||
var out []string
|
||||
err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error {
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
name := d.Name()
|
||||
if d.IsDir() {
|
||||
if path == root {
|
||||
return nil
|
||||
}
|
||||
if skipDirs[name] || strings.HasPrefix(name, ".") {
|
||||
return filepath.SkipDir
|
||||
}
|
||||
return nil
|
||||
}
|
||||
if !strings.HasSuffix(name, ".go") || strings.Contains(name, "_generated.") {
|
||||
return nil
|
||||
}
|
||||
rel, err := filepath.Rel(root, path)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
out = append(out, filepath.ToSlash(rel))
|
||||
return nil
|
||||
})
|
||||
sort.Strings(out)
|
||||
return out, err
|
||||
}
|
||||
|
||||
func parseSource(root, rel string) (*sourceFile, error) {
|
||||
data, err := os.ReadFile(filepath.Join(root, filepath.FromSlash(rel)))
|
||||
if err != nil {
|
||||
return nil, err
|
||||
}
|
||||
return parseBytes(rel, data), nil
|
||||
}
|
||||
|
||||
func parseBytes(rel string, data []byte) *sourceFile {
|
||||
if isGenerated(data) {
|
||||
return nil
|
||||
}
|
||||
fset := token.NewFileSet()
|
||||
file, err := parser.ParseFile(fset, rel, data, parser.ParseComments|parser.SkipObjectResolution)
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
text := strings.ReplaceAll(string(data), "\r\n", "\n")
|
||||
lines := strings.Split(text, "\n")
|
||||
if n := len(lines); n > 0 && lines[n-1] == "" {
|
||||
lines = lines[:n-1]
|
||||
}
|
||||
return &sourceFile{rel: rel, fset: fset, file: file, src: lines, lines: len(lines)}
|
||||
}
|
||||
|
||||
func isGenerated(data []byte) bool {
|
||||
for i, line := range strings.SplitN(string(data), "\n", 12) {
|
||||
if i >= 11 {
|
||||
break
|
||||
}
|
||||
if generatedRe.MatchString(strings.TrimRight(line, "\r")) {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
||||
func (s *sourceFile) isTest() bool { return strings.HasSuffix(s.rel, "_test.go") }
|
||||
|
||||
func (s *sourceFile) line(p token.Pos) int { return s.fset.Position(p).Line }
|
||||
|
||||
type importRef struct {
|
||||
path string
|
||||
line int
|
||||
}
|
||||
|
||||
func (s *sourceFile) importRefs() []importRef {
|
||||
if s.isTest() {
|
||||
return nil
|
||||
}
|
||||
out := make([]importRef, 0, len(s.file.Imports))
|
||||
for _, spec := range s.file.Imports {
|
||||
path, err := strconv.Unquote(spec.Path.Value)
|
||||
if err != nil {
|
||||
continue
|
||||
}
|
||||
out = append(out, importRef{path: path, line: s.line(spec.Pos())})
|
||||
}
|
||||
return out
|
||||
}
|
||||
Reference in new issue
Block a user