Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create multiple .fixed files for diagnostics with multiple suggestions #244

Merged
merged 3 commits into from
Jul 5, 2024
Merged

Create multiple .fixed files for diagnostics with multiple suggestions #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
482e3d9
Move some rustfix code into separate functions
Alexendoo Jul 4, 2024
97555c3
Fix incomplete progress bar assertions when panicking
Alexendoo Jul 5, 2024
7ec0bc7
Create multiple .fixed files for diagnostics with multiple suggestions
Alexendoo Jul 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
269 changes: 156 additions & 113 deletions src/custom_flags/rustfix.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,19 @@

use std::{
collections::HashSet,
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 @@ -48,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 @@ -57,134 +58,170 @@ impl Flag for RustfixMode {
};
let output = output.clone();
let no_run_rustfix = config.find_one_custom("no-rustfix")?;
let fixed_code = (no_run_rustfix.is_none() && global_rustfix.enabled())
.then_some(())
.and_then(|()| {
let suggestions = std::str::from_utf8(&output.stderr)
.unwrap()
.lines()
.flat_map(|line| {
if !line.starts_with('{') {
return vec![];
}
rustfix::get_suggestions_from_json(
line,
&HashSet::new(),
if global_rustfix == RustfixMode::Everything {
rustfix::Filter::Everything
} else {
rustfix::Filter::MachineApplicableOnly
},
)
.unwrap_or_else(|err| {
panic!("could not deserialize diagnostics json for rustfix {err}:{line}")
})
})
.collect::<Vec<_>>();
if suggestions.is_empty() {
None
} else {
let path_str = display(config.status.path());
for sugg in &suggestions {
for snip in &sugg.snippets {
let file_name = snip.file_name.replace('\\', "/");
if file_name != path_str {
return Some(Err(anyhow::anyhow!("cannot apply suggestions for `{file_name}` since main file is `{path_str}`. Please use `//@no-rustfix` to disable rustfix")));
}
}
}
Some(rustfix::apply_suggestions(
&std::fs::read_to_string(config.status.path()).unwrap(),
&suggestions,
).map_err(|e| e.into()))
}
})
.transpose()
.map_err(|err| Errored {
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 {
Vec::new()
};

let rustfix_comments = Comments {
revisions: None,
revisioned: std::iter::once((
vec![],
Revisioned {
span: Span::default(),
ignore: vec![],
only: vec![],
stderr_per_bitwidth: false,
compile_flags: config.collect(|r| r.compile_flags.iter().cloned()),
env_vars: config.collect(|r| r.env_vars.iter().cloned()),
normalize_stderr: vec![],
normalize_stdout: vec![],
error_in_other_files: vec![],
error_matches: vec![],
require_annotations_for_level: Default::default(),
diagnostic_code_prefix: OptWithLine::new(String::new(), Span::default()),
custom: config.comments().flat_map(|r| r.custom.clone()).collect(),
exit_status: OptWithLine::new(0, Span::default()),
require_annotations: OptWithLine::default(),
},
))
.collect(),
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")]
}
_ => fixes
.iter()
.enumerate()
.map(|(i, fix)| {
config.check_output(fix.as_bytes(), &mut errors, &format!("{}.fixed", i + 1))
})
.collect(),
};

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",
);
// 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
.status
.path()
.file_stem()
.unwrap()
.to_str()
.unwrap()
.replace('-', "_");
if fixes.len() != 1 {
// Remove an unused .fixed file
config.check_output(&[], &mut errors, "fixed");
oli-obk marked this conversation as resolved.
Show resolved Hide resolved
}

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),
}));
return Err(Errored {
command: format!("checking {}", display(config.status.path())),
errors,
stderr: vec![],
stdout: vec![],
});
}

compile_fixed(config, build_manager, fixed_paths)
}
}

fn fix(stderr: &[u8], path: &Path, mode: RustfixMode) -> anyhow::Result<Vec<String>> {
let suggestions = std::str::from_utf8(stderr)
.unwrap()
.lines()
.filter_map(|line| {
if !line.starts_with('{') {
return None;
}
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 mode == RustfixMode::Everything {
Filter::Everything
} else {
Filter::MachineApplicableOnly
},
)
})
.collect::<Vec<_>>();
if suggestions.is_empty() {
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()
);
}

if !run {
return Ok(None);
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(),
})?;
}
}

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

let config = TestConfig {
fn compile_fixed(
config: &TestConfig,
build_manager: &BuildManager<'_>,
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
.status
.path()
.file_stem()
.unwrap()
.to_str()
.unwrap()
.replace('-', "_");

let rustfix_comments = Comments {
revisions: None,
revisioned: std::iter::once((
vec![],
Revisioned {
span: Span::default(),
ignore: vec![],
only: vec![],
stderr_per_bitwidth: false,
compile_flags: config.collect(|r| r.compile_flags.iter().cloned()),
env_vars: config.collect(|r| r.env_vars.iter().cloned()),
normalize_stderr: vec![],
normalize_stdout: vec![],
error_in_other_files: vec![],
error_matches: vec![],
require_annotations_for_level: Default::default(),
diagnostic_code_prefix: OptWithLine::new(String::new(), Span::default()),
custom: config.comments().flat_map(|r| r.custom.clone()).collect(),
exit_status: OptWithLine::new(0, Span::default()),
require_annotations: OptWithLine::default(),
},
))
.collect(),
};

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(&rustfix_path),
status: config.status.for_path(&fixed_path),
};

let mut cmd = config.build_command(build_manager)?;
cmd.arg("--crate-name").arg(crate_name);
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),
status: config.status,
}))
let result = if output.status.success() {
Ok(TestOk::Ok)
} else {
let diagnostics = config.process(&output.stderr);
let diagnostics = fixed_config.process(&output.stderr);
Err(Errored {
command: format!("{cmd:?}"),
errors: vec![Error::ExitStatus {
Expand All @@ -204,6 +241,12 @@ impl Flag for RustfixMode {
stderr: diagnostics.rendered,
stdout: output.stdout,
})
}
};
runs.push(TestRun {
result,
status: fixed_config.status,
});
}

Ok(runs)
}
Loading