From 8f65651bea8f7b7bccab76388f9d9a7c5fc2f7ed Mon Sep 17 00:00:00 2001 From: shulaoda Date: Fri, 18 Oct 2024 17:27:57 +0800 Subject: [PATCH 1/3] fix(linter): remove unsafe fixer of no-useless-spread --- .../rules/unicorn/no_useless_spread/mod.rs | 21 ++++++++++++++----- .../src/snapshots/no_useless_spread.snap | 7 ------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs index 294fdc14d7d5e..3052ed008efb7 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs @@ -380,13 +380,26 @@ fn check_useless_clone<'a>( is_array: bool, spread_elem: &SpreadElement<'a>, ctx: &LintContext<'a>, -) -> bool { +) { let span = Span::new(spread_elem.span.start, spread_elem.span.start + 3); let target = spread_elem.argument.get_inner_expression(); // already diagnosed by first check if matches!(target, Expression::ArrayExpression(_) | Expression::ObjectExpression(_)) { - return false; + return; + } + + // unsafe fixer of `[...new Array(1)]` + if let Expression::NewExpression(new_expr) = target { + let is_array_constructor = new_expr + .callee + .without_parentheses() + .get_identifier_reference() + .is_some_and(|id| id.name == "Array"); + + if is_array_constructor && new_expr.arguments.len() == 1 { + return; + } } let hint = target.const_eval(); @@ -397,9 +410,7 @@ fn check_useless_clone<'a>( ctx.diagnostic_with_fix(clone(span, is_array, name), |fixer| { fix_by_removing_array_spread(fixer, &array_or_obj_span, spread_elem) }); - return true; } - false } fn diagnostic_name<'a>(ctx: &LintContext<'a>, expr: &Expression<'a>) -> Option<&'a str> { @@ -577,6 +588,7 @@ fn test() { "[...arr.reduce((set, b) => set.add(b), new Set(iter))]", // NOTE: we may want to consider this a violation in the future "[...(foo ? new Set() : [])]", + "[...new Array(3)]", ]; let fail = vec![ @@ -666,7 +678,6 @@ fn test() { r"[...foo.toSpliced(0, 1)]", r"[...foo.with(0, bar)]", r#"[...foo.split("|")]"#, - r"[...new Array(3)]", r"[...Object.keys(foo)]", r"[...Object.values(foo)]", r"[...Array.from(foo)]", diff --git a/crates/oxc_linter/src/snapshots/no_useless_spread.snap b/crates/oxc_linter/src/snapshots/no_useless_spread.snap index 522474c9e1eec..c8a9ecba3a2aa 100644 --- a/crates/oxc_linter/src/snapshots/no_useless_spread.snap +++ b/crates/oxc_linter/src/snapshots/no_useless_spread.snap @@ -637,13 +637,6 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: `foo.split` returns a new array. Spreading it into an array expression to create a new array is redundant. - ⚠ eslint-plugin-unicorn(no-useless-spread): Using a spread operator here creates a new array unnecessarily. - ╭─[no_useless_spread.tsx:1:2] - 1 │ [...new Array(3)] - · ─── - ╰──── - help: `new Array(3)` returns a new array. Spreading it into an array expression to create a new array is redundant. - ⚠ eslint-plugin-unicorn(no-useless-spread): Using a spread operator here creates a new array unnecessarily. ╭─[no_useless_spread.tsx:1:2] 1 │ [...Object.keys(foo)] From d34dbc568617b400c05a5ce209d539424d74fec2 Mon Sep 17 00:00:00 2001 From: shulaoda Date: Sat, 19 Oct 2024 22:58:25 +0800 Subject: [PATCH 2/3] refactor: improve fixer --- .../rules/unicorn/no_useless_spread/mod.rs | 35 +++++++++++-------- .../src/snapshots/no_useless_spread.snap | 7 ++++ 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs index 3052ed008efb7..a7952e60c9077 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs @@ -389,24 +389,30 @@ fn check_useless_clone<'a>( return; } - // unsafe fixer of `[...new Array(1)]` - if let Expression::NewExpression(new_expr) = target { - let is_array_constructor = new_expr - .callee - .without_parentheses() - .get_identifier_reference() - .is_some_and(|id| id.name == "Array"); - - if is_array_constructor && new_expr.arguments.len() == 1 { - return; - } - } - let hint = target.const_eval(); let hint_matches_expr = if is_array { hint.is_array() } else { hint.is_object() }; if hint_matches_expr { let name = diagnostic_name(ctx, target); + // `[...new Array(1)]` -> `new Array(1).fill()` + if let Expression::NewExpression(new_expr) = target { + let is_array_constructor = new_expr + .callee + .without_parentheses() + .get_identifier_reference() + .is_some_and(|id| id.name == "Array"); + + if is_array_constructor && new_expr.arguments.len() == 1 { + ctx.diagnostic_with_fix(clone(span, is_array, name), |fixer| { + fixer.replace( + array_or_obj_span, + format!("{}.fill()", fixer.source_range(spread_elem.argument.span())), + ) + }); + return; + } + } + ctx.diagnostic_with_fix(clone(span, is_array, name), |fixer| { fix_by_removing_array_spread(fixer, &array_or_obj_span, spread_elem) }); @@ -588,7 +594,6 @@ fn test() { "[...arr.reduce((set, b) => set.add(b), new Set(iter))]", // NOTE: we may want to consider this a violation in the future "[...(foo ? new Set() : [])]", - "[...new Array(3)]", ]; let fail = vec![ @@ -678,6 +683,7 @@ fn test() { r"[...foo.toSpliced(0, 1)]", r"[...foo.with(0, bar)]", r#"[...foo.split("|")]"#, + r"[...new Array(3)]", r"[...Object.keys(foo)]", r"[...Object.values(foo)]", r"[...Array.from(foo)]", @@ -736,6 +742,7 @@ fn test() { // useless clones - simple arrays ("[...foo.map(x => !!x)]", "foo.map(x => !!x)"), ("[...new Array()]", "new Array()"), + ("[...new Array(3)]", "new Array(3).fill()"), ("[...new Array(1, 2, 3)]", "new Array(1, 2, 3)"), // useless clones - complex (r"[...await Promise.all(foo)]", r"await Promise.all(foo)"), diff --git a/crates/oxc_linter/src/snapshots/no_useless_spread.snap b/crates/oxc_linter/src/snapshots/no_useless_spread.snap index c8a9ecba3a2aa..522474c9e1eec 100644 --- a/crates/oxc_linter/src/snapshots/no_useless_spread.snap +++ b/crates/oxc_linter/src/snapshots/no_useless_spread.snap @@ -637,6 +637,13 @@ source: crates/oxc_linter/src/tester.rs ╰──── help: `foo.split` returns a new array. Spreading it into an array expression to create a new array is redundant. + ⚠ eslint-plugin-unicorn(no-useless-spread): Using a spread operator here creates a new array unnecessarily. + ╭─[no_useless_spread.tsx:1:2] + 1 │ [...new Array(3)] + · ─── + ╰──── + help: `new Array(3)` returns a new array. Spreading it into an array expression to create a new array is redundant. + ⚠ eslint-plugin-unicorn(no-useless-spread): Using a spread operator here creates a new array unnecessarily. ╭─[no_useless_spread.tsx:1:2] 1 │ [...Object.keys(foo)] From b6af37afd94e2d158961b99a61efef209ce896ce Mon Sep 17 00:00:00 2001 From: Boshen Date: Mon, 28 Oct 2024 17:04:19 +0800 Subject: [PATCH 3/3] fix_dangerous --- crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs index a7952e60c9077..efa71f8b7bb85 100644 --- a/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs +++ b/crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs @@ -143,7 +143,7 @@ declare_oxc_lint!( /// ``` NoUselessSpread, correctness, - conditional_fix + fix_dangerous ); impl Rule for NoUselessSpread {