diff --git a/internal/manifest/validate.go b/internal/manifest/validate.go index 727c6ba..c09bd34 100644 --- a/internal/manifest/validate.go +++ b/internal/manifest/validate.go @@ -1,6 +1,8 @@ package manifest import ( + "bytes" + "encoding/json" "fmt" "sort" "strings" @@ -41,9 +43,131 @@ func Validate(data []byte) ([]ValidationError, error) { } } + // Check for duplicate flag keys + duplicates := findDuplicateFlagKeys(data) + for _, key := range duplicates { + issues = append(issues, ValidationError{ + Type: "duplicate_key", + Path: fmt.Sprintf("flags.%s", key), + Message: fmt.Sprintf("flag '%s' is defined multiple times in the manifest", key), + }) + } + return issues, nil } +// findDuplicateFlagKeys parses the raw JSON to detect duplicate keys within the "flags" object. +// Standard JSON unmarshaling silently accepts duplicates (taking the last value), so we use +// a token-based approach to detect them. +func findDuplicateFlagKeys(data []byte) []string { + decoder := json.NewDecoder(bytes.NewReader(data)) + + // Navigate to the root object + token, err := decoder.Token() + if err != nil || token != json.Delim('{') { + return nil + } + + // Look for the "flags" key at the top level + for decoder.More() { + keyToken, err := decoder.Token() + if err != nil { + return nil + } + + key, ok := keyToken.(string) + if !ok { + continue + } + + if key == "flags" { + return findDuplicatesInObject(decoder) + } + + // Skip the value for non-"flags" keys + skipValue(decoder) + } + + return nil +} + +// findDuplicatesInObject reads an object from the decoder and returns any duplicate keys. +func findDuplicatesInObject(decoder *json.Decoder) []string { + token, err := decoder.Token() + if err != nil || token != json.Delim('{') { + return nil + } + + seen := make(map[string]bool) + var duplicates []string + + for decoder.More() { + keyToken, err := decoder.Token() + if err != nil { + break + } + + key, ok := keyToken.(string) + if !ok { + continue + } + + if seen[key] { + duplicates = append(duplicates, key) + } else { + seen[key] = true + } + + // Skip the value + skipValue(decoder) + } + + // Consume the closing brace + _, err = decoder.Token() + if err != nil { + return duplicates + } + + // Sort for consistent output + sort.Strings(duplicates) + + return duplicates +} + +// skipValue advances the decoder past one complete JSON value (object, array, or primitive). +func skipValue(decoder *json.Decoder) { + token, err := decoder.Token() + if err != nil { + return + } + + switch t := token.(type) { + case json.Delim: + switch t { + case '{': + // Skip object contents + for decoder.More() { + if _, err := decoder.Token(); err != nil { // key + return + } + skipValue(decoder) + } + if _, err := decoder.Token(); err != nil { // closing } + return + } + case '[': + // Skip array contents + for decoder.More() { + skipValue(decoder) + } + if _, err := decoder.Token(); err != nil { // closing ] + return + } + } + } + // Primitives (string, number, bool, null) are already consumed by the Token() call +} + func FormatValidationError(issues []ValidationError) string { var sb strings.Builder sb.WriteString("flag manifest validation failed:\n\n") diff --git a/internal/manifest/validate_test.go b/internal/manifest/validate_test.go index eaafd4b..5413be8 100644 --- a/internal/manifest/validate_test.go +++ b/internal/manifest/validate_test.go @@ -5,6 +5,189 @@ import ( "testing" ) +func TestValidate_DuplicateFlagKeys(t *testing.T) { + tests := []struct { + name string + manifest string + wantDuplicates []string + }{ + { + name: "no duplicates", + manifest: `{ + "flags": { + "flag-a": {"flagType": "boolean", "defaultValue": true}, + "flag-b": {"flagType": "string", "defaultValue": "hello"} + } + }`, + wantDuplicates: nil, + }, + { + name: "single duplicate", + manifest: `{ + "flags": { + "my-flag": {"flagType": "boolean", "defaultValue": true}, + "my-flag": {"flagType": "string", "defaultValue": "hello"} + } + }`, + wantDuplicates: []string{"my-flag"}, + }, + { + name: "multiple duplicates", + manifest: `{ + "flags": { + "flag-a": {"flagType": "boolean", "defaultValue": true}, + "flag-b": {"flagType": "string", "defaultValue": "hello"}, + "flag-a": {"flagType": "integer", "defaultValue": 42}, + "flag-b": {"flagType": "float", "defaultValue": 3.14} + } + }`, + wantDuplicates: []string{"flag-a", "flag-b"}, + }, + { + name: "triple duplicate of same key", + manifest: `{ + "flags": { + "repeated": {"flagType": "boolean", "defaultValue": true}, + "repeated": {"flagType": "string", "defaultValue": "hello"}, + "repeated": {"flagType": "integer", "defaultValue": 42} + } + }`, + wantDuplicates: []string{"repeated", "repeated"}, + }, + { + name: "empty flags object", + manifest: `{ + "flags": {} + }`, + wantDuplicates: nil, + }, + { + name: "manifest with schema field", + manifest: `{ + "$schema": "https://example.com/schema.json", + "flags": { + "dup": {"flagType": "boolean", "defaultValue": true}, + "dup": {"flagType": "boolean", "defaultValue": false} + } + }`, + wantDuplicates: []string{"dup"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + issues, err := Validate([]byte(tt.manifest)) + if err != nil { + t.Fatalf("Validate() error = %v", err) + } + + var gotDuplicates []string + for _, issue := range issues { + if issue.Type == "duplicate_key" { + // Extract the flag key from the path (format: "flags.key") + parts := strings.SplitN(issue.Path, ".", 2) + if len(parts) == 2 { + gotDuplicates = append(gotDuplicates, parts[1]) + } + } + } + + if len(gotDuplicates) != len(tt.wantDuplicates) { + t.Errorf("got %d duplicates, want %d", len(gotDuplicates), len(tt.wantDuplicates)) + t.Errorf("got duplicates: %v", gotDuplicates) + t.Errorf("want duplicates: %v", tt.wantDuplicates) + return + } + + for i, want := range tt.wantDuplicates { + if gotDuplicates[i] != want { + t.Errorf("duplicate[%d] = %q, want %q", i, gotDuplicates[i], want) + } + } + }) + } +} + +func TestValidate_DuplicateKeyErrorMessage(t *testing.T) { + manifest := `{ + "flags": { + "my-flag": {"flagType": "boolean", "defaultValue": true}, + "my-flag": {"flagType": "string", "defaultValue": "hello"} + } + }` + + issues, err := Validate([]byte(manifest)) + if err != nil { + t.Fatalf("Validate() error = %v", err) + } + + var found bool + for _, issue := range issues { + if issue.Type == "duplicate_key" { + found = true + if issue.Path != "flags.my-flag" { + t.Errorf("expected path 'flags.my-flag', got %q", issue.Path) + } + expectedMsg := "flag 'my-flag' is defined multiple times in the manifest" + if issue.Message != expectedMsg { + t.Errorf("expected message %q, got %q", expectedMsg, issue.Message) + } + } + } + + if !found { + t.Error("expected to find a duplicate_key validation error") + } +} + +func TestFindDuplicateFlagKeys_EdgeCases(t *testing.T) { + tests := []struct { + name string + input string + expected []string + }{ + { + name: "invalid JSON", + input: "not valid json", + expected: nil, + }, + { + name: "array instead of object", + input: `["a", "b", "c"]`, + expected: nil, + }, + { + name: "no flags key", + input: `{"other": "value"}`, + expected: nil, + }, + { + name: "flags is not an object", + input: `{"flags": "string value"}`, + expected: nil, + }, + { + name: "flags is an array", + input: `{"flags": [1, 2, 3]}`, + expected: nil, + }, + { + name: "nested duplicates not detected in flag values", + input: `{"flags": {"flag1": {"nested": 1, "nested": 2}}}`, + expected: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := findDuplicateFlagKeys([]byte(tt.input)) + if len(result) != len(tt.expected) { + t.Errorf("got %v, want %v", result, tt.expected) + } + }) + } +} + // Sample test for FormatValidationError func TestFormatValidationError_SortsByPath(t *testing.T) { issues := []ValidationError{