rustfix: Support inserting new lines.

Cargo.toml

@@ -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"] }

crates/rustfix/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "rustfix"
-version = "0.7.0"
+version = "0.8.0"
 authors = [
     "Pascal Hertleif <[email protected]>",
     "Oliver Schneider <[email protected]>",

crates/rustfix/src/lib.rs

@@ -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/tests/everything/use-insert.fixed.rs

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

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

@@ -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.rs

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

crates/rustfix/tests/parse_and_replace.rs

@@ -1,3 +1,25 @@
+//! Tests that verify rustfix applies the appropriate changes to a file.
+//!
+//! This test works by reading a series of `*.rs` files in the
+//! `tests/everything` directory. For each `.rs` file, it runs `rustc` to
+//! collect JSON diagnostics from the file. It feeds that JSON data into
+//! rustfix and applies the recommended suggestions to the `.rs` file. It then
+//! compares the result with the corresponding `.fixed.rs` file. If they don't
+//! match, then the test fails.
+//!
+//! There are several debugging environment variables for this test that you can set:
+//!
+//! - `RUST_LOG=parse_and_replace=debug`: Print debug information.
+//! - `RUSTFIX_TEST_BLESS=test-name.rs`: When given the name of a test, this
+//!   will overwrite the `.json` and `.fixed.rs` files with the expected
+//!   values. This can be used when adding a new test.
+//! - `RUSTFIX_TEST_RECORD_JSON=1`:  Records the JSON output to
+//!   `*.recorded.json` files. You can then move that to `.json` or whatever
+//!   you need.
+//! - `RUSTFIX_TEST_RECORD_FIXED_RUST=1`: Records the fixed result to
+//!   `*.recorded.rs` files. You can then move that to `.rs` or whatever you
+//!   need.
+
 #![allow(clippy::disallowed_methods, clippy::print_stdout, clippy::print_stderr)]
 
 use anyhow::{anyhow, ensure, Context, Error};
@@ -20,6 +42,7 @@ mod settings {
     pub const CHECK_JSON: &str = "RUSTFIX_TEST_CHECK_JSON";
     pub const RECORD_JSON: &str = "RUSTFIX_TEST_RECORD_JSON";
     pub const RECORD_FIXED_RUST: &str = "RUSTFIX_TEST_RECORD_FIXED_RUST";
+    pub const BLESS: &str = "RUSTFIX_TEST_BLESS";
 }
 
 fn compile(file: &Path) -> Result<Output, Error> {
@@ -79,15 +102,6 @@ fn compiles_without_errors(file: &Path) -> Result<(), Error> {
     }
 }
 
-fn read_file(path: &Path) -> Result<String, Error> {
-    use std::io::Read;
-
-    let mut buffer = String::new();
-    let mut file = fs::File::open(path)?;
-    file.read_to_string(&mut buffer)?;
-    Ok(buffer)
-}
-
 fn diff(expected: &str, actual: &str) -> String {
     use similar::{ChangeTag, TextDiff};
     use std::fmt::Write;
@@ -104,11 +118,7 @@ fn diff(expected: &str, actual: &str) -> String {
                 ChangeTag::Delete => "-",
             };
             if !different {
-                write!(
-                    &mut res,
-                    "differences found (+ == actual, - == expected):\n"
-                )
-                .unwrap();
+                writeln!(&mut res, "differences found (+ == actual, - == expected):").unwrap();
                 different = true;
             }
             write!(&mut res, "{} {}", prefix, change.value()).unwrap();
@@ -133,23 +143,19 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
     };
 
     debug!("next up: {:?}", file);
-    let code = read_file(file).context(format!("could not read {}", file.display()))?;
+    let code = fs::read_to_string(file)?;
     let errors =
         compile_and_get_json_errors(file).context(format!("could compile {}", file.display()))?;
     let suggestions =
         rustfix::get_suggestions_from_json(&errors, &HashSet::new(), filter_suggestions)
             .context("could not load suggestions")?;
 
     if std::env::var(settings::RECORD_JSON).is_ok() {
-        use std::io::Write;
-        let mut recorded_json = fs::File::create(&file.with_extension("recorded.json")).context(
-            format!("could not create recorded.json for {}", file.display()),
-        )?;
-        recorded_json.write_all(errors.as_bytes())?;
+        fs::write(file.with_extension("recorded.json"), &errors)?;
     }
 
     if std::env::var(settings::CHECK_JSON).is_ok() {
-        let expected_json = read_file(&json_file).context(format!(
+        let expected_json = fs::read_to_string(&json_file).context(format!(
             "could not load json fixtures for {}",
             file.display()
         ))?;
@@ -168,16 +174,23 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
     }
 
     let fixed = apply_suggestions(&code, &suggestions)
-        .context(format!("could not apply suggestions to {}", file.display()))?;
+        .context(format!("could not apply suggestions to {}", file.display()))?
+        .replace('\r', "");
 
     if std::env::var(settings::RECORD_FIXED_RUST).is_ok() {
-        use std::io::Write;
-        let mut recorded_rust = fs::File::create(&file.with_extension("recorded.rs"))?;
-        recorded_rust.write_all(fixed.as_bytes())?;
+        fs::write(file.with_extension("recorded.rs"), &fixed)?;
+    }
+
+    if let Some(bless_name) = std::env::var_os(settings::BLESS) {
+        if bless_name == file.file_name().unwrap() {
+            std::fs::write(&json_file, &errors)?;
+            std::fs::write(&fixed_file, &fixed)?;
+        }
     }
 
-    let expected_fixed =
-        read_file(&fixed_file).context(format!("could read fixed file for {}", file.display()))?;
+    let expected_fixed = fs::read_to_string(&fixed_file)
+        .context(format!("could read fixed file for {}", file.display()))?
+        .replace('\r', "");
     ensure!(
         fixed.trim() == expected_fixed.trim(),
         "file {} doesn't look fixed:\n{}",
@@ -191,8 +204,7 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
 }
 
 fn get_fixture_files(p: &str) -> Result<Vec<PathBuf>, Error> {
-    Ok(fs::read_dir(&p)?
-        .into_iter()
+    Ok(fs::read_dir(p)?
         .map(|e| e.unwrap().path())
         .filter(|p| p.is_file())
         .filter(|p| {
@@ -203,7 +215,7 @@ fn get_fixture_files(p: &str) -> Result<Vec<PathBuf>, Error> {
 }
 
 fn assert_fixtures(dir: &str, mode: &str) {
-    let files = get_fixture_files(&dir)
+    let files = get_fixture_files(dir)
         .context(format!("couldn't load dir `{}`", dir))
         .unwrap();
     let mut failures = 0;