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

[new rule] Disallow !?? (Non-null + nullish) #2853

Closed
fregante opened this issue Dec 6, 2020 · 7 comments · Fixed by #3349
Closed

[new rule] Disallow !?? (Non-null + nullish) #2853

fregante opened this issue Dec 6, 2020 · 7 comments · Fixed by #3349
Labels
enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin

Comments

@fregante
Copy link
Contributor

fregante commented Dec 6, 2020

! in TypeScript (non-null assertion operator) cancels the null type and ?? (nullish coalescing operator) actually handles it. The first one is redundant.

This is a suggestion for a new rule:

Fail

return document.querySelector('a')! ?? defaultElement

Pass

return document.querySelector('a') ?? defaultElement

@fregante fregante added package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin triage Waiting for maintainers to take a look labels Dec 6, 2020
@bradzacher
Copy link
Member

Seems like a logical cousin to no-non-null-asserted-optional-chain

@bradzacher bradzacher added enhancement: new plugin rule New rule request for eslint-plugin and removed triage Waiting for maintainers to take a look labels Dec 6, 2020
sonallux added a commit to sonallux/typescript-eslint that referenced this issue May 4, 2021
sonallux added a commit to sonallux/typescript-eslint that referenced this issue May 4, 2021
@ypresto
Copy link
Contributor

ypresto commented Aug 30, 2021

It seems covered by no-unnecessary-conditional?

image

@fregante
Copy link
Contributor Author

fregante commented Aug 30, 2021

You're right, no-unnecessary-condition was disabled in my preset due to

My example is covered by that rule:

Screen Shot 4

This issue can be closed as far as I'm concerned.

@ypresto
Copy link
Contributor

ypresto commented Aug 30, 2021

It would be appreciated if you can paste the code (example) caused false-positive in your project. Just at assigning array element to variable?

@fregante
Copy link
Contributor Author

There's no false positive. My example matches your example except it uses the same API I used initially.

@bradzacher
Copy link
Member

Thanks for pointing that out @ypresto!

For reference this could also be handled via no-restricted-syntax:
https://github.com/typescript-eslint/typescript-eslint/blob/master/docs/getting-started/linting/FAQ.md#how-can-i-ban-specific-language-feature

{
  "no-restricted-syntax": [
    "error",
     {
       "selector": "LogicalExpression[operator='??'] > TSNonNullExpression.left",
       "message": "Why the heck are you non-nulling a nullish-coalesce?"
     },
  ],
}

(dump the config into here to see it working!)

Closing as there are multiple ways to achieve this without a new rule.


For reference - no-non-null-asserted-optional-chain makes sense as a rule because it's not achievable with a simple selector alone due to the complexity of the optional chaining AST.

@fregante
Copy link
Contributor Author

fregante commented Aug 30, 2021

It would be appreciated if you can paste the code (example) caused false-positive in your project

Oh maybe you were referring to this:

I didn't personally encounter the issue; I said "my preset" but I meant "the preset I use"

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 30, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement: new plugin rule New rule request for eslint-plugin package: eslint-plugin Issues related to @typescript-eslint/eslint-plugin
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants