Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(linter/unicorn): allow set spreading in no-useless-spread #4944

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
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