From 218458811b4a2d7ad3ba30debac94f83c18feeb5 Mon Sep 17 00:00:00 2001 From: Boshen Date: Sun, 3 Nov 2024 11:18:36 +0800 Subject: [PATCH] feat(linter): do not bail for unmatched rules yet (#7093) I intend to make a release today, let's hold onto this behaviour. --- apps/oxlint/src/lint.rs | 33 ++++---------------------- crates/oxc_language_server/src/main.rs | 2 -- crates/oxc_linter/src/builder.rs | 26 ++++++++++++-------- crates/oxc_linter/src/tester.rs | 2 +- 4 files changed, 22 insertions(+), 41 deletions(-) diff --git a/apps/oxlint/src/lint.rs b/apps/oxlint/src/lint.rs index 34ebb0f7f058c..32b52130c5a1f 100644 --- a/apps/oxlint/src/lint.rs +++ b/apps/oxlint/src/lint.rs @@ -1,10 +1,10 @@ use std::{env, io::BufWriter, time::Instant}; use ignore::gitignore::Gitignore; -use oxc_diagnostics::{DiagnosticService, Error, GraphicalReportHandler, OxcDiagnostic}; +use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler}; use oxc_linter::{ loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter, LintService, - LintServiceOptions, Linter, LinterBuilder, LinterBuilderError, Oxlintrc, + LintServiceOptions, Linter, LinterBuilder, Oxlintrc, }; use oxc_span::VALID_EXTENSIONS; @@ -119,21 +119,9 @@ 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); - // 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::>().join("\n"); - let error = Error::from(OxcDiagnostic::warn(format!( - "The following rules do not match the currently supported rules:\n{rules}" - ))); - return CliRunResult::LintError { error: format!("{error:?}") }; - } - }, - }; - let builder = builder.with_filters(filter).with_fix(fix_options.fix_kind()); + let builder = LinterBuilder::from_oxlintrc(false, oxlintrc) + .with_filters(filter) + .with_fix(fix_options.fix_kind()); if let Some(basic_config_file) = oxlintrc_for_print { return CliRunResult::PrintConfigResult { @@ -498,12 +486,7 @@ 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", @@ -560,13 +543,7 @@ 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] - #[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", diff --git a/crates/oxc_language_server/src/main.rs b/crates/oxc_language_server/src/main.rs index c5d2ef27800b8..f71f2e45cecc6 100644 --- a/crates/oxc_language_server/src/main.rs +++ b/crates/oxc_language_server/src/main.rs @@ -359,8 +359,6 @@ 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(), ); diff --git a/crates/oxc_linter/src/builder.rs b/crates/oxc_linter/src/builder.rs index 53ffdd1bb3f33..2148411cf5891 100644 --- a/crates/oxc_linter/src/builder.rs +++ b/crates/oxc_linter/src/builder.rs @@ -3,6 +3,7 @@ use std::{ fmt, }; +use oxc_diagnostics::{Error, OxcDiagnostic}; use oxc_span::CompactStr; use rustc_hash::FxHashSet; @@ -80,10 +81,7 @@ impl LinterBuilder { /// 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 { + pub fn from_oxlintrc(start_empty: bool, oxlintrc: Oxlintrc) -> Self { // TODO: monorepo config merging, plugin-based extends, etc. let Oxlintrc { plugins, settings, env, globals, categories, rules: mut oxlintrc_rules } = oxlintrc; @@ -104,13 +102,21 @@ impl LinterBuilder { oxlintrc_rules.override_rules(&mut builder.rules, all_rules.as_slice()); } + #[expect(clippy::print_stderr)] if !oxlintrc_rules.unknown_rules.is_empty() { - return Err(LinterBuilderError::UnknownRules { - rules: std::mem::take(&mut oxlintrc_rules.unknown_rules), - }); + let rules = oxlintrc_rules + .unknown_rules + .iter() + .map(|r| r.full_name()) + .collect::>() + .join("\n"); + let error = Error::from(OxcDiagnostic::warn(format!( + "The following rules do not match the currently supported rules:\n{rules}" + ))); + eprintln!("{error:?}"); } - Ok(builder) + builder } #[inline] @@ -311,7 +317,7 @@ impl TryFrom for LinterBuilder { #[inline] fn try_from(oxlintrc: Oxlintrc) -> Result { - Self::from_oxlintrc(false, oxlintrc) + Ok(Self::from_oxlintrc(false, oxlintrc)) } } @@ -643,7 +649,7 @@ mod test { "#, ) .unwrap(); - let builder = LinterBuilder::from_oxlintrc(false, oxlintrc).unwrap(); + let builder = LinterBuilder::from_oxlintrc(false, oxlintrc); for rule in &builder.rules { let name = rule.name(); let plugin = rule.plugin_name(); diff --git a/crates/oxc_linter/src/tester.rs b/crates/oxc_linter/src/tester.rs index aad7fdf7ae4ea..8403dd394b44d 100644 --- a/crates/oxc_linter/src/tester.rs +++ b/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()).unwrap() + LinterBuilder::from_oxlintrc(true, Oxlintrc::deserialize(v).unwrap()) }) .with_fix(fix.into()) .with_plugins(self.plugins)