Skip to content

Commit

Permalink
Create multiple .fixed files for diagnostics with multiple suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
@Alexendoo
Alexendoo committed Jul 5, 2024
1 parent 97555c3 commit 09ed073
Show file tree
Hide file tree
Showing 17 changed files with 310 additions and 136 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

* Split up `Revisioned::mode` into `Revisioned::exit_status` and `Revisioned::require_annotations`
* `Config::output_conflict_handling` is now `Error` instead of `Bless`
* Rustfix tests now create multiple `.fixed` files if diagnostics contain multiple suggestions
* updated `prettydiff` from 0.6.4 to 0.7.0, which drops `ansi_term` and `winapi*` deps.

### Removed
Expand Down
6 changes: 3 additions & 3 deletions src/custom_flags.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,15 +34,15 @@ pub trait Flag: Send + Sync + UnwindSafe + RefUnwindSafe + std::fmt::Debug {
}

/// Run an action after a test is finished.
/// Returns `None` if no action was taken.
/// Returns an empty [`Vec`] if no action was taken.
fn post_test_action(
&self,
_config: &TestConfig<'_>,
_cmd: &mut Command,
_output: &Output,
_build_manager: &BuildManager<'_>,
) -> Result<Option<TestRun>, Errored> {
Ok(None)
) -> Result<Vec<TestRun>, Errored> {
Ok(Vec::new())
}

/// Whether the flag gets overridden by the same flag in revisions.
Expand Down
11 changes: 6 additions & 5 deletions src/custom_flags/run.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use std::{
};

use crate::{
build_manager::BuildManager, display, per_test_config::TestConfig, test_result::TestRun, Error,
Errored, TestOk,
build_manager::BuildManager, display, per_test_config::TestConfig, Error, Errored, TestOk,
TestRun,
};

use super::Flag;
Expand All @@ -33,7 +33,7 @@ impl Flag for Run {
cmd: &mut Command,
_output: &Output,
_build_manager: &BuildManager<'_>,
) -> Result<Option<TestRun>, Errored> {
) -> Result<Vec<TestRun>, Errored> {
let exit_code = self.exit_code;
let revision = config.extension("run");
let config = TestConfig {
Expand Down Expand Up @@ -80,7 +80,8 @@ impl Flag for Run {
},
})
}
Ok(Some(TestRun {

Ok(vec![TestRun {
result: if errors.is_empty() {
Ok(TestOk::Ok)
} else {
Expand All @@ -92,7 +93,7 @@ impl Flag for Run {
})
},
status: config.status,
}))
}])
}
}

Expand Down
219 changes: 128 additions & 91 deletions src/custom_flags/rustfix.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,19 @@

use std::{
collections::HashSet,
path::Path,
path::{Path, PathBuf},
process::{Command, Output},
};

use rustfix::{CodeFix, Filter, Suggestion};
use spanned::{Span, Spanned};

use crate::{
build_manager::BuildManager,
display,
parser::OptWithLine,
per_test_config::{Comments, Revisioned, TestConfig},
test_result::TestRun,
Error, Errored, TestOk,
Error, Errored, TestOk, TestRun,
};

use super::Flag;
Expand Down Expand Up @@ -49,7 +49,7 @@ impl Flag for RustfixMode {
_cmd: &mut Command,
output: &Output,
build_manager: &BuildManager<'_>,
) -> Result<Option<TestRun>, Errored> {
) -> Result<Vec<TestRun>, Errored> {
let global_rustfix = match config.exit_status()? {
Some(Spanned {
content: 101 | 0, ..
Expand All @@ -58,94 +58,126 @@ impl Flag for RustfixMode {
};
let output = output.clone();
let no_run_rustfix = config.find_one_custom("no-rustfix")?;
let fixed_code = if no_run_rustfix.is_none() && global_rustfix.enabled() {
let fixes = if no_run_rustfix.is_none() && global_rustfix.enabled() {
fix(&output.stderr, config.status.path(), global_rustfix).map_err(|err| Errored {
command: format!("rustfix {}", display(config.status.path())),
errors: vec![Error::Rustfix(err)],
stderr: output.stderr,
stdout: output.stdout,
})?
} else {
None
Vec::new()
};

let run = fixed_code.is_some();
let mut errors = vec![];
let rustfix_path = config.check_output(
// Always check for `.fixed` files, even if there were reasons not to run rustfix.
// We don't want to leave around stray `.fixed` files
fixed_code.unwrap_or_default().as_bytes(),
&mut errors,
"fixed",
);
let mut errors = Vec::new();
let fixed_paths = match fixes.as_slice() {
[] => Vec::new(),
[single] => {
vec![config.check_output(single.as_bytes(), &mut errors, "fixed")]
}
_ => {
config.check_output(&[], &mut errors, "fixed");
fixes
.iter()
.enumerate()
.map(|(i, fix)| {
config.check_output(
fix.as_bytes(),
&mut errors,
&format!("{}.fixed", i + 1),
)
})
.collect()
}
};

if !errors.is_empty() {
return Ok(Some(TestRun {
result: Err(Errored {
command: format!("checking {}", display(config.status.path())),
errors,
stderr: vec![],
stdout: vec![],
}),
status: config.status.for_path(&rustfix_path),
}));
if fixes.len() != 1 {
// Remove an unused .fixed file
config.check_output(&[], &mut errors, "fixed");
}

if !run {
return Ok(None);
if !errors.is_empty() {
return Err(Errored {
command: format!("checking {}", display(config.status.path())),
errors,
stderr: vec![],
stdout: vec![],
});
}

compile_fixed(config, build_manager, &rustfix_path)
compile_fixed(config, build_manager, fixed_paths)
}
}

fn fix(stderr: &[u8], path: &Path, global_rustfix: RustfixMode) -> anyhow::Result<Option<String>> {
fn fix(stderr: &[u8], path: &Path, mode: RustfixMode) -> anyhow::Result<Vec<String>> {
let suggestions = std::str::from_utf8(stderr)
.unwrap()
.lines()
.flat_map(|line| {
.filter_map(|line| {
if !line.starts_with('{') {
return vec![];
return None;
}
rustfix::get_suggestions_from_json(
line,
let diagnostic = serde_json::from_str(line).unwrap_or_else(|err| {
panic!("could not deserialize diagnostics json for rustfix {err}:{line}")
});
rustfix::collect_suggestions(
&diagnostic,
&HashSet::new(),
if global_rustfix == RustfixMode::Everything {
rustfix::Filter::Everything
if mode == RustfixMode::Everything {
Filter::Everything
} else {
rustfix::Filter::MachineApplicableOnly
Filter::MachineApplicableOnly
},
)
.unwrap_or_else(|err| {
panic!("could not deserialize diagnostics json for rustfix {err}:{line}")
})
})
.collect::<Vec<_>>();
if suggestions.is_empty() {
Ok(None)
} else {
let path_str = display(path);
for sugg in &suggestions {
for snip in &sugg.snippets {
let file_name = snip.file_name.replace('\\', "/");
anyhow::ensure!(
file_name == path_str,
"cannot apply suggestions for `{file_name}` since main file is `{path_str}`. Please use `//@no-rustfix` to disable rustfix",
)
}
return Ok(Vec::new());
}

let max_solutions = suggestions
.iter()
.map(|suggestion| suggestion.solutions.len())
.max()
.unwrap();
let src = std::fs::read_to_string(path).unwrap();
let mut fixes = (0..max_solutions)
.map(|_| CodeFix::new(&src))
.collect::<Vec<_>>();
for Suggestion {
message,
snippets,
solutions,
} in suggestions
{
for snippet in &snippets {
anyhow::ensure!(
Path::new(&snippet.file_name) == path,
"cannot apply suggestions for `{}` since main file is `{}`. Please use `//@no-rustfix` to disable rustfix",
snippet.file_name,
path.display()
);
}

let repeat_first = std::iter::from_fn(|| solutions.first());
for (solution, fix) in solutions.iter().chain(repeat_first).zip(&mut fixes) {
// TODO: use CodeFix::apply_solution when rustfix 0.8.5 is published
fix.apply(&Suggestion {
solutions: vec![solution.clone()],
message: message.clone(),
snippets: snippets.clone(),
})?;
}
Ok(Some(rustfix::apply_suggestions(
&std::fs::read_to_string(path).unwrap(),
&suggestions,
)?))
}

fixes.into_iter().map(|fix| Ok(fix.finish()?)).collect()
}

fn compile_fixed(
config: &TestConfig,
build_manager: &BuildManager<'_>,
rustfix_path: &Path,
) -> Result<Option<TestRun>, Errored> {
fixed_paths: Vec<PathBuf>,
) -> Result<Vec<TestRun>, Errored> {
// picking the crate name from the file name is problematic when `.revision_name` is inserted,
// so we compute it here before replacing the path.
let crate_name = config
Expand Down Expand Up @@ -182,41 +214,46 @@ fn compile_fixed(
.collect(),
};

let fixed_config = TestConfig {
config: config.config.clone(),
comments: &rustfix_comments,
aux_dir: config.aux_dir,
status: config.status.for_path(&rustfix_path),
};

let mut cmd = fixed_config.build_command(build_manager)?;
cmd.arg("--crate-name").arg(crate_name);
let output = cmd.output().unwrap();
if output.status.success() {
Ok(Some(TestRun {
result: Ok(TestOk::Ok),
let mut runs = Vec::new();
for fixed_path in fixed_paths {
let fixed_config = TestConfig {
config: config.config.clone(),
comments: &rustfix_comments,
aux_dir: config.aux_dir,
status: config.status.for_path(&fixed_path),
};
let mut cmd = fixed_config.build_command(build_manager)?;
cmd.arg("--crate-name").arg(&crate_name);
let output = cmd.output().unwrap();
let result = if output.status.success() {
Ok(TestOk::Ok)
} else {
let diagnostics = fixed_config.process(&output.stderr);
Err(Errored {
command: format!("{cmd:?}"),
errors: vec![Error::ExitStatus {
expected: 0,
status: output.status,
reason: Spanned::new(
"after rustfix is applied, all errors should be gone, but weren't".into(),
diagnostics
.messages
.iter()
.flatten()
.chain(diagnostics.messages_from_unknown_file_or_line.iter())
.find_map(|message| message.line_col.clone())
.unwrap_or_default(),
),
}],
stderr: diagnostics.rendered,
stdout: output.stdout,
})
};
runs.push(TestRun {
result,
status: fixed_config.status,
}))
} else {
let diagnostics = fixed_config.process(&output.stderr);
Err(Errored {
command: format!("{cmd:?}"),
errors: vec![Error::ExitStatus {
expected: 0,
status: output.status,
reason: Spanned::new(
"after rustfix is applied, all errors should be gone, but weren't".into(),
diagnostics
.messages
.iter()
.flatten()
.chain(diagnostics.messages_from_unknown_file_or_line.iter())
.find_map(|message| message.line_col.clone())
.unwrap_or_default(),
),
}],
stderr: diagnostics.rendered,
stdout: output.stdout,
})
});
}

Ok(runs)
}
Loading

0 comments on commit 09ed073

Please sign in to comment.