Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/oxc_linter/src/generated/rule_runner_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2493,6 +2493,11 @@ impl RuleRunner for crate::rules::unicorn::no_array_reduce::NoArrayReduce {
const ANY_NODE_TYPE: bool = true;
}

impl RuleRunner for crate::rules::unicorn::no_array_reverse::NoArrayReverse {
const NODE_TYPES: &AstTypesBitset = &AstTypesBitset::new();
const ANY_NODE_TYPE: bool = true;
}

impl RuleRunner for crate::rules::unicorn::no_await_expression_member::NoAwaitExpressionMember {
const NODE_TYPES: &AstTypesBitset = &AstTypesBitset::new();
const ANY_NODE_TYPE: bool = true;
Expand Down
2 changes: 2 additions & 0 deletions crates/oxc_linter/src/rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -397,6 +397,7 @@ pub(crate) mod unicorn {
pub mod no_array_for_each;
pub mod no_array_method_this_argument;
pub mod no_array_reduce;
pub mod no_array_reverse;
pub mod no_await_expression_member;
pub mod no_await_in_promise_methods;
pub mod no_console_spaces;
Expand Down Expand Up @@ -1102,6 +1103,7 @@ oxc_macros::declare_all_lint_rules! {
unicorn::explicit_length_check,
unicorn::filename_case,
unicorn::new_for_builtins,
unicorn::no_array_reverse,
unicorn::no_instanceof_builtins,
unicorn::no_array_method_this_argument,
unicorn::no_unnecessary_array_flat_depth,
Expand Down
165 changes: 165 additions & 0 deletions crates/oxc_linter/src/rules/unicorn/no_array_reverse.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
use oxc_ast::{
AstKind,
ast::{ArrayExpressionElement, Expression},
};
use oxc_diagnostics::OxcDiagnostic;
use oxc_macros::declare_oxc_lint;
use oxc_span::Span;
use serde_json::Value;

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

fn no_array_reverse_diagnostic(span: Span) -> OxcDiagnostic {
OxcDiagnostic::warn("Use `Array#toReversed()` instead of `Array#reverse()`.")
.with_help("`Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.")
.with_label(span)
}

#[derive(Debug, Clone)]
pub struct NoArrayReverse {
allow_expression_statement: bool,
}

impl Default for NoArrayReverse {
fn default() -> Self {
Self { allow_expression_statement: true }
}
}

declare_oxc_lint!(
/// ### What it does
///
/// Prefer using `Array#toReversed()` over `Array#reverse()`.
///
/// ### Why is this bad?
///
/// `Array#reverse()` modifies the original array in place, which can lead to unintended side effects—especially
/// when the original array is used elsewhere in the code.
///
/// ### Examples
///
/// Examples of **incorrect** code for this rule:
/// ```js
/// const reversed = [...array].reverse();
/// ```
///
/// Examples of **correct** code for this rule:
/// ```js
/// const reversed = [...array].toReversed();
/// ```
///
/// ### Options
///
/// #### allowExpressionStatement
///
/// `{ type: boolean, default: true }`
///
/// This rule allow `array.reverse()` as an expression statement by default,
/// Pass allowExpressionStatement: false to forbid `Array#reverse()` even it's an expression statement.
///
/// Examples of **incorrect** code for this rule with the `{ "allowExpressionStatement": false }` option:
/// ```js
/// array.reverse();
/// ```
NoArrayReverse,
unicorn,
suspicious,
fix,
);

impl Rule for NoArrayReverse {
fn from_configuration(value: Value) -> Self {
Self {
allow_expression_statement: value
.get(0)
.and_then(|v| v.get("allowExpressionStatement"))
.and_then(Value::as_bool)
.unwrap_or(true),
}
}

fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
let AstKind::CallExpression(call_expr) = node.kind() else {
return;
};
if !call_expr.arguments.is_empty() || call_expr.optional {
return;
}
let Some(member_expr) = call_expr.callee.get_member_expr() else {
return;
};
let Some((span, static_property_name)) = member_expr.static_property_info() else {
return;
};
if static_property_name != "reverse" {
return;
}
let is_spread = match member_expr.object() {
Expression::ArrayExpression(array) => {
array.elements.len() == 1
&& matches!(array.elements[0], ArrayExpressionElement::SpreadElement(_))
}
_ => false,
};
if self.allow_expression_statement && !is_spread {
let parent = ctx.nodes().parent_node(node.id());
let parent_is_expression_statement = match parent.kind() {
AstKind::ExpressionStatement(_) => true,
AstKind::ChainExpression(_) => {
let grand_parent = ctx.nodes().parent_node(parent.id());
matches!(grand_parent.kind(), AstKind::ExpressionStatement(_))
}
_ => false,
};
if parent_is_expression_statement {
return;
}
}
ctx.diagnostic_with_fix(no_array_reverse_diagnostic(span), |fixer| {
fixer.replace(span, "toReversed")
});
}
}

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

let pass = vec![
("reversed =[...array].toReversed()", None),
("reversed =array.toReversed()", None),
("reversed =[...array].reverse", None),
("reversed =[...array].reverse?.()", None),
("array.reverse()", None),
("array.reverse?.()", None),
("array?.reverse()", None),
("if (true) array.reverse()", None),
("reversed = array.reverse(extraArgument)", None),
];

let fail = vec![
("reversed = [...array].reverse()", None),
("reversed = [...array]?.reverse()", None),
("reversed = array.reverse()", None),
("reversed = array?.reverse()", None),
("array.reverse()", Some(serde_json::json!([{"allowExpressionStatement": false}]))),
("array?.reverse()", Some(serde_json::json!([{"allowExpressionStatement": false}]))),
("[...array].reverse()", Some(serde_json::json!([{"allowExpressionStatement": true}]))),
("reversed = [...(0, array)].reverse()", None),
];

let fix = vec![
("reversed = [...array].reverse()", "reversed = [...array].toReversed()", None),
("reversed = [...array]?.reverse()", "reversed = [...array]?.toReversed()", None),
(
"a.reverse()",
"a.toReversed()",
Some(serde_json::json!([{"allowExpressionStatement": false}])),
),
("reversed = array?.reverse()", "reversed = array?.toReversed()", None),
];

Tester::new(NoArrayReverse::NAME, NoArrayReverse::PLUGIN, pass, fail)
.expect_fix(fix)
.test_and_snapshot();
}
58 changes: 58 additions & 0 deletions crates/oxc_linter/src/snapshots/unicorn_no_array_reverse.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
---
source: crates/oxc_linter/src/tester.rs
---
⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:23]
1 │ reversed = [...array].reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:24]
1 │ reversed = [...array]?.reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:18]
1 │ reversed = array.reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:19]
1 │ reversed = array?.reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:7]
1 │ array.reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:8]
1 │ array?.reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:12]
1 │ [...array].reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.

⚠ eslint-plugin-unicorn(no-array-reverse): Use `Array#toReversed()` instead of `Array#reverse()`.
╭─[no_array_reverse.tsx:1:28]
1 │ reversed = [...(0, array)].reverse()
· ───────
╰────
help: `Array#reverse()` mutates the original array. Use `Array#toReversed()` to return a new reversed array without modifying the original.
Loading