search

getsentry / json-schema-diff

Diff changes between JSON schemas
https://docs.rs/json-schema-diff/
16 stars 3 forks source link

false-positive `integer` addition #25

Closed 6293 closed 1 year ago

6293 commented 1 year ago

https://github.com/getsentry/json-schema-diff/blob/1c6b1cbb91356b9bca3b7a8c65c2644364a16d7a/src/lib.rs#L91

I noticed that in this test diff reports false-positive integer addition

untitaker commented 1 year ago

This is intentional and probably should be documented better.

We explode number type into [number, integer]. This is done so that when the type is changed from integer to number, no breaking change is recorded, because integer is a subtype of number.

6293 commented 1 year ago

We explode number type into [number, integer]. This is done so that when the type is changed from integer to number, no breaking change is recorded, because integer is a subtype of number.

We can detect integer -> number transition without exploding exploding trick, by postprocessing the result from diff.

e.g. if diff is going to return

vec![
    Change {
        path: "some_path".to_string(),
        change: ChangeKind::TypeAdd {
            added: JsonSchemaType::Number,
        },
    },
    Change {
        path: "some_path".to_string(),
        change: ChangeKind::TypeRemove {
            removed: JsonSchemaType::Integer,
        },
    },
]

we can find pair of changes that is (removal_of_integer, addition_of_number) which shares the same path. This pair are then squashed into some kind of

vec![
    Change {
        path: "some_path".to_string(),
        change: ChangeKind::TypeLoose {
            kind: Loosen::IntegerToNumber,
        },
    },
]

But I am not obsessed with this idea because the current behavior is not harmful at all

untitaker commented 1 year ago

i'm inclined to not add more processing stages for now. i also want to keep the ChangeKind enum as small as possible because this output is post-processed by a script and it needs to implement support for every variant