Skip to content

Commit

Permalink
Add rustfix::apply_suggestions_separately
Browse files Browse the repository at this point in the history
  • Loading branch information
Alexendoo committed Jun 18, 2024
1 parent 06daef6 commit 50fa303
Show file tree
Hide file tree
Showing 12 changed files with 538 additions and 39 deletions.
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/rustfix/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "rustfix"
version = "0.8.4"
version = "0.8.5"
authors = [
"Pascal Hertleif <killercup@gmail.com>",
"Oliver Schneider <oli-obk@users.noreply.github.com>",
Expand Down
53 changes: 47 additions & 6 deletions crates/rustfix/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ pub fn collect_suggestions<S: ::std::hash::BuildHasher>(
/// 1. Feeds the source of a file to [`CodeFix::new`].
/// 2. Calls [`CodeFix::apply`] to apply suggestions to the source code.
/// 3. Calls [`CodeFix::finish`] to get the "fixed" code.
#[derive(Clone)]
pub struct CodeFix {
data: replace::Data,
/// Whether or not the data has been modified.
Expand All @@ -230,12 +231,18 @@ impl CodeFix {

/// Applies a suggestion to the code.
pub fn apply(&mut self, suggestion: &Suggestion) -> Result<(), Error> {
for sol in &suggestion.solutions {
for r in &sol.replacements {
self.data
.replace_range(r.snippet.range.clone(), r.replacement.as_bytes())?;
self.modified = true;
}
for solution in &suggestion.solutions {
self.apply_solution(solution)?;
}
Ok(())
}

/// Applies an individual solution from a [`Suggestion`]
pub fn apply_solution(&mut self, solution: &Solution) -> Result<(), Error> {
for r in &solution.replacements {
self.data
.replace_range(r.snippet.range.clone(), r.replacement.as_bytes())?;
self.modified = true;
}
Ok(())
}
Expand All @@ -252,6 +259,10 @@ impl CodeFix {
}

/// Applies multiple `suggestions` to the given `code`.
///
/// When a diagnostic has multiple `help` subdiagnostics that offer suggestions
/// they are merged together into a single output, to apply them separately see
/// [`apply_suggestions_separately`]
pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<String, Error> {
let mut already_applied = HashSet::new();
let mut fix = CodeFix::new(code);
Expand All @@ -270,3 +281,33 @@ pub fn apply_suggestions(code: &str, suggestions: &[Suggestion]) -> Result<Strin
}
fix.finish()
}

/// Applies multiple `suggestions` to the given `code`.
///
/// When a diagnostic has multiple subdiagnostics that offer suggestions they
/// are applied separately, each output being the result of only applying the
/// Nth suggestion.
///
/// Currently individual subdiagnostics with multiple suggestions are treated
/// the same as a single multi-span suggestion, see
/// [rust-lang/rust#53934](https://github.com/rust-lang/rust/issues/53934).
pub fn apply_suggestions_separately(
code: &str,
suggestions: &[Suggestion],
) -> Result<Vec<String>, Error> {
let max_solutions = suggestions
.iter()
.map(|suggestion| suggestion.solutions.len())
.max()
.unwrap_or_default();
let mut already_applied = HashSet::new();
let mut fixes = vec![CodeFix::new(code); max_solutions];
for suggestion in suggestions.iter().rev() {
for (solution, fix) in suggestion.solutions.iter().zip(&mut fixes) {
if already_applied.insert(solution) {
fix.apply_solution(solution)?;
}
}
}
fixes.iter().map(CodeFix::finish).collect()
}
87 changes: 56 additions & 31 deletions crates/rustfix/tests/parse_and_replace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
#![allow(clippy::disallowed_methods, clippy::print_stdout, clippy::print_stderr)]

use anyhow::{anyhow, ensure, Context, Error};
use rustfix::apply_suggestions;
use rustfix::{apply_suggestions, apply_suggestions_separately};
use std::collections::HashSet;
use std::env;
use std::ffi::OsString;
Expand All @@ -33,8 +33,12 @@ use std::process::{Command, Output};
use tempfile::tempdir;
use tracing::{debug, info, warn};

mod fixmode {
pub const EVERYTHING: &str = "yolo";
#[derive(Clone, Copy)]
enum ReplaceMode {
/// Combine suggestions into a single `.fixed` file with [`apply_suggestions`]
Combined,
/// Create a separate `.fixed` file per suggestion with [`apply_suggestions_separately`]
Separate,
}

mod settings {
Expand Down Expand Up @@ -151,23 +155,16 @@ fn diff(expected: &str, actual: &str) -> String {
res
}

fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Error> {
fn test_rustfix_with_file<P: AsRef<Path>>(file: P, replace_mode: ReplaceMode) -> Result<(), Error> {
let file: &Path = file.as_ref();
let json_file = file.with_extension("json");
let fixed_file = file.with_extension("fixed.rs");

let filter_suggestions = if mode == fixmode::EVERYTHING {
rustfix::Filter::Everything
} else {
rustfix::Filter::MachineApplicableOnly
};

debug!("next up: {:?}", file);
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)
rustfix::get_suggestions_from_json(&errors, &HashSet::new(), rustfix::Filter::Everything)
.context("could not load suggestions")?;

if std::env::var(settings::RECORD_JSON).is_ok() {
Expand All @@ -179,9 +176,12 @@ fn test_rustfix_with_file<P: AsRef<Path>>(file: P, mode: &str) -> Result<(), Err
"could not load json fixtures for {}",
file.display()
))?;
let expected_suggestions =
rustfix::get_suggestions_from_json(&expected_json, &HashSet::new(), filter_suggestions)
.context("could not load expected suggestions")?;
let expected_suggestions = rustfix::get_suggestions_from_json(
&expected_json,
&HashSet::new(),
rustfix::Filter::Everything,
)
.context("could not load expected suggestions")?;

ensure!(
expected_suggestions == suggestions,
Expand All @@ -193,28 +193,52 @@ 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()))?
.replace('\r', "");
let bless = std::env::var_os(settings::BLESS)
.is_some_and(|bless_name| bless_name == file.file_name().unwrap());

if std::env::var(settings::RECORD_FIXED_RUST).is_ok() {
fs::write(file.with_extension("recorded.rs"), &fixed)?;
if bless {
std::fs::write(&json_file, &errors)?;
}

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)?;
match replace_mode {
ReplaceMode::Combined => {
let fixed = apply_suggestions(&code, &suggestions)
.context(format!("could not apply suggestions to {}", file.display()))?
.replace('\r', "");
check_fixed_file(fixed, file.with_extension("fixed.rs"), bless)?;
}
ReplaceMode::Separate => {
let fixes = apply_suggestions_separately(&code, &suggestions)
.context(format!("could not apply suggestions to {}", file.display()))?;
for (i, fixed) in fixes.iter().enumerate() {
check_fixed_file(
fixed.replace('\r', ""),
file.with_extension(format!("fixed.{i}.rs")),
bless,
)?;
}
}
}

Ok(())
}

fn check_fixed_file(fixed: String, fixed_file: PathBuf, bless: bool) -> Result<(), Error> {
if std::env::var(settings::RECORD_FIXED_RUST).is_ok() {
fs::write(fixed_file.with_extension("recorded.rs"), &fixed)?;
}

if bless {
std::fs::write(&fixed_file, &fixed)?;
}

let expected_fixed = fs::read_to_string(&fixed_file)
.context(format!("could read fixed file for {}", file.display()))?
.context(format!("could read fixed file {}", fixed_file.display()))?
.replace('\r', "");
ensure!(
fixed.trim() == expected_fixed.trim(),
"file {} doesn't look fixed:\n{}",
file.display(),
"file {} doesn't match expected fix:\n{}",
fixed_file.display(),
diff(fixed.trim(), expected_fixed.trim())
);

Expand All @@ -229,12 +253,12 @@ fn get_fixture_files(p: &str) -> Result<Vec<PathBuf>, Error> {
.filter(|p| p.is_file())
.filter(|p| {
let x = p.to_string_lossy();
x.ends_with(".rs") && !x.ends_with(".fixed.rs") && !x.ends_with(".recorded.rs")
x.ends_with(".rs") && !x.contains(".fixed.") && !x.ends_with(".recorded.rs")
})
.collect())
}

fn assert_fixtures(dir: &str, mode: &str) {
fn assert_fixtures(dir: &str, replace_mode: ReplaceMode) {
let files = get_fixture_files(dir)
.context(format!("couldn't load dir `{}`", dir))
.unwrap();
Expand All @@ -254,7 +278,7 @@ fn assert_fixtures(dir: &str, mode: &str) {
info!("skipped: {file:?}");
continue;
}
if let Err(err) = test_rustfix_with_file(file, mode) {
if let Err(err) = test_rustfix_with_file(file, replace_mode) {
println!("failed: {}", file.display());
warn!("{:?}", err);
failures += 1;
Expand All @@ -275,5 +299,6 @@ fn assert_fixtures(dir: &str, mode: &str) {
#[test]
fn everything() {
tracing_subscriber::fmt::init();
assert_fixtures("./tests/everything", fixmode::EVERYTHING);
assert_fixtures("./tests/everything", ReplaceMode::Combined);
assert_fixtures("./tests/separate", ReplaceMode::Separate);
}
8 changes: 8 additions & 0 deletions crates/rustfix/tests/separate/multiple.fixed.0.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let mut a = 0;
a += 1;
a += 2;

// Should be `_unused` in `.fixed.0.rs` but left as `unused` in `.fixed.1.rs`
let _unused = a;
}
8 changes: 8 additions & 0 deletions crates/rustfix/tests/separate/multiple.fixed.1.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
fn main() {
let mut a = 0;
a = a + a + 1;
a = a + a + 2;

// Should be `_unused` in `.fixed.0.rs` but left as `unused` in `.fixed.1.rs`
let unused = a;
}
Loading

0 comments on commit 50fa303

Please sign in to comment.