Skip to content

Commit d760797

Browse files
donislawdevclaude
andcommitted
format: the sentence saying which words a setting takes is written in one place
Two places in the archive package listed the values of a closed set by hand, in the registry's order and the registry's phrasing, for compression and for entry_owner. The registry builds that sentence from the declared set, so these were copies of it with nothing comparing the two. Identical copies are the dangerous kind, because nothing goes red while they agree - and they agreed. The day they stop agreeing is the day somebody adds a value to the declaration: the registry lists it, tfg formats prints it, the window offers it, and the hand written line does not. The second copy arrived through the change that closed the first half of this, which is the clearest evidence available that this copies itself faster than anybody notices. Both now ask the declaration. Nothing a person sees changes: a bad value is refused by the registry first, with exit 4, in the registry's own words, and these branches sit behind that check for a direct caller. Three guards, each proven by mutation. The sentence is built in one place, read from string literals rather than file text so a comment may still name it. A refusal quotes the registry, which catches a copy that was reworded rather than repeated. And every word the declaration offers is one its reader accepts, which closes the hole the fix itself opens: Allows says nothing is wrong with a value it allows, so a word added to the set and not handled by the reader would be refused with an empty reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 1da087f commit d760797

3 files changed

Lines changed: 265 additions & 2 deletions

File tree

‎internal/format/archive/compression.go‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,19 @@ func ReadCompression(id string, r format.Request, locked bool) (Squeeze, error)
7878
Format: id,
7979
Key: Compression,
8080
Value: raw,
81-
Reason: "it takes one of: " + CompressBest + ", " + CompressDefault + ", " + CompressFast + ", " + CompressNone,
81+
// Asked of the declaration rather than written out again. The
82+
// words used to be listed here by hand, in the registry's order
83+
// and the registry's phrasing, and identical is how a copy looks
84+
// right up until it is not - O171. A fifth level added to the
85+
// declaration would have reached tfg formats, reached the window,
86+
// reached the registry's refusal, and not reached this line.
87+
//
88+
// This is empty for a value the declaration allows, and a value
89+
// the declaration allows but levels has no entry for would arrive
90+
// here. That pair cannot come apart without a guard going red:
91+
// every declared word is put through this function and has to be
92+
// accepted.
93+
Reason: axes[Compression].Allows(raw),
8294
Remedy: "Ask for " + CompressNone + " to store the files as they are.",
8395
}
8496
}

‎internal/format/archive/ownership.go‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,13 @@ func ReadOwnership(id string, props map[string]string) (Ownership, error) {
105105
// for the other door: this function is callable directly.
106106
return Ownership{}, &format.PropertyValueError{
107107
Format: id, Key: EntryOwner, Value: raw,
108-
Reason: "it takes one of: " + OwnerRoot + ", " + OwnerUnset + ", " + OwnerUser,
108+
// From the declaration, for the reason the same branch in
109+
// compression.go gives: a hand written list is a copy of the
110+
// registry's sentence with nothing comparing the two - O171.
111+
// A word added to the declaration and not to this switch lands
112+
// here, and the guard that puts every declared word through this
113+
// function is what stops that arriving as an empty reason.
114+
Reason: axes[EntryOwner].Allows(raw),
109115
Remedy: "Write it the way the setting is declared, so root or user, " +
110116
"or leave the line out and the entries carry no owner.",
111117
}
Lines changed: 245 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,245 @@
1+
package guard
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"go/ast"
7+
"go/parser"
8+
"go/token"
9+
"os"
10+
"path/filepath"
11+
"strings"
12+
"testing"
13+
14+
"github.com/donislawdev/TestingFilesGenerator/internal/format"
15+
"github.com/donislawdev/TestingFilesGenerator/internal/format/archive"
16+
)
17+
18+
// What these defend. A setting with a closed set of values is refused in one
19+
// voice, and the voice is the declaration's.
20+
//
21+
// Why they were written. Measured 2026-09-02 while closing O168, and then
22+
// again while closing O171: three places in the tree built the sentence that
23+
// says which words a setting takes. One was the registry, which builds it from
24+
// the declared set. The other two were the archive package writing the same
25+
// list out by hand, in the same order and the same phrasing, for compression
26+
// and for entry_owner - and the second of those was added by the very change
27+
// that closed O168, which is the clearest evidence available that this copies
28+
// itself faster than anybody notices.
29+
//
30+
// Why identical copies are the dangerous kind. Nothing goes red while they
31+
// agree, and they agreed. The day they stop agreeing is the day somebody adds
32+
// a fifth compression level to the declaration: the registry lists five, tfg
33+
// formats prints five, the window offers five, and the hand written line lists
34+
// four. Nobody set out to write two answers - one line simply did not get
35+
// edited, and no test in this repository could have said so.
36+
//
37+
// Why the user never saw it, and why that is not a reason to leave it. Measured
38+
// 2026-09-02 on the built binary: --set compression=TURBO and --set
39+
// entry_owner=USER are both refused by the REGISTRY, exit 4, in the registry's
40+
// words. The hand written branches sit behind that check and no surface reaches
41+
// them. They are there for a direct caller, which is what a guard is - so they
42+
// can be reddened, which is the test this project applies before calling
43+
// anything a defence.
44+
//
45+
// What they do NOT check. Whether the sentence reads well. Three tests can say
46+
// it is built once, that a refusal quotes it, and that every word in it works.
47+
// None of them can say it is the right sentence to put in front of a person.
48+
49+
// One place builds the sentence, and it is the registry.
50+
//
51+
// String literals rather than the file text, the same choice plurals_test.go
52+
// makes and for the same reason: a comment explaining this rule has to be able
53+
// to name the sentence, and reading raw bytes could not tell a comment from a
54+
// message. The parser can.
55+
func TestTheSentenceNamingWhichWordsASettingTakesIsWrittenInOnePlace(t *testing.T) {
56+
root := repoRoot(t)
57+
58+
// The stem rather than the whole sentence. The registry writes it with a
59+
// colon and a trailing space, and a copy that dropped the colon would be
60+
// exactly as much of a copy.
61+
const sentence = "it takes one of"
62+
63+
// The one place allowed to say it, written with forward slashes so this
64+
// reads the same on every system.
65+
const home = "internal/format/format.go"
66+
67+
var found []string
68+
files := 0
69+
for _, dir := range []string{"internal", "cmd"} {
70+
start := filepath.Join(root, dir)
71+
err := filepath.WalkDir(start, func(path string, d os.DirEntry, err error) error {
72+
if err != nil || d.IsDir() {
73+
return err
74+
}
75+
// Guards quote the sentences they assert on, so a guard reading
76+
// guards would fail on its own evidence.
77+
if !strings.HasSuffix(path, ".go") || strings.HasSuffix(path, "_test.go") {
78+
return nil
79+
}
80+
files++
81+
82+
fset := token.NewFileSet()
83+
parsed, perr := parser.ParseFile(fset, path, nil, 0)
84+
if perr != nil {
85+
t.Errorf("parsing %s: %v", path, perr)
86+
return nil
87+
}
88+
rel, _ := filepath.Rel(root, path)
89+
rel = filepath.ToSlash(rel)
90+
91+
ast.Inspect(parsed, func(n ast.Node) bool {
92+
lit, isLit := n.(*ast.BasicLit)
93+
if !isLit || lit.Kind != token.STRING {
94+
return true
95+
}
96+
if strings.Contains(lit.Value, sentence) {
97+
found = append(found, fmt.Sprintf("%s:%d", rel, fset.Position(lit.Pos()).Line))
98+
}
99+
return true
100+
})
101+
return nil
102+
})
103+
if err != nil {
104+
t.Fatalf("walking %s: %v", start, err)
105+
}
106+
}
107+
108+
// A walk that read nothing is a green test about nothing. This has happened
109+
// in this repository often enough to be written down as a trap.
110+
if files == 0 {
111+
t.Fatal("no source file was read, so this proved nothing")
112+
}
113+
114+
switch {
115+
case len(found) == 0:
116+
t.Fatalf("nothing in the tree says %q, so either the registry stopped saying it "+
117+
"or this guard is looking for the wrong words. Both are worth knowing: "+
118+
"the sentence is what a person reads when a setting is refused.", sentence)
119+
case len(found) > 1:
120+
t.Errorf("%d places say %q and there may only be one, which is %s.\n"+
121+
" %s\n"+
122+
"A format that wants this sentence asks its declaration for it - Property.Allows "+
123+
"builds it from the declared set, so a value added to the set reaches the sentence "+
124+
"on its own. A list written out by hand is a second answer to one question, and it "+
125+
"agrees with the first until the day somebody adds a value (O171).",
126+
len(found), sentence, home, strings.Join(found, "\n "))
127+
case !strings.HasPrefix(found[0], home+":"):
128+
t.Errorf("the one place saying %q is %s, and it should be %s.\n"+
129+
"The sentence belongs where the declared set is read, so that every format "+
130+
"refuses in the same words.", sentence, found[0], home)
131+
}
132+
}
133+
134+
// A refusal quotes the registry, whatever the wording is that day.
135+
//
136+
// This is the half that survives a rewording. The test above forbids the exact
137+
// list coming back. This one forbids a differently worded copy, because a
138+
// second sentence saying the same thing is the same defect wearing other words.
139+
func TestAnArchiveRefusesAValueInTheWordsTheRegistryBuilds(t *testing.T) {
140+
t.Run(archive.Compression, func(t *testing.T) {
141+
const bad = "turbo"
142+
_, err := archive.ReadCompression("zip", format.Request{
143+
Properties: map[string]string{archive.Compression: bad},
144+
}, false)
145+
refusedInTheRegistrysWords(t, err, archive.Compression, bad)
146+
})
147+
148+
t.Run(archive.EntryOwner, func(t *testing.T) {
149+
const bad = "nobody"
150+
_, err := archive.ReadOwnership("targz", map[string]string{archive.EntryOwner: bad})
151+
refusedInTheRegistrysWords(t, err, archive.EntryOwner, bad)
152+
})
153+
}
154+
155+
// Every word the declaration offers is a word its reader takes.
156+
//
157+
// This is what the fix above newly depends on, so it is what the fix newly has
158+
// to be guarded against. The sentence now comes from the declared set, which
159+
// means a value added to the set is named as allowed by the refusal that
160+
// refuses it - "compression cannot be turbo, it takes one of: best, default,
161+
// fast, none, turbo" - and worse, a reader with no branch for it returns an
162+
// EMPTY reason, because Allows says nothing is wrong with a value it allows.
163+
//
164+
// Asked of the declaration rather than of a list written here, so a fourth
165+
// closed set added to the archive package is covered on the day it arrives.
166+
func TestEveryWordTheArchiveDeclarationOffersIsOneItsReaderAccepts(t *testing.T) {
167+
readers := map[string]func(word string) error{
168+
archive.Compression: func(word string) error {
169+
_, err := archive.ReadCompression("zip", format.Request{
170+
Properties: map[string]string{archive.Compression: word},
171+
}, false)
172+
return err
173+
},
174+
archive.EntryOwner: func(word string) error {
175+
_, err := archive.ReadOwnership("targz", map[string]string{archive.EntryOwner: word})
176+
return err
177+
},
178+
archive.EntryMode: func(word string) error {
179+
_, err := archive.ReadOwnership("targz", map[string]string{archive.EntryMode: word})
180+
return err
181+
},
182+
}
183+
184+
for key, read := range readers {
185+
t.Run(key, func(t *testing.T) {
186+
p := archive.Axes(key)[0]
187+
if p.Kind != format.PropertyChoice {
188+
t.Fatalf("%s is declared as %v rather than a closed set, so this test is "+
189+
"asking the wrong question about it", key, p.Kind)
190+
}
191+
// An empty set would make the loop below pass without running, which
192+
// is the shape of a green test that proves nothing.
193+
if len(p.Choices) == 0 {
194+
t.Fatalf("%s declares no values, so the loop over them proved nothing", key)
195+
}
196+
197+
for _, word := range p.Choices {
198+
if err := read(word); err != nil {
199+
t.Errorf("the declaration offers %s=%q and the reader refuses it: %v\n"+
200+
"Both halves are read by a person: the window draws this word in a menu "+
201+
"and tfg formats prints it, so a word the reader will not take is a "+
202+
"setting that is offered and does not work.", key, word, err)
203+
}
204+
}
205+
})
206+
}
207+
}
208+
209+
// refusedInTheRegistrysWords checks that err is the refusal that names a
210+
// setting, and that its reason is the one the declaration builds.
211+
func refusedInTheRegistrysWords(t *testing.T, err error, key, bad string) {
212+
t.Helper()
213+
214+
p := archive.Axes(key)[0]
215+
216+
// If the value this test picked has since been declared, the comparison
217+
// below would hold two empty strings against each other and pass. Naming
218+
// that rather than letting it happen: a test whose evidence has evaporated
219+
// reports success in exactly the same words as a test that worked.
220+
want := p.Allows(bad)
221+
if want == "" {
222+
t.Fatalf("%s=%q is a value the declaration now allows, so this test no longer has "+
223+
"a refusal to read. Pick a value the declaration does not contain.", key, bad)
224+
}
225+
226+
if err == nil {
227+
t.Fatalf("%s=%q was accepted, and the declaration does not allow it", key, bad)
228+
}
229+
230+
var value *format.PropertyValueError
231+
if !errors.As(err, &value) {
232+
t.Fatalf("%s=%q was refused as %T rather than as the refusal that names a setting, "+
233+
"so nothing downstream can say which field to point at: %v", key, bad, err, err)
234+
}
235+
236+
if value.Reason != want {
237+
t.Errorf("%s=%q is refused in words of its own rather than the declaration's.\n"+
238+
" refusal says: %q\n"+
239+
" declaration says: %q\n"+
240+
"Ask the declaration - Property.Allows builds this from the declared set, so the "+
241+
"two cannot come apart. Written out by hand they agree until somebody adds a "+
242+
"value to the set and edits one of them (O171).",
243+
key, bad, value.Reason, want)
244+
}
245+
}

0 commit comments

Comments
 (0)