tidy: refactor --extra-checks parsing

Author: lolbinarycat

src/tools/tidy/src/ext_tool_checks.rs

@@ -20,6 +20,7 @@
 use std::ffi::OsStr;
 use std::path::{Path, PathBuf};
 use std::process::Command;
+use std::str::FromStr;
 use std::{fmt, fs, io};
 
 const MIN_PY_REV: (u32, u32) = (3, 9);
@@ -61,25 +62,38 @@ fn check_impl(
 
     // Split comma-separated args up
     let lint_args = match extra_checks {
-        Some(s) => s.strip_prefix("--extra-checks=").unwrap().split(',').collect(),
+        Some(s) => s
+            .strip_prefix("--extra-checks=")
+            .unwrap()
+            .split(',')
+            .map(|s| {
+                if s == "spellcheck:fix" {
+                    eprintln!("warning: `spellcheck:fix` is no longer valid, use `--extra=check=spellcheck --bless`");
+                }
+                (ExtraCheckArg::from_str(s), s)
+            })
+            .filter_map(|(res, src)| match res {
+                Ok(x) => Some(x),
+                Err(err) => {
+                    eprintln!("warning: bad extra check argument {src:?}: {err:?}");
+                    None
+                }
+            })
+            .collect(),
         None => vec![],
     };
 
-    if lint_args.contains(&"spellcheck:fix") {
-        return Err(Error::Generic(
-            "`spellcheck:fix` is no longer valid, use `--extra=check=spellcheck --bless`"
-                .to_string(),
-        ));
+    macro_rules! extra_check {
+        ($lang:ident, $kind:ident) => {
+            lint_args.iter().any(|arg| arg.matches(ExtraCheckLang::$lang, ExtraCheckKind::$kind))
+        };
     }
 
-    let python_all = lint_args.contains(&"py");
-    let python_lint = lint_args.contains(&"py:lint") || python_all;
-    let python_fmt = lint_args.contains(&"py:fmt") || python_all;
-    let shell_all = lint_args.contains(&"shell");
-    let shell_lint = lint_args.contains(&"shell:lint") || shell_all;
-    let cpp_all = lint_args.contains(&"cpp");
-    let cpp_fmt = lint_args.contains(&"cpp:fmt") || cpp_all;
-    let spellcheck = lint_args.contains(&"spellcheck");
+    let python_lint = extra_check!(Py, Lint);
+    let python_fmt = extra_check!(Py, Fmt);
+    let shell_lint = extra_check!(Shell, Lint);
+    let cpp_fmt = extra_check!(Cpp, Fmt);
+    let spellcheck = extra_check!(Spellcheck, None);
 
     let mut py_path = None;
 
@@ -638,3 +652,86 @@ impl From<io::Error> for Error {
         Self::Io(value)
     }
 }
+
+#[derive(Debug)]
+enum ExtraCheckParseError {
+    #[allow(dead_code, reason = "shown through Debug")]
+    UnknownKind(String),
+    #[allow(dead_code)]
+    UnknownLang(String),
+    /// Too many `:`
+    TooManyParts,
+    /// Tried to parse the empty string
+    Empty,
+}
+
+struct ExtraCheckArg {
+    lang: ExtraCheckLang,
+    /// None = run all extra checks for the given lang
+    kind: Option<ExtraCheckKind>,
+}
+
+impl ExtraCheckArg {
+    fn matches(&self, lang: ExtraCheckLang, kind: ExtraCheckKind) -> bool {
+        self.lang == lang && self.kind.map(|k| k == kind).unwrap_or(true)
+    }
+}
+
+impl FromStr for ExtraCheckArg {
+    type Err = ExtraCheckParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        let mut parts = s.split(':');
+        let Some(first) = parts.next() else {
+            return Err(ExtraCheckParseError::Empty);
+        };
+        let second = parts.next();
+        if parts.next().is_some() {
+            return Err(ExtraCheckParseError::TooManyParts);
+        }
+        Ok(Self { lang: first.parse()?, kind: second.map(|s| s.parse()).transpose()? })
+    }
+}
+
+#[derive(PartialEq, Copy, Clone)]
+enum ExtraCheckLang {
+    Py,
+    Shell,
+    Cpp,
+    Spellcheck,
+}
+
+impl FromStr for ExtraCheckLang {
+    type Err = ExtraCheckParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s {
+            "py" => Self::Py,
+            "shell" => Self::Shell,
+            "cpp" => Self::Cpp,
+            "spellcheck" => Self::Spellcheck,
+            _ => return Err(ExtraCheckParseError::UnknownLang(s.to_string())),
+        })
+    }
+}
+
+#[derive(PartialEq, Copy, Clone)]
+enum ExtraCheckKind {
+    Lint,
+    Fmt,
+    /// Never parsed, but used as a placeholder for
+    /// langs that never have a specific kind.
+    None,
+}
+
+impl FromStr for ExtraCheckKind {
+    type Err = ExtraCheckParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Ok(match s {
+            "lint" => Self::Lint,
+            "fmt" => Self::Fmt,
+            _ => return Err(ExtraCheckParseError::UnknownKind(s.to_string())),
+        })
+    }
+}