Skip to content

noUnusedLocals doesn't error when using a ...rest declaration #61748

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

Open
scamden opened this issue May 21, 2025 · 6 comments
Open

noUnusedLocals doesn't error when using a ...rest declaration #61748

scamden opened this issue May 21, 2025 · 6 comments

Comments

@scamden
Copy link

scamden commented May 21, 2025

πŸ”Ž Search Terms

desctructure noUnusedLocals rest

πŸ•— Version & Regression Information

  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about noUnusedLocals

⏯ Playground Link

https://www.typescriptlang.org/play/?noUncheckedIndexedAccess=true&noUnusedLocals=true&noUnusedParameters=true&target=99#code/GYVwdgxgLglg9mABAWznAFASkQbwFCKGIQIDOUiAhogLy7UBciAjADSIBGiTATAL4EiJMOVyd2AOikAnAKai+tKgG5BhNcTIUIS5qoFA

πŸ’» Code

function moo() {
    const A = {a : 1, b : 2}
    // no error on b
    const { b, ...rest } = A;
    // error on a as expected
    const { a } = A
    const c = 1;
}

πŸ™ Actual behavior

no error on an unused local

πŸ™‚ Expected behavior

error on any unused local whether a ...rest param is used or not

Additional information about the issue

No response

@MartinJohns
Copy link
Contributor

MartinJohns commented May 22, 2025

This is working as intended. It's indirectly used by removing the variable from the rest. If you remove it, you will get different behavior (it's suddenly part of the rest again).

@scamden
Copy link
Author

scamden commented May 22, 2025

Hmm yes but the variable itself is unused and I expected an error there.. I can always rename it with an underscore prefix to avoid putting it back in the rest.

I’ve since enabled the typescript-eslint version of this rule which lets me error for this so I’m unblocked personally but this seems pretty unexpected

@MartinJohns
Copy link
Contributor

MartinJohns commented May 22, 2025

Completely expected to me. :-) And intentional, see #12778. Having to write b: _b for every property would really be a bloated bummer.

@scamden
Copy link
Author

scamden commented May 22, 2025

Yes but forgetting to use a variable you intended to use is a correctness bummer which bothers me more than typing a few more characters.

The linked issue seems to assume that destructured properties with a rest sibling are only intended to remove properties from the rest element but in my experience that’s actually the less common intention. Usually, I intend to use both the destructured properties and the rest element.

I can see how this is convenient when your intention is to use destructuring as a poor man’s omit but I would argue that using an actual omit utility would be more clear for that whereas it’s more clear and expected to use all local declarations.

Regardless as I said I’m unblocked but I do feel strongly that trading correctness for convenience is a poor trade off.

@nmain
Copy link

nmain commented May 22, 2025

correctness

Having an unused variable is not a lapse in correctness, and warnings or errors for such situations are closer to lints than true compiler errors. Given that, exactly what to error on is more of a judgment call and convenience is important.

@snarbles2
Copy link

It is inconsistent and would have confused me just the same to discover that noUnusedLocals acts more like someUnusedLocals.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants