diff --git a/AGENTS.md b/AGENTS.md index 82ada50..6572973 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -226,7 +226,9 @@ result, err := parser.ParseExpressions("text with [[expr]]") result, err := parser.EvalExpressions(envStruct) // Check a trait key that did not come from the parser -err := zapscript.ValidateTraitKey(key) +if err := zapscript.ValidateTraitKey(key); err != nil { + return err +} // Match a plain-string key against a parsed Script's traits value, ok := script.Traits[zapscript.NormalizeTraitKey(key)] diff --git a/parser.go b/parser.go index 9815cd1..e551e44 100644 --- a/parser.go +++ b/parser.go @@ -22,6 +22,35 @@ import ( "strings" ) +// mergeJSONTraits merges the decoded members of a **traits object into dst. +// +// Keys are normalized so a trait means the same thing however it was written, +// which lets two distinct JSON keys land on one trait. There is no member +// order left to break the tie by: JSON arguments are normalized through a map +// before they reach here, so a collision would otherwise be settled by Go's +// randomized map iteration and the same script could parse two ways. A +// contradictory object drops the trait instead of picking a winner. +func mergeJSONTraits(dst, src map[string]any) { + normalized := make(map[string]int, len(src)) + for key := range src { + if ValidateTraitKey(key) != nil { + continue + } + normalized[NormalizeTraitKey(key)]++ + } + + for key, value := range src { + if ValidateTraitKey(key) != nil { + continue + } + normalizedKey := NormalizeTraitKey(key) + if normalized[normalizedKey] > 1 { + continue + } + dst[normalizedKey] = value + } +} + func (sr *ScriptReader) parseMediaTitleSyntax() (*mediaTitleParseResult, error) { result := &mediaTitleParseResult{ advArgs: make(map[string]string), @@ -335,12 +364,7 @@ func (sr *ScriptReader) ParseScript() (Script, error) { if script.Traits == nil { script.Traits = make(map[string]any) } - for k, v := range traitsData { - if ValidateTraitKey(k) != nil { - continue - } - script.Traits[NormalizeTraitKey(k)] = v - } + mergeJSONTraits(script.Traits, traitsData) continue } } diff --git a/validate_test.go b/validate_test.go index 2dbde69..4e1ebba 100644 --- a/validate_test.go +++ b/validate_test.go @@ -21,6 +21,8 @@ import ( "testing" "github.com/ZaparooProject/go-zapscript" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateTraitKeyValid(t *testing.T) { @@ -252,3 +254,128 @@ func TestParseTraitsJSONKeysNormalized(t *testing.T) { }) } } + +// Two JSON keys differing only in case normalize to one trait. Merging them +// through a map let Go's randomized iteration order pick the winner, so the +// same script parsed to different results run to run. +func TestParseTraitsJSONKeyCollisionIsDeterministic(t *testing.T) { + t.Parallel() + + inputs := []string{ + `**traits:{"Tap":true,"tap":false,"TAP":"last"}||**echo:hi`, + `**traits:{"Hold":true,"hold":false}||**echo:hi`, + `**traits:{"Keep":1,"kept":2}`, + } + + for _, input := range inputs { + t.Run(input, func(t *testing.T) { + t.Parallel() + + first, err := zapscript.NewParser(input).ParseScript() + require.NoError(t, err) + + for range 200 { + got, parseErr := zapscript.NewParser(input).ParseScript() + require.NoError(t, parseErr) + require.Equal(t, first.Traits, got.Traits, "same input parsed to different traits") + } + }) + } +} + +// JSON arguments are normalized through a map before the traits merge sees +// them, so there is no member order to resolve a collision by. A contradictory +// object drops the trait rather than picking a winner. +func TestParseTraitsJSONKeyCollisionDropsTrait(t *testing.T) { + t.Parallel() + + tests := []struct { + wantTraits map[string]any + name string + input string + }{ + { + name: "collision drops only the contested trait", + input: `**traits:{"Tap":true,"tap":false,"hold":true}`, + wantTraits: map[string]any{"hold": true}, + }, + { + name: "collision on every key leaves no traits", + input: `**traits:{"Tap":true,"tap":false}||**echo:hi`, + wantTraits: map[string]any{}, + }, + { + name: "three way collision drops the trait", + input: `**traits:{"TAP":1,"Tap":2,"tap":3,"hold":true}`, + wantTraits: map[string]any{"hold": true}, + }, + { + name: "unique keys are unaffected", + input: `**traits:{"Tap":true,"Hold":false}`, + wantTraits: map[string]any{"tap": true, "hold": false}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + script, err := zapscript.NewParser(tt.input).ParseScript() + require.NoError(t, err) + assert.Equal(t, tt.wantTraits, script.Traits) + }) + } +} + +func TestParseTraitsJSONDecoding(t *testing.T) { + t.Parallel() + + tests := []struct { + wantTraits map[string]any + name string + input string + wantCmds int + }{ + { + name: "null is consumed and adds nothing", + input: `**traits:null||**echo:hi`, + wantCmds: 1, + wantTraits: map[string]any{}, + }, + { + name: "empty object adds nothing", + input: `**traits:{}||**echo:hi`, + wantCmds: 1, + wantTraits: map[string]any{}, + }, + { + name: "values keep their inferred types", + input: `**traits:{"name":"mario","level":5,"flag":true,"items":["a"]}`, + wantCmds: 0, + wantTraits: map[string]any{ + "name": "mario", "level": float64(5), "flag": true, "items": []any{"a"}, + }, + }, + { + name: "non object json stays a command", + input: `**traits:[1,2]||**echo:hi`, + wantCmds: 2, + wantTraits: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + script, err := zapscript.NewParser(tt.input).ParseScript() + require.NoError(t, err) + assert.Len(t, script.Cmds, tt.wantCmds) + if tt.wantTraits == nil { + assert.Empty(t, script.Traits) + return + } + assert.Equal(t, tt.wantTraits, script.Traits) + }) + } +}