Skip to content

Commit 27fa591

Browse files
donislawdevclaude
andcommitted
recipe: sort the two map iterations whose keys reach the report
Go randomises map order, and two loops in the recipe validator walked a map to name unknown keys as problems. A recipe with two bad keys in a contains entry, or two bad keys inside an expectation, could print its problems in a different order on two runs of the same file. No file bytes were affected - this is the report, not the output - but RC7 promises every problem at once, and a list whose order moves between runs is a poor way to keep that promise. A test asserting on the whole message would also have been flaky for a reason nobody would find. Checked all twelve map iterations in product code rather than these two. Ten already sorted, including the one behind `tfg formats` and the one behind the closed reason list, and one of them carries a comment saying why. So the convention was already here and these two were the exception rather than a decision. They now go through one shared sortedKeys. Also fixed: the doc comment on Parse had come adrift from Parse when MaxBytes was added above it, and was documenting the constant instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent a2d4a31 commit 27fa591

1 file changed

Lines changed: 21 additions & 7 deletions

File tree

‎internal/recipe/recipe.go‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ func contentGroups(p *problems, where string, raw []map[string]any) []Content {
8888
at := fmt.Sprintf("%s: contains entry %d", where, i+1)
8989
g := Content{Count: 1}
9090

91-
for key := range item {
91+
// Sorted, because Go randomises map order and these keys become
92+
// problems in the report. Unsorted, the same broken recipe would print
93+
// its problems in a different order on two runs.
94+
for _, key := range sortedKeys(item) {
9295
switch key {
9396
case "format", "count", "size":
9497
default:
@@ -161,11 +164,6 @@ type Output struct {
161164
Manifest string
162165
}
163166

164-
// Parse reads a recipe and returns it only when every check passes.
165-
//
166-
// Nothing is written before this succeeds, and it reports every problem at
167-
// once rather than the first one. Fixing a recipe one error per run is the
168-
// cheapest way to make someone stop using the tool.
169167
// MaxBytes is the largest recipe this build will read.
170168
//
171169
// A recipe comes from somebody else's repository - it can arrive in a pull
@@ -195,6 +193,11 @@ func (e *TooLargeError) Error() string {
195193
e.Name, e.Bytes, MaxBytes)
196194
}
197195

196+
// Parse reads a recipe and returns it only when every check passes.
197+
//
198+
// Nothing is written before this succeeds, and it reports every problem at
199+
// once rather than the first one. Fixing a recipe one error per run is the
200+
// cheapest way to make someone stop using the tool.
198201
func Parse(src []byte, name string) (*Recipe, error) {
199202
// Checked here as well as before the read, because this is the door every
200203
// caller comes through - including the fuzz target, which hands over bytes
@@ -561,6 +564,17 @@ var reasons = map[string]bool{
561564
"malware_signature": true, "duplicate": true, "none": true,
562565
}
563566

567+
// sortedKeys is map iteration with the randomness taken out, for the places
568+
// where the keys become text somebody reads.
569+
func sortedKeys(m map[string]any) []string {
570+
out := make([]string, 0, len(m))
571+
for k := range m {
572+
out = append(out, k)
573+
}
574+
sort.Strings(out)
575+
return out
576+
}
577+
564578
func reasonList() string {
565579
out := make([]string, 0, len(reasons))
566580
for r := range reasons {
@@ -639,7 +653,7 @@ func expectation(p *problems, where string, v any) (string, string) {
639653

640654
// Any key other than the two we carry would be dropped on the way to
641655
// the manifest, and a dropped expectation is one nobody ever checks.
642-
for k := range x {
656+
for _, k := range sortedKeys(x) {
643657
switch k {
644658
case "outcome", "reason":
645659
default:

0 commit comments

Comments
 (0)