From f4c49b53cf30850902ff1f7e7ea492e51ac1e291 Mon Sep 17 00:00:00 2001 From: Don Isaac Date: Wed, 11 Sep 2024 17:06:29 -0400 Subject: [PATCH] refactor(linter): Add `LinterBuilder` This will replace `OxlintOptions` in an upstream PR. This also adds `plugins` to `Oxlintrc`. This field gets respected by the builder, but not by `OxlintOptions`. --- crates/oxc_linter/src/builder.rs | 288 ++++++++++++++++++ crates/oxc_linter/src/config/mod.rs | 2 +- crates/oxc_linter/src/config/oxlintrc.rs | 4 +- crates/oxc_linter/src/lib.rs | 12 +- crates/oxc_linter/src/options/mod.rs | 6 +- crates/oxc_linter/src/options/plugins.rs | 2 + .../oxc_linter/src/snapshots/schema_json.snap | 19 ++ npm/oxlint/configuration_schema.json | 19 ++ .../src/linter/snapshots/schema_markdown.snap | 16 + 9 files changed, 361 insertions(+), 7 deletions(-) create mode 100644 crates/oxc_linter/src/builder.rs diff --git a/crates/oxc_linter/src/builder.rs b/crates/oxc_linter/src/builder.rs new file mode 100644 index 0000000000000..afc1c11bbee40 --- /dev/null +++ b/crates/oxc_linter/src/builder.rs @@ -0,0 +1,288 @@ +use std::{ + cell::{Ref, RefCell}, + fmt, +}; + +use rustc_hash::FxHashSet; + +use crate::{ + options::LintPlugins, rules::RULES, AllowWarnDeny, FixKind, FrameworkFlags, LintConfig, + LintFilter, LintFilterKind, LintOptions, Linter, Oxlintrc, RuleCategory, RuleEnum, + RuleWithSeverity, +}; + +#[must_use = "You dropped your builder without building a Linter! Did you mean to call .build()?"] +pub struct LinterBuilder { + rules: FxHashSet, + options: LintOptions, + config: LintConfig, + cache: RulesCache, +} + +impl Default for LinterBuilder { + fn default() -> Self { + Self { rules: Self::warn_correctness(LintPlugins::default()), ..Self::empty() } + } +} + +impl LinterBuilder { + /// Create a [`LinterBuilder`] with default plugins enabled and no + /// configured rules. + /// + /// You can think of this as `oxlint -A all`. + pub fn empty() -> Self { + let options = LintOptions::default(); + let cache = RulesCache::new(options.plugins); + Self { rules: FxHashSet::default(), options, config: LintConfig::default(), cache } + } + + /// Warn on all rules in all plugins and categories, including those in `nursery`. + /// This is the kitchen sink. + /// + /// You can think of this as `oxlint -W all -W nursery`. + pub fn all() -> Self { + let options = LintOptions { plugins: LintPlugins::all(), ..LintOptions::default() }; + let cache = RulesCache::new(options.plugins); + Self { + rules: RULES + .iter() + .map(|rule| RuleWithSeverity { rule: rule.clone(), severity: AllowWarnDeny::Warn }) + .collect(), + options, + config: LintConfig::default(), + cache, + } + } + + /// Create a [`LinterBuilder`] from a loaded or manually built [`Oxlintrc`]. + /// `start_empty` will configure the builder to contain only the + /// configuration settings from the config. When this is `false`, the config + /// will be applied on top of a default [`Oxlintrc`]. + /// + /// # Example + /// Here's how to create a [`Linter`] from a `.oxlintrc.json` file. + /// ``` + /// use oxc_linter::{LinterBuilder, Oxlintrc}; + /// let oxlintrc = Oxlintrc::from_file("path/to/.oxlintrc.json").unwrap(); + /// let linter = LinterBuilder::from_oxlintrc(true, oxlintrc).build(); + /// // you can use `From` as a shorthand for `from_oxlintrc(false, oxlintrc)` + /// let linter = LinterBuilder::from(oxlintrc).build(); + /// ``` + pub fn from_oxlintrc(start_empty: bool, oxlintrc: Oxlintrc) -> Self { + // TODO: monorepo config merging, plugin-based extends, etc. + let Oxlintrc { plugins, settings, env, globals, rules: oxlintrc_rules } = oxlintrc; + + let config = LintConfig { settings, env, globals }; + let options = LintOptions { plugins, ..Default::default() }; + let rules = + if start_empty { FxHashSet::default() } else { Self::warn_correctness(plugins) }; + let cache = RulesCache::new(options.plugins); + let mut builder = Self { rules, options, config, cache }; + + { + let all_rules = builder.cache.borrow(); + oxlintrc_rules.override_rules(&mut builder.rules, all_rules.as_slice()); + } + + builder + } + + #[inline] + pub fn with_framework_hints(mut self, flags: FrameworkFlags) -> Self { + self.options.framework_hints = flags; + self + } + + #[inline] + pub fn and_framework_hints(mut self, flags: FrameworkFlags) -> Self { + self.options.framework_hints |= flags; + self + } + + #[inline] + pub fn with_fix(mut self, fix: FixKind) -> Self { + self.options.fix = fix; + self + } + + #[inline] + pub fn with_plugins(mut self, plugins: LintPlugins) -> Self { + self.options.plugins = plugins; + self.cache.set_plugins(plugins); + self + } + + #[inline] + pub fn and_plugins(mut self, plugins: LintPlugins, enabled: bool) -> Self { + self.options.plugins.set(plugins, enabled); + self.cache.set_plugins(self.options.plugins); + self + } + + pub fn with_filters>(mut self, filters: I) -> Self { + for filter in filters { + self = self.with_filter(filter); + } + self + } + + pub fn with_filter(mut self, filter: LintFilter) -> Self { + let (severity, filter) = filter.into(); + let all_rules = self.cache.borrow(); + + match severity { + AllowWarnDeny::Deny | AllowWarnDeny::Warn => match filter { + LintFilterKind::Category(category) => { + self.rules.extend( + all_rules + .iter() + .filter(|rule| rule.category() == category) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } + LintFilterKind::Rule(_, name) => { + self.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" { + self.rules.extend( + all_rules + .iter() + .filter(|rule| rule.category() != RuleCategory::Nursery) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } else { + self.rules.extend( + all_rules + .iter() + .filter(|rule| rule.name() == name_or_category) + .map(|rule| RuleWithSeverity::new(rule.clone(), severity)), + ); + } + } + }, + AllowWarnDeny::Allow => match filter { + LintFilterKind::Category(category) => { + self.rules.retain(|rule| rule.category() != category); + } + LintFilterKind::Rule(_, name) => { + self.rules.retain(|rule| rule.name() != name); + } + LintFilterKind::Generic(name_or_category) => { + if name_or_category == "all" { + self.rules.clear(); + } else { + self.rules.retain(|rule| rule.name() != name_or_category); + } + } + }, + } + drop(all_rules); + + self + } + + #[must_use] + pub fn build(self) -> Linter { + let mut rules = self.rules.into_iter().collect::>(); + rules.sort_unstable_by_key(|r| r.id()); + Linter::new(rules, self.options, self.config) + } + + /// Warn for all correctness rules in the given set of plugins. + fn warn_correctness(plugins: LintPlugins) -> FxHashSet { + RULES + .iter() + .filter(|rule| { + // NOTE: this logic means there's no way to disable ESLint + // correctness rules. I think that's fine for now. + rule.category() == RuleCategory::Correctness + && plugins.contains(LintPlugins::from(rule.plugin_name())) + }) + .map(|rule| RuleWithSeverity { rule: rule.clone(), severity: AllowWarnDeny::Warn }) + .collect() + } +} + +impl From for LinterBuilder { + #[inline] + fn from(oxlintrc: Oxlintrc) -> Self { + Self::from_oxlintrc(false, oxlintrc) + } +} + +impl fmt::Debug for LinterBuilder { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("LinterBuilder") + .field("rules", &self.rules) + .field("options", &self.options) + .field("config", &self.config) + .finish_non_exhaustive() + } +} + +struct RulesCache(RefCell>>, LintPlugins); +impl RulesCache { + #[inline] + #[must_use] + pub fn new(plugins: LintPlugins) -> Self { + Self(RefCell::new(None), plugins) + } + + pub fn set_plugins(&mut self, plugins: LintPlugins) { + self.1 = plugins; + self.clear(); + } + + #[must_use] + fn borrow(&self) -> Ref<'_, Vec> { + let cached = self.0.borrow(); + if cached.is_some() { + Ref::map(cached, |cached| cached.as_ref().unwrap()) + } else { + drop(cached); + self.initialize(); + Ref::map(self.0.borrow(), |cached| cached.as_ref().unwrap()) + } + } + + /// # Panics + /// If the cache cell is currently borrowed. + fn clear(&self) { + *self.0.borrow_mut() = None; + } + + /// Forcefully initialize this cache with all rules in all plugins currently + /// enabled. + /// + /// This will clobber whatever value is currently stored. It should only be + /// called when the cache is not populated, either because it has not been + /// initialized yet or it was cleared with [`Self::clear`]. + /// + /// # Panics + /// If the cache cell is currently borrowed. + fn initialize(&self) { + debug_assert!( + self.0.borrow().is_none(), + "Cannot re-initialize a populated rules cache. It must be cleared first." + ); + + let mut all_rules: Vec<_> = if self.1.is_all() { + RULES.clone() + } else { + RULES + .iter() + .filter(|rule| self.1.contains(LintPlugins::from(rule.plugin_name()))) + .cloned() + .collect() + }; + all_rules.sort_unstable(); // TODO: do we need to sort? is is already sorted? + + *self.0.borrow_mut() = Some(all_rules); + } +} diff --git a/crates/oxc_linter/src/config/mod.rs b/crates/oxc_linter/src/config/mod.rs index bab88cfbe0962..229c31fa8912b 100644 --- a/crates/oxc_linter/src/config/mod.rs +++ b/crates/oxc_linter/src/config/mod.rs @@ -76,7 +76,7 @@ mod test { })); assert!(config.is_ok()); - let Oxlintrc { rules, settings, env, globals } = config.unwrap(); + let Oxlintrc { rules, settings, env, globals, .. } = config.unwrap(); assert!(!rules.is_empty()); assert_eq!( settings.jsx_a11y.polymorphic_prop_name.as_ref().map(CompactStr::as_str), diff --git a/crates/oxc_linter/src/config/oxlintrc.rs b/crates/oxc_linter/src/config/oxlintrc.rs index e4cb9c599043f..86f58df275700 100644 --- a/crates/oxc_linter/src/config/oxlintrc.rs +++ b/crates/oxc_linter/src/config/oxlintrc.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; use super::{env::OxlintEnv, globals::OxlintGlobals, rules::OxlintRules, settings::OxlintSettings}; -use crate::utils::read_to_string; +use crate::{options::LintPlugins, utils::read_to_string}; /// Oxlint Configuration File /// @@ -42,7 +42,9 @@ use crate::utils::read_to_string; /// ``` #[derive(Debug, Default, Deserialize, Serialize, JsonSchema)] #[serde(default)] +#[non_exhaustive] pub struct Oxlintrc { + pub plugins: LintPlugins, /// See [Oxlint Rules](https://oxc.rs/docs/guide/usage/linter/rules.html). pub rules: OxlintRules, pub settings: OxlintSettings, diff --git a/crates/oxc_linter/src/lib.rs b/crates/oxc_linter/src/lib.rs index 11688179362bc..692986122a2c0 100644 --- a/crates/oxc_linter/src/lib.rs +++ b/crates/oxc_linter/src/lib.rs @@ -4,6 +4,7 @@ mod tester; mod ast_util; +mod builder; mod config; mod context; mod disable_directives; @@ -28,11 +29,12 @@ use oxc_diagnostics::Error; use oxc_semantic::{AstNode, Semantic}; pub use crate::{ + builder::LinterBuilder, config::Oxlintrc, context::LintContext, fixer::FixKind, frameworks::FrameworkFlags, - options::{AllowWarnDeny, InvalidFilterKind, LintFilter, OxlintOptions}, + options::{AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind, OxlintOptions}, rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleWithSeverity}, service::{LintService, LintServiceOptions}, }; @@ -66,6 +68,14 @@ impl Default for Linter { } impl Linter { + pub(crate) fn new( + rules: Vec, + options: LintOptions, + config: LintConfig, + ) -> Self { + Self { rules, options, config: Arc::new(config) } + } + /// # Errors /// /// Returns `Err` if there are any errors parsing the configuration file. diff --git a/crates/oxc_linter/src/options/mod.rs b/crates/oxc_linter/src/options/mod.rs index 7558c133de080..9fe7f133e8890 100644 --- a/crates/oxc_linter/src/options/mod.rs +++ b/crates/oxc_linter/src/options/mod.rs @@ -4,14 +4,12 @@ mod plugins; use std::{convert::From, path::PathBuf}; -use filter::LintFilterKind; use oxc_diagnostics::Error; -use plugins::LintPlugins; use rustc_hash::FxHashSet; pub use allow_warn_deny::AllowWarnDeny; -pub use filter::{InvalidFilterKind, LintFilter}; -pub use plugins::LintPluginOptions; +pub use filter::{InvalidFilterKind, LintFilter, LintFilterKind}; +pub use plugins::{LintPluginOptions, LintPlugins}; use crate::{ config::{LintConfig, Oxlintrc}, diff --git a/crates/oxc_linter/src/options/plugins.rs b/crates/oxc_linter/src/options/plugins.rs index 52355892fa633..d873a8d0dc172 100644 --- a/crates/oxc_linter/src/options/plugins.rs +++ b/crates/oxc_linter/src/options/plugins.rs @@ -6,6 +6,8 @@ bitflags! { // NOTE: may be increased to a u32 if needed #[derive(Debug, Clone, Copy, PartialEq, Hash)] pub struct LintPlugins: u16 { + /// Not really a plugin. Included for completeness. + const ESLINT = 0; /// `eslint-plugin-react`, plus `eslint-plugin-react-hooks` const REACT = 1 << 0; /// `eslint-plugin-unicorn` diff --git a/crates/oxc_linter/src/snapshots/schema_json.snap b/crates/oxc_linter/src/snapshots/schema_json.snap index 2ca0e388dc03a..792e860337bad 100644 --- a/crates/oxc_linter/src/snapshots/schema_json.snap +++ b/crates/oxc_linter/src/snapshots/schema_json.snap @@ -28,6 +28,19 @@ expression: json } ] }, + "plugins": { + "default": [ + "react", + "unicorn", + "typescript", + "oxc" + ], + "allOf": [ + { + "$ref": "#/definitions/LintPlugins" + } + ] + }, "rules": { "description": "See [Oxlint Rules](https://oxc.rs/docs/guide/usage/linter/rules.html).", "default": {}, @@ -222,6 +235,12 @@ expression: json } } }, + "LintPlugins": { + "type": "array", + "items": { + "type": "string" + } + }, "NextPluginSettings": { "type": "object", "properties": { diff --git a/npm/oxlint/configuration_schema.json b/npm/oxlint/configuration_schema.json index d5dadb9fe6337..23c32c74fa96a 100644 --- a/npm/oxlint/configuration_schema.json +++ b/npm/oxlint/configuration_schema.json @@ -24,6 +24,19 @@ } ] }, + "plugins": { + "default": [ + "react", + "unicorn", + "typescript", + "oxc" + ], + "allOf": [ + { + "$ref": "#/definitions/LintPlugins" + } + ] + }, "rules": { "description": "See [Oxlint Rules](https://oxc.rs/docs/guide/usage/linter/rules.html).", "default": {}, @@ -218,6 +231,12 @@ } } }, + "LintPlugins": { + "type": "array", + "items": { + "type": "string" + } + }, "NextPluginSettings": { "type": "object", "properties": { diff --git a/tasks/website/src/linter/snapshots/schema_markdown.snap b/tasks/website/src/linter/snapshots/schema_markdown.snap index e716a23f8c985..f6a6e801867c3 100644 --- a/tasks/website/src/linter/snapshots/schema_markdown.snap +++ b/tasks/website/src/linter/snapshots/schema_markdown.snap @@ -70,6 +70,22 @@ You may also use `"readable"` or `false` to represent `"readonly"`, and `"writea +## plugins + +type: `array` + + + + +### plugins[n] + +type: `string` + + + + + + ## rules type: `object`