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

- closes https://github.com/oxc-project/oxc/issues/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 https://github.com/oxc-project/oxc/issues/7025

this also fixes https://github.com/oxc-project/oxc/issues/7025, where we were reporting rules as unmatched simply because they had been disabled prior to being configured. similar to https://github.com/oxc-project/oxc/issues/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)
+        let builder = LinterBuilder::from_oxlintrc(false, oxlintrc);
-            .with_filters(filter)
+        // Gracefully report any linter builder errors as CLI errors
-            .with_fix(fix_options.fix_kind());
+        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,7 +501,12 @@ 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",
@@ -491,10 +514,7 @@ mod test {
             "--disable-typescript-plugin",
             "fixtures/typescript_eslint/test.ts",
         ];
-        let result = test(args);
+        test(args);
-        assert_eq!(result.number_of_files, 1);
-        assert_eq!(result.number_of_warnings, 2);
-        assert_eq!(result.number_of_errors, 0);
     }
 
     #[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);
+        test(args);
-        assert_eq!(result.number_of_files, 1);
+    }
-        assert_eq!(result.number_of_errors, 0);
 
+    #[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 },
+    InvalidOptions {
-    PathNotFound { paths: Vec<PathBuf> },
+        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 },
+    TypeCheckResult {
-    PrintConfigResult { config_file: String },
+        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);
 

crates/oxc_linter/src/config/rules.rs

@@ -1,4 +1,4 @@
-use std::{borrow::Cow, fmt, ops::Deref};
+use std::{borrow::Cow, fmt};
 
 use oxc_diagnostics::{Error, OxcDiagnostic};
 use rustc_hash::{FxHashMap, FxHashSet};
@@ -24,50 +24,53 @@ type RuleSet = FxHashSet<RuleWithSeverity>;
 // Note: when update document comment, also update `DummyRuleMap`'s description in this file.
 #[derive(Debug, Clone, Default)]
 #[cfg_attr(test, derive(PartialEq))]
-pub struct OxlintRules(Vec<ESLintRule>);
+pub struct OxlintRules {
+    /// List of all configured rules
+    pub(crate) rules: Vec<ESLintRule>,
+    /// List of rules that didn't match any known rules
+    pub unknown_rules: Vec<ESLintRule>,
+}
 
 impl OxlintRules {
     pub fn new(rules: Vec<ESLintRule>) -> Self {
-        Self(rules)
+        Self { rules, unknown_rules: Vec::new() }
+    }
+
+    /// Returns `true` if there are no rules.
+    pub fn is_empty(&self) -> bool {
+        self.rules.is_empty()
     }
 }
 
+/// A fully qualified rule name, e.g. `eslint/no-console` or `react/rule-of-hooks`.
+/// Includes the plugin name, the rule name, and the configuration for the rule (if any).
+/// This does not imply the rule is known to the linter as that, only that it is configured.
 #[derive(Debug, Clone)]
 #[cfg_attr(test, derive(PartialEq))]
 pub struct ESLintRule {
+    /// Name of the plugin: `eslint`, `react`, etc.
     pub plugin_name: String,
+    /// Name of the rule: `no-console`, `prefer-const`, etc.
     pub rule_name: String,
+    /// Severity of the rule: `off`, `warn`, `error`, etc.
     pub severity: AllowWarnDeny,
+    /// JSON configuration for the rule, if any.
     pub config: Option<serde_json::Value>,
 }
 
-impl Deref for OxlintRules {
-    type Target = Vec<ESLintRule>;
-
-    fn deref(&self) -> &Self::Target {
-        &self.0
-    }
-}
-
-impl IntoIterator for OxlintRules {
-    type Item = ESLintRule;
-    type IntoIter = <Vec<ESLintRule> as IntoIterator>::IntoIter;
-
-    fn into_iter(self) -> Self::IntoIter {
-        self.0.into_iter()
-    }
-}
-
 impl OxlintRules {
     #[allow(clippy::option_if_let_else, clippy::print_stderr)]
-    pub(crate) fn override_rules(&self, rules_for_override: &mut RuleSet, all_rules: &[RuleEnum]) {
+    pub(crate) fn override_rules(
+        &mut self,
+        rules_for_override: &mut RuleSet,
+        all_rules: &[RuleEnum],
+    ) {
         use itertools::Itertools;
         let mut rules_to_replace: Vec<RuleWithSeverity> = vec![];
         let mut rules_to_remove: Vec<RuleWithSeverity> = vec![];
-        let mut rules_not_matched: Vec<&str> = vec![];
 
         // Rules can have the same name but different plugin names
-        let lookup = self.iter().into_group_map_by(|r| r.rule_name.as_str());
+        let lookup = self.rules.iter().into_group_map_by(|r| r.rule_name.as_str());
 
         for (name, rule_configs) in &lookup {
             match rule_configs.len() {
@@ -89,7 +92,12 @@ impl OxlintRules {
                                 let rule = rule.read_json(config);
                                 rules_to_replace.push(RuleWithSeverity::new(rule, severity));
                             } else {
-                                rules_not_matched.push(rule_name);
+                                self.unknown_rules.push(ESLintRule {
+                                    plugin_name: plugin_name.to_string(),
+                                    rule_name: rule_name.to_string(),
+                                    severity,
+                                    config: rule_config.config.clone(),
+                                });
                             }
                         }
                         AllowWarnDeny::Allow => {
@@ -99,8 +107,23 @@ impl OxlintRules {
                             {
                                 let rule = rule.clone();
                                 rules_to_remove.push(rule);
+                            }
+                            // If the given rule is not found in the rule list (for example, if all rules are disabled),
+                            // then look it up in the entire rules list and add it.
+                            else if let Some(rule) = all_rules
+                                .iter()
+                                .find(|r| r.name() == rule_name && r.plugin_name() == plugin_name)
+                            {
+                                let config = rule_config.config.clone().unwrap_or_default();
+                                let rule = rule.read_json(config);
+                                rules_to_remove.push(RuleWithSeverity::new(rule, severity));
                             } else {
-                                rules_not_matched.push(rule_name);
+                                self.unknown_rules.push(ESLintRule {
+                                    plugin_name: plugin_name.to_string(),
+                                    rule_name: rule_name.to_string(),
+                                    severity,
+                                    config: rule_config.config.clone(),
+                                });
                             }
                         }
                     }
@@ -141,14 +164,6 @@ impl OxlintRules {
         for rule in rules_to_replace {
             rules_for_override.replace(rule);
         }
-
-        if !rules_not_matched.is_empty() {
-            let rules = rules_not_matched.join("\n");
-            let error = Error::from(OxcDiagnostic::warn(format!(
-                "The following rules do not match the currently supported rules:\n{rules}"
-            )));
-            eprintln!("{error:?}");
-        }
     }
 }
 
@@ -197,9 +212,9 @@ impl Serialize for OxlintRules {
     where
         S: serde::Serializer,
     {
-        let mut rules = s.serialize_map(Some(self.len()))?;
+        let mut rules = s.serialize_map(Some(self.rules.len()))?;
 
-        for rule in &self.0 {
+        for rule in &self.rules {
             let key = rule.full_name();
             match rule.config.as_ref() {
                 // e.g. unicorn/some-rule: ["warn", { foo: "bar" }]
@@ -247,7 +262,7 @@ impl<'de> Deserialize<'de> for OxlintRules {
                     rules.push(ESLintRule { plugin_name, rule_name, severity, config });
                 }
 
-                Ok(OxlintRules(rules))
+                Ok(OxlintRules { rules, unknown_rules: Vec::new() })
             }
         }
 
@@ -328,8 +343,9 @@ fn failed_to_parse_rule_value(value: &str, err: &str) -> OxcDiagnostic {
 
 impl ESLintRule {
     /// Returns `<plugin_name>/<rule_name>` for non-eslint rules. For eslint rules, returns
-    /// `<rule_name>`. This is effectively the inverse operation for [`parse_rule_key`].
+    /// `<rule_name>`.
-    fn full_name(&self) -> Cow<'_, str> {
+    // This is effectively the inverse operation for `parse_rule_key`.
+    pub fn full_name(&self) -> Cow<'_, str> {
         if self.plugin_name == "eslint" {
             Cow::Borrowed(self.rule_name.as_str())
         } else {
@@ -359,7 +375,7 @@ mod test {
             "@next/next/noop": 2,
         }))
         .unwrap();
-        let mut rules = rules.iter();
+        let mut rules = rules.rules.iter();
 
         let r1 = rules.next().unwrap();
         assert_eq!(r1.rule_name, "no-console");
@@ -386,6 +402,34 @@ mod test {
         assert!(r4.config.is_none());
     }
 
+    #[test]
+    fn test_parse_unknown_rules() {
+        let config = json!({
+            "no-console": "off",
+            "foo/no-unused-vars": [1],
+            "dummy": ["error", "arg1", "args2"],
+        });
+        let mut rules = OxlintRules::deserialize(&config).unwrap();
+        let mut rule_set = RuleSet::default();
+
+        rules.override_rules(&mut rule_set, &RULES);
+
+        rules.unknown_rules.sort_by(|a, b| a.rule_name.cmp(&b.rule_name));
+        let mut rules = rules.unknown_rules.iter();
+
+        let r = rules.next().unwrap();
+        assert_eq!(r.rule_name, "dummy");
+        assert_eq!(r.plugin_name, "unknown_plugin");
+        assert!(r.severity.is_warn_deny());
+        assert_eq!(r.config, Some(serde_json::json!(["arg1", "args2"])));
+
+        let r = rules.next().unwrap();
+        assert_eq!(r.rule_name, "no-unused-vars");
+        assert_eq!(r.plugin_name, "foo");
+        assert!(r.severity.is_warn_deny());
+        assert!(r.config.is_none());
+    }
+
     #[test]
     fn test_parse_rules_default() {
         let rules = OxlintRules::default();
@@ -393,7 +437,7 @@ mod test {
     }
 
     fn r#override(rules: &mut RuleSet, rules_rc: &Value) {
-        let rules_config = OxlintRules::deserialize(rules_rc).unwrap();
+        let mut rules_config = OxlintRules::deserialize(rules_rc).unwrap();
         rules_config.override_rules(rules, &RULES);
     }
 

crates/oxc_linter/src/lib.rs

@@ -30,8 +30,8 @@ use oxc_semantic::{AstNode, Semantic};
 use utils::iter_possible_jest_call_node;
 
 pub use crate::{
-    builder::LinterBuilder,
+    builder::{LinterBuilder, LinterBuilderError},
-    config::{LintPlugins, Oxlintrc},
+    config::{ESLintRule, LintPlugins, Oxlintrc},
     context::LintContext,
     fixer::FixKind,
     frameworks::FrameworkFlags,

crates/oxc_linter/src/tester.rs

@@ -432,7 +432,7 @@ impl Tester {
         let linter = eslint_config
             .as_ref()
             .map_or_else(LinterBuilder::empty, |v| {
-                LinterBuilder::from_oxlintrc(true, Oxlintrc::deserialize(v).unwrap())
+                LinterBuilder::from_oxlintrc(true, Oxlintrc::deserialize(v).unwrap()).unwrap()
             })
             .with_fix(fix.into())
             .with_plugins(self.plugins)