From 2fde2255f954d350b10e5a590dd5973a4f507766 Mon Sep 17 00:00:00 2001 From: Kei Sakamoto <6201376+Thiry1@users.noreply.github.com> Date: Thu, 17 Aug 2023 18:08:03 +0900 Subject: [PATCH] feat(linter): implement eslint-plugin-unicorn/no-instanceof-array (#752) implement [eslint-plugin-unicorn/no-instanceof-array](https://github.com/sindresorhus/eslint-plugin-unicorn/blob/main/docs/rules/no-instanceof-array.md). Related issue: #684 --- crates/oxc_linter/src/rules.rs | 5 + .../src/rules/unicorn/no_instanceof_array.rs | 107 ++++++++++++++++++ .../src/snapshots/no_instanceof_array.snap | 62 ++++++++++ justfile | 3 + tasks/rulegen/src/main.rs | 7 ++ tasks/rulegen/src/template.rs | 1 + 6 files changed, 185 insertions(+) create mode 100644 crates/oxc_linter/src/rules/unicorn/no_instanceof_array.rs create mode 100644 crates/oxc_linter/src/snapshots/no_instanceof_array.snap diff --git a/crates/oxc_linter/src/rules.rs b/crates/oxc_linter/src/rules.rs index 8eb344df0dc83..fabab7a90d459 100644 --- a/crates/oxc_linter/src/rules.rs +++ b/crates/oxc_linter/src/rules.rs @@ -98,6 +98,10 @@ mod jest { pub mod valid_describe_callback; } +mod unicorn { + pub mod no_instanceof_array; +} + oxc_macros::declare_all_lint_rules! { deepscan::bad_array_method_on_arguments, deepscan::bad_bitwise_operator, @@ -180,4 +184,5 @@ oxc_macros::declare_all_lint_rules! { jest::no_focused_tests, jest::valid_describe_callback, jest::no_commented_out_tests, + unicorn::no_instanceof_array, } diff --git a/crates/oxc_linter/src/rules/unicorn/no_instanceof_array.rs b/crates/oxc_linter/src/rules/unicorn/no_instanceof_array.rs new file mode 100644 index 0000000000000..c5f7cb95c63a9 --- /dev/null +++ b/crates/oxc_linter/src/rules/unicorn/no_instanceof_array.rs @@ -0,0 +1,107 @@ +use oxc_ast::ast::Expression; +use oxc_ast::AstKind; +use oxc_diagnostics::{ + miette::{self, Diagnostic}, + thiserror::Error, +}; +use oxc_formatter::Gen; +use oxc_macros::declare_oxc_lint; +use oxc_span::Span; +use oxc_syntax::operator::BinaryOperator; + +use crate::{context::LintContext, fixer::Fix, rule::Rule, AstNode}; + +#[derive(Debug, Error, Diagnostic)] +#[error("Use `Array.isArray()` instead of `instanceof Array`.")] +#[diagnostic(severity(warning), help("The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js."))] +struct NoInstanceofArrayDiagnostic(#[label] pub Span); + +#[derive(Debug, Default, Clone)] +pub struct NoInstanceofArray; + +declare_oxc_lint!( + /// ### What it does + /// Require `Array.isArray()` instead of `instanceof Array`. + /// + /// ### Why is this bad? + /// The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + /// + /// ### Example + /// ```javascript + /// array instanceof Array; + /// [1,2,3] instanceof Array; + /// ``` + NoInstanceofArray, + correctness +); + +impl Rule for NoInstanceofArray { + fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) { + let AstKind::BinaryExpression(expr) = node.kind() else { return }; + if expr.operator != BinaryOperator::Instanceof { + return; + } + + match &expr.right { + Expression::Identifier(identifier) if identifier.name == "Array" => { + ctx.diagnostic_with_fix(NoInstanceofArrayDiagnostic(expr.span), || { + let modified_code = { + let mut formatter = ctx.formatter(); + formatter.print_str(b"Array.isArray("); + expr.left.gen(&mut formatter); + formatter.print(b')'); + formatter.into_code() + }; + Fix::new(modified_code, expr.span) + }); + } + _ => {} + } + } +} + +#[test] +fn test() { + use crate::tester::Tester; + + let pass = vec![ + ("Array.isArray(arr)", None), + ("arr instanceof Object", None), + ("arr instanceof array", None), + ("a instanceof 'array'", None), + ("a instanceof ArrayA", None), + ("a.x[2] instanceof foo()", None), + ("Array.isArray([1,2,3]) === true", None), + ("\"arr instanceof Array\"", None), + ]; + + let fail = vec![ + ("arr instanceof Array", None), + ("[] instanceof Array", None), + ("[1,2,3] instanceof Array === true", None), + ("fun.call(1, 2, 3) instanceof Array", None), + ("obj.arr instanceof Array", None), + ("foo.bar[2] instanceof Array", None), + ("(0, array) instanceof Array", None), + ("function foo(){return [] instanceof Array}", None), + ]; + + let fix = vec![ + ("arr instanceof Array", "Array.isArray(arr)", None), + ("[] instanceof Array", "Array.isArray([])", None), + ("[1,2,3] instanceof Array === true", "Array.isArray([1, 2, 3]) === true", None), + ("fun.call(1, 2, 3) instanceof Array", "Array.isArray(fun.call(1, 2, 3))", None), + ("obj.arr instanceof Array", "Array.isArray(obj.arr)", None), + ("foo.bar[2] instanceof Array", "Array.isArray(foo.bar[2])", None), + ("(0, array) instanceof Array", "Array.isArray((0, array))", None), + ( + "function foo(){return [] instanceof Array}", + "function foo(){return Array.isArray([])}", + None, + ), + ]; + + let mut tester = Tester::new(NoInstanceofArray::NAME, pass, fail); + tester.test_and_snapshot(); + tester.test_fix(fix); +} diff --git a/crates/oxc_linter/src/snapshots/no_instanceof_array.snap b/crates/oxc_linter/src/snapshots/no_instanceof_array.snap new file mode 100644 index 0000000000000..c5599c7badc48 --- /dev/null +++ b/crates/oxc_linter/src/snapshots/no_instanceof_array.snap @@ -0,0 +1,62 @@ +--- +source: crates/oxc_linter/src/tester.rs +assertion_line: 80 +expression: no_instanceof_array +--- + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ arr instanceof Array + · ──────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ [] instanceof Array + · ─────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ [1,2,3] instanceof Array === true + · ──────────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ fun.call(1, 2, 3) instanceof Array + · ────────────────────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ obj.arr instanceof Array + · ──────────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ foo.bar[2] instanceof Array + · ─────────────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ (0, array) instanceof Array + · ─────────────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + ⚠ Use `Array.isArray()` instead of `instanceof Array`. + ╭─[no_instanceof_array.tsx:1:1] + 1 │ function foo(){return [] instanceof Array} + · ─────────────────── + ╰──── + help: The instanceof Array check doesn't work across realms/contexts, for example, frames/windows in browsers or the vm module in Node.js. + + diff --git a/justfile b/justfile index caeafaf5209a8..ee0bfedfb37a8 100755 --- a/justfile +++ b/justfile @@ -74,6 +74,9 @@ new-jest-rule name: new-ts-rule name: cargo run -p rulegen {{name}} typescript +new-unicorn-rule name: + cargo run -p rulegen {{name}} unicorn + # Sync all submodules with their own remote repos (this is for Boshen updating the submodules) sync: git submodule update --init --remote diff --git a/tasks/rulegen/src/main.rs b/tasks/rulegen/src/main.rs index 254c95984d10f..a61897dee56ff 100644 --- a/tasks/rulegen/src/main.rs +++ b/tasks/rulegen/src/main.rs @@ -32,6 +32,9 @@ const JEST_TEST_PATH: &str = const TYPESCRIPT_ESLINT_TEST_PATH: &str = "https://raw.githubusercontent.com/typescript-eslint/typescript-eslint/main/packages/eslint-plugin/tests/rules"; +const UNICORN_TEST_PATH: &str = + "https://raw.githubusercontent.com/sindresorhus/eslint-plugin-unicorn/main/test"; + struct TestCase<'a> { source_text: &'a str, code: Option>, @@ -262,6 +265,7 @@ pub enum RuleKind { ESLint, Jest, Typescript, + Unicorn, } impl RuleKind { @@ -269,6 +273,7 @@ impl RuleKind { match kind { "jest" => Self::Jest, "typescript" => Self::Typescript, + "unicorn" => Self::Unicorn, _ => Self::ESLint, } } @@ -280,6 +285,7 @@ impl Display for RuleKind { Self::ESLint => write!(f, "eslint"), Self::Typescript => write!(f, "typescript-eslint"), Self::Jest => write!(f, "eslint-plugin-jest"), + Self::Unicorn => write!(f, "eslint-plugin-unicorn"), } } } @@ -297,6 +303,7 @@ fn main() { RuleKind::ESLint => format!("{ESLINT_TEST_PATH}/{kebab_rule_name}.js"), RuleKind::Jest => format!("{JEST_TEST_PATH}/{kebab_rule_name}.test.ts"), RuleKind::Typescript => format!("{TYPESCRIPT_ESLINT_TEST_PATH}/{kebab_rule_name}.test.ts"), + RuleKind::Unicorn => format!("{UNICORN_TEST_PATH}/{kebab_rule_name}.mjs"), }; println!("Reading test file from {rule_test_path}"); diff --git a/tasks/rulegen/src/template.rs b/tasks/rulegen/src/template.rs index 49b11fd11143d..39796714bcf53 100644 --- a/tasks/rulegen/src/template.rs +++ b/tasks/rulegen/src/template.rs @@ -33,6 +33,7 @@ impl<'a> Template<'a> { RuleKind::ESLint => Path::new("crates/oxc_linter/src/rules/eslint"), RuleKind::Jest => Path::new("crates/oxc_linter/src/rules/jest"), RuleKind::Typescript => Path::new("crates/oxc_linter/src/rules/typescript"), + RuleKind::Unicorn => Path::new("crates/oxc_linter/src/rules/unicorn"), }; let out_path = path.join(format!("{}.rs", self.context.rule_name));