Skip to content

refactor(linter): remove OxlintOptions #6098

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graphite-app merged 1 commit into from
Oct 11, 2024
Merged

refactor(linter): remove OxlintOptions #6098

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 7 additions & 6 deletions crates/oxc_language_server/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use futures::future::join_all;
use globset::Glob;
use ignore::gitignore::Gitignore;
use log::{debug, error, info};
use oxc_linter::{FixKind, Linter, OxlintOptions};
use oxc_linter::{FixKind, LinterBuilder, Oxlintrc};
use serde::{Deserialize, Serialize};
use tokio::sync::{Mutex, OnceCell, RwLock, SetError};
use tower_lsp::{
Expand Down Expand Up @@ -345,12 +345,13 @@ impl Backend {
if let Some(config_path) = config_path {
let mut linter = self.server_linter.write().await;
*linter = ServerLinter::new_with_linter(
Linter::from_options(
OxlintOptions::default()
.with_fix(FixKind::SafeFix)
.with_config_path(Some(config_path)),
LinterBuilder::from_oxlintrc(
true,
Oxlintrc::from_file(&config_path)
.expect("should have initialized linter with new options"),
)
.expect("should have initialized linter with new options"),
.with_fix(FixKind::SafeFix)
.build(),
);
}
}
Expand Down
15 changes: 2 additions & 13 deletions crates/oxc_linter/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use std::{io::Write, path::Path, rc::Rc, sync::Arc};
use config::LintConfig;
use context::ContextHost;
use options::LintOptions;
use oxc_diagnostics::Error;
use oxc_semantic::{AstNode, Semantic};

pub use crate::{
Expand All @@ -35,9 +34,7 @@ pub use crate::{
context::LintContext,
fixer::FixKind,
frameworks::FrameworkFlags,
options::{
AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind, LintPlugins, OxlintOptions,
},
options::{AllowWarnDeny, InvalidFilterKind, LintFilter, LintFilterKind, LintPlugins},
rule::{RuleCategory, RuleFixMeta, RuleMeta, RuleWithSeverity},
service::{LintService, LintServiceOptions},
};
Expand Down Expand Up @@ -66,7 +63,7 @@ pub struct Linter {

impl Default for Linter {
fn default() -> Self {
Self::from_options(OxlintOptions::default()).unwrap()
LinterBuilder::default().build()
}
}

Expand All @@ -79,14 +76,6 @@ impl Linter {
Self { rules, options, config: Arc::new(config) }
}

/// # Errors
///
/// Returns `Err` if there are any errors parsing the configuration file.
pub fn from_options(options: OxlintOptions) -> Result<Self, Error> {
let (rules, config) = options.derive_rules_and_config()?;
Ok(Self { rules, options: options.into(), config: Arc::new(config) })
}

#[cfg(test)]
#[must_use]
pub fn with_rules(mut self, rules: Vec<RuleWithSeverity>) -> Self {
Expand Down
289 changes: 3 additions & 286 deletions crates/oxc_linter/src/options/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,300 +2,17 @@ mod allow_warn_deny;
mod filter;
mod plugins;

use std::{convert::From, path::PathBuf};

use oxc_diagnostics::Error;
use rustc_hash::FxHashSet;

pub use allow_warn_deny::AllowWarnDeny;
pub use filter::{InvalidFilterKind, LintFilter, LintFilterKind};
pub use plugins::{LintPluginOptions, LintPlugins};
pub use plugins::LintPlugins;

use crate::{
config::{LintConfig, Oxlintrc},
fixer::FixKind,
rules::RULES,
utils::is_jest_rule_adapted_to_vitest,
FrameworkFlags, RuleCategory, RuleEnum, RuleWithSeverity,
};
use crate::{fixer::FixKind, FrameworkFlags};

/// Subset of options used directly by the [`Linter`]. Derived from
/// [`OxlintOptions`], which is the public-facing API. Do not expose this
/// outside of this crate.
///
/// [`Linter`]: crate::Linter
/// Subset of options used directly by the linter.
#[derive(Debug, Default, Clone, Copy)]
#[cfg_attr(test, derive(PartialEq))]
pub(crate) struct LintOptions {
pub fix: FixKind,
pub framework_hints: FrameworkFlags,
pub plugins: LintPlugins,
}

impl From<OxlintOptions> for LintOptions {
fn from(options: OxlintOptions) -> Self {
Self {
fix: options.fix,
framework_hints: options.framework_hints,
plugins: options.plugins.into(),
}
}
}

#[derive(Debug)]
pub struct OxlintOptions {
/// Allow / Deny rules in order. [("allow" / "deny", rule name)]
/// Defaults to [("deny", "correctness")]
pub filter: Vec<LintFilter>,
pub config_path: Option<PathBuf>,
/// Enable automatic code fixes. Set to [`None`] to disable.
///
/// The kind represents the riskiest fix that the linter can apply.
pub fix: FixKind,

pub plugins: LintPluginOptions,

pub framework_hints: FrameworkFlags,
}

impl Default for OxlintOptions {
fn default() -> Self {
Self {
filter: vec![LintFilter::warn(RuleCategory::Correctness)],
config_path: None,
fix: FixKind::None,
plugins: LintPluginOptions::default(),
framework_hints: FrameworkFlags::default(),
}
}
}

impl OxlintOptions {
#[must_use]
pub fn with_filter(mut self, filter: Vec<LintFilter>) -> Self {
if !filter.is_empty() {
self.filter = filter;
}
self
}

#[must_use]
pub fn with_config_path(mut self, filter: Option<PathBuf>) -> Self {
self.config_path = filter;
self
}

/// Set the kind of auto fixes to apply.
///
/// # Example
///
/// ```
/// use oxc_linter::{LintOptions, FixKind};
///
/// // turn off all auto fixes. This is default behavior.
/// LintOptions::default().with_fix(FixKind::None);
/// ```
#[must_use]
pub fn with_fix(mut self, kind: FixKind) -> Self {
self.fix = kind;
self
}

#[must_use]
pub fn with_react_plugin(mut self, yes: bool) -> Self {
self.plugins.react = yes;
self
}

#[must_use]
pub fn with_unicorn_plugin(mut self, yes: bool) -> Self {
self.plugins.unicorn = yes;
self
}

#[must_use]
pub fn with_typescript_plugin(mut self, yes: bool) -> Self {
self.plugins.typescript = yes;
self
}

#[must_use]
pub fn with_oxc_plugin(mut self, yes: bool) -> Self {
self.plugins.oxc = yes;
self
}

#[must_use]
pub fn with_import_plugin(mut self, yes: bool) -> Self {
self.plugins.import = yes;
self
}

#[must_use]
pub fn with_jsdoc_plugin(mut self, yes: bool) -> Self {
self.plugins.jsdoc = yes;
self
}

#[must_use]
pub fn with_jest_plugin(mut self, yes: bool) -> Self {
self.plugins.jest = yes;
self
}

#[must_use]
pub fn with_vitest_plugin(mut self, yes: bool) -> Self {
self.plugins.vitest = yes;
self
}

#[must_use]
pub fn with_jsx_a11y_plugin(mut self, yes: bool) -> Self {
self.plugins.jsx_a11y = yes;
self
}

#[must_use]
pub fn with_nextjs_plugin(mut self, yes: bool) -> Self {
self.plugins.nextjs = yes;
self
}

#[must_use]
pub fn with_react_perf_plugin(mut self, yes: bool) -> Self {
self.plugins.react_perf = yes;
self
}

#[must_use]
pub fn with_promise_plugin(mut self, yes: bool) -> Self {
self.plugins.promise = yes;
self
}

#[must_use]
pub fn with_node_plugin(mut self, yes: bool) -> Self {
self.plugins.node = yes;
self
}

#[must_use]
pub fn with_security_plugin(mut self, yes: bool) -> Self {
self.plugins.security = yes;
self
}
}

impl OxlintOptions {
/// # Errors
///
/// * Returns `Err` if there are any errors parsing the configuration file.
pub(crate) fn derive_rules_and_config(
&self,
) -> Result<(Vec<RuleWithSeverity>, LintConfig), Error> {
let config = self.config_path.as_ref().map(|path| Oxlintrc::from_file(path)).transpose()?;

let mut rules: FxHashSet<RuleWithSeverity> = FxHashSet::default();
let all_rules = self.get_filtered_rules();

for (severity, filter) in self.filter.iter().map(Into::into) {
match severity {
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.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 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);
}
}
},
}
}

if let Some(config) = &config {
config.rules.override_rules(&mut rules, &all_rules);
}

let mut rules = rules.into_iter().collect::<Vec<_>>();

// for stable diagnostics output ordering
rules.sort_unstable_by_key(|rule| rule.id());

Ok((rules, config.map(Into::into).unwrap_or_default()))
}

/// Get final filtered rules by reading `self.xxx_plugin`
fn get_filtered_rules(&self) -> Vec<RuleEnum> {
RULES
.iter()
.filter(|rule| match rule.plugin_name() {
"react" => self.plugins.react,
"unicorn" => self.plugins.unicorn,
"typescript" => self.plugins.typescript,
"import" => self.plugins.import,
"jsdoc" => self.plugins.jsdoc,
"jest" => {
if self.plugins.jest {
return true;
}
if self.plugins.vitest && is_jest_rule_adapted_to_vitest(rule.name()) {
return true;
}
false
}
"vitest" => self.plugins.vitest,
"jsx_a11y" => self.plugins.jsx_a11y,
"nextjs" => self.plugins.nextjs,
"react_perf" => self.plugins.react_perf,
"oxc" => self.plugins.oxc,
"eslint" | "tree_shaking" => true,
"promise" => self.plugins.promise,
"node" => self.plugins.node,
"security" => self.plugins.security,
name => panic!("Unhandled plugin: {name}"),
})
.cloned()
.collect::<Vec<_>>()
}
}