Skip to content

Commit

Permalink
feat(linter): do not bail for unmatched rules yet (#7093)
Browse files Browse the repository at this point in the history
I intend to make a release today, let's hold onto this behaviour.
  • Loading branch information
Boshen authored Nov 3, 2024
1 parent 7872927 commit 2184588
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 41 deletions.
33 changes: 5 additions & 28 deletions apps/oxlint/src/lint.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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::<Vec<_>>().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 {
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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",
Expand Down
2 changes: 0 additions & 2 deletions crates/oxc_language_server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);
Expand Down
26 changes: 16 additions & 10 deletions crates/oxc_linter/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::{
fmt,
};

use oxc_diagnostics::{Error, OxcDiagnostic};
use oxc_span::CompactStr;
use rustc_hash::FxHashSet;

Expand Down Expand Up @@ -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<Self, LinterBuilderError> {
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;
Expand All @@ -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::<Vec<_>>()
.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]
Expand Down Expand Up @@ -311,7 +317,7 @@ impl TryFrom<Oxlintrc> for LinterBuilder {

#[inline]
fn try_from(oxlintrc: Oxlintrc) -> Result<Self, Self::Error> {
Self::from_oxlintrc(false, oxlintrc)
Ok(Self::from_oxlintrc(false, oxlintrc))
}
}

Expand Down Expand Up @@ -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();
Expand Down
2 changes: 1 addition & 1 deletion crates/oxc_linter/src/tester.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 2184588

Please sign in to comment.