fix: prevented false overlap errors by deduplicating identical edits
- Enabled deduplication of byte-identical edits in `apply_edits` to prevent false overlap errors. - Added logic in `ast_edit_blocking` to filter out duplicate rewrite matches before staging. - Updated `apply_edits` to collapse multiple identical replacements into a single deterministic operation. - Added regression tests for both `apply_edits` and `ast_edit_blocking` to ensure identical matches resolve cleanly.
This commit is contained in:
@@ -187,7 +187,20 @@ pub fn rewrite_source(
|
||||
|
||||
pub fn apply_edits(content: &str, edits: &[Edit<String>]) -> Result<String> {
|
||||
let mut sorted: Vec<&Edit<String>> = edits.iter().collect();
|
||||
sorted.sort_by_key(|edit| edit.position);
|
||||
sorted.sort_by(|a, b| {
|
||||
a.position
|
||||
.cmp(&b.position)
|
||||
.then(a.deleted_length.cmp(&b.deleted_length))
|
||||
.then(a.inserted_text.cmp(&b.inserted_text))
|
||||
});
|
||||
// Byte-identical edits (same span, same replacement) are one deterministic
|
||||
// edit: multiple patterns matching the same node collapse instead of
|
||||
// tripping the overlap check. Only divergent overlaps are ambiguous.
|
||||
sorted.dedup_by(|a, b| {
|
||||
a.position == b.position
|
||||
&& a.deleted_length == b.deleted_length
|
||||
&& a.inserted_text == b.inserted_text
|
||||
});
|
||||
let mut prev_end = 0usize;
|
||||
for edit in &sorted {
|
||||
if edit.position < prev_end {
|
||||
@@ -299,4 +312,15 @@ mod tests {
|
||||
];
|
||||
assert!(apply_edits(source, &edits).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn apply_edits_dedupes_identical_edits() {
|
||||
let source = "abcdef";
|
||||
let edits = vec![
|
||||
Edit::<String> { position: 1, deleted_length: 3, inserted_text: b"x".to_vec() },
|
||||
Edit::<String> { position: 1, deleted_length: 3, inserted_text: b"x".to_vec() },
|
||||
];
|
||||
let output = apply_edits(source, &edits).expect("identical edits should collapse to one");
|
||||
assert_eq!(output, "axef");
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1065,12 +1065,23 @@ fn ast_edit_blocking(
|
||||
'patterns: for (_pattern, rewrite, compiled) in &compiled_rules {
|
||||
for matched in ast.root().find_all(compiled.clone()) {
|
||||
ct.heartbeat()?;
|
||||
let edit = matched.replace_by(rewrite.as_str());
|
||||
// Multiple rules matching the same node with the same output are one
|
||||
// deterministic edit; list and count it once instead of staging a
|
||||
// duplicate that trips the apply-time overlap check.
|
||||
let duplicate = file_changes.iter().any(|entry: &PendingFileChange| {
|
||||
entry.edit.position == edit.position
|
||||
&& entry.edit.deleted_length == edit.deleted_length
|
||||
&& entry.edit.inserted_text == edit.inserted_text
|
||||
});
|
||||
if duplicate {
|
||||
continue;
|
||||
}
|
||||
if changes.len() + file_changes.len() >= max_replacements as usize {
|
||||
limit_reached = true;
|
||||
reached_max_replacements = true;
|
||||
break 'patterns;
|
||||
}
|
||||
let edit = matched.replace_by(rewrite.as_str());
|
||||
let range = matched.range();
|
||||
let start = matched.start_pos();
|
||||
let end = matched.end_pos();
|
||||
@@ -1334,6 +1345,58 @@ mod tests {
|
||||
assert!(apply_edits(source, &edits).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dedupes_byte_identical_edits() {
|
||||
let source = "abcdef";
|
||||
let edits = vec![
|
||||
Edit::<String> { position: 1, deleted_length: 3, inserted_text: b"x".to_vec() },
|
||||
Edit::<String> { position: 1, deleted_length: 3, inserted_text: b"x".to_vec() },
|
||||
];
|
||||
let output = apply_edits(source, &edits).expect("identical edits should collapse to one");
|
||||
assert_eq!(output, "axef");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ast_edit_dedupes_identical_matches_across_rules() {
|
||||
let unique = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
.expect("system time should be after UNIX_EPOCH")
|
||||
.as_nanos();
|
||||
let root = std::env::temp_dir().join(format!("pi-ast-dedupe-{unique}"));
|
||||
fs::create_dir_all(&root).expect("temp dedupe dir should be created");
|
||||
let tree = TempTree { root };
|
||||
let file_path = tree.root.join("a.ts");
|
||||
fs::write(&file_path, "const b = foo(bar);\n").expect("temp file a.ts should be written");
|
||||
|
||||
// Both rules match the same call node and produce the byte-identical
|
||||
// replacement; the deterministic edit must apply once, not error as an
|
||||
// ambiguous overlap.
|
||||
let mut rewrites = HashMap::new();
|
||||
rewrites.insert("foo($X)".to_string(), "qux($X)".to_string());
|
||||
rewrites.insert("foo(bar)".to_string(), "qux(bar)".to_string());
|
||||
|
||||
let result = ast_edit_blocking(
|
||||
task::CancelToken::default(),
|
||||
Some(rewrites),
|
||||
Some("ts".to_string()),
|
||||
Some(tree.root.to_string_lossy().into_owned()),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
Some(false),
|
||||
None,
|
||||
None,
|
||||
None,
|
||||
)
|
||||
.expect("identical duplicate matches should apply cleanly");
|
||||
|
||||
assert_eq!(result.total_replacements, 1, "duplicate match must be counted once");
|
||||
assert_eq!(
|
||||
fs::read_to_string(&file_path).expect("a.ts should be readable"),
|
||||
"const b = qux(bar);\n",
|
||||
);
|
||||
}
|
||||
|
||||
fn make_apply_failure_tree() -> TempTree {
|
||||
let unique = SystemTime::now()
|
||||
.duration_since(UNIX_EPOCH)
|
||||
|
||||
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `ast_edit` rejecting byte-identical duplicate replacements as "Overlapping replacements detected": multiple rewrite ops matching the same node with the same output now collapse into one deterministic edit (deduped in both the preview listing/counts and the apply pass), so only genuinely divergent overlaps error.
|
||||
|
||||
### Added
|
||||
|
||||
- Added an in-process `readlink` shell builtin (vendored from uutils coreutils 0.8.0), supporting `-f`/`-e`/`-m` canonicalization, `-n`/`-z` delimiters, and `-v`/`-q`/`-s` verbosity, with path operands resolved against the shell working directory.
|
||||
|
||||
Reference in New Issue
Block a user