-
Notifications
You must be signed in to change notification settings - Fork 2
Description
Hello! The preprocess_array function called when comparing json arrays sorts them if any regexes for keys to ignore are passed in, even if users set sort_arrays to false when calling the entry points into the json differ. This can lead to confusing diffs that while not wrong from the code's perspective, are not what users intend and make for confusing results.
Take the function below as an example
fn confusing_diff() {
let lhs = json!([
{"a":170},
]);
let rhs = json!([
{"a":171},
{"a":null}
]);
let diff = compare_serde_values(&lhs, &rhs, false, &[]);
println!(
"Diff output {:#?}",
diff.expect("Simple diff, no errors").all_diffs()
);
}
Without any keys to ignore, the diff is
Diff output [
(
Mismatch,
DiffEntry {
path: [
ArrayEntry(
0,
),
Object(
"a",
),
],
values: Some(
(
Number(170),
Number(171),
),
),
},
),
(
Mismatch,
DiffEntry {
path: [
ArrayEntry(
1,
),
],
values: Some(
(
Null,
Object {
"a": Null,
},
),
),
},
),
]
To the user, this does correctly compare the first elements of each array, and detects the second element of the right hand array as the extra. Logically, having no keys to ignore and having keys to ignore, but which don't appear in the input should lead to the same diff, but that is not the case. If instead of &[],
let ignore_keys = {
let re = lazy_regex!("non-matching regex");
vec![re.clone()]
};
is passed in as an array of keys to ignore, the diff is
Diff output [
(
Mismatch,
DiffEntry {
path: [
ArrayEntry(
0,
),
Object(
"a",
),
],
values: Some(
(
Number(170),
Null,
),
),
},
),
(
Mismatch,
DiffEntry {
path: [
ArrayEntry(
1,
),
],
values: Some(
(
Null,
Object {
"a": Number(171),
},
),
),
},
),
]
To the user, this diff compares the first entry in the left hand array with the second entry in the right hand array, and puts the first element of the right hand array last. While it is easy to tell that sorting is happening in this simple example, with bigger json structures, the diffs can very quickly become unintelligible to users as elements move around and get compared to each other out of their original orders.
Would it be possible to only sort arrays if the user sets sort_arrays to true?