Skip to content

Commit

Permalink
fix(linter/unicorn): allow set spreading in no-useless-spread (#4944)
Browse files Browse the repository at this point in the history
Closes #4872
  • Loading branch information
DonIsaac committed Aug 16, 2024
1 parent 4c989d0 commit e99836d
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ use crate::ast_util::{is_method_call, is_new_expression};
pub(super) enum ValueHint {
NewObject,
NewArray,
/// A non-array iterable.
///
/// Note that typed arrays are considered arrays, not iterables.
NewIterable,
Promise(Box<ValueHint>),
Unknown,
}
Expand All @@ -34,9 +38,12 @@ impl ValueHint {
impl std::ops::BitAnd for ValueHint {
type Output = Self;
fn bitand(self, rhs: Self) -> Self::Output {
// NOTE: what about (NewArray, NewIterable), e.g. in
// `foo ? new Set() : []`
match (self, rhs) {
(Self::NewArray, Self::NewArray) => Self::NewArray,
(Self::NewObject, Self::NewObject) => Self::NewObject,
(Self::NewIterable, Self::NewIterable) => Self::NewIterable,
_ => Self::Unknown,
}
}
Expand Down Expand Up @@ -81,8 +88,10 @@ impl<'a> ConstEval for Argument<'a> {

impl<'a> ConstEval for NewExpression<'a> {
fn const_eval(&self) -> ValueHint {
if is_new_array(self) || is_new_map_or_set(self) || is_new_typed_array(self) {
if is_new_array(self) || is_new_typed_array(self) {
ValueHint::NewArray
} else if is_new_map_or_set(self) {
ValueHint::NewIterable
} else if is_new_object(self) {
ValueHint::NewObject
} else {
Expand Down
24 changes: 5 additions & 19 deletions crates/oxc_linter/src/rules/unicorn/no_useless_spread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,23 +492,6 @@ fn get_method_name(call_expr: &CallExpression) -> Option<String> {
Some(format!("{}.{}", object_name, callee.static_property_name().unwrap()))
}

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

let pass = vec![];

let fail = vec![
// "[...arr.reduce(f, new Set())]",
"const obj = { a, ...{ b, c } }",
// "const promise = Promise.all([...iterable])",
// "const obj = { ...(foo ? { a: 1 } : { a: 2 }) }",
// "const array = [...[a]]",
];

Tester::new(NoUselessSpread::NAME, pass, fail).test();
}

#[test]
fn test() {
use crate::tester::Tester;
Expand Down Expand Up @@ -578,13 +561,18 @@ fn test() {
// r"[...Int8Array.from(foo)]",
// r"[...Int8Array.of()]",
// r"[...new Int8Array(3)]",
r"[...new Set(iter)]",
r"[...Promise.all(foo)]",
r"[...Promise.allSettled(foo)]",
r"[...await Promise.all(foo, extraArgument)]",
// Complex object spreads
r"const obj = { ...obj, ...(addFoo ? { foo: 'foo' } : {}) }",
r"<Button {...(isLoading ? { data: undefined } : { data: dataFromApi })} />",
r"const obj = { ...(foo ? getObjectInOpaqueManner() : { a: 2 }) }",
"[...arr.reduce((set, b) => set.add(b), new Set())]",
"[...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() : [])]",
];

let fail = vec![
Expand Down Expand Up @@ -707,8 +695,6 @@ fn test() {
"[...arr.reduce((a, b) => a.push(b), Array.from(iter))]",
"[...arr.reduce((a, b) => a.push(b), foo.map(x => x))]",
"[...arr.reduce((a, b) => a.push(b), await Promise.all(promises))]",
"[...arr.reduce((set, b) => set.add(b), new Set())]",
"[...arr.reduce((set, b) => set.add(b), new Set(iter))]",
// useless object clones with complex expressions
r"const obj = { ...(foo ? { a: 1 } : { a: 2 }) }",
r"const obj = { ...(foo ? Object.entries(obj).reduce(fn, {}) : { a: 2 }) }",
Expand Down
14 changes: 0 additions & 14 deletions crates/oxc_linter/src/snapshots/no_useless_spread.snap
Original file line number Diff line number Diff line change
Expand Up @@ -788,20 +788,6 @@ source: crates/oxc_linter/src/tester.rs
╰────
help: `arr.reduce` 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 │ [...arr.reduce((set, b) => set.add(b), new Set())]
· ───
╰────
help: `arr.reduce` 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 │ [...arr.reduce((set, b) => set.add(b), new Set(iter))]
· ───
╰────
help: `arr.reduce` 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 object unnecessarily.
╭─[no_useless_spread.tsx:1:15]
1 │ const obj = { ...(foo ? { a: 1 } : { a: 2 }) }
Expand Down

0 comments on commit e99836d

Please sign in to comment.