From 6ed0f242155d433b2247b1259a88f1732762416e Mon Sep 17 00:00:00 2001 From: Adam Jones Date: Tue, 7 Oct 2025 13:41:03 +0000 Subject: [PATCH] feat: Add enum support with context-aware breaking change detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Implements detection of JSON Schema enum changes, resolving issue #38. Changes: - Add EnumAdd and EnumRemove change types with context flags - Track whether enum constraint is being added/removed entirely - Smart breaking change logic: - Adding values to existing enum: non-breaking (accepts more) - Removing values from existing enum: breaking (rejects data) - Adding enum constraint: breaking (restricts values) - Removing enum constraint: non-breaking (relaxes values) - Comprehensive test coverage with 9 test cases including the real-world scenario from the original issue Fixes #38 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- src/diff_walker.rs | 34 +++++++++++++++++ src/types.rs | 22 +++++++++++ tests/fixtures/enum/add.json | 10 +++++ tests/fixtures/enum/add_and_remove.json | 10 +++++ tests/fixtures/enum/add_constraint.json | 9 +++++ tests/fixtures/enum/mixed_types.json | 8 ++++ tests/fixtures/enum/number_values.json | 10 +++++ .../fixtures/enum/real_world_event_type.json | 10 +++++ tests/fixtures/enum/remove.json | 10 +++++ tests/fixtures/enum/remove_constraint.json | 9 +++++ tests/fixtures/enum/unchanged.json | 10 +++++ .../test__from_fixtures@enum__add.json.snap | 29 ++++++++++++++ ...om_fixtures@enum__add_and_remove.json.snap | 35 +++++++++++++++++ ...om_fixtures@enum__add_constraint.json.snap | 38 +++++++++++++++++++ ..._from_fixtures@enum__mixed_types.json.snap | 29 ++++++++++++++ ...rom_fixtures@enum__number_values.json.snap | 29 ++++++++++++++ ...ures@enum__real_world_event_type.json.snap | 31 +++++++++++++++ ...test__from_fixtures@enum__remove.json.snap | 29 ++++++++++++++ ...fixtures@enum__remove_constraint.json.snap | 38 +++++++++++++++++++ ...t__from_fixtures@enum__unchanged.json.snap | 20 ++++++++++ 20 files changed, 420 insertions(+) create mode 100644 tests/fixtures/enum/add.json create mode 100644 tests/fixtures/enum/add_and_remove.json create mode 100644 tests/fixtures/enum/add_constraint.json create mode 100644 tests/fixtures/enum/mixed_types.json create mode 100644 tests/fixtures/enum/number_values.json create mode 100644 tests/fixtures/enum/real_world_event_type.json create mode 100644 tests/fixtures/enum/remove.json create mode 100644 tests/fixtures/enum/remove_constraint.json create mode 100644 tests/fixtures/enum/unchanged.json create mode 100644 tests/snapshots/test__from_fixtures@enum__add.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__add_and_remove.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__add_constraint.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__mixed_types.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__number_values.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__real_world_event_type.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__remove.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__remove_constraint.json.snap create mode 100644 tests/snapshots/test__from_fixtures@enum__unchanged.json.snap diff --git a/src/diff_walker.rs b/src/diff_walker.rs index ad2b47d..fac1a7c 100644 --- a/src/diff_walker.rs +++ b/src/diff_walker.rs @@ -387,6 +387,39 @@ impl DiffWalker { } } + fn diff_enum(&mut self, json_path: &str, lhs: &mut SchemaObject, rhs: &mut SchemaObject) { + let lhs_enum = lhs.enum_values.as_deref().unwrap_or(&[]); + let rhs_enum = rhs.enum_values.as_deref().unwrap_or(&[]); + let lhs_has_no_enum = lhs.enum_values.is_none(); + let rhs_has_no_enum = rhs.enum_values.is_none(); + + // Find removed enum values (in lhs but not in rhs) + for lhs_value in lhs_enum { + if !rhs_enum.contains(lhs_value) { + (self.cb)(Change { + path: json_path.to_owned(), + change: ChangeKind::EnumRemove { + removed: lhs_value.clone(), + rhs_has_no_enum, + }, + }); + } + } + + // Find added enum values (in rhs but not in lhs) + for rhs_value in rhs_enum { + if !lhs_enum.contains(rhs_value) { + (self.cb)(Change { + path: json_path.to_owned(), + change: ChangeKind::EnumAdd { + added: rhs_value.clone(), + lhs_has_no_enum, + }, + }); + } + } + } + fn resolve_references( &self, lhs: &mut SchemaObject, @@ -496,6 +529,7 @@ impl DiffWalker { } self.diff_const(json_path, lhs, rhs); self.diff_format(json_path, lhs, rhs); + self.diff_enum(json_path, lhs, rhs); // If we split the types, we don't want to compare type-specific properties // because they are already compared in the `Self::diff_any_of` if !is_lhs_split && !is_rhs_split { diff --git a/src/types.rs b/src/types.rs index bf213f5..f4548e8 100644 --- a/src/types.rs +++ b/src/types.rs @@ -124,6 +124,22 @@ pub enum ChangeKind { /// The new format value. new_format: String, }, + /// An enum value has been added to the allowed values. + EnumAdd { + /// The value that was added to the enum. + added: serde_json::Value, + /// Whether the enum constraint was added (lhs had no enum). + /// If true, this is breaking as it adds a new constraint. + lhs_has_no_enum: bool, + }, + /// An enum value has been removed from the allowed values. + EnumRemove { + /// The value that was removed from the enum. + removed: serde_json::Value, + /// Whether the entire enum constraint was removed (rhs has no enum). + /// If true, this is non-breaking as it relaxes the constraint. + rhs_has_no_enum: bool, + }, } impl ChangeKind { @@ -170,6 +186,12 @@ impl ChangeKind { Self::FormatAdd { .. } => true, Self::FormatRemove { .. } => false, Self::FormatChange { .. } => true, + // EnumAdd is breaking only if it adds a new enum constraint (lhs had no enum). + // Adding values to an existing enum is non-breaking (accepts more data). + Self::EnumAdd { lhs_has_no_enum, .. } => *lhs_has_no_enum, + // EnumRemove is breaking if removing values from a surviving enum constraint. + // Removing the entire enum constraint is non-breaking (accepts more data). + Self::EnumRemove { rhs_has_no_enum, .. } => !rhs_has_no_enum, } } } diff --git a/tests/fixtures/enum/add.json b/tests/fixtures/enum/add.json new file mode 100644 index 0000000..6ca025e --- /dev/null +++ b/tests/fixtures/enum/add.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "string", + "enum": ["error", "warning", "info"] + }, + "rhs": { + "type": "string", + "enum": ["error", "warning", "info", "debug"] + } +} diff --git a/tests/fixtures/enum/add_and_remove.json b/tests/fixtures/enum/add_and_remove.json new file mode 100644 index 0000000..9d66e7e --- /dev/null +++ b/tests/fixtures/enum/add_and_remove.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "string", + "enum": ["error", "warning", "debug"] + }, + "rhs": { + "type": "string", + "enum": ["error", "info", "debug"] + } +} diff --git a/tests/fixtures/enum/add_constraint.json b/tests/fixtures/enum/add_constraint.json new file mode 100644 index 0000000..e65970c --- /dev/null +++ b/tests/fixtures/enum/add_constraint.json @@ -0,0 +1,9 @@ +{ + "lhs": { + "type": "string" + }, + "rhs": { + "type": "string", + "enum": ["error", "warning", "info"] + } +} diff --git a/tests/fixtures/enum/mixed_types.json b/tests/fixtures/enum/mixed_types.json new file mode 100644 index 0000000..a39f986 --- /dev/null +++ b/tests/fixtures/enum/mixed_types.json @@ -0,0 +1,8 @@ +{ + "lhs": { + "enum": ["error", 1, null, true] + }, + "rhs": { + "enum": ["error", 1, null, true, "warning"] + } +} diff --git a/tests/fixtures/enum/number_values.json b/tests/fixtures/enum/number_values.json new file mode 100644 index 0000000..c334e06 --- /dev/null +++ b/tests/fixtures/enum/number_values.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "integer", + "enum": [1, 2, 3] + }, + "rhs": { + "type": "integer", + "enum": [1, 2, 3, 4] + } +} diff --git a/tests/fixtures/enum/real_world_event_type.json b/tests/fixtures/enum/real_world_event_type.json new file mode 100644 index 0000000..d7e90f3 --- /dev/null +++ b/tests/fixtures/enum/real_world_event_type.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "string", + "enum": ["expectct", "expectstaple", "transaction", "default"] + }, + "rhs": { + "type": "string", + "enum": ["expectct", "expectstaple", "transaction", "default", "generic"] + } +} diff --git a/tests/fixtures/enum/remove.json b/tests/fixtures/enum/remove.json new file mode 100644 index 0000000..ad286b8 --- /dev/null +++ b/tests/fixtures/enum/remove.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "string", + "enum": ["error", "warning", "info", "debug"] + }, + "rhs": { + "type": "string", + "enum": ["error", "warning", "info"] + } +} diff --git a/tests/fixtures/enum/remove_constraint.json b/tests/fixtures/enum/remove_constraint.json new file mode 100644 index 0000000..5e70d58 --- /dev/null +++ b/tests/fixtures/enum/remove_constraint.json @@ -0,0 +1,9 @@ +{ + "lhs": { + "type": "string", + "enum": ["error", "warning", "info"] + }, + "rhs": { + "type": "string" + } +} diff --git a/tests/fixtures/enum/unchanged.json b/tests/fixtures/enum/unchanged.json new file mode 100644 index 0000000..20c6996 --- /dev/null +++ b/tests/fixtures/enum/unchanged.json @@ -0,0 +1,10 @@ +{ + "lhs": { + "type": "string", + "enum": ["error", "warning", "info"] + }, + "rhs": { + "type": "string", + "enum": ["error", "warning", "info"] + } +} diff --git a/tests/snapshots/test__from_fixtures@enum__add.json.snap b/tests/snapshots/test__from_fixtures@enum__add.json.snap new file mode 100644 index 0000000..331d0a7 --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__add.json.snap @@ -0,0 +1,29 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - warning + - info + type: string + rhs: + enum: + - error + - warning + - info + - debug + type: string +input_file: tests/fixtures/enum/add.json +--- +[ + Change { + path: "", + change: EnumAdd { + added: String("debug"), + lhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__add_and_remove.json.snap b/tests/snapshots/test__from_fixtures@enum__add_and_remove.json.snap new file mode 100644 index 0000000..85d8fe3 --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__add_and_remove.json.snap @@ -0,0 +1,35 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - warning + - debug + type: string + rhs: + enum: + - error + - info + - debug + type: string +input_file: tests/fixtures/enum/add_and_remove.json +--- +[ + Change { + path: "", + change: EnumRemove { + removed: String("warning"), + rhs_has_no_enum: false, + }, + }, + Change { + path: "", + change: EnumAdd { + added: String("info"), + lhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__add_constraint.json.snap b/tests/snapshots/test__from_fixtures@enum__add_constraint.json.snap new file mode 100644 index 0000000..c0a935d --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__add_constraint.json.snap @@ -0,0 +1,38 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + type: string + rhs: + enum: + - error + - warning + - info + type: string +input_file: tests/fixtures/enum/add_constraint.json +--- +[ + Change { + path: "", + change: EnumAdd { + added: String("error"), + lhs_has_no_enum: true, + }, + }, + Change { + path: "", + change: EnumAdd { + added: String("warning"), + lhs_has_no_enum: true, + }, + }, + Change { + path: "", + change: EnumAdd { + added: String("info"), + lhs_has_no_enum: true, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__mixed_types.json.snap b/tests/snapshots/test__from_fixtures@enum__mixed_types.json.snap new file mode 100644 index 0000000..1b279e4 --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__mixed_types.json.snap @@ -0,0 +1,29 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - 1 + - ~ + - true + rhs: + enum: + - error + - 1 + - ~ + - true + - warning +input_file: tests/fixtures/enum/mixed_types.json +--- +[ + Change { + path: "", + change: EnumAdd { + added: String("warning"), + lhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__number_values.json.snap b/tests/snapshots/test__from_fixtures@enum__number_values.json.snap new file mode 100644 index 0000000..6e8792f --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__number_values.json.snap @@ -0,0 +1,29 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - 1 + - 2 + - 3 + type: integer + rhs: + enum: + - 1 + - 2 + - 3 + - 4 + type: integer +input_file: tests/fixtures/enum/number_values.json +--- +[ + Change { + path: "", + change: EnumAdd { + added: Number(4), + lhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__real_world_event_type.json.snap b/tests/snapshots/test__from_fixtures@enum__real_world_event_type.json.snap new file mode 100644 index 0000000..469211e --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__real_world_event_type.json.snap @@ -0,0 +1,31 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - expectct + - expectstaple + - transaction + - default + type: string + rhs: + enum: + - expectct + - expectstaple + - transaction + - default + - generic + type: string +input_file: tests/fixtures/enum/real_world_event_type.json +--- +[ + Change { + path: "", + change: EnumAdd { + added: String("generic"), + lhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__remove.json.snap b/tests/snapshots/test__from_fixtures@enum__remove.json.snap new file mode 100644 index 0000000..1cd3c38 --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__remove.json.snap @@ -0,0 +1,29 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - warning + - info + - debug + type: string + rhs: + enum: + - error + - warning + - info + type: string +input_file: tests/fixtures/enum/remove.json +--- +[ + Change { + path: "", + change: EnumRemove { + removed: String("debug"), + rhs_has_no_enum: false, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__remove_constraint.json.snap b/tests/snapshots/test__from_fixtures@enum__remove_constraint.json.snap new file mode 100644 index 0000000..b974af3 --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__remove_constraint.json.snap @@ -0,0 +1,38 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - warning + - info + type: string + rhs: + type: string +input_file: tests/fixtures/enum/remove_constraint.json +--- +[ + Change { + path: "", + change: EnumRemove { + removed: String("error"), + rhs_has_no_enum: true, + }, + }, + Change { + path: "", + change: EnumRemove { + removed: String("warning"), + rhs_has_no_enum: true, + }, + }, + Change { + path: "", + change: EnumRemove { + removed: String("info"), + rhs_has_no_enum: true, + }, + }, +] diff --git a/tests/snapshots/test__from_fixtures@enum__unchanged.json.snap b/tests/snapshots/test__from_fixtures@enum__unchanged.json.snap new file mode 100644 index 0000000..bad510d --- /dev/null +++ b/tests/snapshots/test__from_fixtures@enum__unchanged.json.snap @@ -0,0 +1,20 @@ +--- +source: tests/test.rs +assertion_line: 12 +expression: diff +info: + lhs: + enum: + - error + - warning + - info + type: string + rhs: + enum: + - error + - warning + - info + type: string +input_file: tests/fixtures/enum/unchanged.json +--- +[]