Skip to content

Commit

Permalink
feat(linter) eslint plugin unicorn: no array foreach
Browse files Browse the repository at this point in the history
  • Loading branch information
camc314 committed Dec 3, 2023
1 parent cfe207f commit 9e8ab1f
Show file tree
Hide file tree
Showing 3 changed files with 233 additions and 0 deletions.
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ mod unicorn {
pub mod filename_case;
pub mod new_for_builtins;
pub mod no_abusive_eslint_disable;
pub mod no_array_for_each;
pub mod no_array_reduce;
pub mod no_await_expression_member;
pub mod no_console_spaces;
Expand Down Expand Up @@ -338,6 +339,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::new_for_builtins,
unicorn::no_abusive_eslint_disable,
unicorn::no_array_reduce,
unicorn::no_array_for_each,
unicorn::no_await_expression_member,
unicorn::no_console_spaces,
unicorn::no_document_cookie,
Expand Down
128 changes: 128 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/no_array_for_each.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
use oxc_ast::{ast::Expression, AstKind};
use oxc_diagnostics::{
miette::{self, Diagnostic},
thiserror::Error,
};
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;

use crate::{ast_util::is_method_call, context::LintContext, rule::Rule, AstNode};

use phf::phf_set;

#[derive(Debug, Error, Diagnostic)]
#[error("eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`")]
#[diagnostic(severity(warning), help("Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early."))]
struct NoArrayForEachDiagnostic(#[label] pub Span);

#[derive(Debug, Default, Clone)]
pub struct NoArrayForEach;

declare_oxc_lint!(
/// ### What it does
///
/// Forbids the use of `Array#forEach` in favor of a for loop.
///
/// ### Why is this bad?
///
/// Benefits of [`for…of` statement](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/for...of) over the `forEach` method can include:
///
/// - Faster
/// - Better readability
/// - Ability to exit early with `break` or `return`
///
/// Additionally, using `for…of` has great benefits if you are using TypeScript, because it does not cause a function boundary to be crossed. This means that type-narrowing earlier on in the current scope will work properly while inside of the loop (without having to re-type-narrow). Furthermore, any mutated variables inside of the loop will picked up on for the purposes of determining if a variable is being used.
///
/// ### Example
/// ```javascript
/// // Bad
/// const foo = [1, 2, 3];
/// foo.forEach((element) => { /* ... */ });
///
/// // Good
/// const foo = [1, 2, 3];
/// for (const element of foo) { /* ... */ }
/// ```
NoArrayForEach,
restriction,
);

impl Rule for NoArrayForEach {
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};

let Some(member_expr) = (call_expr).callee.get_member_expr() else {
return;
};

let Some((_span, _)) = member_expr.static_property_info() else {
return;
};

if is_method_call(call_expr, None, Some(&["forEach"]), None, None)
&& !member_expr.is_computed()
{
let object = member_expr.object();

match object {
Expression::Identifier(ident) => {
if IGNORED_OBJECTS.contains(ident.name.as_str()) {
return;
}
}
Expression::MemberExpression(v) => {
if let Some(name) = v.static_property_name() {
if IGNORED_OBJECTS.contains(name) {
return;
}
}
}
_ => {}
}

let Some((span, _)) = member_expr.static_property_info() else {
return;
};

ctx.diagnostic(NoArrayForEachDiagnostic(span));
}
}
}

pub const IGNORED_OBJECTS: phf::Set<&'static str> = phf_set! {
"Children",
"r",
"pIteration",
};

#[test]
fn test() {
use crate::tester::Tester;

let pass = vec![
r"new foo.forEach(element => bar())",
r"forEach(element => bar())",
r"foo.notForEach(element => bar())",
r"React.Children.forEach(children, (child) => {});",
r"Children.forEach(children, (child) => {});",
];

let fail = vec![
r"foo.forEach?.(element => bar(element))",
r"1?.forEach((a, b) => call(a, b))",
r"array.forEach((arrayInArray) => arrayInArray.forEach(element => bar(element)));",
r"array.forEach((arrayInArray) => arrayInArray?.forEach(element => bar(element)));",
r"array.forEach((element, index = element) => {})",
r"array.forEach(({foo}, index = foo) => {})",
r"array.forEach((element, {bar = element}) => {})",
r"array.forEach(({foo}, {bar = foo}) => {})",
r"foo.forEach(function(element, element) {})",
r"foo.forEach(function element(element, element) {})",
r"this._listeners.forEach((listener: () => void) => listener());",
r"return foo.forEach(element => {bar(element)});",
];

Tester::new_without_config(NoArrayForEach::NAME, pass, fail).test_and_snapshot();
}
103 changes: 103 additions & 0 deletions crates/oxc_linter/src/snapshots/no_array_for_each.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
---
source: crates/oxc_linter/src/tester.rs
expression: no_array_for_each
---
eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1foo.forEach?.(element => bar(element))
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ 1?.forEach((a, b) => call(a, b))
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1array.forEach((arrayInArray) => arrayInArray.forEach(element => bar(element)));
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ array.forEach((arrayInArray) => arrayInArray.forEach(element => bar(element)));
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1array.forEach((arrayInArray) => arrayInArray?.forEach(element => bar(element)));
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ array.forEach((arrayInArray) => arrayInArray?.forEach(element => bar(element)));
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1array.forEach((element, index = element) => {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ array.forEach(({foo}, index = foo) => {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1array.forEach((element, {bar = element}) => {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ array.forEach(({foo}, {bar = foo}) => {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1foo.forEach(function(element, element) {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ foo.forEach(function element(element, element) {})
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.

eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1this._listeners.forEach((listener: () => void) => listener());
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.
⚠ eslint-plugin-unicorn(no-array-for-each): Do not use `Array#forEach`
╭─[no_array_for_each.tsx:1:1]
1 │ return foo.forEach(element => {bar(element)});
· ───────
╰────
help: Replace it with a for` loop. For loop is faster, more readable, and you can use `break` or `return` to exit early.


0 comments on commit 9e8ab1f

Please sign in to comment.