refactor(linter): add LintFilter (#5685)

Re-creation of #5329
This commit is contained in:
DonIsaac 2024-09-11 03:19:04 +00:00
parent 731ffaa00e
commit 9e9435f03b
8 changed files with 452 additions and 81 deletions

View file

@ -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<Vec<LintFilter>, 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 <plugin>/<rule>"
),
});
}
Err(InvalidFilterKind::RuleMissing(filter)) => {
return Err(CliRunResult::InvalidOptions {
message: format!(
"Failed to {severity} filter {filter}: Rule name is missing. Expected <plugin>/<rule>"
),
});
}
}
}
Ok(filters)
}
}
#[cfg(all(test, not(target_os = "windows")))]

View file

@ -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},
};

View file

@ -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;

View file

@ -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 `<plugin>/<rule>` filter but is missing either the
/// plugin or the rule.
pub fn new<F: TryInto<LintFilterKind>>(
severity: AllowWarnDeny,
kind: F,
) -> Result<Self, <F as TryInto<LintFilterKind>>::Error> {
Ok(Self { severity, kind: kind.try_into()? })
}
#[must_use]
pub fn allow<F: Into<LintFilterKind>>(kind: F) -> Self {
Self { severity: AllowWarnDeny::Allow, kind: kind.into() }
}
#[must_use]
pub fn warn<F: Into<LintFilterKind>>(kind: F) -> Self {
Self { severity: AllowWarnDeny::Warn, kind: kind.into() }
}
#[must_use]
pub fn deny<F: Into<LintFilterKind>>(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<LintFilter> 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 `<plugin>/<rule>` filter but is missing either the
/// plugin or the rule.
pub fn parse(filter: Cow<'static, str>) -> Result<Self, InvalidFilterKind> {
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<String> for LintFilterKind {
type Error = InvalidFilterKind;
#[inline]
fn try_from(filter: String) -> Result<Self, Self::Error> {
Self::parse(Cow::Owned(filter))
}
}
impl TryFrom<&'static str> for LintFilterKind {
type Error = InvalidFilterKind;
#[inline]
fn try_from(filter: &'static str) -> Result<Self, Self::Error> {
Self::parse(Cow::Borrowed(filter))
}
}
impl TryFrom<Cow<'static, str>> for LintFilterKind {
type Error = InvalidFilterKind;
#[inline]
fn try_from(filter: Cow<'static, str>) -> Result<Self, Self::Error> {
Self::parse(filter)
}
}
impl From<RuleCategory> 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 <plugin>/<rule> but is missing a plugin name."
)
}
Self::RuleMissing(filter) => {
write!(
f,
"Filter '{filter}' must match <plugin>/<rule> 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()
);
}
}
}

View file

@ -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<OxlintOptions> 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<LintFilter>,
pub config_path: Option<PathBuf>,
/// 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<LintFilter>) -> Self {
if !filter.is_empty() {
self.filter = filter;
}
@ -191,48 +195,58 @@ impl OxlintOptions {
let mut rules: FxHashSet<RuleWithSeverity> = 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)),
),
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.category() == *category)
.map(|rule| RuleWithSeverity::new(rule.clone(), severity)),
);
}
LintFilterKind::Rule(_, name) => {
rules.extend(
all_rules
.iter()
.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);
}
};
}
}
},
}
}

View file

@ -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<S: AsRef<str>> FromIterator<(S, bool)> for LintPluginOptions {
fn from_iter<I: IntoIterator<Item = (S, bool)>>(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
}
}

View file

@ -81,19 +81,6 @@ pub enum RuleCategory {
}
impl RuleCategory {
pub fn from(input: &str) -> Option<Self> {
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<Self, Self::Error> {
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 {

View file

@ -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)