Skip to content

feat(eslint-plugin): [no-misused-spread] add new rule #10551

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

Conversation

JoshuaKGoldberg
Copy link
Member

PR Checklist

Overview

Continuation of @StyleShit's #8509 as indicated in #8443 (comment).

Co-authored-by: @StyleShit

@JoshuaKGoldberg JoshuaKGoldberg requested a review from a team December 26, 2024 19:48
@JoshuaKGoldberg JoshuaKGoldberg force-pushed the no-misused-spread-ii-electric-boogaloo branch from 61e64b5 to b0c3919 Compare December 26, 2024 20:04
Copy link
Member

@ronami ronami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really excited to enable this on some of the projects I work on 🎉

Comment on lines 46 to 61
noArraySpreadInObject:
'Using the spread operator on an array in an object will result in a list of indices.',
noClassDeclarationSpreadInObject:
'Using the spread operator on class declarations will spread only their static properties, and will lose their class prototype.',
noClassInstanceSpreadInObject:
'Using the spread operator on class instances will lose their class prototype.',
noFunctionSpreadInObject:
'Using the spread operator on a function without additional properties can cause unexpected behavior. Did you forget to call the function?',
noIterableSpreadInObject:
'Using the spread operator on an Iterable in an object can cause unexpected behavior.',
noMapSpreadInObject:
'Using the spread operator on a Map in an object will result in an empty object. Did you mean to use `Object.fromEntries(map)` instead?',
noPromiseSpreadInObject:
'Using the spread operator on Promise in an object can cause unexpected behavior. Did you forget to await the promise?',
noStringSpreadInArray:
"Using the spread operator on a string can cause unexpected behavior. Prefer `String.split('')` instead.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Feat]: What do you think about adding a suggestion fixer for some of these? Some of the error messages suggest a particular way of fixing the warning.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really like the idea, but think it will end up being tricky like a lot of suggestion fixers. Would you be ok with it as a followup?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, that sounds great 👍

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For posterity: #10642

Copy link

codecov bot commented Dec 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.95%. Comparing base (d63ea3b) to head (b52123d).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10551      +/-   ##
==========================================
+ Coverage   86.90%   86.95%   +0.05%     
==========================================
  Files         446      447       +1     
  Lines       15505    15566      +61     
  Branches     4517     4535      +18     
==========================================
+ Hits        13475    13536      +61     
  Misses       1675     1675              
  Partials      355      355              
Flag Coverage Δ
unittest 86.95% <100.00%> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...kages/eslint-plugin/src/rules/no-misused-spread.ts 100.00% <100.00%> (ø)

@JoshuaKGoldberg JoshuaKGoldberg requested a review from ronami January 7, 2025 15:38
Copy link
Member

@ronami ronami left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks amazing, thank you 🥇

@JoshuaKGoldberg JoshuaKGoldberg added the 1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge label Jan 13, 2025
@JoshuaKGoldberg JoshuaKGoldberg merged commit 04166e0 into typescript-eslint:main Jan 13, 2025
65 checks passed
@JoshuaKGoldberg JoshuaKGoldberg deleted the no-misused-spread-ii-electric-boogaloo branch January 13, 2025 00:04
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Jan 13, 2025
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)

##### 🚀 Features

-   **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))
-   **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))
-   **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))

##### 🩹 Fixes

-   **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))
-   **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))

##### ❤️ Thank You

-   Josh Goldberg ✨
-   Ronen Amiel
-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)

You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Jan 13, 2025
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)

##### 🚀 Features

-   **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))
-   **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))
-   **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))

##### 🩹 Fixes

-   **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))
-   **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))

##### ❤️ Thank You

-   Josh Goldberg ✨
-   Ronen Amiel
-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)

You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Jan 14, 2025
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)

##### 🚀 Features

-   **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))
-   **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))
-   **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))

##### 🩹 Fixes

-   **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))
-   **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))

##### ❤️ Thank You

-   Josh Goldberg ✨
-   Ronen Amiel
-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)

You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
renovate bot added a commit to mmkal/eslint-plugin-mmkal that referenced this pull request Jan 15, 2025
##### [v8.20.0](https://github.com/typescript-eslint/typescript-eslint/blob/HEAD/packages/eslint-plugin/CHANGELOG.md#8200-2025-01-13)

##### 🚀 Features

-   **eslint-plugin:** \[consistent-type-assertions] add arrayLiteralTypeAssertions options ([#10565](typescript-eslint/typescript-eslint#10565))
-   **eslint-plugin:** \[no-deprecated] add allow options ([#10585](typescript-eslint/typescript-eslint#10585))
-   **eslint-plugin:** \[no-misused-spread] add new rule ([#10551](typescript-eslint/typescript-eslint#10551))

##### 🩹 Fixes

-   **eslint-plugin:** \[no-unnecessary-condition] don't flag optional chaining for union types with an unconstrained type parameters ([#10602](typescript-eslint/typescript-eslint#10602))
-   **eslint-plugin:** \[no-shadow] ignore ordering of type declarations ([#10593](typescript-eslint/typescript-eslint#10593))

##### ❤️ Thank You

-   Josh Goldberg ✨
-   Ronen Amiel
-   YeonJuan [@yeonjuan](https://github.com/yeonjuan)

You can read about our [versioning strategy](https://main--typescript-eslint.netlify.app/users/versioning) and [releases](https://main--typescript-eslint.netlify.app/users/releases) on our website.
@boris-petrov
Copy link

Once I updated, I got a conflict with unicorn/prefer-spread. They link to a comment in SO where the first example is:

// DO NOT USE THIS!
const a = '𝟘𝟙𝟚𝟛'.split('');
console.log(a);
// Output: ["�","�","�","�","�","�","�","�"]

So... who do I believe? 😄

@bradzacher
Copy link
Member

As per our contributing guide - please don't comment on closed PRs. If you think the rule is wrong, please file an issue and we can discuss there.

@typescript-eslint typescript-eslint locked and limited conversation to collaborators Jan 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1 approval >=1 team member has approved this PR; we're now leaving it open for more reviews before we merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rule proposal: prevent array, iterable and function spreads into objects
6 participants