Merge pull request #213 from oli-obk/cleanups

Cleanups

CHANGELOG.md

@@ -9,6 +9,26 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Added
 
+* `Config::custom_comments`
+* `Revisioned::custom`
+
+### Fixed
+
+### Changed
+
+* removed `Revisioned::no_rustfix` in favor of turning that into a rustc-specific custom flag
+* removed `Revisioned::edition` in favor of turning that into a rustc-specific custom flag
+* removed `Revisioned::needs_asm_support` in favor of turning that into a rustc-specific custom flag
+* replaced `Mode::Run` with a rustc-specific run flag
+* replaced rustfix with a rustc-specific rustfix flag
+* replaced `rustfix` fields of `Mode::Fail` and `Mode::Yolo` by instead overwriting the rustc-specific custom flag
+
+### Removed
+
+## [0.22.3] - 2024-04-05
+
+### Added
+
 * Reexporting `eyre::Result` at the root level
 
 ### Fixed

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "ui_test"
-version = "0.22.3"
+version = "0.23.0"
 edition = "2021"
 license = "MIT OR Apache-2.0"
 description = "A test framework for testing rustc diagnostics output"

src/config.rs

@@ -1,18 +1,22 @@
 use regex::bytes::Regex;
-use spanned::{Span, Spanned};
+use spanned::Spanned;
 
 use crate::{
+    core::Flag,
     dependencies::build_dependencies,
     filter::Match,
-    per_test_config::{Comments, Condition},
-    CommandBuilder, Mode, RustfixMode,
+    parser::CommandParserFunc,
+    per_test_config::{Comments, Condition, Run, TestConfig},
+    CommandBuilder, Errored, Mode, RustfixMode,
 };
 pub use color_eyre;
 use color_eyre::eyre::Result;
 use std::{
+    collections::HashMap,
     ffi::OsString,
     num::NonZeroUsize,
     path::{Path, PathBuf},
+    process::{Command, Output},
 };
 
 mod args;
@@ -55,17 +59,79 @@ pub struct Config {
     pub filter_exact: bool,
     /// The default settings settable via `@` comments
     pub comment_defaults: Comments,
+    /// Custom comment parsers
+    pub custom_comments: HashMap<&'static str, CommandParserFunc>,
 }
 
 impl Config {
     /// Create a configuration for testing the output of running
     /// `rustc` on the test files.
     pub fn rustc(root_dir: impl Into<PathBuf>) -> Self {
         let mut comment_defaults = Comments::default();
+
+        #[derive(Debug)]
+        struct Edition(String);
+
+        impl Flag for Edition {
+            fn clone_inner(&self) -> Box<dyn Flag> {
+                Box::new(Edition(self.0.clone()))
+            }
+
+            fn apply(&self, cmd: &mut std::process::Command) {
+                cmd.arg("--edition").arg(&self.0);
+            }
+        }
+
+        #[derive(Debug)]
+        struct NeedsAsmSupport;
+
+        impl Flag for NeedsAsmSupport {
+            fn clone_inner(&self) -> Box<dyn Flag> {
+                Box::new(NeedsAsmSupport)
+            }
+            fn test_condition(&self, config: &Config) -> bool {
+                let target = config.target.as_ref().unwrap();
+                static ASM_SUPPORTED_ARCHS: &[&str] = &[
+                    "x86", "x86_64", "arm", "aarch64", "riscv32",
+                    "riscv64",
+                    // These targets require an additional asm_experimental_arch feature.
+                    // "nvptx64", "hexagon", "mips", "mips64", "spirv", "wasm32",
+                ];
+                !ASM_SUPPORTED_ARCHS.iter().any(|arch| target.contains(arch))
+            }
+        }
+
+        impl Flag for RustfixMode {
+            fn clone_inner(&self) -> Box<dyn Flag> {
+                Box::new(*self)
+            }
+            fn post_test_action(
+                &self,
+                config: &TestConfig<'_>,
+                cmd: Command,
+                output: &Output,
+            ) -> Result<Option<Command>, Errored> {
+                let global_rustfix = match *config.mode()? {
+                    Mode::Pass | Mode::Panic => RustfixMode::Disabled,
+                    Mode::Fail { .. } | Mode::Yolo => *self,
+                };
+
+                if config.run_rustfix(output.clone(), global_rustfix)? {
+                    Ok(None)
+                } else {
+                    Ok(Some(cmd))
+                }
+            }
+        }
+
         let _ = comment_defaults
             .base()
-            .edition
-            .set("2021".into(), Span::default());
+            .custom
+            .insert("edition", Spanned::dummy(Box::new(Edition("2021".into()))));
+        let _ = comment_defaults.base().custom.insert(
+            "rustfix",
+            Spanned::dummy(Box::new(RustfixMode::MachineApplicable)),
+        );
         let filters = vec![
             (Match::PathBackslash, b"/".to_vec()),
             #[cfg(windows)]
@@ -77,10 +143,9 @@ impl Config {
         comment_defaults.base().normalize_stdout = filters;
         comment_defaults.base().mode = Spanned::dummy(Mode::Fail {
             require_patterns: true,
-            rustfix: RustfixMode::MachineApplicable,
         })
         .into();
-        Self {
+        let mut config = Self {
             host: None,
             target: None,
             root_dir: root_dir.into(),
@@ -100,22 +165,83 @@ impl Config {
             run_only_ignored: false,
             filter_exact: false,
             comment_defaults,
-        }
+            custom_comments: Default::default(),
+        };
+        config
+            .custom_comments
+            .insert("no-rustfix", |parser, _args, span| {
+                // args are ignored (can be used as comment)
+                let prev = parser
+                    .custom
+                    .insert("no-rustfix", Spanned::new(Box::new(()), span.clone()));
+                parser.check(span, prev.is_none(), "cannot specify `no-rustfix` twice");
+            });
+
+        config
+            .custom_comments
+            .insert("edition", |parser, args, span| {
+                let prev = parser.custom.insert(
+                    "edition",
+                    Spanned::new(Box::new(Edition((*args).into())), args.span()),
+                );
+                parser.check(span, prev.is_none(), "cannot specify `edition` twice");
+            });
+
+        config
+            .custom_comments
+            .insert("needs-asm-support", |parser, args, span| {
+                let prev = parser.custom.insert(
+                    "needs-asm-support",
+                    Spanned::new(Box::new(NeedsAsmSupport), args.span()),
+                );
+                parser.check(
+                    span,
+                    prev.is_none(),
+                    "cannot specify `needs-asm-support` twice",
+                );
+            });
+
+        config.custom_comments.insert("run", |parser, args, span| {
+            parser.check(
+                span.clone(),
+                parser.mode.is_none(),
+                "cannot specify test mode changes twice",
+            );
+            let set = |exit_code| {
+                parser.custom.insert(
+                    "run",
+                    Spanned::new(Box::new(Run { exit_code }), args.span()),
+                );
+                parser.mode = Spanned::new(Mode::Pass, args.span()).into();
+
+                let prev = parser
+                    .custom
+                    .insert("no-rustfix", Spanned::new(Box::new(()), span.clone()));
+                parser.check(span, prev.is_none(), "`run` implies `no-rustfix`");
+            };
+            if args.is_empty() {
+                set(0);
+            } else {
+                match args.content.parse() {
+                    Ok(exit_code) => {
+                        set(exit_code);
+                    }
+                    Err(err) => parser.error(args.span(), err.to_string()),
+                }
+            }
+        });
+        config
     }
 
     /// Create a configuration for testing the output of running
     /// `cargo` on the test `Cargo.toml` files.
     pub fn cargo(root_dir: impl Into<PathBuf>) -> Self {
         let mut this = Self {
             program: CommandBuilder::cargo(),
+            custom_comments: Default::default(),
             ..Self::rustc(root_dir)
         };
-        this.comment_defaults.base().edition = Default::default();
-        this.comment_defaults.base().mode = Spanned::dummy(Mode::Fail {
-            require_patterns: true,
-            rustfix: RustfixMode::Disabled,
-        })
-        .into();
+        this.comment_defaults.base().custom.clear();
         this
     }
 
@@ -269,18 +395,6 @@ impl Config {
                 .expect("target should have been filled in")
     }
 
-    pub(crate) fn has_asm_support(&self) -> bool {
-        static ASM_SUPPORTED_ARCHS: &[&str] = &[
-            "x86", "x86_64", "arm", "aarch64", "riscv32",
-            "riscv64",
-            // These targets require an additional asm_experimental_arch feature.
-            // "nvptx64", "hexagon", "mips", "mips64", "spirv", "wasm32",
-        ];
-        ASM_SUPPORTED_ARCHS
-            .iter()
-            .any(|arch| self.target.as_ref().unwrap().contains(arch))
-    }
-
     pub(crate) fn get_pointer_width(&self) -> u8 {
         // Taken 1:1 from compiletest-rs
         fn get_pointer_width(triple: &str) -> u8 {
@@ -320,7 +434,7 @@ impl Config {
         }
         if comments
             .for_revision(revision)
-            .any(|r| r.needs_asm_support && !self.has_asm_support())
+            .any(|r| r.custom.values().any(|flag| flag.test_condition(self)))
         {
             return self.run_only_ignored;
         }

src/core.rs

@@ -1,12 +1,15 @@
 //! Basic operations useful for building a testsuite
 
+use crate::per_test_config::TestConfig;
 use crate::test_result::Errored;
+use crate::Config;
 use bstr::ByteSlice as _;
 use color_eyre::eyre::Result;
 use crossbeam_channel::unbounded;
 use crossbeam_channel::Receiver;
 use crossbeam_channel::Sender;
 use std::num::NonZeroUsize;
+use std::panic::UnwindSafe;
 use std::path::Component;
 use std::path::Path;
 use std::path::Prefix;
@@ -119,3 +122,40 @@ pub fn run_and_collect<SUBMISSION: Send, RESULT: Send>(
         Ok(())
     })
 }
+
+/// Tester-specific flag that gets parsed from `//@` comments.
+pub trait Flag: Send + Sync + UnwindSafe + std::fmt::Debug {
+    /// Clone the boxed value and create a new box.
+    fn clone_inner(&self) -> Box<dyn Flag>;
+
+    /// Modify a command to what the flag specifies
+    fn apply(&self, _cmd: &mut Command) {}
+
+    /// Whether this flag causes a test to be filtered out
+    fn test_condition(&self, _config: &Config) -> bool {
+        false
+    }
+
+    /// Run an action after a test is finished.
+    /// Returns the `cmd` back if no action was taken.
+    fn post_test_action(
+        &self,
+        _config: &TestConfig<'_>,
+        cmd: Command,
+        _output: &Output,
+    ) -> Result<Option<Command>, Errored> {
+        Ok(Some(cmd))
+    }
+}
+
+impl Flag for () {
+    fn clone_inner(&self) -> Box<dyn Flag> {
+        Box::new(())
+    }
+}
+
+impl Clone for Box<dyn Flag> {
+    fn clone(&self) -> Self {
+        self.clone_inner()
+    }
+}

src/dependencies.rs

@@ -338,7 +338,7 @@ impl<'a> BuildManager<'a> {
             stderr: err.to_string().into_bytes(),
             stdout: vec![],
         })?;
-        let comments = Comments::parse(&file_contents, config.comment_defaults.clone(), aux_file)
+        let comments = Comments::parse(&file_contents, config, aux_file)
             .map_err(|errors| Errored::new(errors, "parse aux comments"))?;
         assert_eq!(
             comments.revisions, None,

src/error.rs

@@ -1,7 +1,6 @@
 use crate::{
     parser::{Pattern, Span, Spanned},
     rustc_stderr::Message,
-    Mode,
 };
 use std::{num::NonZeroUsize, path::PathBuf, process::ExitStatus};
 
@@ -12,7 +11,7 @@ pub enum Error {
     /// Got an invalid exit status for the given mode.
     ExitStatus {
         /// The expected mode.
-        mode: Mode,
+        mode: String,
         /// The exit status of the command.
         status: ExitStatus,
         /// The expected exit status as set in the file or derived from the mode.

src/lib.rs

@@ -10,6 +10,7 @@
 
 pub use color_eyre;
 use color_eyre::eyre::eyre;
+use color_eyre::eyre::Context as _;
 pub use color_eyre::eyre::Result;
 pub use core::run_and_collect;
 pub use core::CrateType;
@@ -126,7 +127,9 @@ pub fn test_command(mut config: Config, path: &Path) -> Result<Command> {
     config.fill_host_and_target()?;
     let extra_args = config.build_dependencies()?;
 
-    let comments = Comments::parse_file(config.comment_defaults.clone(), path)?
+    let content =
+        std::fs::read(path).wrap_err_with(|| format!("failed to read {}", path.display()))?;
+    let comments = Comments::parse(&content, &config, path)
         .map_err(|errors| color_eyre::eyre::eyre!("{errors:#?}"))?;
     let config = TestConfig {
         config,
@@ -301,12 +304,8 @@ fn parse_and_test_file(
     mut config: Config,
     file_contents: Vec<u8>,
 ) -> Result<Vec<TestRun>, Errored> {
-    let comments = Comments::parse(
-        &file_contents,
-        config.comment_defaults.clone(),
-        status.path(),
-    )
-    .map_err(|errors| Errored::new(errors, "parse comments"))?;
+    let comments = Comments::parse(&file_contents, &config, status.path())
+        .map_err(|errors| Errored::new(errors, "parse comments"))?;
     const EMPTY: &[String] = &[String::new()];
     // Run the test for all revisions
     let revisions = comments.revisions.as_deref().unwrap_or(EMPTY);