From b3ba466268feba5389f2c9047e65235966b2ad64 Mon Sep 17 00:00:00 2001 From: Nils Lehnen <30603423+iderex@users.noreply.github.com> Date: Wed, 12 Aug 2026 02:57:53 +0200 Subject: [PATCH] Hold the three exit-code declarations to one contract (#125) Record 0011 fixes four codes and their meanings, and this tree declares them in three places because a code lives next to the thing that can return it. Each declaration was right about itself and nothing was right about all of them together, so a number that moved in one of them compiled, passed the tests of the package it moved in, and changed the meaning of every reader keyed on it without their files being edited. The exit-code leg in internal/invariants reads every exit-code constant out of the tree and refuses a code declared with two numbers, or a number declared under two codes. The second direction is the one that reaches the harness code: it is declared exactly once, so there is no second copy of it for the first direction to compare against, and what can go wrong is a collision with a meaning the runner's three already carry. The reason is written at each of the three declarations rather than only at the leg, because that is where somebody moving a number is looking. Both directions have a case that trips exactly this property and no other, and both name the same near neighbour, which is that case with the one character or the one identifier repaired. Signed-off-by: Nils Lehnen <30603423+iderex@users.noreply.github.com> --- cmd/lab/main.go | 10 + cmd/pullrequest/main.go | 9 + internal/hardware/hardware.go | 9 + internal/invariants/exitcodes.go | 214 ++++++++++++++++++ internal/invariants/invariants.go | 12 + .../expected | 1 + .../expected-refusals | 1 + .../near-neighbour | 1 + .../tree/NOTICE.md | 5 + .../tree/README.md | 3 + .../tree/cmd/one/main.go | 10 + .../tree/cmd/two/main.go | 11 + .../expected | 1 + .../expected-refusals | 0 .../tree/NOTICE.md | 5 + .../tree/README.md | 3 + .../tree/cmd/one/main.go | 10 + .../tree/cmd/two/main.go | 10 + .../expected | 1 + .../expected-refusals | 1 + .../near-neighbour | 1 + .../tree/NOTICE.md | 5 + .../tree/README.md | 3 + .../tree/cmd/one/main.go | 10 + .../tree/cmd/two/main.go | 11 + 25 files changed, 347 insertions(+) create mode 100644 internal/invariants/exitcodes.go create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/expected create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/expected-refusals create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/near-neighbour create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/tree/NOTICE.md create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/tree/README.md create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/one/main.go create mode 100644 testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/two/main.go create mode 100644 testdata/invariants/exit-code-declarations-that-agree/expected create mode 100644 testdata/invariants/exit-code-declarations-that-agree/expected-refusals create mode 100644 testdata/invariants/exit-code-declarations-that-agree/tree/NOTICE.md create mode 100644 testdata/invariants/exit-code-declarations-that-agree/tree/README.md create mode 100644 testdata/invariants/exit-code-declarations-that-agree/tree/cmd/one/main.go create mode 100644 testdata/invariants/exit-code-declarations-that-agree/tree/cmd/two/main.go create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/expected create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/expected-refusals create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/near-neighbour create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/tree/NOTICE.md create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/tree/README.md create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/one/main.go create mode 100644 testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/two/main.go diff --git a/cmd/lab/main.go b/cmd/lab/main.go index e9b9be3..3b56333 100644 --- a/cmd/lab/main.go +++ b/cmd/lab/main.go @@ -22,6 +22,16 @@ import ( // harness and is declared where its producer is, in internal/hardware. Record // 0011 refuses a code written into the runner as a branch nothing can reach, // and that is what one here would be. +// +// WHAT HOLDS THIS DECLARATION TO THE OTHERS. Keeping a code beside its producer +// means the contract is written down in more than one place, and each place was +// right about itself while nothing was right about all of them together. The +// exit-code leg in internal/invariants reads every declaration in the tree and +// refuses a code declared with two numbers, or a number declared under two +// codes, so a number that moves here is red in the default suite rather than +// green in this package. It finds a declaration by the shape of its name, so a +// code named something that does not read like one is invisible to it: keep the +// convention these three follow. const ( // exitClean means the run completed and refused nothing. It does not // mean the tree is good, only that nothing this runner judges was found diff --git a/cmd/pullrequest/main.go b/cmd/pullrequest/main.go index 6a27481..eb51a98 100644 --- a/cmd/pullrequest/main.go +++ b/cmd/pullrequest/main.go @@ -30,6 +30,15 @@ import ( // they are written here with the record named rather than invented. The // integration-hardware harness declares its own code where its producer is, and // this follows that shape: a code lives next to the thing that can return it. +// +// WHAT HOLDS THIS DECLARATION TO THE OTHERS. The exit-code leg in +// internal/invariants reads every exit-code constant in the tree and refuses a +// code declared with two numbers, or a number declared under two codes. Without +// it these three numbers being the same as the runner's is a thing a reader +// checked once, and a number that moved here would compile, pass this package's +// own tests and change the meaning of anything keyed on it. It finds a +// declaration by the shape of its name, so keep the convention these three +// follow. const ( // exitClean means the run completed and refused nothing. It does not mean // the pull request is good, only that nothing this check judges was found diff --git a/internal/hardware/hardware.go b/internal/hardware/hardware.go index 27b367f..5b781b8 100644 --- a/internal/hardware/hardware.go +++ b/internal/hardware/hardware.go @@ -42,6 +42,15 @@ const Name = "integration-hardware" // returns. It lives here rather than beside those three because this is where // its producer is, and that record refuses a code written into the runner as a // branch nothing can reach. +// +// WHAT HOLDS THIS DECLARATION TO THE OTHERS. The exit-code leg in +// internal/invariants reads every exit-code constant in the tree and refuses a +// number declared under two codes, which is the direction that reaches this +// one: it is declared exactly once, so there is no second copy of it for +// anything to compare against, and what can go wrong is this number colliding +// with a meaning one of the runner's three already has. It finds a declaration +// by the shape of its name, so keep the convention this one and those three +// follow. const ExitAskedAndDeliveredNothing = 3 // BuildTag is the constraint its tests are behind. The default run does not diff --git a/internal/invariants/exitcodes.go b/internal/invariants/exitcodes.go new file mode 100644 index 0000000..c5161c1 --- /dev/null +++ b/internal/invariants/exitcodes.go @@ -0,0 +1,214 @@ +// The leg that holds record 0011's contract to itself. +// +// The numbers are declared in more than one place on purpose. A code lives next +// to the thing that can return it, which is what keeps the code the hardware +// harness returns out of the runner rather than written there as a branch +// nothing can reach. What that shape costs is that each declaration is right +// about itself and nothing was right about all of them together: a number that +// moved in one of them compiled, passed the tests of the package it moved in, +// and changed the meaning of every reader keyed on it without their files being +// edited. +// +// This leg is the other half of that shape rather than a replacement for it. +// The declarations stay where their producers are and this reads them all and +// requires them to say one thing. + +package invariants + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "regexp" + "sort" + "strconv" + "strings" +) + +// goExtension is what this leg reads. Everything else the scan carries is prose +// and declares no constants. +const goExtension = ".go" + +// exitCodeName is how a declaration of one of record 0011's codes is +// recognised. It is a naming convention read out of the tree rather than a list +// of files, so a fourth declaration written somewhere nobody has thought of yet +// is held to the same contract as the three that exist, and this leg does not +// have to be edited to notice it. +// +// The cost of a convention is that it reaches only what follows it. A code +// declared under a name that does not read like one is invisible here, and +// nothing in this tree can see that it is a code, which is why the convention is +// stated at each declaration rather than only in this file. +var exitCodeName = regexp.MustCompile(`^[Ee]xit[A-Z]`) + +// An exitCodeDeclaration is one exit-code constant as one file declares it. +type exitCodeDeclaration struct { + // name is the constant, which is what groups two declarations of one code. + name string + + // value is the number, which is what the contract fixes. + value int + + // path is the file as the walk reached it, which is what a reader opens. + path string + + // line is where in that file, because a reader hitting this refusal is + // looking for two numbers and needs to be sent to both of them. + line int +} + +// exitCodesLeg holds every declaration of record 0011's contract to every other +// one, in both directions a disagreement can run. +// +// THE BOUND, and it is the one this package's harness already carries for the +// notice leg. Both directions refuse under one property, because the repair is +// one repair: make the declarations say what the record says. A case comparing +// property sets cannot tell them apart, so a change that loses one of the two +// stays green on the other's case. Each direction has its own case anyway, and +// that is a convention here rather than something the harness requires. +func exitCodesLeg(texts []textFile) (Leg, []Refusal) { + declarations, examined := exitCodeDeclarations(texts) + leg := Leg{Name: "the exit codes", Asked: true, Examined: examined} + + var refusals []Refusal + refusals = append(refusals, oneCodeWithTwoNumbers(declarations)...) + refusals = append(refusals, oneNumberWithTwoCodes(declarations)...) + return leg, refusals +} + +// oneCodeWithTwoNumbers refuses a code whose declarations do not agree on the +// number. This is the drift the shape invites: the constant is spelled the same +// everywhere, so a reader comparing two files by eye is looking at the one +// character that differs. +func oneCodeWithTwoNumbers(declarations []exitCodeDeclaration) []Refusal { + first := make(map[string]exitCodeDeclaration) + var refusals []Refusal + + for _, declaration := range declarations { + reference, seen := first[declaration.name] + if !seen { + first[declaration.name] = declaration + continue + } + if declaration.value == reference.value { + continue + } + refusals = append(refusals, Refusal{ + Property: ExitCodeDeclarationsDisagree, + Subject: declaration.path, + Detail: fmt.Sprintf("it declares %s as %d at line %d, and %s declares the same code as %d, so the contract record 0011 fixes has two numbers in this tree", + declaration.name, declaration.value, declaration.line, reference.path, reference.value), + }) + } + return refusals +} + +// oneNumberWithTwoCodes refuses one number declared under two codes. This is +// the direction that reaches a code declared exactly once, which the direction +// above cannot: a number written into a new constant collides with the meaning +// the record already gave it, and there is no second declaration of that +// constant for anything to compare it against. +func oneNumberWithTwoCodes(declarations []exitCodeDeclaration) []Refusal { + first := make(map[int]exitCodeDeclaration) + var refusals []Refusal + + for _, declaration := range declarations { + reference, seen := first[declaration.value] + if !seen { + first[declaration.value] = declaration + continue + } + if declaration.name == reference.name { + continue + } + refusals = append(refusals, Refusal{ + Property: ExitCodeDeclarationsDisagree, + Subject: declaration.path, + Detail: fmt.Sprintf("it declares %s as %d at line %d, and %s declares %s as the same number, so one code in record 0011's contract answers to two names", + declaration.name, declaration.value, declaration.line, reference.path, reference.name), + }) + } + return refusals +} + +// exitCodeDeclarations reads every Go file the scan carries and returns the +// exit-code constants it declares, with how many files were parsed. +// +// It reads the text the scan already read rather than the filesystem again, so +// the declarations it compares are the ones in the tree the report is about. +func exitCodeDeclarations(texts []textFile) ([]exitCodeDeclaration, int) { + var found []exitCodeDeclaration + parsed := 0 + + for _, file := range texts { + if !strings.HasSuffix(file.relative, goExtension) { + continue + } + fset := token.NewFileSet() + syntax, err := parser.ParseFile(fset, file.relative, file.text, 0) + if err != nil { + // A file the parser cannot read is not judged here. The compiler + // refuses it, and a leg that refused it too would be a second copy + // of a rule that already has a better mechanism. + continue + } + parsed++ + + for _, declaration := range syntax.Decls { + group, ok := declaration.(*ast.GenDecl) + if !ok || group.Tok != token.CONST { + continue + } + for _, spec := range group.Specs { + values, ok := spec.(*ast.ValueSpec) + if !ok { + continue + } + for i, name := range values.Names { + if i >= len(values.Values) || !exitCodeName.MatchString(name.Name) { + continue + } + number, ok := integerLiteral(values.Values[i]) + if !ok { + continue + } + found = append(found, exitCodeDeclaration{ + name: name.Name, + value: number, + path: file.path, + line: fset.Position(name.Pos()).Line, + }) + } + } + } + } + + sort.Slice(found, func(i, j int) bool { + if found[i].path != found[j].path { + return found[i].path < found[j].path + } + return found[i].line < found[j].line + }) + return found, parsed +} + +// integerLiteral reads a constant's value where that value is a plain number, +// and says no for everything else. +// +// A constant whose name reads like an exit code and whose value is not a number +// is not an exit code. That is not a hypothetical: the property identifier this +// leg refuses under is spelled ExitCodeDeclarationsDisagree, it sits in this +// package, and a leg that took every matching name would refuse this repository +// for declaring the rule it is refusing under. +func integerLiteral(expression ast.Expr) (int, bool) { + literal, ok := expression.(*ast.BasicLit) + if !ok || literal.Kind != token.INT { + return 0, false + } + number, err := strconv.Atoi(literal.Value) + if err != nil { + return 0, false + } + return number, true +} diff --git a/internal/invariants/invariants.go b/internal/invariants/invariants.go index 256be17..fd7d11d 100644 --- a/internal/invariants/invariants.go +++ b/internal/invariants/invariants.go @@ -116,6 +116,14 @@ const ( // this for experiment records; this is the same rule over the documents a // visitor reads first, which is where a dead pointer does the most damage. DocumentNamesAPathThatDoesNotResolve = "document-names-a-path-that-does-not-resolve" + + // ExitCodeDeclarationsDisagree refuses a tree whose exit-code constants do + // not say one thing. Record 0011 fixes four numbers and their meanings, and + // the tree declares them in more than one place because a code lives next + // to the thing that can return it. Each declaration is right about itself + // and nothing held them to each other, so a number that moved in one of + // them compiled and stayed green in the package it moved in. + ExitCodeDeclarationsDisagree = "exit-code-declarations-disagree" ) // A Refusal is one rule refusing one subject. It carries the subject separately @@ -226,6 +234,10 @@ func Scan(root, declaredLicence string) (Report, error) { rep.Legs = append(rep.Legs, leg) rep.Refusals = append(rep.Refusals, refusals...) + leg, refusals = exitCodesLeg(texts) + rep.Legs = append(rep.Legs, leg) + rep.Refusals = append(rep.Refusals, refusals...) + return rep, nil } diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/expected b/testdata/invariants/an-exit-code-declared-with-two-numbers/expected new file mode 100644 index 0000000..3d7cc38 --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/expected @@ -0,0 +1 @@ +text-files 4 diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/expected-refusals b/testdata/invariants/an-exit-code-declared-with-two-numbers/expected-refusals new file mode 100644 index 0000000..34fe4fd --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/expected-refusals @@ -0,0 +1 @@ +exit-code-declarations-disagree diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/near-neighbour b/testdata/invariants/an-exit-code-declared-with-two-numbers/near-neighbour new file mode 100644 index 0000000..be8ea6c --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/near-neighbour @@ -0,0 +1 @@ +exit-code-declarations-that-agree diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/NOTICE.md b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/NOTICE.md new file mode 100644 index 0000000..e3cfa99 --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/NOTICE.md @@ -0,0 +1,5 @@ +# Notice + +This software is developed for lawful use. Operators and users are responsible +for making sure that their deployment and use comply with the laws that apply +to them. diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/README.md b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/README.md new file mode 100644 index 0000000..315a136 --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/README.md @@ -0,0 +1,3 @@ +# a tree + +See [NOTICE.md](NOTICE.md) for the intended-use notice. diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/one/main.go b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/one/main.go new file mode 100644 index 0000000..266b242 --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/one/main.go @@ -0,0 +1,10 @@ +// Command one is a fixture. It declares two of the codes the contract fixes, +// beside the thing that returns them. +package main + +const ( + exitClean = 0 + exitRefused = 1 +) + +func main() {} diff --git a/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/two/main.go b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/two/main.go new file mode 100644 index 0000000..ce329ff --- /dev/null +++ b/testdata/invariants/an-exit-code-declared-with-two-numbers/tree/cmd/two/main.go @@ -0,0 +1,11 @@ +// Command two is a fixture. It spells the second code the way command one +// spells it and gives it a different number, which is the drift a reader +// comparing the two files by eye has to catch one character of. +package main + +const ( + exitClean = 0 + exitRefused = 5 +) + +func main() {} diff --git a/testdata/invariants/exit-code-declarations-that-agree/expected b/testdata/invariants/exit-code-declarations-that-agree/expected new file mode 100644 index 0000000..3d7cc38 --- /dev/null +++ b/testdata/invariants/exit-code-declarations-that-agree/expected @@ -0,0 +1 @@ +text-files 4 diff --git a/testdata/invariants/exit-code-declarations-that-agree/expected-refusals b/testdata/invariants/exit-code-declarations-that-agree/expected-refusals new file mode 100644 index 0000000..e69de29 diff --git a/testdata/invariants/exit-code-declarations-that-agree/tree/NOTICE.md b/testdata/invariants/exit-code-declarations-that-agree/tree/NOTICE.md new file mode 100644 index 0000000..e3cfa99 --- /dev/null +++ b/testdata/invariants/exit-code-declarations-that-agree/tree/NOTICE.md @@ -0,0 +1,5 @@ +# Notice + +This software is developed for lawful use. Operators and users are responsible +for making sure that their deployment and use comply with the laws that apply +to them. diff --git a/testdata/invariants/exit-code-declarations-that-agree/tree/README.md b/testdata/invariants/exit-code-declarations-that-agree/tree/README.md new file mode 100644 index 0000000..315a136 --- /dev/null +++ b/testdata/invariants/exit-code-declarations-that-agree/tree/README.md @@ -0,0 +1,3 @@ +# a tree + +See [NOTICE.md](NOTICE.md) for the intended-use notice. diff --git a/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/one/main.go b/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/one/main.go new file mode 100644 index 0000000..266b242 --- /dev/null +++ b/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/one/main.go @@ -0,0 +1,10 @@ +// Command one is a fixture. It declares two of the codes the contract fixes, +// beside the thing that returns them. +package main + +const ( + exitClean = 0 + exitRefused = 1 +) + +func main() {} diff --git a/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/two/main.go b/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/two/main.go new file mode 100644 index 0000000..ee71c4d --- /dev/null +++ b/testdata/invariants/exit-code-declarations-that-agree/tree/cmd/two/main.go @@ -0,0 +1,10 @@ +// Command two is a fixture. It declares the same two codes as command one, +// with the same numbers, which is the whole of what this leg asks for. +package main + +const ( + exitClean = 0 + exitRefused = 1 +) + +func main() {} diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/expected b/testdata/invariants/one-number-declared-under-two-exit-codes/expected new file mode 100644 index 0000000..3d7cc38 --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/expected @@ -0,0 +1 @@ +text-files 4 diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/expected-refusals b/testdata/invariants/one-number-declared-under-two-exit-codes/expected-refusals new file mode 100644 index 0000000..34fe4fd --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/expected-refusals @@ -0,0 +1 @@ +exit-code-declarations-disagree diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/near-neighbour b/testdata/invariants/one-number-declared-under-two-exit-codes/near-neighbour new file mode 100644 index 0000000..be8ea6c --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/near-neighbour @@ -0,0 +1 @@ +exit-code-declarations-that-agree diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/tree/NOTICE.md b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/NOTICE.md new file mode 100644 index 0000000..e3cfa99 --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/NOTICE.md @@ -0,0 +1,5 @@ +# Notice + +This software is developed for lawful use. Operators and users are responsible +for making sure that their deployment and use comply with the laws that apply +to them. diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/tree/README.md b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/README.md new file mode 100644 index 0000000..315a136 --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/README.md @@ -0,0 +1,3 @@ +# a tree + +See [NOTICE.md](NOTICE.md) for the intended-use notice. diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/one/main.go b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/one/main.go new file mode 100644 index 0000000..266b242 --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/one/main.go @@ -0,0 +1,10 @@ +// Command one is a fixture. It declares two of the codes the contract fixes, +// beside the thing that returns them. +package main + +const ( + exitClean = 0 + exitRefused = 1 +) + +func main() {} diff --git a/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/two/main.go b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/two/main.go new file mode 100644 index 0000000..de8962f --- /dev/null +++ b/testdata/invariants/one-number-declared-under-two-exit-codes/tree/cmd/two/main.go @@ -0,0 +1,11 @@ +// Command two is a fixture. It gives the number command one already uses a +// second name, so the contract carries one code answering to two of them and +// neither declaration is wrong about itself. +package main + +const ( + exitClean = 0 + exitDenied = 1 +) + +func main() {}