From 1dcd2297de37b61337cee105a10943b049a2e2c4 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 08:09:15 +0200 Subject: [PATCH 1/3] guard: a type has a ceiling, because splitting its file does not shrink it The three ceilings here measure files, functions and branching, and a type is none of them. codeshape_test.go records three lowerings of longestFile, 503 to 457 to 433, and every one was earned the same way: code moved out of engine.go into a new file beside it. The watched number fell each time and nothing asked whether the type a reader has to hold in their head fell with it. A type is the unit of state. Twenty two fields is twenty two things any of its twenty eight methods may have changed, and splitting the file those methods live in does not divide that by anything. Measured on this tree: 271 types, worst is internal/gui/window.runner at 28 methods and 22 fields. Three types reach the method band and four reach the field band. All four numbers are the measurement rather than a number above it, which a third guard pins. The bands are a count of things rather than a share of the ceiling, the local convention, and the predicate is the existing crowding() rather than a second copy of it. Build constraints were measured rather than assumed, because a pinned count that moves with the environment would go red on one platform only: three files declare types behind a tag and every one of them is far below both ceilings and both bands. Co-Authored-By: Claude Opus 5 --- internal/guard/typeshape_test.go | 323 +++++++++++++++++++++++++++++++ 1 file changed, 323 insertions(+) create mode 100644 internal/guard/typeshape_test.go diff --git a/internal/guard/typeshape_test.go b/internal/guard/typeshape_test.go new file mode 100644 index 0000000..516ec6b --- /dev/null +++ b/internal/guard/typeshape_test.go @@ -0,0 +1,323 @@ +package guard + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "sort" + "strings" + "testing" +) + +// The fourth axis: the TYPE. +// +// The three ceilings already here measure files, functions and branching, and a +// type is none of them. That is not a gap in principle - it is a hole this tree +// can be shown to have. codeshape_test.go records three separate lowerings of +// longestFile, 503 to 457 to 433, and every one of them was earned the same way: +// code moved out of engine.go into a new file beside it. The number that is +// watched went down each time. Nothing asked whether the type a reader has to +// hold in their head went down with it, because nothing here can ask that. +// +// What this buys that a file ceiling cannot: a type is the unit of state. +// Twenty two fields is twenty two things any of its twenty eight methods may +// have changed, and splitting the FILE those methods live in does not divide +// that by anything. +// +// Today's numbers are healthy - the worst type here has 28 methods, not the +// ninety a long lived window class grows - so this ceiling buys nothing today +// and is bought for the reason deepestNesting was bought: it costs nothing +// while nothing grows, and by the time it would have been worth adding, the +// number it would have to be set to is already the problem. +// +// Down is routine. Up is the owner's decision, the same as every other ceiling +// in this package. +const ( + // Measured 2026-09-07. Both are internal/gui/window.runner, which is the + // screen that drives a run - it holds the widgets, the progress state and + // the cancel plumbing at once. + mostMethods = 28 + mostFields = 22 + + // What counts as crowding, in the shape this package already uses + // everywhere else: an ABSOLUTE number rather than a percentage of the + // ceiling. That is a local convention and it is deliberate here, because a + // band written as a fraction reshapes itself every time the ceiling moves, + // and a band that moves under the thing it is watching says nothing. + // + // Set at roughly three quarters of the ceiling and then measured, which is + // the order that matters - a threshold picked first and measured second is + // a guess with a gate around it. + crowdingMethods = 21 + crowdingFields = 17 + + // Caps measured on the tree of 2026-09-07 and then frozen. Like every count + // in this package these only go down, and raising one to turn a run green is + // the same act as raising a ceiling. + crowdedMethodTypes = 3 + crowdedFieldTypes = 4 +) + +// typeSize is one named type with the two counts this axis watches. +// +// METHODS are the declarations whose receiver is this type. A function inside +// one of them is that method's business and not another entry in this type's +// surface, which is why only top level declarations are read. +// +// FIELDS counts what a struct declares, embedded ones included, because an +// embedded type is state the reader still has to know about. A type that is not +// a struct has no fields and simply does not appear in that half. +type typeSize struct { + name string // internal/gui/window.runner + where string // internal/gui/window/run.go:112 + methods int + fields int +} + +// measureTypes reads every named type of the shipped tree. +// +// Scope is packages(t) and its files field, which is production code only and +// excludes _test.go - the same tree longestFile and longestFunction measure. A +// guard measuring a different tree from its neighbours would be a fourth axis +// answering about a fourth codebase. +// +// 🔴 What build constraints do to this number, measured 2026-09-07 rather than +// assumed, because a pinned count that moves with the environment is the shape +// this project has in its own table of environmental noise. build.ImportDir +// applies the constraints of the machine it runs on, so a type behind a tag is +// invisible on a build that does not carry the tag. Two files declare types +// that way: internal/gui/run_cgo.go behind cgo, holding desktop with 5 methods +// and stored with 4, and internal/format/avif/requiretag.go behind !noasm, +// holding a struct with no fields at all. Every one of them is far below both +// ceilings and below both bands, so CGO_ENABLED and the build tags cannot move +// any number in this file. A large type added behind a tag would break that, +// and it would show up as a pinning failure on one platform only. +func measureTypes(t *testing.T) []typeSize { + t.Helper() + root := repoRoot(t) + + methods := map[string]int{} + fields := map[string]int{} + where := map[string]string{} + + for _, p := range packages(t) { + for _, path := range p.files { + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parsing %s: %v", path, err) + } + rel, err := filepath.Rel(root, path) + if err != nil { + rel = path + } + rel = filepath.ToSlash(rel) + + for _, decl := range file.Decls { + switch node := decl.(type) { + case *ast.FuncDecl: + if node.Recv == nil || len(node.Recv.List) == 0 { + continue + } + owner := receiver(node.Recv.List[0].Type) + if owner == "" { + continue + } + methods[qualify(p.rel, owner)]++ + case *ast.GenDecl: + for _, spec := range node.Specs { + ts, ok := spec.(*ast.TypeSpec) + if !ok { + continue + } + name := qualify(p.rel, ts.Name.Name) + where[name] = fmt.Sprintf("%s:%d", rel, fset.Position(ts.Pos()).Line) + st, ok := ts.Type.(*ast.StructType) + if !ok || st.Fields == nil { + continue + } + fields[name] = structFields(st) + } + } + } + } + } + + names := map[string]bool{} + for name := range methods { + names[name] = true + } + for name := range fields { + names[name] = true + } + + out := make([]typeSize, 0, len(names)) + for name := range names { + out = append(out, typeSize{ + name: name, + where: where[name], + methods: methods[name], + fields: fields[name], + }) + } + sort.Slice(out, func(i, j int) bool { return out[i].name < out[j].name }) + return out +} + +// qualify names a type the way a person would have to say it out loud. +// +// Keyed by package AND name rather than by name alone, which is not tidiness: +// this tree has a type called generator in every format package, and a map +// keyed by the bare name adds twenty of them together. The first measurement +// taken for this file did exactly that and reported a type with 48 methods that +// does not exist. +func qualify(pkg, name string) string { + if pkg == "" { + return name + } + return pkg + "." + name +} + +// receiver reports the type a method is declared on, through a pointer and +// through the type parameters of a generic type. +func receiver(e ast.Expr) string { + switch node := e.(type) { + case *ast.Ident: + return node.Name + case *ast.StarExpr: + return receiver(node.X) + case *ast.IndexExpr: + return receiver(node.X) + case *ast.IndexListExpr: + return receiver(node.X) + } + return "" +} + +// structFields counts what a struct declares. An embedded type carries no name +// of its own and counts as one. +func structFields(st *ast.StructType) int { + n := 0 + for _, f := range st.Fields.List { + if len(f.Names) == 0 { + n++ + continue + } + n += len(f.Names) + } + return n +} + +func TestNoTypeHasGrownPastWhatAPersonCanHoldInTheirHead(t *testing.T) { + sizes := measureTypes(t) + + // The canary every guard in this package carries. A walk that reads nothing + // satisfies every ceiling ever set and looks exactly like a walk that works. + if len(sizes) < 100 { + t.Fatalf("the type scan found %d types, which is too few to be this tree - it read nothing", len(sizes)) + } + + var over []string + for _, ts := range sizes { + if ts.methods > mostMethods { + over = append(over, fmt.Sprintf( + "%s has %d methods and the ceiling is %d - move behaviour out, do not raise the number (%s)", + ts.name, ts.methods, mostMethods, ts.where)) + } + if ts.fields > mostFields { + over = append(over, fmt.Sprintf( + "%s holds %d fields and the ceiling is %d - move state out, do not raise the number (%s)", + ts.name, ts.fields, mostFields, ts.where)) + } + } + + // Every one at once rather than the first, for the reason RC7 gives about + // recipes: being sent back four times for four answers is its own defect. + if len(over) > 0 { + sort.Strings(over) + t.Errorf("%d type(s) have grown past the ceiling:\n %s", + len(over), strings.Join(over, "\n ")) + } +} + +// typeCrowd names the types already inside each band. +// +// One copy of the question, called by the count and by the pinning test below, +// because two copies of "what counts as crowded" is two places for it to drift. +// The predicate itself is crowding(), shared with the file and function bands - +// a band that stopped being a number of things and went back to a share of the +// ceiling would reshape itself under every axis at once, and one mutation +// already watches exactly that. +func typeCrowd(sizes []typeSize) (byMethods, byFields []string) { + for _, ts := range sizes { + if crowding(ts.methods, crowdingMethods) { + byMethods = append(byMethods, fmt.Sprintf("%s (%d)", ts.name, ts.methods)) + } + if crowding(ts.fields, crowdingFields) { + byFields = append(byFields, fmt.Sprintf("%s (%d)", ts.name, ts.fields)) + } + } + sort.Strings(byMethods) + sort.Strings(byFields) + return byMethods, byFields +} + +func TestNoSecondTypeIsCreepingUpOnTheTypeCeilings(t *testing.T) { + // The second knob, for the reason crowding_test.go gives at length: a + // ceiling on the worst single type sees one thing growing to a record and is + // blind to two of them climbing together, neither a record. + byMethods, byFields := typeCrowd(measureTypes(t)) + + if len(byMethods) > crowdedMethodTypes { + t.Errorf("%d type(s) reach %d methods and at most %d may - move behaviour out of one before adding another:\n %s", + len(byMethods), crowdingMethods, crowdedMethodTypes, strings.Join(byMethods, "\n ")) + } + if len(byFields) > crowdedFieldTypes { + t.Errorf("%d type(s) reach %d fields and at most %d may - move state out of one before adding another:\n %s", + len(byFields), crowdingFields, crowdedFieldTypes, strings.Join(byFields, "\n ")) + } +} + +func TestTheTypeCeilingsAreTodaysMeasurementAndNotALooserNumber(t *testing.T) { + // The other half of both ceilings, and the reason ratchet_test.go exists at + // all: a number parked above the truth grants headroom nobody decided to + // grant, and the next arrival slips in under it in silence. So shrinking a + // type comes with a two character chore - bring its number down with it. + sizes := measureTypes(t) + + worstMethods, worstFields := 0, 0 + methodHolder, fieldHolder := "", "" + for _, ts := range sizes { + if ts.methods > worstMethods { + worstMethods, methodHolder = ts.methods, ts.name + } + if ts.fields > worstFields { + worstFields, fieldHolder = ts.fields, ts.name + } + } + inMethodBand, inFieldBand := typeCrowd(sizes) + crowdedByMethods, crowdedByFields := len(inMethodBand), len(inFieldBand) + + if worstMethods != mostMethods { + t.Errorf("mostMethods is %d and the widest type is %s at %d - move the ceiling to %d.", + mostMethods, methodHolder, worstMethods, worstMethods) + } + if worstFields != mostFields { + t.Errorf("mostFields is %d and the widest type is %s at %d - move the ceiling to %d.", + mostFields, fieldHolder, worstFields, worstFields) + } + if crowdedByMethods != crowdedMethodTypes { + t.Errorf("crowdedMethodTypes is %d and %d type(s) reach %d methods - lower it to %d.", + crowdedMethodTypes, crowdedByMethods, crowdingMethods, crowdedByMethods) + } + if crowdedByFields != crowdedFieldTypes { + t.Errorf("crowdedFieldTypes is %d and %d type(s) reach %d fields - lower it to %d.", + crowdedFieldTypes, crowdedByFields, crowdingFields, crowdedByFields) + } + + t.Logf("%d types, worst %d methods (%s) and %d fields (%s), crowding %d by methods and %d by fields", + len(sizes), worstMethods, methodHolder, worstFields, fieldHolder, + crowdedByMethods, crowdedByFields) +} From 2024b80a344dbaaa114dd191e9d18905d332bbbf Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 08:16:17 +0200 Subject: [PATCH 2/3] guard: the console belongs to the surface, and to nothing under it The layer map says which package may import which. It cannot see a line printed from the engine, because fmt is on everybody's list and always will be. That matters here rather than as a matter of taste. This tool is run by somebody else's CI and --json promises a document a script parses. One stray line on the SUCCESS path does not fail a run, does not move an exit code, and is invisible to the guard that a failed run prints nothing on stdout, because that one watches the failure path. It turns a machine readable report into text no parser accepts, and the reader finds out in their pipeline. Layer 4 is the surface and layer 5 is a main package. Everything at 3 or below is a library and does not reach the console by name - not fmt.Print, not os.Stdout or os.Stderr, and not the builtins, which are the hardest to notice because nothing has to be imported for them to work. Passing an io.Writer stays allowed and is the point: cmd/tfg hands os.Stdout to cli.Run. Read from the syntax tree rather than from the text, which is a measurement. internal/oracle keeps Python scripts inside Go raw strings and calls print() thirty times in them, and a text scan reports every one. Co-Authored-By: Claude Opus 5 --- internal/guard/consolereach_test.go | 132 ++++++++++++++++++++++++++++ 1 file changed, 132 insertions(+) create mode 100644 internal/guard/consolereach_test.go diff --git a/internal/guard/consolereach_test.go b/internal/guard/consolereach_test.go new file mode 100644 index 0000000..e630d84 --- /dev/null +++ b/internal/guard/consolereach_test.go @@ -0,0 +1,132 @@ +package guard + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "path/filepath" + "sort" + "strings" + "testing" +) + +// The console belongs to the surface, and to nothing under it. +// +// layers_test.go asks which package may IMPORT which. This asks a different +// question that a layer number cannot answer: a package may import only +// downwards and still write a line to standard output, because fmt is on +// everybody's list and always will be. +// +// Why it matters here rather than as a matter of taste. This tool exists to be +// run by somebody else's CI, and `--json` promises a document a script parses. +// One stray line printed from the engine on the SUCCESS path does not fail a +// run, does not trip "a failed run prints nothing on stdout", and turns a +// machine readable report into text that no parser accepts. The reader finds +// out in their pipeline rather than in ours. +// +// What is allowed and is not a hole: writing to an io.Writer that was handed +// in. cmd/tfg passes os.Stdout into cli.Run and everything below takes a +// writer as an argument, which is the whole point - the caller decides where +// the words go. This guard refuses the console reached DIRECTLY, by name. +// +// 🔴 Read from the syntax tree rather than by searching the text, and that is a +// measurement rather than caution. internal/oracle holds Python scripts inside +// Go raw strings and they call print() thirty times over. A text scan reports +// every one of them, and the honest fix for a guard that shouts at correct code +// is a guard that reads what the compiler reads. oracle is test only and out of +// the layer map anyway, but the next embedded script will not be. +func TestNothingBelowASurfaceReachesTheConsoleDirectly(t *testing.T) { + // Layer 4 is the surface - internal/cli and the window - and layer 5 is a + // main package. Those own the console by construction. Everything at 3 or + // below is a library, and a library that prints has taken a decision that + // belongs to whoever called it. + const surface = 4 + + var offenders []string + packagesRead, filesRead := 0, 0 + + for _, p := range packages(t) { + depth, known := layer[p.rel] + if !known || depth >= surface { + continue + } + packagesRead++ + + for _, path := range p.files { + filesRead++ + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, 0) + if err != nil { + t.Fatalf("parsing %s: %v", path, err) + } + rel, err := filepath.Rel(repoRoot(t), path) + if err != nil { + rel = path + } + rel = filepath.ToSlash(rel) + + ast.Inspect(file, func(n ast.Node) bool { + said := consoleReach(n) + if said == "" { + return true + } + offenders = append(offenders, fmt.Sprintf( + "%s:%d %s reaches the console from layer %d - take an io.Writer instead and let the caller decide", + rel, fset.Position(n.Pos()).Line, said, depth)) + return true + }) + } + } + + // The canary every guard here carries. A walk that reads nothing finds no + // offender and looks exactly like a walk that works. + if packagesRead < 5 || filesRead < 20 { + t.Fatalf("the console scan read %d package(s) and %d file(s), which is too few to be this tree", + packagesRead, filesRead) + } + + if len(offenders) > 0 { + sort.Strings(offenders) + t.Errorf("%d place(s) below the surface write straight to the console:\n %s", + len(offenders), strings.Join(offenders, "\n ")) + } +} + +// consoleReach names what a node does to the console, or "" when it does +// nothing. Three shapes, because there are three ways to get there. +func consoleReach(n ast.Node) string { + switch node := n.(type) { + case *ast.SelectorExpr: + // os.Stdout and os.Stderr, wherever they appear - passed, assigned or + // written to. Holding one is already the decision this refuses. + pkg, ok := node.X.(*ast.Ident) + if !ok || pkg.Name != "os" { + return "" + } + if node.Sel.Name == "Stdout" || node.Sel.Name == "Stderr" { + return "os." + node.Sel.Name + } + case *ast.CallExpr: + switch fn := node.Fun.(type) { + case *ast.Ident: + // The builtins. They go to standard error and survive every + // refactor because nothing has to be imported for them to work, + // which is exactly what makes a forgotten one hard to see. + if fn.Name == "print" || fn.Name == "println" { + return fn.Name + "()" + } + case *ast.SelectorExpr: + pkg, ok := fn.X.(*ast.Ident) + if !ok || pkg.Name != "fmt" { + return "" + } + // Print, Printf and Println only. Fprint and its family take a + // writer and are the sanctioned way to say something. + if strings.HasPrefix(fn.Sel.Name, "Print") { + return "fmt." + fn.Sel.Name + } + } + } + return "" +} From ba21aa6cdb3ff21d591ed3c1b6d03ef90886da5f Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Mon, 7 Sep 2026 08:28:28 +0200 Subject: [PATCH 3/3] guard: a symbol in a comment is text that ships, so it goes The punctuation guard reads every file in the repository and rule 13 covers comments in code. The project's own notes use these marks and are outside the repository, which is why the habit travelled. Co-Authored-By: Claude Opus 5 --- internal/guard/consolereach_test.go | 2 +- internal/guard/typeshape_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/guard/consolereach_test.go b/internal/guard/consolereach_test.go index e630d84..45ee7f6 100644 --- a/internal/guard/consolereach_test.go +++ b/internal/guard/consolereach_test.go @@ -30,7 +30,7 @@ import ( // writer as an argument, which is the whole point - the caller decides where // the words go. This guard refuses the console reached DIRECTLY, by name. // -// 🔴 Read from the syntax tree rather than by searching the text, and that is a +// Read from the syntax tree rather than by searching the text, and that is a // measurement rather than caution. internal/oracle holds Python scripts inside // Go raw strings and they call print() thirty times over. A text scan reports // every one of them, and the honest fix for a guard that shouts at correct code diff --git a/internal/guard/typeshape_test.go b/internal/guard/typeshape_test.go index 516ec6b..23dfff7 100644 --- a/internal/guard/typeshape_test.go +++ b/internal/guard/typeshape_test.go @@ -83,7 +83,7 @@ type typeSize struct { // guard measuring a different tree from its neighbours would be a fourth axis // answering about a fourth codebase. // -// 🔴 What build constraints do to this number, measured 2026-09-07 rather than +// What build constraints do to this number, measured 2026-09-07 rather than // assumed, because a pinned count that moves with the environment is the shape // this project has in its own table of environmental noise. build.ImportDir // applies the constraints of the machine it runs on, so a type behind a tag is