Skip to content

Commit e0bcc8c

Browse files
committed
feat(linter/unicorn): implement no-unnecessary-array-splice-count
1 parent b3b482a commit e0bcc8c

File tree

4 files changed

+395
-0
lines changed

4 files changed

+395
-0
lines changed

crates/oxc_linter/src/generated/rule_runner_impls.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,6 +2513,13 @@ impl RuleRunner
25132513
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
25142514
}
25152515

2516+
impl RuleRunner
2517+
for crate::rules::unicorn::no_unnecessary_array_splice_count::NoUnnecessaryArraySpliceCount
2518+
{
2519+
const NODE_TYPES: Option<&AstTypesBitset> =
2520+
Some(&AstTypesBitset::from_types(&[AstType::CallExpression]));
2521+
}
2522+
25162523
impl RuleRunner for crate::rules::unicorn::no_unnecessary_await::NoUnnecessaryAwait {
25172524
const NODE_TYPES: Option<&AstTypesBitset> =
25182525
Some(&AstTypesBitset::from_types(&[AstType::AwaitExpression]));

crates/oxc_linter/src/rules.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -432,6 +432,7 @@ pub(crate) mod unicorn {
432432
pub mod no_this_assignment;
433433
pub mod no_typeof_undefined;
434434
pub mod no_unnecessary_array_flat_depth;
435+
pub mod no_unnecessary_array_splice_count;
435436
pub mod no_unnecessary_await;
436437
pub mod no_unnecessary_slice_end;
437438
pub mod no_unreadable_array_destructuring;
@@ -1129,6 +1130,7 @@ oxc_macros::declare_all_lint_rules! {
11291130
unicorn::explicit_length_check,
11301131
unicorn::filename_case,
11311132
unicorn::new_for_builtins,
1133+
unicorn::no_unnecessary_array_splice_count,
11321134
unicorn::no_array_callback_reference,
11331135
unicorn::no_array_sort,
11341136
unicorn::no_array_reverse,
Lines changed: 258 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,258 @@
1+
use std::borrow::Cow;
2+
3+
use oxc_ast::{
4+
AstKind,
5+
ast::{Argument, Expression, MemberExpression, StaticMemberExpression},
6+
match_member_expression,
7+
};
8+
use oxc_diagnostics::OxcDiagnostic;
9+
use oxc_macros::declare_oxc_lint;
10+
use oxc_span::{GetSpan, Span};
11+
12+
use crate::{
13+
AstNode, ast_util::is_method_call, context::LintContext, rule::Rule, utils::is_same_expression,
14+
};
15+
16+
fn no_unnecessary_array_splice_count_diagnostic(span: Span, arg_str: &str) -> OxcDiagnostic {
17+
OxcDiagnostic::warn(format!(
18+
"Passing `{arg_str}` as the `deleteCount` argument is unnecessary."
19+
))
20+
.with_help("Omit the argument to delete all elements after the start index.")
21+
.with_label(span)
22+
}
23+
24+
#[derive(Debug, Default, Clone)]
25+
pub struct NoUnnecessaryArraySpliceCount;
26+
27+
// See <https://github.com/oxc-project/oxc/issues/6050> for documentation details.
28+
declare_oxc_lint!(
29+
/// ### What it does
30+
///
31+
/// Disallows passing `.length` or `Infinity` as the `deleteCount` or `skipCount` argument of `Array#splice()` or `Array#toSpliced()`.
32+
///
33+
/// ### Why is this bad?
34+
///
35+
/// When calling `Array#splice(start, deleteCount)` or `Array#toSpliced(start, skipCount)`,
36+
/// omitting the `deleteCount` or `skipCount` argument will delete or skip all elements after `start`.
37+
/// Using `.length` or `Infinity` is unnecessary and makes the code more verbose.
38+
///
39+
/// ### Examples
40+
///
41+
/// Examples of **incorrect** code for this rule:
42+
/// ```js
43+
/// array.splice(1, array.length);
44+
/// array.splice(1, Infinity);
45+
/// array.splice(1, Number.POSITIVE_INFINITY);
46+
/// array.toSpliced(1, array.length);
47+
/// ```
48+
///
49+
/// Examples of **correct** code for this rule:
50+
/// ```js
51+
/// array.splice(1);
52+
/// array.toSpliced(1);
53+
/// ```
54+
NoUnnecessaryArraySpliceCount,
55+
unicorn,
56+
pedantic,
57+
fix
58+
);
59+
60+
impl Rule for NoUnnecessaryArraySpliceCount {
61+
fn run<'a>(&self, node: &AstNode<'a>, ctx: &LintContext<'a>) {
62+
let AstKind::CallExpression(call_expr) = node.kind() else {
63+
return;
64+
};
65+
66+
if call_expr.optional
67+
|| !is_method_call(call_expr, None, Some(&["splice", "toSpliced"]), Some(2), Some(2))
68+
{
69+
return;
70+
}
71+
72+
let Some(MemberExpression::StaticMemberExpression(member_expr)) =
73+
call_expr.callee.as_member_expression()
74+
else {
75+
return;
76+
};
77+
78+
if matches!(member_expr.object, Expression::CallExpression(_))
79+
|| call_expr.arguments.iter().any(|arg| matches!(arg, Argument::SpreadElement(_)))
80+
{
81+
return;
82+
}
83+
84+
let [first_arg, second_arg] = call_expr.arguments.as_slice() else {
85+
return;
86+
};
87+
88+
let Some(arg_expr) = second_arg.as_expression().map(Expression::without_parentheses) else {
89+
return;
90+
};
91+
92+
match arg_expr {
93+
Expression::Identifier(ident) if ident.name.as_str() == "Infinity" => {
94+
ctx.diagnostic_with_fix(
95+
no_unnecessary_array_splice_count_diagnostic(second_arg.span(), "Infinity"),
96+
|fixer| {
97+
fixer.delete_range(Span::new(first_arg.span().end, second_arg.span().end))
98+
},
99+
);
100+
}
101+
Expression::ChainExpression(chain_expr) => {
102+
if let Some(expr) = chain_expr.expression.as_member_expression()
103+
&& let Some(msg) =
104+
check_expression_and_get_diagnostic(member_expr, expr, true, ctx)
105+
{
106+
ctx.diagnostic_with_fix(
107+
no_unnecessary_array_splice_count_diagnostic(second_arg.span(), &msg),
108+
|fixer| {
109+
fixer.delete_range(Span::new(
110+
first_arg.span().end,
111+
second_arg.span().end,
112+
))
113+
},
114+
);
115+
}
116+
}
117+
match_member_expression!(Expression) => {
118+
let expr = arg_expr.to_member_expression();
119+
if let Some(msg) =
120+
check_expression_and_get_diagnostic(member_expr, expr, false, ctx)
121+
{
122+
ctx.diagnostic_with_fix(
123+
no_unnecessary_array_splice_count_diagnostic(second_arg.span(), &msg),
124+
|fixer| {
125+
fixer.delete_range(Span::new(
126+
first_arg.span().end,
127+
second_arg.span().end,
128+
))
129+
},
130+
);
131+
}
132+
}
133+
_ => {}
134+
}
135+
}
136+
}
137+
138+
fn check_expression_and_get_diagnostic<'a>(
139+
left: &StaticMemberExpression,
140+
right: &MemberExpression,
141+
is_chain_expr: bool,
142+
ctx: &'a LintContext,
143+
) -> Option<Cow<'a, str>> {
144+
let MemberExpression::StaticMemberExpression(right) = right else {
145+
return None;
146+
};
147+
let property = right.property.name.as_str();
148+
if ctx.source_range(right.span()) == "Number.POSITIVE_INFINITY" {
149+
return Some("Number.POSITIVE_INFINITY".into());
150+
}
151+
if property == "length" && is_same_expression(&left.object, &right.object, ctx) {
152+
if matches!(left.object, Expression::Identifier(_)) {
153+
return Some(ctx.source_range(right.span()).into());
154+
}
155+
let operator = if is_chain_expr { "?." } else { "." };
156+
return Some(format!("…{operator}length").into());
157+
}
158+
None
159+
}
160+
161+
#[test]
162+
fn test() {
163+
use crate::tester::Tester;
164+
165+
let pass = vec![
166+
"foo.splice?.(1, foo.length)",
167+
"foo.splice(foo.length, 1)",
168+
"foo.splice()",
169+
"foo.splice(1)",
170+
"foo.splice(1, foo.length - 1)",
171+
"foo.splice(1, foo.length, extraArgument)",
172+
"foo.splice(...[1], foo.length)",
173+
"foo.not_splice(1, foo.length)",
174+
"new foo.splice(1, foo.length)",
175+
"splice(1, foo.length)",
176+
"foo.splice(1, foo.notLength)",
177+
"foo.splice(1, length)",
178+
"foo[splice](1, foo.length)",
179+
"foo.splice(1, foo[length])",
180+
"foo.splice(1, bar.length)",
181+
"foo?.splice(1, NotInfinity)",
182+
"foo?.splice(1, Number.NOT_POSITIVE_INFINITY)",
183+
"foo?.splice(1, Not_Number.POSITIVE_INFINITY)",
184+
"foo?.splice(1, Number?.POSITIVE_INFINITY)",
185+
"foo().splice(1, foo().length)",
186+
"foo.toSpliced?.(1, foo.length)",
187+
"foo.toSpliced(foo.length, 1)",
188+
"foo.toSpliced()",
189+
"foo.toSpliced(1)",
190+
"foo.toSpliced(1, foo.length - 1)",
191+
"foo.toSpliced(1, foo.length, extraArgument)",
192+
"foo.toSpliced(...[1], foo.length)",
193+
"foo.not_toSpliced(1, foo.length)",
194+
"new foo.toSpliced(1, foo.length)",
195+
"toSpliced(1, foo.length)",
196+
"foo.toSpliced(1, foo.notLength)",
197+
"foo.toSpliced(1, length)",
198+
"foo[toSpliced](1, foo.length)",
199+
"foo.toSpliced(1, foo[length])",
200+
"foo.toSpliced(1, bar.length)",
201+
"foo?.toSpliced(1, NotInfinity)",
202+
"foo?.toSpliced(1, Number.NOT_POSITIVE_INFINITY)",
203+
"foo?.toSpliced(1, Not_Number.POSITIVE_INFINITY)",
204+
"foo?.toSpliced(1, Number?.POSITIVE_INFINITY)",
205+
"foo().toSpliced(1, foo().length)",
206+
];
207+
208+
let fail = vec![
209+
"foo.splice(1, foo.length)",
210+
"foo?.splice(1, foo.length)",
211+
"foo.splice(1, foo.length,)",
212+
"foo.splice(1, (( foo.length )))",
213+
"foo.splice(1, foo?.length)",
214+
"foo?.splice(1, foo?.length)",
215+
"foo?.splice(1, Infinity)",
216+
"foo?.splice(1, Number.POSITIVE_INFINITY)",
217+
"foo.bar.splice(1, foo.bar.length)",
218+
"foo.toSpliced(1, foo.length)",
219+
"foo?.toSpliced(1, foo.length)",
220+
"foo.toSpliced(1, foo.length,)",
221+
"foo.toSpliced(1, (( foo.length )))",
222+
"foo.toSpliced(1, foo?.length)",
223+
"foo?.toSpliced(1, foo?.length)",
224+
"foo?.toSpliced(1, Infinity)",
225+
"foo?.toSpliced(1, Number.POSITIVE_INFINITY)",
226+
"foo.bar.toSpliced(1, foo.bar.length)",
227+
];
228+
229+
let fix = vec![
230+
("foo.splice(1, foo.length)", "foo.splice(1)"),
231+
("foo?.splice(1, foo.length)", "foo?.splice(1)"),
232+
("foo.splice(1, foo.length,)", "foo.splice(1,)"),
233+
("foo.splice(1, (( foo.length )))", "foo.splice(1)"),
234+
("foo.splice(1, foo?.length)", "foo.splice(1)"),
235+
("foo?.splice(1, foo?.length)", "foo?.splice(1)"),
236+
("foo?.splice(1, Infinity)", "foo?.splice(1)"),
237+
("foo?.splice(1, Number.POSITIVE_INFINITY)", "foo?.splice(1)"),
238+
("foo.bar.splice(1, foo.bar.length)", "foo.bar.splice(1)"),
239+
("foo.toSpliced(1, foo.length)", "foo.toSpliced(1)"),
240+
("foo?.toSpliced(1, foo.length)", "foo?.toSpliced(1)"),
241+
("foo.toSpliced(1, foo.length,)", "foo.toSpliced(1,)"),
242+
("foo.toSpliced(1, (( foo.length )))", "foo.toSpliced(1)"),
243+
("foo.toSpliced(1, foo?.length)", "foo.toSpliced(1)"),
244+
("foo?.toSpliced(1, foo?.length)", "foo?.toSpliced(1)"),
245+
("foo?.toSpliced(1, Infinity)", "foo?.toSpliced(1)"),
246+
("foo?.toSpliced(1, Number.POSITIVE_INFINITY)", "foo?.toSpliced(1)"),
247+
("foo.bar.toSpliced(1, foo.bar.length)", "foo.bar.toSpliced(1)"),
248+
];
249+
250+
Tester::new(
251+
NoUnnecessaryArraySpliceCount::NAME,
252+
NoUnnecessaryArraySpliceCount::PLUGIN,
253+
pass,
254+
fail,
255+
)
256+
.expect_fix(fix)
257+
.test_and_snapshot();
258+
}

0 commit comments

Comments
 (0)