Skip to content
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.

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

feat(linter)!: support plugins in oxlint config files #6088

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 7 additions & 0 deletions apps/oxlint/fixtures/import/.oxlintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"plugins": ["import"],
Boshen marked this conversation as resolved.
Show resolved Hide resolved
"rules": {
"import/no-default-export": "error",
"import/namespace": "allow"
}
}
9 changes: 9 additions & 0 deletions apps/oxlint/fixtures/import/test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import * as foo from './foo';
// ^ import/namespace

console.log(foo);

// import/no-default-export
export default function foo() {}


2 changes: 2 additions & 0 deletions apps/oxlint/fixtures/typescript_eslint/eslintrc.json
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
{
// NOTE: enabled by default
"plugins": ["@typescript-eslint"],
"rules": {
"no-loss-of-precision": "off",
"@typescript-eslint/no-loss-of-precision": "error",
Expand Down
199 changes: 169 additions & 30 deletions apps/oxlint/src/command/lint.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::{path::PathBuf, str::FromStr};

use bpaf::Bpaf;
use oxc_linter::{AllowWarnDeny, FixKind};
use oxc_linter::{AllowWarnDeny, FixKind, LintPlugins};

use super::{
expand_glob,
Expand Down Expand Up @@ -211,64 +211,203 @@ impl FromStr for OutputFormat {

/// Enable Plugins
#[allow(clippy::struct_field_names)]
#[derive(Debug, Clone, Bpaf)]
#[derive(Debug, Default, Clone, Bpaf)]
pub struct EnablePlugins {
/// Disable react plugin, which is turned on by default
#[bpaf(long("disable-react-plugin"), flag(false, true), hide_usage)]
pub react_plugin: bool,
#[bpaf(
long("disable-react-plugin"),
flag(OverrideToggle::Disable, OverrideToggle::NotSet),
hide_usage
)]
pub react_plugin: OverrideToggle,

/// Disable unicorn plugin, which is turned on by default
#[bpaf(long("disable-unicorn-plugin"), flag(false, true), hide_usage)]
pub unicorn_plugin: bool,
#[bpaf(
long("disable-unicorn-plugin"),
flag(OverrideToggle::Disable, OverrideToggle::NotSet),
hide_usage
)]
pub unicorn_plugin: OverrideToggle,

/// Disable oxc unique rules, which is turned on by default
#[bpaf(long("disable-oxc-plugin"), flag(false, true), hide_usage)]
pub oxc_plugin: bool,
#[bpaf(
long("disable-oxc-plugin"),
flag(OverrideToggle::Disable, OverrideToggle::NotSet),
hide_usage
)]
pub oxc_plugin: OverrideToggle,

/// Disable TypeScript plugin, which is turned on by default
#[bpaf(long("disable-typescript-plugin"), flag(false, true), hide_usage)]
pub typescript_plugin: bool,
#[bpaf(
long("disable-typescript-plugin"),
flag(OverrideToggle::Disable, OverrideToggle::NotSet),
hide_usage
)]
pub typescript_plugin: OverrideToggle,

/// Enable the experimental import plugin and detect ESM problems.
/// It is recommended to use along side with the `--tsconfig` option.
#[bpaf(switch, hide_usage)]
pub import_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub import_plugin: OverrideToggle,

/// Enable the experimental jsdoc plugin and detect JSDoc problems
#[bpaf(switch, hide_usage)]
pub jsdoc_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub jsdoc_plugin: OverrideToggle,

/// Enable the Jest plugin and detect test problems
#[bpaf(switch, hide_usage)]
pub jest_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub jest_plugin: OverrideToggle,

/// Enable the Vitest plugin and detect test problems
#[bpaf(switch, hide_usage)]
pub vitest_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub vitest_plugin: OverrideToggle,

/// Enable the JSX-a11y plugin and detect accessibility problems
#[bpaf(switch, hide_usage)]
pub jsx_a11y_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub jsx_a11y_plugin: OverrideToggle,

/// Enable the Next.js plugin and detect Next.js problems
#[bpaf(switch, hide_usage)]
pub nextjs_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub nextjs_plugin: OverrideToggle,

/// Enable the React performance plugin and detect rendering performance problems
#[bpaf(switch, hide_usage)]
pub react_perf_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub react_perf_plugin: OverrideToggle,

/// Enable the promise plugin and detect promise usage problems
#[bpaf(switch, hide_usage)]
pub promise_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub promise_plugin: OverrideToggle,

/// Enable the node plugin and detect node usage problems
#[bpaf(switch, hide_usage)]
pub node_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub node_plugin: OverrideToggle,

/// Enable the security plugin and detect security problems
#[bpaf(switch, hide_usage)]
pub security_plugin: bool,
#[bpaf(flag(OverrideToggle::Enable, OverrideToggle::NotSet), hide_usage)]
pub security_plugin: OverrideToggle,
}

/// Enables or disables a boolean option, or leaves it unset.
///
/// We want CLI flags to modify whatever's set in the user's config file, but we don't want them
/// changing default behavior if they're not explicitly passed by the user. This scheme is a bit
/// convoluted, but needed due to architectural constraints imposed by `bpaf`.
#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)]
#[allow(clippy::enum_variant_names)]
pub enum OverrideToggle {
/// Override the option to enabled
Enable,
/// Override the option to disabled
Disable,
/// Do not override.
#[default]
NotSet,
}

impl From<Option<bool>> for OverrideToggle {
fn from(value: Option<bool>) -> Self {
match value {
Some(true) => Self::Enable,
Some(false) => Self::Disable,
None => Self::NotSet,
}
}
}

impl From<OverrideToggle> for Option<bool> {
fn from(value: OverrideToggle) -> Self {
match value {
OverrideToggle::Enable => Some(true),
OverrideToggle::Disable => Some(false),
OverrideToggle::NotSet => None,
}
}
}

impl OverrideToggle {
#[inline]
pub fn is_enabled(self) -> bool {
matches!(self, Self::Enable)
}

#[inline]
pub fn is_not_set(self) -> bool {
matches!(self, Self::NotSet)
}
DonIsaac marked this conversation as resolved.
Show resolved Hide resolved

pub fn inspect<F>(self, f: F)
where
F: FnOnce(bool),
{
DonIsaac marked this conversation as resolved.
Show resolved Hide resolved
if let Some(v) = self.into() {
f(v);
}
}
}

impl EnablePlugins {
pub fn apply_overrides(&self, plugins: &mut LintPlugins) {
self.react_plugin.inspect(|yes| plugins.set(LintPlugins::REACT, yes));
self.unicorn_plugin.inspect(|yes| plugins.set(LintPlugins::UNICORN, yes));
self.oxc_plugin.inspect(|yes| plugins.set(LintPlugins::OXC, yes));
self.typescript_plugin.inspect(|yes| plugins.set(LintPlugins::TYPESCRIPT, yes));
self.import_plugin.inspect(|yes| plugins.set(LintPlugins::IMPORT, yes));
self.jsdoc_plugin.inspect(|yes| plugins.set(LintPlugins::JSDOC, yes));
self.jest_plugin.inspect(|yes| plugins.set(LintPlugins::JEST, yes));
self.vitest_plugin.inspect(|yes| plugins.set(LintPlugins::VITEST, yes));
self.jsx_a11y_plugin.inspect(|yes| plugins.set(LintPlugins::JSX_A11Y, yes));
self.nextjs_plugin.inspect(|yes| plugins.set(LintPlugins::NEXTJS, yes));
self.react_perf_plugin.inspect(|yes| plugins.set(LintPlugins::REACT_PERF, yes));
self.promise_plugin.inspect(|yes| plugins.set(LintPlugins::PROMISE, yes));
self.node_plugin.inspect(|yes| plugins.set(LintPlugins::NODE, yes));
self.security_plugin.inspect(|yes| plugins.set(LintPlugins::SECURITY, yes));

// Without this, jest plugins adapted to vitest will not be enabled.
if self.vitest_plugin.is_enabled() && self.jest_plugin.is_not_set() {
plugins.set(LintPlugins::JEST, true);
}
}
}

#[cfg(test)]
mod plugins {
use super::{EnablePlugins, OverrideToggle};
use oxc_linter::LintPlugins;

#[test]
fn test_override_default() {
let mut plugins = LintPlugins::default();
let enable = EnablePlugins::default();

enable.apply_overrides(&mut plugins);
assert_eq!(plugins, LintPlugins::default());
}

#[test]
fn test_overrides() {
let mut plugins = LintPlugins::default();
let enable = EnablePlugins {
react_plugin: OverrideToggle::Enable,
unicorn_plugin: OverrideToggle::Disable,
..EnablePlugins::default()
};
let expected =
LintPlugins::default().union(LintPlugins::REACT).difference(LintPlugins::UNICORN);

enable.apply_overrides(&mut plugins);
assert_eq!(plugins, expected);
}

#[test]
fn test_override_vitest() {
let mut plugins = LintPlugins::default();
let enable =
EnablePlugins { vitest_plugin: OverrideToggle::Enable, ..EnablePlugins::default() };
let expected = LintPlugins::default() | LintPlugins::VITEST | LintPlugins::JEST;

enable.apply_overrides(&mut plugins);
assert_eq!(plugins, expected);
}
}

#[cfg(test)]
Expand Down
64 changes: 33 additions & 31 deletions apps/oxlint/src/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use ignore::gitignore::Gitignore;
use oxc_diagnostics::{DiagnosticService, GraphicalReportHandler};
use oxc_linter::{
loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter, LintService,
LintServiceOptions, Linter, OxlintOptions,
LintServiceOptions, Linter, LinterBuilder, Oxlintrc,
};
use oxc_span::VALID_EXTENSIONS;

Expand Down Expand Up @@ -98,39 +98,32 @@ impl Runner for LintRunner {
let number_of_files = paths.len();

let cwd = std::env::current_dir().unwrap();
let mut options =
LintServiceOptions::new(cwd, paths).with_cross_module(enable_plugins.import_plugin);
let lint_options = OxlintOptions::default()
.with_filter(filter)
.with_config_path(basic_options.config)
.with_fix(fix_options.fix_kind())
.with_react_plugin(enable_plugins.react_plugin)
.with_unicorn_plugin(enable_plugins.unicorn_plugin)
.with_typescript_plugin(enable_plugins.typescript_plugin)
.with_oxc_plugin(enable_plugins.oxc_plugin)
.with_import_plugin(enable_plugins.import_plugin)
.with_jsdoc_plugin(enable_plugins.jsdoc_plugin)
.with_jest_plugin(enable_plugins.jest_plugin)
.with_vitest_plugin(enable_plugins.vitest_plugin)
.with_jsx_a11y_plugin(enable_plugins.jsx_a11y_plugin)
.with_nextjs_plugin(enable_plugins.nextjs_plugin)
.with_react_perf_plugin(enable_plugins.react_perf_plugin)
.with_promise_plugin(enable_plugins.promise_plugin)
.with_node_plugin(enable_plugins.node_plugin)
.with_security_plugin(enable_plugins.security_plugin);

let linter = match Linter::from_options(lint_options) {
Ok(lint_service) => lint_service,
Err(diagnostic) => {
let handler = GraphicalReportHandler::new();
let mut err = String::new();
handler.render_report(&mut err, diagnostic.as_ref()).unwrap();
return CliRunResult::InvalidOptions {
message: format!("Failed to parse configuration file.\n{err}"),
};

let mut oxlintrc = if let Some(config_path) = basic_options.config.as_ref() {
match Oxlintrc::from_file(config_path) {
Ok(config) => config,
Err(diagnostic) => {
let handler = GraphicalReportHandler::new();
let mut err = String::new();
handler.render_report(&mut err, &diagnostic).unwrap();
return CliRunResult::InvalidOptions {
message: format!("Failed to parse configuration file.\n{err}"),
};
}
}
} else {
Oxlintrc::default()
};

enable_plugins.apply_overrides(&mut oxlintrc.plugins);
let builder = LinterBuilder::from_oxlintrc(false, oxlintrc)
.with_filters(filter)
.with_fix(fix_options.fix_kind());

let mut options =
LintServiceOptions::new(cwd, paths).with_cross_module(builder.plugins().has_import());
let linter = builder.build();

let tsconfig = basic_options.tsconfig;
if let Some(path) = tsconfig.as_ref() {
if path.is_file() {
Expand Down Expand Up @@ -562,4 +555,13 @@ mod test {
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_errors, 1);
}

#[test]
fn test_import_plugin_enabled_in_config() {
let args = &["-c", "fixtures/import/.oxlintrc.json", "fixtures/import/test.js"];
let result = test(args);
assert_eq!(result.number_of_files, 1);
assert_eq!(result.number_of_warnings, 0);
assert_eq!(result.number_of_errors, 1);
}
}
1 change: 1 addition & 0 deletions crates/oxc_linter/src/config/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use serde::{Deserialize, Serialize};
/// environments](https://eslint.org/docs/v8.x/use/configure/language-options#specifying-environments)
/// for what environments are available and what each one provides.
#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)]
#[cfg_attr(test, derive(PartialEq))]
pub struct OxlintEnv(FxHashMap<String, bool>);

impl OxlintEnv {
Expand Down
Loading