edit tool: Handle over-indentation in replace_with_flexible_indent (#29153)

Release Notes:

- agent: Correct over-indentation in search/replace strings from model

crates/assistant_tools/src/replace.rs

@@ -59,10 +59,8 @@ pub fn replace_with_flexible_indent(old: &str, new: &str, buffer: &BufferSnapsho
 
     let max_row = buffer.max_point().row;
 
-    'windows: for start_row in 0..max_row.saturating_sub(old_lines.len() as u32 - 1) {
-        let mut common_leading = None;
-
-        let end_row = start_row + old_lines.len() as u32 - 1;
+    'windows: for start_row in 0..max_row + 1 {
+        let end_row = start_row + old_lines.len().saturating_sub(1) as u32;
 
         if end_row > max_row {
             // The buffer ends before fully matching the pattern
@@ -77,6 +75,14 @@ pub fn replace_with_flexible_indent(old: &str, new: &str, buffer: &BufferSnapsho
         let mut window_lines = window_text.lines();
         let mut old_lines_iter = old_lines.iter();
 
+        let mut common_mismatch = None;
+
+        #[derive(Eq, PartialEq)]
+        enum Mismatch {
+            OverIndented(String),
+            UnderIndented(String),
+        }
+
         while let (Some(window_line), Some(old_line)) = (window_lines.next(), old_lines_iter.next())
         {
             let line_trimmed = window_line.trim_start();
@@ -89,18 +95,24 @@ pub fn replace_with_flexible_indent(old: &str, new: &str, buffer: &BufferSnapsho
                 continue;
             }
 
-            let line_leading = &window_line[..window_line.len() - old_line.len()];
+            let line_mismatch = if window_line.len() > old_line.len() {
+                let prefix = window_line[..window_line.len() - old_line.len()].to_string();
+                Mismatch::UnderIndented(prefix)
+            } else {
+                let prefix = old_line[..old_line.len() - window_line.len()].to_string();
+                Mismatch::OverIndented(prefix)
+            };
 
-            match &common_leading {
-                Some(common_leading) if common_leading != line_leading => {
+            match &common_mismatch {
+                Some(common_mismatch) if common_mismatch != &line_mismatch => {
                     continue 'windows;
                 }
                 Some(_) => (),
-                None => common_leading = Some(line_leading.to_string()),
+                None => common_mismatch = Some(line_mismatch),
             }
         }
 
-        if let Some(common_leading) = common_leading {
+        if let Some(common_mismatch) = &common_mismatch {
             let line_ending = buffer.line_ending();
             let replacement = new_lines
                 .iter()
@@ -108,7 +120,13 @@ pub fn replace_with_flexible_indent(old: &str, new: &str, buffer: &BufferSnapsho
                     if new_line.trim().is_empty() {
                         new_line.to_string()
                     } else {
-                        common_leading.to_string() + new_line
+                        match common_mismatch {
+                            Mismatch::UnderIndented(prefix) => prefix.to_string() + new_line,
+                            Mismatch::OverIndented(prefix) => new_line
+                                .strip_prefix(prefix)
+                                .unwrap_or(new_line)
+                                .to_string(),
+                        }
                     }
                 })
                 .collect::<Vec<_>>()
@@ -150,14 +168,123 @@ fn lines_with_min_indent(input: &str) -> (Vec<&str>, usize) {
 }
 
 #[cfg(test)]
-mod tests {
+mod replace_exact_tests {
+    use super::*;
+    use gpui::TestAppContext;
+    use gpui::prelude::*;
+
+    #[gpui::test]
+    async fn basic(cx: &mut TestAppContext) {
+        let result = test_replace_exact(cx, "let x = 41;", "let x = 41;", "let x = 42;").await;
+        assert_eq!(result, Some("let x = 42;".to_string()));
+    }
+
+    #[gpui::test]
+    async fn no_match(cx: &mut TestAppContext) {
+        let result = test_replace_exact(cx, "let x = 41;", "let y = 42;", "let y = 43;").await;
+        assert_eq!(result, None);
+    }
+
+    #[gpui::test]
+    async fn multi_line(cx: &mut TestAppContext) {
+        let whole = "fn example() {\n    let x = 41;\n    println!(\"x = {}\", x);\n}";
+        let old_text = "    let x = 41;\n    println!(\"x = {}\", x);";
+        let new_text = "    let x = 42;\n    println!(\"x = {}\", x);";
+        let result = test_replace_exact(cx, whole, old_text, new_text).await;
+        assert_eq!(
+            result,
+            Some("fn example() {\n    let x = 42;\n    println!(\"x = {}\", x);\n}".to_string())
+        );
+    }
+
+    #[gpui::test]
+    async fn multiple_occurrences(cx: &mut TestAppContext) {
+        let whole = "let x = 41;\nlet y = 41;\nlet z = 41;";
+        let result = test_replace_exact(cx, whole, "let x = 41;", "let x = 42;").await;
+        assert_eq!(
+            result,
+            Some("let x = 42;\nlet y = 41;\nlet z = 41;".to_string())
+        );
+    }
+
+    #[gpui::test]
+    async fn empty_buffer(cx: &mut TestAppContext) {
+        let result = test_replace_exact(cx, "", "let x = 41;", "let x = 42;").await;
+        assert_eq!(result, None);
+    }
+
+    #[gpui::test]
+    async fn partial_match(cx: &mut TestAppContext) {
+        let whole = "let x = 41; let y = 42;";
+        let result = test_replace_exact(cx, whole, "let x = 41", "let x = 42").await;
+        assert_eq!(result, Some("let x = 42; let y = 42;".to_string()));
+    }
+
+    #[gpui::test]
+    async fn whitespace_sensitive(cx: &mut TestAppContext) {
+        let result = test_replace_exact(cx, "let x = 41;", " let x = 41;", "let x = 42;").await;
+        assert_eq!(result, None);
+    }
+
+    #[gpui::test]
+    async fn entire_buffer(cx: &mut TestAppContext) {
+        let result = test_replace_exact(cx, "let x = 41;", "let x = 41;", "let x = 42;").await;
+        assert_eq!(result, Some("let x = 42;".to_string()));
+    }
+
+    async fn test_replace_exact(
+        cx: &mut TestAppContext,
+        whole: &str,
+        old: &str,
+        new: &str,
+    ) -> Option<String> {
+        let buffer = cx.new(|cx| language::Buffer::local(whole, cx));
+
+        let buffer_snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact(old, new, &buffer_snapshot).await;
+        diff.map(|diff| {
+            buffer.update(cx, |buffer, cx| {
+                let _ = buffer.apply_diff(diff, cx);
+                buffer.text()
+            })
+        })
+    }
+}
+
+#[cfg(test)]
+mod flexible_indent_tests {
     use super::*;
     use gpui::TestAppContext;
     use gpui::prelude::*;
     use unindent::Unindent;
 
     #[gpui::test]
-    fn test_replace_consistent_indentation(cx: &mut TestAppContext) {
+    fn test_underindented_single_line(cx: &mut TestAppContext) {
+        let cur = "        let a = 41;".to_string();
+        let old = "    let a = 41;".to_string();
+        let new = "    let a = 42;".to_string();
+        let exp = "        let a = 42;".to_string();
+
+        let result = test_replace_with_flexible_indent(cx, &cur, &old, &new);
+
+        assert_eq!(result, Some(exp.to_string()))
+    }
+
+    #[gpui::test]
+    fn test_overindented_single_line(cx: &mut TestAppContext) {
+        let cur = "    let a = 41;".to_string();
+        let old = "        let a = 41;".to_string();
+        let new = "        let a = 42;".to_string();
+        let exp = "    let a = 42;".to_string();
+
+        let result = test_replace_with_flexible_indent(cx, &cur, &old, &new);
+
+        assert_eq!(result, Some(exp.to_string()))
+    }
+
+    #[gpui::test]
+    fn test_underindented_multi_line(cx: &mut TestAppContext) {
         let whole = r#"
             fn test() {
                 let x = 5;
@@ -194,6 +321,33 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    fn test_overindented_multi_line(cx: &mut TestAppContext) {
+        let cur = r#"
+            fn foo() {
+                let a = 41;
+                let b = 3.13;
+            }
+        "#
+        .unindent();
+
+        // 6 space indent instead of 4
+        let old = "      let a = 41;\n      let b = 3.13;";
+        let new = "      let a = 42;\n      let b = 3.14;";
+
+        let expected = r#"
+            fn foo() {
+                let a = 42;
+                let b = 3.14;
+            }
+        "#
+        .unindent();
+
+        let result = test_replace_with_flexible_indent(cx, &cur, &old, &new);
+
+        assert_eq!(result, Some(expected.to_string()))
+    }
+
     #[gpui::test]
     fn test_replace_inconsistent_indentation(cx: &mut TestAppContext) {
         let whole = r#"
@@ -266,7 +420,6 @@ mod tests {
 
     #[gpui::test]
     fn test_replace_no_match(cx: &mut TestAppContext) {
-        // Test with no match
         let whole = r#"
             fn test() {
                 let x = 5;
@@ -317,6 +470,71 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    fn test_replace_whole_is_shorter_than_old(cx: &mut TestAppContext) {
+        let whole = r#"
+            let x = 5;
+        "#
+        .unindent();
+
+        let old = r#"
+            let x = 5;
+            let y = 10;
+        "#
+        .unindent();
+
+        let new = r#"
+            let x = 5;
+            let y = 20;
+        "#
+        .unindent();
+
+        assert_eq!(
+            test_replace_with_flexible_indent(cx, &whole, &old, &new),
+            None
+        );
+    }
+
+    #[gpui::test]
+    fn test_replace_old_is_empty(cx: &mut TestAppContext) {
+        let whole = r#"
+            fn test() {
+                let x = 5;
+            }
+        "#
+        .unindent();
+
+        let old = "";
+        let new = r#"
+            let y = 10;
+        "#
+        .unindent();
+
+        assert_eq!(
+            test_replace_with_flexible_indent(cx, &whole, &old, &new),
+            None
+        );
+    }
+
+    #[gpui::test]
+    fn test_replace_whole_is_empty(cx: &mut TestAppContext) {
+        let whole = "";
+        let old = r#"
+            let x = 5;
+        "#
+        .unindent();
+
+        let new = r#"
+            let x = 10;
+        "#
+        .unindent();
+
+        assert_eq!(
+            test_replace_with_flexible_indent(cx, &whole, &old, &new),
+            None
+        );
+    }
+
     #[test]
     fn test_lines_with_min_indent() {
         // Empty string
@@ -504,6 +722,133 @@ mod tests {
         );
     }
 
+    #[gpui::test]
+    async fn test_replace_exact_basic(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("let x = 41;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact("let x = 41;", "let x = 42;", &snapshot).await;
+        assert!(diff.is_some());
+
+        let diff = diff.unwrap();
+        assert_eq!(diff.edits.len(), 1);
+
+        let result = buffer.update(cx, |buffer, cx| {
+            let _ = buffer.apply_diff(diff, cx);
+            buffer.text()
+        });
+
+        assert_eq!(result, "let x = 42;");
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_no_match(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("let x = 41;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact("let y = 42;", "let y = 43;", &snapshot).await;
+        assert!(diff.is_none());
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_multi_line(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| {
+            language::Buffer::local(
+                "fn example() {\n    let x = 41;\n    println!(\"x = {}\", x);\n}",
+                cx,
+            )
+        });
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let old_text = "    let x = 41;\n    println!(\"x = {}\", x);";
+        let new_text = "    let x = 42;\n    println!(\"x = {}\", x);";
+        let diff = replace_exact(old_text, new_text, &snapshot).await;
+        assert!(diff.is_some());
+
+        let diff = diff.unwrap();
+        let result = buffer.update(cx, |buffer, cx| {
+            let _ = buffer.apply_diff(diff, cx);
+            buffer.text()
+        });
+
+        assert_eq!(
+            result,
+            "fn example() {\n    let x = 42;\n    println!(\"x = {}\", x);\n}"
+        );
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_multiple_occurrences(cx: &mut TestAppContext) {
+        let buffer =
+            cx.new(|cx| language::Buffer::local("let x = 41;\nlet y = 41;\nlet z = 41;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        // Should replace only the first occurrence
+        let diff = replace_exact("let x = 41;", "let x = 42;", &snapshot).await;
+        assert!(diff.is_some());
+
+        let diff = diff.unwrap();
+        let result = buffer.update(cx, |buffer, cx| {
+            let _ = buffer.apply_diff(diff, cx);
+            buffer.text()
+        });
+
+        assert_eq!(result, "let x = 42;\nlet y = 41;\nlet z = 41;");
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_empty_buffer(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact("let x = 41;", "let x = 42;", &snapshot).await;
+        assert!(diff.is_none());
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_partial_match(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("let x = 41; let y = 42;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        // Verify substring replacement actually works
+        let diff = replace_exact("let x = 41", "let x = 42", &snapshot).await;
+        assert!(diff.is_some());
+
+        let diff = diff.unwrap();
+        let result = buffer.update(cx, |buffer, cx| {
+            let _ = buffer.apply_diff(diff, cx);
+            buffer.text()
+        });
+
+        assert_eq!(result, "let x = 42; let y = 42;");
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_whitespace_sensitive(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("let x = 41;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact(" let x = 41;", "let x = 42;", &snapshot).await;
+        assert!(diff.is_none());
+    }
+
+    #[gpui::test]
+    async fn test_replace_exact_entire_buffer(cx: &mut TestAppContext) {
+        let buffer = cx.new(|cx| language::Buffer::local("let x = 41;", cx));
+        let snapshot = buffer.read_with(cx, |buffer, _cx| buffer.snapshot());
+
+        let diff = replace_exact("let x = 41;", "let x = 42;", &snapshot).await;
+        assert!(diff.is_some());
+
+        let diff = diff.unwrap();
+        let result = buffer.update(cx, |buffer, cx| {
+            let _ = buffer.apply_diff(diff, cx);
+            buffer.text()
+        });
+
+        assert_eq!(result, "let x = 42;");
+    }
+
     fn test_replace_with_flexible_indent(
         cx: &mut TestAppContext,
         whole: &str,