Move mode to comment defaults

Author: oli-obk

CHANGELOG.md

@@ -24,5 +24,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 * `Config::edition` (replaced by `config.comment_defaults.base().edition`)
 * `Config::filter_stdout` (replaced by `config.comment_defaults.base().normalize_stdout`)
 * `Config::filter_stderr` (replaced by `config.comment_defaults.base().normalize_stderr`)
+* `Config::mode` (replaced by `config.comment_defaults.base().mode`)
 
 ## [0.21.2] - 2023-09-27

src/config.rs

@@ -1,5 +1,5 @@
 use regex::bytes::Regex;
-use spanned::Span;
+use spanned::{Span, Spanned};
 
 use crate::{
     dependencies::build_dependencies, per_test_config::Comments, CommandBuilder, Match, Mode,
@@ -25,8 +25,6 @@ pub struct Config {
     pub target: Option<String>,
     /// The folder in which to start searching for .rs files
     pub root_dir: PathBuf,
-    /// The mode in which to run the tests.
-    pub mode: Mode,
     /// The binary to actually execute.
     pub program: CommandBuilder,
     /// The command to run to obtain the cfgs that the output is supposed to
@@ -73,16 +71,17 @@ impl Config {
             #[cfg(windows)]
             (Match::Exact(br"\\?\".to_vec()), b"".to_vec()),
         ];
-        let _ = comment_defaults.base().normalize_stderr = filters.clone();
-        let _ = comment_defaults.base().normalize_stdout = filters;
+        comment_defaults.base().normalize_stderr = filters.clone();
+        comment_defaults.base().normalize_stdout = filters;
+        comment_defaults.base().mode = Spanned::dummy(Mode::Fail {
+            require_patterns: true,
+            rustfix: RustfixMode::MachineApplicable,
+        })
+        .into();
         Self {
             host: None,
             target: None,
             root_dir: root_dir.into(),
-            mode: Mode::Fail {
-                require_patterns: true,
-                rustfix: RustfixMode::MachineApplicable,
-            },
             program: CommandBuilder::rustc(),
             cfgs: CommandBuilder::cfgs(),
             output_conflict_handling: OutputConflictHandling::Bless,
@@ -107,13 +106,14 @@ impl Config {
     pub fn cargo(root_dir: impl Into<PathBuf>) -> Self {
         let mut this = Self {
             program: CommandBuilder::cargo(),
-            mode: Mode::Fail {
-                require_patterns: true,
-                rustfix: RustfixMode::Disabled,
-            },
             ..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
     }
 

src/dependencies.rs

@@ -59,7 +59,15 @@ pub(crate) fn build_dependencies(config: &Config) -> Result<Dependencies> {
     }
 
     // Reusable closure for setting up the environment both for artifact generation and `cargo_metadata`
-    let set_locking = |cmd: &mut Command| match (&config.output_conflict_handling, &config.mode) {
+    let set_locking = |cmd: &mut Command| match (
+        &config.output_conflict_handling,
+        config
+            .comment_defaults
+            .base_immut()
+            .mode
+            .as_deref()
+            .unwrap(),
+    ) {
         (_, Mode::Yolo { .. }) => {}
         (OutputConflictHandling::Error(_), _) => {
             cmd.arg("--locked");

src/error.rs

@@ -30,7 +30,7 @@ pub enum Error {
     /// A ui test checking for success has failure patterns
     PatternFoundInPassTest {
         /// Span of a flag changing the mode (if changed from default).
-        mode: Option<Span>,
+        mode: Span,
         /// Span of the pattern
         span: Span,
     },
@@ -59,6 +59,8 @@ pub enum Error {
         /// The character range in which it was defined.
         span: Span,
     },
+    /// An invalid setting was used.
+    ConfigError(String),
     /// Conflicting comments
     MultipleRevisionsWithResults {
         /// The comment being looked for

src/lib.rs

@@ -14,7 +14,7 @@ use color_eyre::eyre::{eyre, Result};
 use crossbeam_channel::{unbounded, Receiver, Sender};
 use dependencies::{Build, BuildManager};
 use lazy_static::lazy_static;
-use parser::{ErrorMatch, MaybeSpanned, OptWithLine, Revisioned, Spanned};
+use parser::{ErrorMatch, OptWithLine, Revisioned, Spanned};
 use regex::bytes::{Captures, Regex};
 use rustc_stderr::{Level, Message};
 use spanned::Span;
@@ -48,6 +48,8 @@ pub use config::*;
 pub use error::*;
 pub use mode::*;
 
+pub use spanned;
+
 /// A filter's match rule.
 #[derive(Clone, Debug)]
 pub enum Match {
@@ -657,7 +659,7 @@ impl dyn TestStatus {
 
         let (cmd, status, stderr, stdout) = self.run_command(cmd)?;
 
-        let mode = config.mode.maybe_override(comments, revision)?;
+        let mode = comments.mode(revision)?;
         let cmd = check_test_result(
             cmd,
             match *mode {
@@ -767,7 +769,7 @@ fn build_aux_files(
 }
 
 fn run_test_binary(
-    mode: MaybeSpanned<Mode>,
+    mode: Spanned<Mode>,
     path: &Path,
     revision: &str,
     comments: &Comments,
@@ -1009,7 +1011,6 @@ fn check_test_result(
         diagnostics.messages_from_unknown_file_or_line,
         path,
         &mut errors,
-        config,
         revision,
         comments,
     )?;
@@ -1045,7 +1046,6 @@ fn check_annotations(
     mut messages_from_unknown_file_or_line: Vec<Message>,
     path: &Path,
     errors: &mut Errors,
-    config: &Config,
     revision: &str,
     comments: &Comments,
 ) -> Result<(), Errored> {
@@ -1122,9 +1122,9 @@ fn check_annotations(
         msgs
     };
 
-    let mode = config.mode.maybe_override(comments, revision)?;
+    let mode = comments.mode(revision)?;
 
-    if !matches!(config.mode, Mode::Yolo { .. }) {
+    if !matches!(*mode, Mode::Yolo { .. }) {
         let messages_from_unknown_file_or_line = filter(messages_from_unknown_file_or_line);
         if !messages_from_unknown_file_or_line.is_empty() {
             errors.push(Error::ErrorsWithoutPattern {

src/mode.rs

@@ -1,7 +1,4 @@
 use super::Error;
-use crate::parser::Comments;
-use crate::parser::MaybeSpanned;
-use crate::Errored;
 use std::fmt::Display;
 use std::process::ExitStatus;
 
@@ -68,16 +65,6 @@ impl Mode {
             })
         }
     }
-    pub(crate) fn maybe_override(
-        self,
-        comments: &Comments,
-        revision: &str,
-    ) -> Result<MaybeSpanned<Self>, Errored> {
-        let mode = comments.find_one_for_revision(revision, "mode changes", |r| r.mode.clone())?;
-        Ok(mode
-            .into_inner()
-            .map_or(MaybeSpanned::new_config(self), Into::into))
-    }
 }
 
 impl Display for Mode {

src/parser.rs

@@ -53,7 +53,10 @@ impl Comments {
     ) -> Result<OptWithLine<T>, Errored> {
         let mut result = None;
         let mut errors = vec![];
-        for rev in self.for_revision(revision) {
+        for (k, rev) in &self.revisioned {
+            if !k.iter().any(|r| r == revision) {
+                continue;
+            }
             if let Some(found) = f(rev).into_inner() {
                 if result.is_some() {
                     errors.push(found.line());
@@ -62,6 +65,9 @@ impl Comments {
                 }
             }
         }
+        if result.is_none() {
+            result = f(&self.revisioned[&[][..]]).into_inner();
+        }
         if errors.is_empty() {
             Ok(result.into())
         } else {
@@ -99,6 +105,26 @@ impl Comments {
     pub fn base(&mut self) -> &mut Revisioned {
         self.revisioned.get_mut(&[][..]).unwrap()
     }
+
+    /// The comments set for all revisions
+    pub fn base_immut(&self) -> &Revisioned {
+        self.revisioned.get(&[][..]).unwrap()
+    }
+
+    pub(crate) fn mode(&self, revision: &str) -> Result<Spanned<Mode>, Errored> {
+        let mode = self
+            .find_one_for_revision(revision, "`mode` annotations", |r| r.mode.clone())?
+            .into_inner()
+            .ok_or_else(|| Errored {
+                command: Command::new(format!("<finding mode for revision `{revision}`>")),
+                errors: vec![Error::ConfigError(
+                    "no mode set up in Config::comment_defaults".into(),
+                )],
+                stderr: vec![],
+                stdout: vec![],
+            })?;
+        Ok(mode)
+    }
 }
 
 #[derive(Debug, Clone, Default)]
@@ -133,7 +159,7 @@ pub struct Revisioned {
     pub aux_builds: Vec<Spanned<PathBuf>>,
     /// Set the `--edition` flag on the test.
     pub edition: OptWithLine<String>,
-    /// Overwrites the mode from `Config`.
+    /// The mode this test is being run in.
     pub mode: OptWithLine<Mode>,
     pub(crate) needs_asm_support: bool,
     /// Don't run [`rustfix`] for this test
@@ -238,6 +264,8 @@ impl Comments {
             commands: CommentParser::<_>::commands(),
         };
 
+        let defaults = std::mem::take(parser.comments.revisioned.get_mut(&[][..]).unwrap());
+
         let mut fallthrough_to = None; // The line that a `|` will refer to.
         for (l, line) in content.as_ref().lines().enumerate() {
             let l = NonZeroUsize::new(l + 1).unwrap(); // enumerate starts at 0, but line numbers start at 1
@@ -274,6 +302,51 @@ impl Comments {
                 }
             }
         }
+        let Revisioned {
+            span,
+            ignore,
+            only,
+            stderr_per_bitwidth,
+            compile_flags,
+            env_vars,
+            normalize_stderr,
+            normalize_stdout,
+            error_in_other_files,
+            error_matches,
+            require_annotations_for_level,
+            aux_builds,
+            edition,
+            mode,
+            needs_asm_support,
+            no_rustfix,
+        } = parser.comments.base();
+        if span.is_dummy() {
+            *span = defaults.span;
+        }
+        ignore.extend(defaults.ignore);
+        only.extend(defaults.only);
+        *stderr_per_bitwidth |= defaults.stderr_per_bitwidth;
+        compile_flags.extend(defaults.compile_flags);
+        env_vars.extend(defaults.env_vars);
+        normalize_stderr.extend(defaults.normalize_stderr);
+        normalize_stdout.extend(defaults.normalize_stdout);
+        error_in_other_files.extend(defaults.error_in_other_files);
+        error_matches.extend(defaults.error_matches);
+        aux_builds.extend(defaults.aux_builds);
+        if require_annotations_for_level.is_none() {
+            *require_annotations_for_level = defaults.require_annotations_for_level;
+        }
+        if edition.is_none() {
+            *edition = defaults.edition;
+        }
+        if mode.is_none() {
+            *mode = defaults.mode;
+        }
+        if no_rustfix.is_none() {
+            *no_rustfix = defaults.no_rustfix;
+        }
+        *needs_asm_support |= defaults.needs_asm_support;
+
         if parser.errors.is_empty() {
             Ok(parser.comments)
         } else {
@@ -418,21 +491,7 @@ impl CommentParser<Comments> {
                 .entry(revisions)
                 .or_insert_with(|| Revisioned {
                     span,
-                    ignore: Default::default(),
-                    only: Default::default(),
-                    stderr_per_bitwidth: Default::default(),
-                    compile_flags: Default::default(),
-                    env_vars: Default::default(),
-                    normalize_stderr: Default::default(),
-                    normalize_stdout: Default::default(),
-                    error_in_other_files: Default::default(),
-                    error_matches: Default::default(),
-                    require_annotations_for_level: Default::default(),
-                    aux_builds: Default::default(),
-                    edition: Default::default(),
-                    mode: Default::default(),
-                    needs_asm_support: Default::default(),
-                    no_rustfix: Default::default(),
+                    ..Default::default()
                 }),
         };
         f(&mut this);

src/parser/spanned.rs

@@ -1,39 +1,5 @@
 pub use spanned::*;
 
-#[derive(Default, Debug, Clone)]
-pub struct MaybeSpanned<T> {
-    data: T,
-    span: Option<Span>,
-}
-
-impl<T> std::ops::Deref for MaybeSpanned<T> {
-    type Target = T;
-
-    fn deref(&self) -> &Self::Target {
-        &self.data
-    }
-}
-
-impl<T> MaybeSpanned<T> {
-    /// Values from the `Config` struct don't have lines.
-    pub fn new_config(data: T) -> Self {
-        Self { data, span: None }
-    }
-
-    pub fn span(&self) -> Option<Span> {
-        self.span.clone()
-    }
-}
-
-impl<T> From<Spanned<T>> for MaybeSpanned<T> {
-    fn from(value: Spanned<T>) -> Self {
-        Self {
-            data: value.content,
-            span: Some(value.span),
-        }
-    }
-}
-
 #[derive(Debug, Clone)]
 pub struct OptWithLine<T>(Option<Spanned<T>>);