feat(linter)!: report unmatched rules with error exit code (#7027)

- closes #6988

we now return an error exit code when there are unmatched rules. previously, we would print an error to stderr and continue running. however, this masked errors in some tests that actually had unmatched rules in them. these test cases now trigger a panic (in tests only, not at runtime), and help ensure that we are reporting an error message to the user for unknown rules, which we did not have any tests cases for before.

- fixes #7025

this also fixes #7025, where we were reporting rules as unmatched simply because they had been disabled prior to being configured. similar to #7009.

apps/oxlint/src/lint.rs

@@ -1,10 +1,10 @@
 use std::{env, io::BufWriter, time::Instant};
 
 use ignore::gitignore::Gitignore;
-use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler};
+use oxc_diagnostics::{DiagnosticService, Error, GraphicalReportHandler, OxcDiagnostic};
 use oxc_linter::{
     loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter, LintService,
-    LintServiceOptions, Linter, LinterBuilder, Oxlintrc,
+    LintServiceOptions, Linter, LinterBuilder, LinterBuilderError, Oxlintrc,
 };
 use oxc_span::VALID_EXTENSIONS;
 
@@ -119,9 +119,24 @@ impl Runner for LintRunner {
 
         let oxlintrc_for_print =
             if misc_options.print_config { Some(oxlintrc.clone()) } else { None };
-        let builder = LinterBuilder::from_oxlintrc(false, oxlintrc)
-            .with_filters(filter)
-            .with_fix(fix_options.fix_kind());
+        let builder = LinterBuilder::from_oxlintrc(false, oxlintrc);
+        // Gracefully report any linter builder errors as CLI errors
+        let builder = match builder {
+            Ok(builder) => builder,
+            Err(err) => match err {
+                LinterBuilderError::UnknownRules { rules } => {
+                    let rules = rules.iter().map(|r| r.full_name()).collect::<Vec<_>>().join("\n");
+                    let error = Error::from(
+                        OxcDiagnostic::warn(format!(
+                        "The following rules do not match the currently supported rules:\n{rules}"
+                    ))
+                        .with_help("Check that the plugin that contains this rule is enabled."),
+                    );
+                    return CliRunResult::LintError { error: format!("{error:?}") };
+                }
+            },
+        };
+        let builder = builder.with_filters(filter).with_fix(fix_options.fix_kind());
 
         if let Some(basic_config_file) = oxlintrc_for_print {
             return CliRunResult::PrintConfigResult {
@@ -244,6 +259,9 @@ mod test {
         let options = lint_command().run_inner(new_args.as_slice()).unwrap();
         match LintRunner::new(options).run() {
             CliRunResult::LintResult(lint_result) => lint_result,
+            CliRunResult::LintError { error } => {
+                panic!("{error}")
+            }
             other => panic!("{other:?}"),
         }
     }
@@ -483,18 +501,20 @@ mod test {
         assert_eq!(result.number_of_errors, 0);
     }
 
+    // Previously, this test would pass and the unmatched rule would be ignored, but now we report that
+    // there was unmatched rule, because the typescript plugin has been disabled and we are trying to configure it.
     #[test]
+    #[should_panic(
+        expected = "The following rules do not match the currently supported rules:\n  | typescript/no-namespace"
+    )]
     fn typescript_eslint_off() {
         let args = &[
             "-c",
             "fixtures/typescript_eslint/eslintrc.json",
             "--disable-typescript-plugin",
             "fixtures/typescript_eslint/test.ts",
         ];
-        let result = test(args);
-        assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_warnings, 2);
-        assert_eq!(result.number_of_errors, 0);
+        test(args);
     }
 
     #[test]
@@ -543,17 +563,24 @@ mod test {
             .contains("oxc/tsconfig.json\" does not exist, Please provide a valid tsconfig file."));
     }
 
+    // Previously, we used to not report errors when enabling a rule that did not have the corresponding plugin enabled,
+    // but now this is reported as an unmatched rule.
     #[test]
-    fn test_enable_vitest_plugin() {
+    #[should_panic(
+        // FIXME: We should probably report the original rule name error, not the mapped jest rule name?
+        expected = "The following rules do not match the currently supported rules:\n  | jest/no-disabled-tests\n"
+    )]
+    fn test_enable_vitest_rule_without_plugin() {
         let args = &[
             "-c",
             "fixtures/eslintrc_vitest_replace/eslintrc.json",
             "fixtures/eslintrc_vitest_replace/foo.test.js",
         ];
-        let result = test(args);
-        assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_errors, 0);
+        test(args);
+    }
 
+    #[test]
+    fn test_enable_vitest_plugin() {
         let args = &[
             "--vitest-plugin",
             "-c",

apps/oxlint/src/result.rs

@@ -7,23 +7,46 @@ use std::{
 #[derive(Debug)]
 pub enum CliRunResult {
     None,
-    InvalidOptions { message: String },
-    PathNotFound { paths: Vec<PathBuf> },
+    InvalidOptions {
+        message: String,
+    },
+    PathNotFound {
+        paths: Vec<PathBuf>,
+    },
+    /// Indicates that there was an error trying to run the linter and it was
+    /// not able to complete linting successfully.
+    LintError {
+        error: String,
+    },
     LintResult(LintResult),
     FormatResult(FormatResult),
-    TypeCheckResult { duration: Duration, number_of_diagnostics: usize },
-    PrintConfigResult { config_file: String },
+    TypeCheckResult {
+        duration: Duration,
+        number_of_diagnostics: usize,
+    },
+    PrintConfigResult {
+        config_file: String,
+    },
 }
 
+/// A summary of a complete linter run.
 #[derive(Debug, Default)]
 pub struct LintResult {
+    /// The total time it took to run the linter.
     pub duration: Duration,
+    /// The number of lint rules that were run.
     pub number_of_rules: usize,
+    /// The number of files that were linted.
     pub number_of_files: usize,
+    /// The number of warnings that were found.
     pub number_of_warnings: usize,
+    /// The number of errors that were found.
     pub number_of_errors: usize,
+    /// Whether or not the maximum number of warnings was exceeded.
     pub max_warnings_exceeded: bool,
+    /// Whether or not warnings should be treated as errors (from `--deny-warnings` for example)
     pub deny_warnings: bool,
+    /// Whether or not to print a summary of the results
     pub print_summary: bool,
 }
 
@@ -34,7 +57,7 @@ pub struct FormatResult {
 }
 
 impl Termination for CliRunResult {
-    #[allow(clippy::print_stdout)]
+    #[allow(clippy::print_stdout, clippy::print_stderr)]
     fn report(self) -> ExitCode {
         match self {
             Self::None => ExitCode::from(0),
@@ -46,6 +69,10 @@ impl Termination for CliRunResult {
                 println!("Path {paths:?} does not exist.");
                 ExitCode::from(1)
             }
+            Self::LintError { error } => {
+                eprintln!("Error: {error}");
+                ExitCode::from(1)
+            }
             Self::LintResult(LintResult {
                 duration,
                 number_of_rules,

crates/oxc_language_server/src/main.rs

@@ -359,6 +359,8 @@ impl Backend {
                     Oxlintrc::from_file(&config_path)
                         .expect("should have initialized linter with new options"),
                 )
+                // FIXME: Handle this error more gracefully and report it properly
+                .expect("failed to build linter from oxlint config")
                 .with_fix(FixKind::SafeFix)
                 .build(),
             );

crates/oxc_linter/src/builder.rs

@@ -74,9 +74,18 @@ impl LinterBuilder {
     /// // you can use `From` as a shorthand for `from_oxlintrc(false, oxlintrc)`
     /// let linter = LinterBuilder::from(oxlintrc).build();
     /// ```
-    pub fn from_oxlintrc(start_empty: bool, oxlintrc: Oxlintrc) -> Self {
+    ///
+    /// # Errors
+    ///
+    /// Will return a [`LinterBuilderError::UnknownRules`] if there are unknown rules in the
+    /// config. This can happen if the plugin for a rule is not enabled, or the rule name doesn't
+    /// match any recognized rules.
+    pub fn from_oxlintrc(
+        start_empty: bool,
+        oxlintrc: Oxlintrc,
+    ) -> Result<Self, LinterBuilderError> {
         // TODO: monorepo config merging, plugin-based extends, etc.
-        let Oxlintrc { plugins, settings, env, globals, categories, rules: oxlintrc_rules } =
+        let Oxlintrc { plugins, settings, env, globals, categories, rules: mut oxlintrc_rules } =
             oxlintrc;
 
         let config = LintConfig { plugins, settings, env, globals };
@@ -95,7 +104,13 @@ impl LinterBuilder {
             oxlintrc_rules.override_rules(&mut builder.rules, all_rules.as_slice());
         }
 
-        builder
+        if !oxlintrc_rules.unknown_rules.is_empty() {
+            return Err(LinterBuilderError::UnknownRules {
+                rules: std::mem::take(&mut oxlintrc_rules.unknown_rules),
+            });
+        }
+
+        Ok(builder)
     }
 
     #[inline]
@@ -258,6 +273,7 @@ impl LinterBuilder {
         let previous_rules = std::mem::take(&mut oxlintrc.rules);
 
         let rule_name_to_rule = previous_rules
+            .rules
             .into_iter()
             .map(|r| (get_name(&r.plugin_name, &r.rule_name), r))
             .collect::<rustc_hash::FxHashMap<_, _>>();
@@ -290,9 +306,11 @@ fn get_name(plugin_name: &str, rule_name: &str) -> CompactStr {
     }
 }
 
-impl From<Oxlintrc> for LinterBuilder {
+impl TryFrom<Oxlintrc> for LinterBuilder {
+    type Error = LinterBuilderError;
+
     #[inline]
-    fn from(oxlintrc: Oxlintrc) -> Self {
+    fn try_from(oxlintrc: Oxlintrc) -> Result<Self, Self::Error> {
         Self::from_oxlintrc(false, oxlintrc)
     }
 }
@@ -307,6 +325,29 @@ impl fmt::Debug for LinterBuilder {
     }
 }
 
+/// An error that can occur while building a [`Linter`] from an [`Oxlintrc`].
+#[derive(Debug, Clone)]
+pub enum LinterBuilderError {
+    /// There were unknown rules that could not be matched to any known plugins/rules.
+    UnknownRules { rules: Vec<ESLintRule> },
+}
+
+impl std::fmt::Display for LinterBuilderError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            LinterBuilderError::UnknownRules { rules } => {
+                write!(f, "unknown rules: ")?;
+                for rule in rules {
+                    write!(f, "{}", rule.full_name())?;
+                }
+                Ok(())
+            }
+        }
+    }
+}
+
+impl std::error::Error for LinterBuilderError {}
+
 struct RulesCache {
     all_rules: RefCell<Option<Vec<RuleEnum>>>,
     plugins: LintPlugins,
@@ -602,7 +643,7 @@ mod test {
         "#,
         )
         .unwrap();
-        let builder = LinterBuilder::from_oxlintrc(false, oxlintrc);
+        let builder = LinterBuilder::from_oxlintrc(false, oxlintrc).unwrap();
         for rule in &builder.rules {
             let name = rule.name();
             let plugin = rule.plugin_name();

crates/oxc_linter/src/config/mod.rs

@@ -101,7 +101,7 @@ mod test {
     fn test_vitest_rule_replace() {
         let fixture_path: std::path::PathBuf =
             env::current_dir().unwrap().join("fixtures/eslint_config_vitest_replace.json");
-        let config = Oxlintrc::from_file(&fixture_path).unwrap();
+        let mut config = Oxlintrc::from_file(&fixture_path).unwrap();
         let mut set = FxHashSet::default();
         config.rules.override_rules(&mut set, &RULES);