rustfix: Support inserting new lines.

If rustfix received a suggestion which inserts new lines without
replacing existing lines, it would ignore the suggestion. This is
because `parse_snippet` would immediately return if the `lines` to
replace was empty.

The solution here is to just drop the code which messes with the
original text line. `cargo fix` (and compiletest) currently do not use
this. This was originally added back in the days when rustfix supported
an interactive UI which showed color highlighting of what it looks like
with the replacement. My feeling is that when we add something like this
back in, I would prefer to instead use a real diff library and display
instead of trying to do various text manipulation for display. This
particular code has generally been buggy, and has been a problem several
times.

The included test fails without this fix because the changes do not
apply, and the code cannot compile.

Author: ehuss

Cargo.lock

@@ -77,7 +77,7 @@ pulldown-cmark = { version = "0.9.3", default-features = false }
 rand = "0.8.5"
 regex = "1.10.2"
 rusqlite = { version = "0.30.0", features = ["bundled"] }
-rustfix = { version = "0.7.0", path = "crates/rustfix" }
+rustfix = { version = "0.8.0", path = "crates/rustfix" }
 same-file = "1.0.6"
 security-framework = "2.9.2"
 semver = { version = "1.0.20", features = ["serde"] }

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustfix"
-version = "0.7.0"
+version = "0.8.0"
 authors = [
     "Pascal Hertleif <killercup@gmail.com>",
     "Oliver Schneider <oli-obk@users.noreply.github.com>",

crates/rustfix/Cargo.toml

@@ -104,10 +104,6 @@ pub struct Snippet {
     pub file_name: String,
     pub line_range: LineRange,
     pub range: Range<usize>,
-    /// leading surrounding text, text to replace, trailing surrounding text
-    ///
-    /// This split is useful for highlighting the part that gets replaced
-    pub text: (String, String, String),
 }
 
 /// Represents a replacement of a `snippet`.
@@ -119,58 +115,9 @@ pub struct Replacement {
     pub replacement: String,
 }
 
-/// Parses a [`Snippet`] from a diagnostic span item.
-fn parse_snippet(span: &DiagnosticSpan) -> Option<Snippet> {
-    // unindent the snippet
-    let indent = span
-        .text
-        .iter()
-        .map(|line| {
-            let indent = line
-                .text
-                .chars()
-                .take_while(|&c| char::is_whitespace(c))
-                .count();
-            std::cmp::min(indent, line.highlight_start - 1)
-        })
-        .min()?;
-
-    let text_slice = span.text[0].text.chars().collect::<Vec<char>>();
-
-    // We subtract `1` because these highlights are 1-based
-    // Check the `min` so that it doesn't attempt to index out-of-bounds when
-    // the span points to the "end" of the line. For example, a line of
-    // "foo\n" with a highlight_start of 5 is intended to highlight *after*
-    // the line. This needs to compensate since the newline has been removed
-    // from the text slice.
-    let start = (span.text[0].highlight_start - 1).min(text_slice.len());
-    let end = (span.text[0].highlight_end - 1).min(text_slice.len());
-    let lead = text_slice[indent..start].iter().collect();
-    let mut body: String = text_slice[start..end].iter().collect();
-
-    for line in span.text.iter().take(span.text.len() - 1).skip(1) {
-        body.push('\n');
-        body.push_str(&line.text[indent..]);
-    }
-    let mut tail = String::new();
-    let last = &span.text[span.text.len() - 1];
-
-    // If we get a DiagnosticSpanLine where highlight_end > text.len(), we prevent an 'out of
-    // bounds' access by making sure the index is within the array bounds.
-    // `saturating_sub` is used in case of an empty file
-    let last_tail_index = last.highlight_end.min(last.text.len()).saturating_sub(1);
-    let last_slice = last.text.chars().collect::<Vec<char>>();
-
-    if span.text.len() > 1 {
-        body.push('\n');
-        body.push_str(
-            &last_slice[indent..last_tail_index]
-                .iter()
-                .collect::<String>(),
-        );
-    }
-    tail.push_str(&last_slice[last_tail_index..].iter().collect::<String>());
-    Some(Snippet {
+/// Converts a [`DiagnosticSpan`] to a [`Snippet`].
+fn span_to_snippet(span: &DiagnosticSpan) -> Snippet {
+    Snippet {
         file_name: span.file_name.clone(),
         line_range: LineRange {
             start: LinePosition {
@@ -183,13 +130,12 @@ fn parse_snippet(span: &DiagnosticSpan) -> Option<Snippet> {
             },
         },
         range: (span.byte_start as usize)..(span.byte_end as usize),
-        text: (lead, body, tail),
-    })
+    }
 }
 
 /// Converts a [`DiagnosticSpan`] into a [`Replacement`].
 fn collect_span(span: &DiagnosticSpan) -> Option<Replacement> {
-    let snippet = parse_snippet(span)?;
+    let snippet = span_to_snippet(span);
     let replacement = span.suggested_replacement.clone()?;
     Some(Replacement {
         snippet,
@@ -217,7 +163,7 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
         }
     }
 
-    let snippets = diagnostic.spans.iter().filter_map(parse_snippet).collect();
+    let snippets = diagnostic.spans.iter().map(span_to_snippet).collect();
 
     let solutions: Vec<_> = diagnostic
         .children

crates/rustfix/src/lib.rs

@@ -0,0 +1,9 @@
+use a::f;
+
+mod a {
+    pub fn f() {}
+}
+
+fn main() {
+    f();
+}

crates/rustfix/tests/everything/use-insert.fixed.rs

@@ -0,0 +1,3 @@
+{"$message_type":"diagnostic","message":"cannot find function `f` in this scope","code":{"code":"E0425","explanation":"An unresolved name was used.\n\nErroneous code examples:\n\n```compile_fail,E0425\nsomething_that_doesnt_exist::foo;\n// error: unresolved name `something_that_doesnt_exist::foo`\n\n// or:\n\ntrait Foo {\n    fn bar() {\n        Self; // error: unresolved name `Self`\n    }\n}\n\n// or:\n\nlet x = unknown_variable;  // error: unresolved name `unknown_variable`\n```\n\nPlease verify that the name wasn't misspelled and ensure that the\nidentifier being referred to is valid for the given situation. Example:\n\n```\nenum something_that_does_exist {\n    Foo,\n}\n```\n\nOr:\n\n```\nmod something_that_does_exist {\n    pub static foo : i32 = 0i32;\n}\n\nsomething_that_does_exist::foo; // ok!\n```\n\nOr:\n\n```\nlet unknown_variable = 12u32;\nlet x = unknown_variable; // ok!\n```\n\nIf the item is not defined in the current module, it must be imported using a\n`use` statement, like so:\n\n```\n# mod foo { pub fn bar() {} }\n# fn main() {\nuse foo::bar;\nbar();\n# }\n```\n\nIf the item you are importing is not defined in some super-module of the\ncurrent module, then it must also be declared as public (e.g., `pub fn`).\n"},"level":"error","spans":[{"file_name":"./tests/everything/use-insert.rs","byte_start":45,"byte_end":46,"line_start":6,"line_end":6,"column_start":5,"column_end":6,"is_primary":true,"text":[{"text":"    f();","highlight_start":5,"highlight_end":6}],"label":"not found in this scope","suggested_replacement":null,"suggestion_applicability":null,"expansion":null}],"children":[{"message":"consider importing this function","code":null,"level":"help","spans":[{"file_name":"./tests/everything/use-insert.rs","byte_start":0,"byte_end":0,"line_start":1,"line_end":1,"column_start":1,"column_end":1,"is_primary":true,"text":[],"label":null,"suggested_replacement":"use a::f;\n\n","suggestion_applicability":"MaybeIncorrect","expansion":null}],"children":[],"rendered":null}],"rendered":"error[E0425]: cannot find function `f` in this scope\n --> ./tests/everything/use-insert.rs:6:5\n  |\n6 |     f();\n  |     ^ not found in this scope\n  |\nhelp: consider importing this function\n  |\n1 + use a::f;\n  |\n\n"}
+{"$message_type":"diagnostic","message":"aborting due to 1 previous error","code":null,"level":"error","spans":[],"children":[],"rendered":"error: aborting due to 1 previous error\n\n"}
+{"$message_type":"diagnostic","message":"For more information about this error, try `rustc --explain E0425`.","code":null,"level":"failure-note","spans":[],"children":[],"rendered":"For more information about this error, try `rustc --explain E0425`.\n"}

crates/rustfix/tests/everything/use-insert.json

@@ -0,0 +1,7 @@
+mod a {
+    pub fn f() {}
+}
+
+fn main() {
+    f();
+}