diff --git a/Cargo.lock b/Cargo.lock index 72ae4db6ac7..9b2b4ff40a5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2864,7 +2864,7 @@ checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" [[package]] name = "rustfix" -version = "0.7.0" +version = "0.8.0" dependencies = [ "anyhow", "proptest", diff --git a/Cargo.toml b/Cargo.toml index 310704da7e3..0b73e3ad983 100644 --- a/Cargo.toml +++ b/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"] } diff --git a/crates/rustfix/Cargo.toml b/crates/rustfix/Cargo.toml index 5843cf98bcd..f7be56fb436 100644 --- a/crates/rustfix/Cargo.toml +++ b/crates/rustfix/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "rustfix" -version = "0.7.0" +version = "0.8.0" authors = [ "Pascal Hertleif ", "Oliver Schneider ", diff --git a/crates/rustfix/src/lib.rs b/crates/rustfix/src/lib.rs index 3538a8d130a..dd3ebbd63e9 100644 --- a/crates/rustfix/src/lib.rs +++ b/crates/rustfix/src/lib.rs @@ -104,10 +104,6 @@ pub struct Snippet { pub file_name: String, pub line_range: LineRange, pub range: Range, - /// 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 { - // 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::>(); - - // 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::>(); - - if span.text.len() > 1 { - body.push('\n'); - body.push_str( - &last_slice[indent..last_tail_index] - .iter() - .collect::(), - ); - } - tail.push_str(&last_slice[last_tail_index..].iter().collect::()); - 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 { }, }, 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 { - 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( } } - 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 diff --git a/crates/rustfix/tests/everything/use-insert.fixed.rs b/crates/rustfix/tests/everything/use-insert.fixed.rs new file mode 100644 index 00000000000..c6ece90433b --- /dev/null +++ b/crates/rustfix/tests/everything/use-insert.fixed.rs @@ -0,0 +1,9 @@ +use a::f; + +mod a { + pub fn f() {} +} + +fn main() { + f(); +} diff --git a/crates/rustfix/tests/everything/use-insert.json b/crates/rustfix/tests/everything/use-insert.json new file mode 100644 index 00000000000..8496feed7fb --- /dev/null +++ b/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"} diff --git a/crates/rustfix/tests/everything/use-insert.rs b/crates/rustfix/tests/everything/use-insert.rs new file mode 100644 index 00000000000..99368721e38 --- /dev/null +++ b/crates/rustfix/tests/everything/use-insert.rs @@ -0,0 +1,7 @@ +mod a { + pub fn f() {} +} + +fn main() { + f(); +} diff --git a/crates/rustfix/tests/parse_and_replace.rs b/crates/rustfix/tests/parse_and_replace.rs index 902275b64e5..949903b238f 100644 --- a/crates/rustfix/tests/parse_and_replace.rs +++ b/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 { @@ -79,15 +102,6 @@ fn compiles_without_errors(file: &Path) -> Result<(), Error> { } } -fn read_file(path: &Path) -> Result { - 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,7 +143,7 @@ fn test_rustfix_with_file>(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 = @@ -141,15 +151,11 @@ fn test_rustfix_with_file>(file: P, mode: &str) -> Result<(), Err .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>(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>(file: P, mode: &str) -> Result<(), Err } fn get_fixture_files(p: &str) -> Result, 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, 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;