Skip to content

Commit

Permalink
feat(linter): support plugins in oxlint config files
Browse files Browse the repository at this point in the history
This PR migrates the linter crate and oxlint app to use the new `LinterBuilder` API. This PR has the following effects:

1. `plugins` in oxlint config files are now supported
2. irons out weirdness when using CLI args and config files together. CLI args are now always applied on top of config file settings, overriding them.
  • Loading branch information
DonIsaac committed Sep 26, 2024
1 parent 6edcd5c commit d08cf2c
Show file tree
Hide file tree
Showing 16 changed files with 296 additions and 82 deletions.
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"],
"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
173 changes: 143 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,177 @@ 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(Enable::Disable, Enable::NotSet), hide_usage)]
pub react_plugin: Enable,

/// 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(Enable::Disable, Enable::NotSet), hide_usage)]
pub unicorn_plugin: Enable,

/// 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(Enable::Disable, Enable::NotSet), hide_usage)]
pub oxc_plugin: Enable,

/// 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(Enable::Disable, Enable::NotSet), hide_usage)]
pub typescript_plugin: Enable,

/// 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(Enable::Enable, Enable::NotSet), hide_usage)]
pub import_plugin: Enable,

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

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

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

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

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

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

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

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

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

#[derive(Default, Debug, Clone, Copy, PartialEq, Eq)]
#[allow(clippy::enum_variant_names)]
pub enum Enable {
Enable,
Disable,
#[default]
NotSet,
}

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

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

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

#[inline]
pub fn is_not_set(self) -> bool {
matches!(self, Self::NotSet)
}

pub fn inspect<F>(self, f: F)
where
F: FnOnce(bool),
{
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::{Enable, EnablePlugins};
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 mut enable = EnablePlugins::default();
enable.react_plugin = Enable::Enable;
enable.unicorn_plugin = Enable::Disable;
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 mut enable = EnablePlugins::default();
enable.vitest_plugin = Enable::Enable;
let expected = LintPlugins::default() | LintPlugins::VITEST | LintPlugins::JEST;

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

#[cfg(test)]
Expand Down
68 changes: 36 additions & 32 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::{
partial_loader::LINT_PARTIAL_LOADER_EXT, AllowWarnDeny, InvalidFilterKind, LintFilter,
LintService, LintServiceOptions, Linter, OxlintOptions,
LintService, LintServiceOptions, Linter, LinterBuilder, Oxlintrc,
};
use oxc_span::VALID_EXTENSIONS;

Expand Down Expand Up @@ -82,7 +82,7 @@ impl Runner for LintRunner {
}

let filter = match Self::get_filters(filter) {
Ok(filter) => filter,
Ok(filter) => dbg!(filter),
Err(e) => return e,
};

Expand All @@ -98,39 +98,34 @@ 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()
};

println!("before overrides: {oxlintrc:#?}");
enable_plugins.apply_overrides(&mut oxlintrc.plugins);
println!("after overrides: {oxlintrc:#?}");
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 +557,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
46 changes: 46 additions & 0 deletions crates/oxc_linter/src/config/oxlintrc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,3 +90,49 @@ impl Oxlintrc {
Ok(config)
}
}

#[cfg(test)]
mod test {
use super::*;
use serde_json::json;

#[test]
fn test_oxlintrc_de_empty() {
let config: Oxlintrc = serde_json::from_value(json!({})).unwrap();
assert_eq!(config.plugins, LintPlugins::default());
assert_eq!(config.rules, OxlintRules::default());
assert!(config.rules.is_empty());
assert_eq!(config.settings, OxlintSettings::default());
assert_eq!(config.env, OxlintEnv::default());
}

#[test]
fn test_oxlintrc_de_plugins_empty_array() {
let config: Oxlintrc = serde_json::from_value(json!({ "plugins": [] })).unwrap();
assert_eq!(config.plugins, LintPlugins::default());
}

#[test]
fn test_oxlintrc_de_plugins_enabled_by_default() {
// NOTE(@DonIsaac): creating a Value with `json!` then deserializing it with serde_json::from_value
// Errs with "invalid type: string \"eslint\", expected a borrowed string" and I can't
// figure out why. This seems to work. Why???
let configs = [
r#"{ "plugins": ["eslint"] }"#,
r#"{ "plugins": ["oxc"] }"#,
r#"{ "plugins": ["deepscan"] }"#, // alias for oxc
];
// ^ these plugins are enabled by default already
for oxlintrc in configs {
let config: Oxlintrc = serde_json::from_str(oxlintrc).unwrap();
assert_eq!(config.plugins, LintPlugins::default());
}
}

#[test]
fn test_oxlintrc_de_plugins_new() {
// let config: Oxlintrc = serde_json::from_value(json!({ "plugins": ["import"] })).unwrap();
let config: Oxlintrc = serde_json::from_str(r#"{ "plugins": ["import"] }"#).unwrap();
assert_eq!(config.plugins, LintPlugins::default().union(LintPlugins::IMPORT));
}
}
Loading

0 comments on commit d08cf2c

Please sign in to comment.