From 654395847fdbe2ee2f3174e2c17c8a1946900c74 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Tue, 30 Jul 2024 22:51:51 -0400 Subject: [PATCH] feat(linter): add auto-fix metadata to RuleMeta (#4557) Add a `FIX` constant to `RuleMeta` that describes what kinds of auto fixes a rule can perform (if any). This PR also updates `declare_oxc_lint` to accept a fix capabilities field. Follow-up PRs in this stack will update existing rules and update other parts of the codebase to use this new field. The end goal of this stack is to 1. automate creation of #4179 in a similar way we auto-update rule progress, 2. use it in rule documentation pages when we eventually add lint rules to the website --- crates/oxc_linter/src/fixer/fix.rs | 2 +- crates/oxc_linter/src/rule.rs | 73 ++++++++++++++++++++++- crates/oxc_macros/src/declare_oxc_lint.rs | 42 ++++++++++++- 3 files changed, 112 insertions(+), 5 deletions(-) diff --git a/crates/oxc_linter/src/fixer/fix.rs b/crates/oxc_linter/src/fixer/fix.rs index 7b5e7c775c2b4..e2e65c0c87729 100644 --- a/crates/oxc_linter/src/fixer/fix.rs +++ b/crates/oxc_linter/src/fixer/fix.rs @@ -12,7 +12,7 @@ bitflags! { /// /// [`FixKind`] is designed to be interopable with [`bool`]. `true` turns /// into [`FixKind::Fix`] (applies only safe fixes) and `false` turns into - /// `FixKind::None` (do not apply any fixes or suggestions). + /// [`FixKind::None`] (do not apply any fixes or suggestions). #[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] pub struct FixKind: u8 { /// An automatic code fix. Most of these are applied with `--fix` diff --git a/crates/oxc_linter/src/rule.rs b/crates/oxc_linter/src/rule.rs index cf7ff27f8ab2e..2ca25ecd8f3cb 100644 --- a/crates/oxc_linter/src/rule.rs +++ b/crates/oxc_linter/src/rule.rs @@ -1,4 +1,5 @@ use std::{ + borrow::Cow, fmt, hash::{Hash, Hasher}, ops::Deref, @@ -6,7 +7,7 @@ use std::{ use oxc_semantic::SymbolId; -use crate::{context::LintContext, AllowWarnDeny, AstNode, RuleEnum}; +use crate::{context::LintContext, AllowWarnDeny, AstNode, FixKind, RuleEnum}; pub trait Rule: Sized + Default + fmt::Debug { /// Initialize from eslint json configuration @@ -40,6 +41,9 @@ pub trait RuleMeta { const CATEGORY: RuleCategory; + /// What kind of auto-fixing can this rule do? + const FIX: RuleFixMeta = RuleFixMeta::None; + fn documentation() -> Option<&'static str> { None } @@ -111,6 +115,73 @@ impl fmt::Display for RuleCategory { } } +// NOTE: this could be packed into a single byte if we wanted. I don't think +// this is needed, but we could do it if it would have a performance impact. +/// Describes the auto-fixing capabilities of a [`Rule`]. +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)] +pub enum RuleFixMeta { + /// An auto-fix is not available. + #[default] + None, + /// An auto-fix could be implemented, but it has not been yet. + FixPending, + /// An auto-fix is available. + Fixable(FixKind), +} + +impl RuleFixMeta { + /// Does this [`Rule`] have some kind of auto-fix available? + /// + /// Also returns `true` for suggestions. + #[inline] + pub fn has_fix(self) -> bool { + matches!(self, Self::Fixable(_)) + } + + pub fn description(self) -> Cow<'static, str> { + match self { + Self::None => Cow::Borrowed("No auto-fix is available for this rule."), + Self::FixPending => Cow::Borrowed("An auto-fix is still under development."), + Self::Fixable(kind) => { + // e.g. an auto-fix is available for this rule + // e.g. a suggestion is available for this rule + // e.g. a dangerous auto-fix is available for this rule + // e.g. an auto-fix and a suggestion are available for this rule + let noun = match (kind.contains(FixKind::Fix), kind.contains(FixKind::Suggestion)) { + (true, true) => "auto-fix and a suggestion are available for this rule", + (true, false) => "auto-fix is available for this rule", + (false, true) => "suggestion is available for this rule", + _ => unreachable!(), + }; + let message = + if kind.is_dangerous() { format!("dangerous {noun}") } else { noun.into() }; + + let article = match message.chars().next().unwrap() { + 'a' | 'e' | 'i' | 'o' | 'u' => "An", + _ => "A", + }; + + Cow::Owned(format!("{article} {message}")) + } + } + } +} + +impl TryFrom<&str> for RuleFixMeta { + type Error = (); + fn try_from(value: &str) -> Result { + match value { + "none" => Ok(Self::None), + "pending" => Ok(Self::FixPending), + "fix" => Ok(Self::Fixable(FixKind::Fix)), + "fix-dangerous" => Ok(Self::Fixable(FixKind::DangerousFix)), + "suggestion" => Ok(Self::Fixable(FixKind::Suggestion)), + "suggestion-dangerous" => Ok(Self::Fixable(FixKind::Suggestion | FixKind::Dangerous)), + _ => Err(()), + } + } +} + #[derive(Debug, Clone)] pub struct RuleWithSeverity { pub rule: RuleEnum, diff --git a/crates/oxc_macros/src/declare_oxc_lint.rs b/crates/oxc_macros/src/declare_oxc_lint.rs index 9508809ca2cf7..b7c73190f6178 100644 --- a/crates/oxc_macros/src/declare_oxc_lint.rs +++ b/crates/oxc_macros/src/declare_oxc_lint.rs @@ -9,6 +9,8 @@ use syn::{ pub struct LintRuleMeta { name: Ident, category: Ident, + /// Describes what auto-fixing capabilities the rule has + fix: Option, documentation: String, pub used_in_test: bool, } @@ -32,10 +34,20 @@ impl Parse for LintRuleMeta { input.parse::()?; let category = input.parse()?; + // Parse FixMeta if it's specified. It will otherwise be excluded from + // the RuleMeta impl, falling back on default set by RuleMeta itself. + // Do not provide a default value here so that it can be set there instead. + let fix: Option = if input.peek(Token!(,)) { + input.parse::()?; + input.parse()? + } else { + None + }; + // Ignore the rest input.parse::()?; - Ok(Self { name: struct_name, category, documentation, used_in_test: false }) + Ok(Self { name: struct_name, category, fix, documentation, used_in_test: false }) } } @@ -44,7 +56,7 @@ fn rule_name_converter() -> Converter { } pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { - let LintRuleMeta { name, category, documentation, used_in_test } = metadata; + let LintRuleMeta { name, category, fix, documentation, used_in_test } = metadata; let canonical_name = rule_name_converter().convert(name.to_string()); let category = match category.to_string().as_str() { @@ -57,11 +69,17 @@ pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { "nursery" => quote! { RuleCategory::Nursery }, _ => panic!("invalid rule category"), }; + let fix = fix.as_ref().map(Ident::to_string).map(|fix| { + let fix = parse_fix(&fix); + quote! { + const FIX: RuleFixMeta = #fix; + } + }); let import_statement = if used_in_test { None } else { - Some(quote! { use crate::rule::{RuleCategory, RuleMeta}; }) + Some(quote! { use crate::rule::{RuleCategory, RuleMeta, RuleFixMeta}; }) }; let output = quote! { @@ -72,6 +90,8 @@ pub fn declare_oxc_lint(metadata: LintRuleMeta) -> TokenStream { const CATEGORY: RuleCategory = #category; + #fix + fn documentation() -> Option<&'static str> { Some(#documentation) } @@ -97,3 +117,19 @@ fn parse_attr<'a, const LEN: usize>( } None } + +fn parse_fix(s: &str) -> proc_macro2::TokenStream { + match s { + "none" => quote! { RuleFixMeta::None }, + "pending" => quote! { RuleFixMeta::FixPending }, + "fix" => quote! { RuleFixMeta::Fixable(FixKind::Fix) }, + "fix-dangerous" => quote! { RuleFixMeta::Fixable(FixKind::Fix.union(FixKind::Dangerous)) }, + "suggestion" => quote! { RuleFixMeta::Fixable(FixKind::Suggestion) }, + "suggestion-dangerous" => quote! { RuleFixMeta::Fixable(FixKind::Suggestion.union(FixKind::Dangerous)) }, + "None" => panic!("Invalid fix kind. Did you mean 'none'?"), + "Pending" => panic!("Invalid fix kind. Did you mean 'pending'?"), + "Fix" => panic!("Invalid fix kind. Did you mean 'fix'?"), + "Suggestion" => panic!("Invalid fix kind. Did you mean 'suggestion'?"), + invalid => panic!("invalid fix kind: {invalid}. Valid kinds are none, pending, fix, fix-dangerous, suggestion, and suggestion-dangerous"), + } +}