diff --git a/apps/oxlint/src/lint/mod.rs b/apps/oxlint/src/lint/mod.rs index a7f93c3877ae4..2ffbd34b29ae0 100644 --- a/apps/oxlint/src/lint/mod.rs +++ b/apps/oxlint/src/lint/mod.rs @@ -3,7 +3,8 @@ use std::{env, io::BufWriter, time::Instant}; use ignore::gitignore::Gitignore; use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler}; use oxc_linter::{ - partial_loader::LINT_PARTIAL_LOADER_EXT, LintService, LintServiceOptions, Linter, OxlintOptions, + partial_loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter, + LintService, LintServiceOptions, Linter, OxlintOptions, }; use oxc_span::VALID_EXTENSIONS; @@ -80,6 +81,11 @@ impl Runner for LintRunner { } } + let filter = match Self::get_filters(filter) { + Ok(filter) => filter, + Err(e) => return e, + }; + let extensions = VALID_EXTENSIONS .iter() .chain(LINT_PARTIAL_LOADER_EXT.iter()) @@ -184,6 +190,43 @@ impl LintRunner { } diagnostic_service } + + // moved into a separate function for readability, but it's only ever used + // in one place. + fn get_filters( + filters_arg: Vec<(AllowWarnDeny, String)>, + ) -> Result, CliRunResult> { + let mut filters = Vec::with_capacity(filters_arg.len()); + + for (severity, filter_arg) in filters_arg { + match LintFilter::new(severity, filter_arg) { + Ok(filter) => { + filters.push(filter); + } + Err(InvalidFilterKind::Empty) => { + return Err(CliRunResult::InvalidOptions { + message: format!("Cannot {severity} an empty filter."), + }); + } + Err(InvalidFilterKind::PluginMissing(filter)) => { + return Err(CliRunResult::InvalidOptions { + message: format!( + "Failed to {severity} filter {filter}: Plugin name is missing. Expected /" + ), + }); + } + Err(InvalidFilterKind::RuleMissing(filter)) => { + return Err(CliRunResult::InvalidOptions { + message: format!( + "Failed to {severity} filter {filter}: Rule name is missing. Expected /" + ), + }); + } + } + } + + Ok(filters) + } } #[cfg(all(test, not(target_os = "windows")))] diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 91214504e2600..e3272067e6682 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -32,7 +32,7 @@ pub use crate::{ context::LintContext, fixer::FixKind, frameworks::FrameworkFlags, - options::{AllowWarnDeny, OxlintOptions}, + options::{AllowWarnDeny, InvalidFilterKind, LintFilter, OxlintOptions}, rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleWithSeverity}, service::{LintService, LintServiceOptions}, }; diff --git a/crates/oxc_linter/src/options/allow_warn_deny.rs b/crates/oxc_linter/src/options/allow_warn_deny.rs index 1237fe05f92da..4de811b7c1c88 100644 --- a/crates/oxc_linter/src/options/allow_warn_deny.rs +++ b/crates/oxc_linter/src/options/allow_warn_deny.rs @@ -1,4 +1,4 @@ -use std::convert::From; +use std::{convert::From, fmt}; use oxc_diagnostics::{OxcDiagnostic, Severity}; use schemars::{schema::SchemaObject, JsonSchema}; @@ -29,6 +29,13 @@ impl AllowWarnDeny { } } +impl fmt::Display for AllowWarnDeny { + #[inline] + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.as_str().fmt(f) + } +} + impl TryFrom<&str> for AllowWarnDeny { type Error = OxcDiagnostic; diff --git a/crates/oxc_linter/src/options/filter.rs b/crates/oxc_linter/src/options/filter.rs new file mode 100644 index 0000000000000..8ba6b80fd5e5e --- /dev/null +++ b/crates/oxc_linter/src/options/filter.rs @@ -0,0 +1,280 @@ +use crate::RuleCategory; + +use super::{plugins::LintPlugins, AllowWarnDeny}; +use std::{borrow::Cow, fmt}; + +/// Enables, disables, and sets the severity of lint rules. +/// +/// Filters come in 3 forms: +/// 1. Filter by rule name and/or plugin: `no-const-assign`, `eslint/no-const-assign` +/// 2. Filter an entire category: `correctness` +/// 3. Some unknow filter. This is a fallback used when parsing a filter string, +/// and is interpreted uniquely by the linter. +#[derive(Debug, Clone)] +#[cfg_attr(test, derive(PartialEq))] +pub struct LintFilter { + severity: AllowWarnDeny, + kind: LintFilterKind, +} + +impl LintFilter { + /// # Errors + /// + /// If `kind` is an empty string, or is a `/` filter but is missing either the + /// plugin or the rule. + pub fn new>( + severity: AllowWarnDeny, + kind: F, + ) -> Result>::Error> { + Ok(Self { severity, kind: kind.try_into()? }) + } + + #[must_use] + pub fn allow>(kind: F) -> Self { + Self { severity: AllowWarnDeny::Allow, kind: kind.into() } + } + + #[must_use] + pub fn warn>(kind: F) -> Self { + Self { severity: AllowWarnDeny::Warn, kind: kind.into() } + } + + #[must_use] + pub fn deny>(kind: F) -> Self { + Self { severity: AllowWarnDeny::Deny, kind: kind.into() } + } + + #[inline] + pub fn severity(&self) -> AllowWarnDeny { + self.severity + } + + #[inline] + pub fn kind(&self) -> &LintFilterKind { + &self.kind + } +} + +impl Default for LintFilter { + fn default() -> Self { + Self { + severity: AllowWarnDeny::Warn, + kind: LintFilterKind::Category(RuleCategory::Correctness), + } + } +} + +impl From for (AllowWarnDeny, LintFilterKind) { + fn from(val: LintFilter) -> Self { + (val.severity, val.kind) + } +} + +impl<'a> From<&'a LintFilter> for (AllowWarnDeny, &'a LintFilterKind) { + fn from(val: &'a LintFilter) -> Self { + (val.severity, &val.kind) + } +} + +#[derive(Debug, Clone)] +#[cfg_attr(test, derive(PartialEq))] +pub enum LintFilterKind { + Generic(Cow<'static, str>), + /// e.g. `no-const-assign` or `eslint/no-const-assign` + Rule(LintPlugins, Cow<'static, str>), + /// e.g. `correctness` + Category(RuleCategory), + // TODO: plugin + category? e.g `-A react:correctness` +} + +impl LintFilterKind { + /// # Errors + /// + /// If `filter` is an empty string, or is a `/` filter but is missing either the + /// plugin or the rule. + pub fn parse(filter: Cow<'static, str>) -> Result { + if filter.is_empty() { + return Err(InvalidFilterKind::Empty); + } + + if filter.contains('/') { + // this is an unfortunate amount of code duplication, but it needs to be done for + // `filter` to live long enough to avoid a String allocation for &'static str + let (plugin, rule) = match filter { + Cow::Borrowed(filter) => { + let mut parts = filter.splitn(2, '/'); + + let plugin = parts + .next() + .ok_or(InvalidFilterKind::PluginMissing(Cow::Borrowed(filter)))?; + if plugin.is_empty() { + return Err(InvalidFilterKind::PluginMissing(Cow::Borrowed(filter))); + } + + let rule = parts + .next() + .ok_or(InvalidFilterKind::RuleMissing(Cow::Borrowed(filter)))?; + if rule.is_empty() { + return Err(InvalidFilterKind::RuleMissing(Cow::Borrowed(filter))); + } + + (LintPlugins::from(plugin), Cow::Borrowed(rule)) + } + Cow::Owned(filter) => { + let mut parts = filter.splitn(2, '/'); + + let plugin = parts + .next() + .ok_or_else(|| InvalidFilterKind::PluginMissing(filter.clone().into()))?; + if plugin.is_empty() { + return Err(InvalidFilterKind::PluginMissing(filter.into())); + } + + let rule = parts + .next() + .ok_or_else(|| InvalidFilterKind::RuleMissing(filter.clone().into()))?; + if rule.is_empty() { + return Err(InvalidFilterKind::RuleMissing(filter.into())); + } + + (LintPlugins::from(plugin), Cow::Owned(rule.to_string())) + } + }; + Ok(LintFilterKind::Rule(plugin, rule)) + } else { + match RuleCategory::try_from(filter.as_ref()) { + Ok(category) => Ok(LintFilterKind::Category(category)), + Err(()) => Ok(LintFilterKind::Generic(filter)), + } + } + } +} + +impl TryFrom for LintFilterKind { + type Error = InvalidFilterKind; + + #[inline] + fn try_from(filter: String) -> Result { + Self::parse(Cow::Owned(filter)) + } +} + +impl TryFrom<&'static str> for LintFilterKind { + type Error = InvalidFilterKind; + + #[inline] + fn try_from(filter: &'static str) -> Result { + Self::parse(Cow::Borrowed(filter)) + } +} + +impl TryFrom> for LintFilterKind { + type Error = InvalidFilterKind; + + #[inline] + fn try_from(filter: Cow<'static, str>) -> Result { + Self::parse(filter) + } +} + +impl From for LintFilterKind { + #[inline] + fn from(category: RuleCategory) -> Self { + LintFilterKind::Category(category) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum InvalidFilterKind { + Empty, + PluginMissing(Cow<'static, str>), + RuleMissing(Cow<'static, str>), +} + +impl fmt::Display for InvalidFilterKind { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + match self { + Self::Empty => "Filter cannot be empty.".fmt(f), + Self::PluginMissing(filter) => { + write!( + f, + "Filter '{filter}' must match / but is missing a plugin name." + ) + } + Self::RuleMissing(filter) => { + write!( + f, + "Filter '{filter}' must match / but is missing a rule name." + ) + } + } + } +} + +impl std::error::Error for InvalidFilterKind {} + +#[cfg(test)] +mod test { + use super::*; + + #[test] + fn test_from_category() { + let correctness: LintFilter = LintFilter::new(AllowWarnDeny::Warn, "correctness").unwrap(); + assert_eq!(correctness.severity(), AllowWarnDeny::Warn); + assert!( + matches!(correctness.kind(), LintFilterKind::Category(RuleCategory::Correctness)), + "{:?}", + correctness.kind() + ); + } + + #[test] + fn test_eslint_deny() { + let filter = LintFilter::deny(LintFilterKind::try_from("no-const-assign").unwrap()); + assert_eq!(filter.severity(), AllowWarnDeny::Deny); + assert_eq!(filter.kind(), &LintFilterKind::Generic("no-const-assign".into())); + + let filter = LintFilter::deny(LintFilterKind::try_from("eslint/no-const-assign").unwrap()); + assert_eq!(filter.severity(), AllowWarnDeny::Deny); + assert_eq!( + filter.kind(), + &LintFilterKind::Rule(LintPlugins::from("eslint"), "no-const-assign".into()) + ); + assert!(matches!(filter.kind(), LintFilterKind::Rule(_, _))); + } + + #[test] + fn test_parse() { + let test_cases: Vec<(&'static str, LintFilterKind)> = vec![ + ("import/namespace", LintFilterKind::Rule(LintPlugins::IMPORT, "namespace".into())), + ( + "react-hooks/exhaustive-deps", + LintFilterKind::Rule(LintPlugins::REACT, "exhaustive-deps".into()), + ), + // categories + ("nursery", LintFilterKind::Category("nursery".try_into().unwrap())), + ("perf", LintFilterKind::Category("perf".try_into().unwrap())), + // misc + ("not-a-valid-filter", LintFilterKind::Generic("not-a-valid-filter".into())), + ]; + + for (input, expected) in test_cases { + let actual = LintFilterKind::try_from(input).unwrap(); + assert_eq!(actual, expected, "input: {input}"); + } + } + + #[test] + fn test_parse_invalid() { + let test_cases = vec!["/rules-of-hooks", "import/", "", "/", "//"]; + + for input in test_cases { + let actual = LintFilterKind::parse(Cow::Borrowed(input)); + assert!( + actual.is_err(), + "input '{input}' produced filter '{:?}' but it should have errored", + actual.unwrap() + ); + } + } +} diff --git a/crates/oxc_linter/src/options/mod.rs b/crates/oxc_linter/src/options/mod.rs index 8f54003983bd5..3461542c6bb46 100644 --- a/crates/oxc_linter/src/options/mod.rs +++ b/crates/oxc_linter/src/options/mod.rs @@ -1,14 +1,18 @@ mod allow_warn_deny; +mod filter; mod plugins; use std::{convert::From, path::PathBuf}; -pub use allow_warn_deny::AllowWarnDeny; +use filter::LintFilterKind; use oxc_diagnostics::Error; -pub use plugins::LintPluginOptions; use plugins::LintPlugins; use rustc_hash::FxHashSet; +pub use allow_warn_deny::AllowWarnDeny; +pub use filter::{InvalidFilterKind, LintFilter}; +pub use plugins::LintPluginOptions; + use crate::{ config::{LintConfig, OxlintConfig}, fixer::FixKind, @@ -44,7 +48,7 @@ impl From for LintOptions { pub struct OxlintOptions { /// Allow / Deny rules in order. [("allow" / "deny", rule name)] /// Defaults to [("deny", "correctness")] - pub filter: Vec<(AllowWarnDeny, String)>, + pub filter: Vec, pub config_path: Option, /// Enable automatic code fixes. Set to [`None`] to disable. /// @@ -59,7 +63,7 @@ pub struct OxlintOptions { impl Default for OxlintOptions { fn default() -> Self { Self { - filter: vec![(AllowWarnDeny::Warn, String::from("correctness"))], + filter: vec![LintFilter::warn(RuleCategory::Correctness)], config_path: None, fix: FixKind::None, plugins: LintPluginOptions::default(), @@ -70,7 +74,7 @@ impl Default for OxlintOptions { impl OxlintOptions { #[must_use] - pub fn with_filter(mut self, filter: Vec<(AllowWarnDeny, String)>) -> Self { + pub fn with_filter(mut self, filter: Vec) -> Self { if !filter.is_empty() { self.filter = filter; } @@ -191,48 +195,58 @@ impl OxlintOptions { let mut rules: FxHashSet = FxHashSet::default(); let all_rules = self.get_filtered_rules(); - for (severity, name_or_category) in &self.filter { - let maybe_category = RuleCategory::from(name_or_category.as_str()); + for (severity, filter) in self.filter.iter().map(Into::into) { match severity { - AllowWarnDeny::Deny | AllowWarnDeny::Warn => { - match maybe_category { - Some(category) => rules.extend( + AllowWarnDeny::Deny | AllowWarnDeny::Warn => match filter { + LintFilterKind::Category(category) => { + rules.extend( + all_rules + .iter() + .filter(|rule| rule.category() == *category) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } + LintFilterKind::Rule(_, name) => { + rules.extend( all_rules .iter() - .filter(|rule| rule.category() == category) - .map(|rule| RuleWithSeverity::new(rule.clone(), *severity)), - ), - None => { - if name_or_category == "all" { - rules.extend( - all_rules - .iter() - .filter(|rule| rule.category() != RuleCategory::Nursery) - .map(|rule| RuleWithSeverity::new(rule.clone(), *severity)), - ); - } else { - rules.extend( - all_rules - .iter() - .filter(|rule| rule.name() == name_or_category) - .map(|rule| RuleWithSeverity::new(rule.clone(), *severity)), - ); - } + .filter(|rule| rule.name() == name) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } + LintFilterKind::Generic(name_or_category) => { + if name_or_category == "all" { + rules.extend( + all_rules + .iter() + .filter(|rule| rule.category() != RuleCategory::Nursery) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } else { + rules.extend( + all_rules + .iter() + .filter(|rule| rule.name() == name_or_category) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); } - }; - } - AllowWarnDeny::Allow => { - match maybe_category { - Some(category) => rules.retain(|rule| rule.category() != category), - None => { - if name_or_category == "all" { - rules.clear(); - } else { - rules.retain(|rule| rule.name() != name_or_category); - } + } + }, + AllowWarnDeny::Allow => match filter { + LintFilterKind::Category(category) => { + rules.retain(|rule| rule.category() != *category); + } + LintFilterKind::Rule(_, name) => { + rules.retain(|rule| rule.name() != name); + } + LintFilterKind::Generic(name_or_category) => { + if name_or_category == "all" { + rules.clear(); + } else { + rules.retain(|rule| rule.name() != name_or_category); } - }; - } + } + }, } } diff --git a/crates/oxc_linter/src/options/plugins.rs b/crates/oxc_linter/src/options/plugins.rs index 785bea0d2f797..52e38be6b47cc 100644 --- a/crates/oxc_linter/src/options/plugins.rs +++ b/crates/oxc_linter/src/options/plugins.rs @@ -3,7 +3,7 @@ use bitflags::bitflags; bitflags! { // NOTE: may be increased to a u32 if needed #[derive(Debug, Clone, Copy, PartialEq, Hash)] - pub(crate) struct LintPlugins: u16 { + pub struct LintPlugins: u16 { /// `eslint-plugin-react`, plus `eslint-plugin-react-hooks` const REACT = 1 << 0; /// `eslint-plugin-unicorn` @@ -79,6 +79,32 @@ impl LintPlugins { } } +impl From<&str> for LintPlugins { + fn from(value: &str) -> Self { + match value { + "react" | "react-hooks" | "react_hooks" => LintPlugins::REACT, + "unicorn" => LintPlugins::UNICORN, + "typescript" | "typescript-eslint" | "typescript_eslint" | "@typescript-eslint" => { + LintPlugins::TYPESCRIPT + } + // deepscan for backwards compatibility. Those rules have been moved into oxc + "oxc" | "deepscan" => LintPlugins::OXC, + "import" => LintPlugins::IMPORT, + "jsdoc" => LintPlugins::JSDOC, + "jest" => LintPlugins::JEST, + "vitest" => LintPlugins::VITEST, + "jsx-a11y" | "jsx_a11y" => LintPlugins::JSX_A11Y, + "nextjs" => LintPlugins::NEXTJS, + "react-perf" | "react_perf" => LintPlugins::REACT_PERF, + "promise" => LintPlugins::PROMISE, + "node" => LintPlugins::NODE, + // "eslint" is not really a plugin, so it's 'empty'. This has the added benefit of + // making it the default value. + _ => LintPlugins::empty(), + } + } +} + #[derive(Debug)] #[non_exhaustive] pub struct LintPluginOptions { @@ -167,27 +193,25 @@ impl> FromIterator<(S, bool)> for LintPluginOptions { fn from_iter>(iter: I) -> Self { let mut options = Self::default(); for (s, enabled) in iter { - match s.as_ref() { - "react" | "react-hooks" => options.react = enabled, - "unicorn" => options.unicorn = enabled, - "typescript" | "typescript-eslint" | "@typescript-eslint" => { - options.typescript = enabled; - } - // deepscan for backwards compatibility. Those rules have been - // moved into oxc - "oxc" | "deepscan" => options.oxc = enabled, - "import" => options.import = enabled, - "jsdoc" => options.jsdoc = enabled, - "jest" => options.jest = enabled, - "vitest" => options.vitest = enabled, - "jsx-a11y" => options.jsx_a11y = enabled, - "nextjs" => options.nextjs = enabled, - "react-perf" => options.react_perf = enabled, - "promise" => options.promise = enabled, - "node" => options.node = enabled, - _ => { /* ignored */ } + let flags = LintPlugins::from(s.as_ref()); + match flags { + LintPlugins::REACT => options.react = enabled, + LintPlugins::UNICORN => options.unicorn = enabled, + LintPlugins::TYPESCRIPT => options.typescript = enabled, + LintPlugins::OXC => options.oxc = enabled, + LintPlugins::IMPORT => options.import = enabled, + LintPlugins::JSDOC => options.jsdoc = enabled, + LintPlugins::JEST => options.jest = enabled, + LintPlugins::VITEST => options.vitest = enabled, + LintPlugins::JSX_A11Y => options.jsx_a11y = enabled, + LintPlugins::NEXTJS => options.nextjs = enabled, + LintPlugins::REACT_PERF => options.react_perf = enabled, + LintPlugins::PROMISE => options.promise = enabled, + LintPlugins::NODE => options.node = enabled, + _ => {} // ignored } } + options } } diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index 31662f8407e7a..2d041e4480803 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -81,19 +81,6 @@ pub enum RuleCategory { } impl RuleCategory { - pub fn from(input: &str) -> Option { - match input { - "correctness" => Some(Self::Correctness), - "suspicious" => Some(Self::Suspicious), - "pedantic" => Some(Self::Pedantic), - "perf" => Some(Self::Perf), - "style" => Some(Self::Style), - "restriction" => Some(Self::Restriction), - "nursery" => Some(Self::Nursery), - _ => None, - } - } - pub fn description(self) -> &'static str { match self { Self::Correctness => "Code that is outright wrong or useless.", @@ -109,6 +96,22 @@ impl RuleCategory { } } +impl TryFrom<&str> for RuleCategory { + type Error = (); + fn try_from(value: &str) -> Result { + match value { + "correctness" => Ok(Self::Correctness), + "suspicious" => Ok(Self::Suspicious), + "pedantic" => Ok(Self::Pedantic), + "perf" => Ok(Self::Perf), + "style" => Ok(Self::Style), + "restriction" => Ok(Self::Restriction), + "nursery" => Ok(Self::Nursery), + _ => Err(()), + } + } +} + impl fmt::Display for RuleCategory { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { diff --git a/tasks/benchmark/benches/linter.rs b/tasks/benchmark/benches/linter.rs index d8c2a4fcd1d59..48e2e9a4a2375 100644 --- a/tasks/benchmark/benches/linter.rs +++ b/tasks/benchmark/benches/linter.rs @@ -2,7 +2,7 @@ use std::{env, path::Path, rc::Rc}; use oxc_allocator::Allocator; use oxc_benchmark::{criterion_group, criterion_main, BenchmarkId, Criterion}; -use oxc_linter::{AllowWarnDeny, FixKind, Linter, OxlintOptions}; +use oxc_linter::{AllowWarnDeny, FixKind, LintFilter, Linter, OxlintOptions}; use oxc_parser::Parser; use oxc_semantic::SemanticBuilder; use oxc_span::SourceType; @@ -34,8 +34,8 @@ fn bench_linter(criterion: &mut Criterion) { .build_module_record(Path::new(""), program) .build(program); let filter = vec![ - (AllowWarnDeny::Deny, "all".into()), - (AllowWarnDeny::Deny, "nursery".into()), + LintFilter::new(AllowWarnDeny::Deny, "all").unwrap(), + LintFilter::new(AllowWarnDeny::Deny, "nursery").unwrap(), ]; let lint_options = OxlintOptions::default() .with_filter(filter)