Create multiple .fixed files for diagnostics with multiple suggestions

CHANGELOG.md

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

src/custom_flags.rs

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

src/custom_flags/run.rs

@@ -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;
@@ -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 {
@@ -80,7 +80,8 @@ impl Flag for Run {
                 },
             })
         }
-        Ok(Some(TestRun {
+
+        Ok(vec![TestRun {
             result: if errors.is_empty() {
                 Ok(TestOk::Ok)
             } else {
@@ -92,7 +93,7 @@ impl Flag for Run {
                 })
             },
             status: config.status,
-        }))
+        }])
     }
 }
 

src/custom_flags/rustfix.rs

@@ -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;
@@ -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, ..
@@ -58,94 +58,119 @@ 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")]
+            }
+            _ => 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
@@ -182,41 +207,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)
 }