Skip to content

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