Skip to content

Commit

Permalink
feat(linter): implement eslint-plugin-unicorn/no-instanceof-array (#752)
Browse files Browse the repository at this point in the history
  • Loading branch information
Thiry1 authored Aug 17, 2023
1 parent 6ab4ce0 commit 2fde225
Show file tree
Hide file tree
Showing 6 changed files with 185 additions and 0 deletions.
5 changes: 5 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
}
107 changes: 107 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/no_instanceof_array.rs
Original file line number Diff line number Diff line change
@@ -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);
}
62 changes: 62 additions & 0 deletions crates/oxc_linter/src/snapshots/no_instanceof_array.snap
Original file line number Diff line number Diff line change
@@ -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]
1arr 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]
1fun.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]
1obj.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]
1foo.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]
1function 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.


3 changes: 3 additions & 0 deletions justfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions tasks/rulegen/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<Cow<'a, str>>,
Expand Down Expand Up @@ -262,13 +265,15 @@ pub enum RuleKind {
ESLint,
Jest,
Typescript,
Unicorn,
}

impl RuleKind {
fn from(kind: &str) -> Self {
match kind {
"jest" => Self::Jest,
"typescript" => Self::Typescript,
"unicorn" => Self::Unicorn,
_ => Self::ESLint,
}
}
Expand All @@ -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"),
}
}
}
Expand All @@ -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}");

Expand Down
1 change: 1 addition & 0 deletions tasks/rulegen/src/template.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down

0 comments on commit 2fde225

Please sign in to comment.