Skip to content

Commit

Permalink
clippy fix
Browse files Browse the repository at this point in the history
  • Loading branch information
DonIsaac committed Sep 27, 2024
1 parent 29c00c5 commit 27c99ab
Show file tree
Hide file tree
Showing 16 changed files with 294 additions and 81 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
174 changes: 144 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,178 @@ 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 enable = EnablePlugins {
react_plugin: Enable::Enable,
unicorn_plugin: Enable::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: Enable::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::{
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 @@ -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
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));
}
}
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/config/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,11 @@ type RuleSet = FxHashSet<RuleWithSeverity>;
// <https://github.com/eslint/eslint/blob/ce838adc3b673e52a151f36da0eedf5876977514/lib/shared/types.js#L12>
// Note: when update document comment, also update `DummyRuleMap`'s description in this file.
#[derive(Debug, Clone, Default)]
#[cfg_attr(test, derive(PartialEq))]
pub struct OxlintRules(Vec<ESLintRule>);

#[derive(Debug, Clone)]
#[cfg_attr(test, derive(PartialEq))]
pub struct ESLintRule {
pub plugin_name: String,
pub rule_name: String,
Expand Down
Loading

0 comments on commit 27c99ab

Please sign in to comment.