From b7464bda4f04717c5f5dcf2453a26a8f18136be3 Mon Sep 17 00:00:00 2001 From: vickydotbat Date: Thu, 25 Jun 2026 22:26:04 +0200 Subject: [PATCH] feat(topdata): reject unsupported canonical json properties --- internal/topdata/json_contract_test.go | 455 +++++++++++++++++++++++++ internal/topdata/topdata.go | 105 +++--- 2 files changed, 520 insertions(+), 40 deletions(-) diff --git a/internal/topdata/json_contract_test.go b/internal/topdata/json_contract_test.go index 1525d48..c7f62e3 100644 --- a/internal/topdata/json_contract_test.go +++ b/internal/topdata/json_contract_test.go @@ -1,6 +1,7 @@ package topdata import ( + "path/filepath" "reflect" "slices" "strings" @@ -227,3 +228,457 @@ func TestGlobalConditionGroupsMatch(t *testing.T) { t.Fatal("expected unless_present match to reject") } } + +func TestValidateProjectRejectsUnsupportedCanonicalJSON(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, root string) + fragments []string + }{ + { + name: "base root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsBase(t, root, `{ + "output": "spells.2da", + "columns": ["Label"], + "rows": [ + {"id": 0, "key": "spells:acid_fog", "Label": "AcidFog"} + ], + "typo": true +}`+"\n") + }, + fragments: []string{"base.json root", "typo", "unsupported"}, + }, + { + name: "plain rows root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "categories.json"), `{ + "output": "categories.2da", + "columns": ["Category"], + "rows": [ + {"id": 0, "key": "categories:harmful", "Category": "Harmful"} + ], + "typo": true +}`+"\n") + }, + fragments: []string{"plain rows root", "typo", "unsupported"}, + }, + { + name: "module root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "modules", "entry.json"), `{ + "entries": { + "spells:burning_hands": {"Label": "BurningHands"} + }, + "typo": true +}`+"\n") + }, + fragments: []string{"module root", "typo", "unsupported"}, + }, + { + name: "global root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "global.json"), `{ + "position": "append", + "typo": true +}`+"\n") + }, + fragments: []string{"global.json root", "typo", "unsupported"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + tc.setup(t, root) + + report := ValidateProject(testProject(root)) + if !report.HasErrors() { + t.Fatalf("expected canonical unsupported-key validation error, got:\n%s", diagnosticsText(report.Diagnostics)) + } + assertDiagnosticTextContainsAll(t, report.Diagnostics, tc.fragments...) + }) + } +} + +func TestValidateProjectRejectsUnsupportedGlobalJSON(t *testing.T) { + tests := []struct { + name string + globalJSON string + fragments []string + }{ + { + name: "injection object is closed", + globalJSON: `{ + "injections": [ + { + "row": {}, + "when_present": [] + } + ] +}` + "\n", + fragments: []string{"global injection 0", "when_present", "unsupported"}, + }, + { + name: "condition object is closed", + globalJSON: `{ + "injections": [ + { + "row": {}, + "require_present": [ + {"field": "FeatIndex", "id": "feat:x", "typo": true} + ] + } + ] +}` + "\n", + fragments: []string{"require_present[0]", "typo", "unsupported"}, + }, + { + name: "default rule is closed", + globalJSON: `{ + "defaults": [ + { + "match": "all", + "values": {}, + "typo": true + } + ] +}` + "\n", + fragments: []string{"global default 0", "typo", "unsupported"}, + }, + { + name: "default match object is closed", + globalJSON: `{ + "defaults": [ + { + "match": {"source": "entries", "typo": true}, + "values": {} + } + ] +}` + "\n", + fragments: []string{"global default 0 match", "typo", "unsupported"}, + }, + { + name: "default format object is closed", + globalJSON: `{ + "defaults": [ + { + "match": "all", + "values": { + "ImpactScript": {"format": "ss_{id}", "typo": true} + } + } + ] +}` + "\n", + fragments: []string{"global default 0 values.ImpactScript", "typo", "unsupported"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "global.json"), tc.globalJSON) + + report := ValidateProject(testProject(root)) + if !report.HasErrors() { + t.Fatalf("expected global unsupported-key validation error, got:\n%s", diagnosticsText(report.Diagnostics)) + } + assertDiagnosticTextContainsAll(t, report.Diagnostics, tc.fragments...) + }) + } +} + +func TestValidateProjectAcceptsAnyPresent(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "global.json"), `{ + "injections": [ + { + "row": {"ImpactScript": "ss_new_spell"}, + "any_present": [ + {"field": "FeatIndex", "id": "feat:gate"} + ] + } + ] +}`+"\n") + + report := ValidateProject(testProject(root)) + if report.HasErrors() { + t.Fatalf("expected non-empty any_present to validate cleanly, got:\n%s", diagnosticsText(report.Diagnostics)) + } +} + +func TestValidateProjectRejectsInvalidAnyPresent(t *testing.T) { + tests := []struct { + name string + injection string + fragments []string + }{ + { + name: "string", + injection: `{ + "row": {}, + "any_present": "feat:gate" +}`, + fragments: []string{"any_present", "array"}, + }, + { + name: "empty list", + injection: `{ + "row": {}, + "any_present": [] +}`, + fragments: []string{"any_present", "at least one condition"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "global.json"), `{ + "injections": [ +`+tc.injection+` + ] +}`+"\n") + + report := ValidateProject(testProject(root)) + if !report.HasErrors() { + t.Fatalf("expected invalid any_present validation error, got:\n%s", diagnosticsText(report.Diagnostics)) + } + assertDiagnosticTextContainsAll(t, report.Diagnostics, tc.fragments...) + }) + } +} + +func TestValidateProjectValidatesEveryCanonicalModuleContainer(t *testing.T) { + root := testProjectRoot(t) + writeCanonicalContractScaffold(t, root) + writeCanonicalRepadjustDataset(t, root) + modulePath := filepath.Join(root, "topdata", "data", "repadjust", "modules", "combined.json") + writeFile(t, modulePath, `{ + "entries": { + "repadjust:burning_hands": {"Label": "BurningHands"} + }, + "overrides": [ + {"key": "repadjust:test", "WIKISUMMARY": "deprecated"} + ] +}`+"\n") + + report := ValidateProject(testProject(root)) + if !report.HasErrors() { + t.Fatalf("expected combined entries+overrides module to validate every container, got:\n%s", diagnosticsText(report.Diagnostics)) + } + assertDiagnosticTextContainsAll(t, report.Diagnostics, "deprecated wiki field", "WIKISUMMARY") + + writeFile(t, modulePath, `{ + "entries": { + "repadjust:burning_hands": {"Label": "BurningHands"} + }, + "overrides": [ + {"key": "repadjust:test", "Label": "FixedLabel"} + ] +}`+"\n") + + report = ValidateProject(testProject(root)) + if report.HasErrors() { + t.Fatalf("expected fixed combined entries+overrides module to validate cleanly, got:\n%s", diagnosticsText(report.Diagnostics)) + } +} + +func TestValidateProjectAcceptsCanonicalJSONContracts(t *testing.T) { + tests := []struct { + name string + setup func(t *testing.T, root string) + }{ + { + name: "columns only module", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "modules", "columns.json"), `{ + "columns": ["ImpactScript"] +}`+"\n") + }, + }, + { + name: "entries only module", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "modules", "entries.json"), `{ + "entries": { + "spells:burning_hands": {"Label": "BurningHands"} + } +}`+"\n") + }, + }, + { + name: "overrides only module", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "modules", "overrides.json"), `{ + "overrides": [ + {"key": "spells:acid_fog", "ImpactScript": "ss_acid_fog"} + ] +}`+"\n") + }, + }, + { + name: "rows module", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "modules", "rows.json"), `{ + "rows": [ + {"key": "spells:burning_hands", "Label": "BurningHands"} + ] +}`+"\n") + }, + }, + { + name: "current base root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsBase(t, root, `{ + "output": "spells.2da", + "columns": ["Label"], + "rows": [ + {"id": 0, "key": "spells:acid_fog", "Label": "AcidFog"} + ] +}`+"\n") + }, + }, + { + name: "loose plain rows root", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "ruleset.json"), `{ + "columns": ["Name", "Value"], + "rows": [ + {"id": 0, "Name": "TEST_RULE", "Value": "1"} + ] +}`+"\n") + }, + }, + { + name: "plain rows root compare reference false", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "categories.json"), `{ + "output": "categories.2da", + "compare_reference": false, + "columns": ["Category"], + "rows": [ + {"id": 0, "key": "categories:harmful", "Category": "Harmful"} + ] +}`+"\n") + }, + }, + { + name: "row like payload helpers remain valid", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsBase(t, root, `{ + "output": "spells.2da", + "columns": ["Name", "Parent", "ImpactScript"], + "rows": [ + { + "id": 0, + "key": "spells:acid_fog", + "Parent": "spells:acid_fog", + "ImpactScript": "ss_acid_fog", + "inherit": {"from": "Parent", "fields": ["ImpactScript"]}, + "_tlk": {"name": "Acid Fog"}, + "meta": {} + } + ] +}`+"\n") + }, + }, + { + name: "global injection row payload stays open", + setup: func(t *testing.T, root string) { + writeCanonicalContractScaffold(t, root) + writeCanonicalSpellsDataset(t, root) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "global.json"), `{ + "injections": [ + { + "row": {"ImpactScript": "ss_new_spell"} + } + ] +}`+"\n") + }, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + root := testProjectRoot(t) + tc.setup(t, root) + + report := ValidateProject(testProject(root)) + if report.HasErrors() { + t.Fatalf("expected canonical contract to validate cleanly, got:\n%s", diagnosticsText(report.Diagnostics)) + } + }) + } +} + +func writeCanonicalContractScaffold(t *testing.T, root string) { + t.Helper() + mkdirAll(t, filepath.Join(root, "topdata", "data")) + writeFile(t, filepath.Join(root, "topdata", "base_dialog.json"), "{}\n") +} + +func writeCanonicalSpellsDataset(t *testing.T, root string) { + t.Helper() + writeCanonicalSpellsBase(t, root, `{ + "output": "spells.2da", + "columns": ["Label", "ImpactScript"], + "rows": [ + {"id": 0, "key": "spells:acid_fog", "Label": "AcidFog", "ImpactScript": "ss_acid_fog"} + ] +}`+"\n") + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "lock.json"), `{"spells:acid_fog":0}`+"\n") + mkdirAll(t, filepath.Join(root, "topdata", "data", "spells", "modules")) +} + +func writeCanonicalSpellsBase(t *testing.T, root string, content string) { + t.Helper() + mkdirAll(t, filepath.Join(root, "topdata", "data", "spells")) + writeFile(t, filepath.Join(root, "topdata", "data", "spells", "base.json"), content) +} + +func writeCanonicalRepadjustDataset(t *testing.T, root string) { + t.Helper() + mkdirAll(t, filepath.Join(root, "topdata", "data", "repadjust", "modules")) + writeFile(t, filepath.Join(root, "topdata", "data", "repadjust", "base.json"), `{ + "output": "repadjust.2da", + "columns": ["Label"], + "rows": [ + {"id": 0, "key": "repadjust:test", "Label": "TestLabel"} + ] +}`+"\n") +} + +func assertDiagnosticTextContainsAll(t *testing.T, diags []Diagnostic, fragments ...string) { + t.Helper() + text := diagnosticsText(diags) + for _, fragment := range fragments { + if !strings.Contains(text, fragment) { + t.Fatalf("expected diagnostics to contain %q, got:\n%s", fragment, text) + } + } +} diff --git a/internal/topdata/topdata.go b/internal/topdata/topdata.go index a70e563..59e4032 100644 --- a/internal/topdata/topdata.go +++ b/internal/topdata/topdata.go @@ -333,6 +333,17 @@ func validateDuplicateJSONKeys(path string, raw []byte, report *ValidationReport } } +func reportTopdataContractError(path string, err error, report *ValidationReport) { + if err == nil { + return + } + report.Diagnostics = append(report.Diagnostics, Diagnostic{ + Severity: SeverityError, + Path: path, + Message: err.Error(), + }) +} + func scanJSONValueForDuplicateKeys(decoder *json.Decoder, path string, report *ValidationReport) error { token, err := decoder.Token() if err != nil { @@ -425,35 +436,40 @@ func validateDataObject(path string, obj map[string]any, report *ValidationRepor return } if base == "base.json" { + reportTopdataContractError(path, unsupportedObjectKeysError("base.json root", obj, baseOrPlainRootKeys...), report) validateRowsFile(path, obj, report, true) return } if inModules { - if _, hasEntries := obj["entries"]; hasEntries { - validateEntriesFile(path, obj, report) - return - } - if _, hasOverrides := obj["overrides"]; hasOverrides { - validateOverridesFile(path, obj, report) - return - } - if _, hasRows := obj["rows"]; hasRows { - validateRowsFile(path, obj, report, false) - return - } - if _, hasColumns := obj["columns"]; hasColumns { + reportTopdataContractError(path, unsupportedObjectKeysError("module root", obj, canonicalModuleRootKeys...), report) + _, hasColumns := obj["columns"] + _, hasEntries := obj["entries"] + _, hasOverrides := obj["overrides"] + _, hasRows := obj["rows"] + if hasColumns { validateColumnsFile(path, obj, report) - return } - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityWarning, - Path: path, - Message: "module file does not use a recognized canonical shape yet; expected one of columns, entries, overrides, or rows", - }) + if hasEntries { + validateEntriesFileWithColumns(path, obj, report, false) + } + if hasOverrides { + validateOverridesFileWithColumns(path, obj, report, false) + } + if hasRows { + validateRowsFileWithColumns(path, obj, report, false, false) + } + if !hasColumns && !hasEntries && !hasOverrides && !hasRows { + report.Diagnostics = append(report.Diagnostics, Diagnostic{ + Severity: SeverityWarning, + Path: path, + Message: "module file does not use a recognized canonical shape yet; expected one of columns, entries, overrides, or rows", + }) + } return } if _, hasRows := obj["rows"]; hasRows { + reportTopdataContractError(path, unsupportedObjectKeysError("plain rows root", obj, baseOrPlainRootKeys...), report) validateRowsFile(path, obj, report, false) return } @@ -826,6 +842,10 @@ func validateLockObject(path string, obj map[string]any, report *ValidationRepor } func validateRowsFile(path string, obj map[string]any, report *ValidationReport, requireColumns bool) { + validateRowsFileWithColumns(path, obj, report, requireColumns, true) +} + +func validateRowsFileWithColumns(path string, obj map[string]any, report *ValidationReport, requireColumns bool, validateColumns bool) { rows, ok := obj["rows"] if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -845,7 +865,7 @@ func validateRowsFile(path string, obj map[string]any, report *ValidationReport, if rowsList, ok := rows.([]any); ok { validateRowCollection(path, obj, rowsList, report) } - if _, ok := obj["columns"]; ok { + if _, ok := obj["columns"]; ok && validateColumns { validateColumnsFile(path, obj, report) } else if requireColumns { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -896,6 +916,10 @@ func validateColumnsFile(path string, obj map[string]any, report *ValidationRepo } func validateEntriesFile(path string, obj map[string]any, report *ValidationReport) { + validateEntriesFileWithColumns(path, obj, report, true) +} + +func validateEntriesFileWithColumns(path string, obj map[string]any, report *ValidationReport, validateColumns bool) { entries, ok := obj["entries"] if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -931,12 +955,16 @@ func validateEntriesFile(path string, obj map[string]any, report *ValidationRepo validateInlineTextUsage(path, key, entry, report) } } - if _, ok := obj["columns"]; ok { + if _, ok := obj["columns"]; ok && validateColumns { validateColumnsFile(path, obj, report) } } func validateOverridesFile(path string, obj map[string]any, report *ValidationReport) { + validateOverridesFileWithColumns(path, obj, report, true) +} + +func validateOverridesFileWithColumns(path string, obj map[string]any, report *ValidationReport, validateColumns bool) { overrides, ok := obj["overrides"] if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -957,12 +985,13 @@ func validateOverridesFile(path string, obj map[string]any, report *ValidationRe container := map[string]any{"rows": overrideList} validateRowCollection(path, container, overrideList, report) } - if _, ok := obj["columns"]; ok { + if _, ok := obj["columns"]; ok && validateColumns { validateColumnsFile(path, obj, report) } } func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationReport) { + reportTopdataContractError(path, unsupportedObjectKeysError("global.json root", obj, globalRootKeys...), report) if _, ok := obj["columns"]; ok { validateColumnsFile(path, obj, report) } @@ -1019,6 +1048,8 @@ func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationR }) continue } + err := unsupportedObjectKeysError(fmt.Sprintf("global injection %d", index), injection, globalInjectionKeys...) + reportTopdataContractError(path, err, report) row, ok := injection["row"].(map[string]any) if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{ @@ -1029,8 +1060,10 @@ func validateGlobalJSONFile(path string, obj map[string]any, report *ValidationR } else { rows = append(rows, row) } - validateGlobalConditionList(path, fmt.Sprintf("global injection %d require_present", index), injection["require_present"], report) - validateGlobalConditionList(path, fmt.Sprintf("global injection %d unless_present", index), injection["unless_present"], report) + if err == nil { + _, conditionErr := parseGlobalConditionGroups(injection) + reportTopdataContractError(path, conditionErr, report) + } } validateRowCollection(path, obj, rows, report) } @@ -1099,6 +1132,7 @@ func validateGlobalDefaults(path string, obj map[string]any, report *ValidationR }) continue } + reportTopdataContractError(path, unsupportedObjectKeysError(fmt.Sprintf("global default %d", index), defaultRule, globalDefaultRuleKeys...), report) validateGlobalDefaultMatch(path, index, defaultRule["match"], report) rawValues, ok := defaultRule["values"] if !ok { @@ -1155,14 +1189,12 @@ func validateGlobalDefaultMatch(path string, index int, rawMatch any, report *Va }) return } - for key := range match { - if key != "source" { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("global default %d match.%s is not supported", index, key), - }) - } + for _, key := range unsupportedObjectKeys(match, globalDefaultMatchKeys...) { + report.Diagnostics = append(report.Diagnostics, Diagnostic{ + Severity: SeverityError, + Path: path, + Message: fmt.Sprintf("global default %d match.%s is not supported (unsupported key; supported keys: %s)", index, key, strings.Join(globalDefaultMatchKeys, ", ")), + }) } source, ok := match["source"].(string) if !ok || strings.TrimSpace(source) == "" { @@ -1193,14 +1225,7 @@ func validateGlobalDefaultValue(path string, index int, field string, value any, if !hasFormat { return } - if len(obj) != 1 { - report.Diagnostics = append(report.Diagnostics, Diagnostic{ - Severity: SeverityError, - Path: path, - Message: fmt.Sprintf("global default %d values.%s format object must contain only format", index, field), - }) - return - } + reportTopdataContractError(path, unsupportedObjectKeysError(fmt.Sprintf("global default %d values.%s", index, field), obj, globalDefaultFormatKeys...), report) format, ok := rawFormat.(string) if !ok { report.Diagnostics = append(report.Diagnostics, Diagnostic{