Skip to content
This repository has been archived by the owner on Mar 25, 2021. It is now read-only.

Allow fast null checks in no-unused-expression rule does not cover some cases #4021

Closed
gnapse opened this issue Jul 9, 2018 · 9 comments
Closed

Comments

@gnapse
Copy link

gnapse commented Jul 9, 2018

The allow-fast-null-checks option for the no-unused-expression rule, which was added in #1638, is currently not covering all cases. Today it's only meant for allowing expressions that check if a variable is truth before calling a method on that variable:

// This is what's currently supported
e && e.preventDefault()

I propose that it should also cover cases that check if a variable is truth before calling it as a function:

// This is not currently supported, but it should be.
callback && callback();

And perhaps there are other cases as well. If someone can think of any, please add them here so it can be covered in a future PR for this issue as well.

@alamothe
Copy link

This pattern does not work either:

this.props.onClick && this.props.onClick()

@gnapse
Copy link
Author

gnapse commented Oct 27, 2018

@alamothe that is effectively the same pattern the issue raised. It doesn't matter if the nullable thing is a variable, or a property inside an object.

@alamothe
Copy link

Right, sorry about that, I guess my point was that the pattern is fairly common inside React.

@XeniaSiskaki
Copy link

XeniaSiskaki commented Apr 17, 2019

x && y() also doesn't work

@gnapse
Copy link
Author

gnapse commented Apr 17, 2019

@XeniaSiskaki why would you expect that to work? If x is not null, that does not tell you anything about the ability to call y as a function.

@XeniaSiskaki
Copy link

It's the short way of saying if (x) y(); which is pretty useful. I'm just saying that there should be a way to not have tslint complain.

@gnapse
Copy link
Author

gnapse commented Apr 17, 2019

tslint complains if the type of y makes it potentially null or undefined. And the if (x) at the front of it does not remove the possibility of y not being a function at the moment of it being called. If you really want not to be warned about it, then why use TS at all? Or if you're sure you know what you're doing and want TS out of your way in this particular case (for instance if you know that if x is not null, then y is also not null for sure, but TS has no way of knowing), then do this:

if (x) y!();

But my point is, this is not an issue with TS, and it's not related to the issue I brought up here, which is a real thing tslint could improve.

@JoshuaKGoldberg
Copy link
Contributor

☠️ TSLint's time has come! ☠️

TSLint is no longer accepting most feature requests per #4534. See typescript-eslint.io for the new, shiny way to lint your TypeScript code with ESLint. ✨

It was a pleasure open sourcing with you all!

@JoshuaKGoldberg
Copy link
Contributor

🤖 Beep boop! 👉 TSLint is deprecated 👈 (#4534) and you should switch to typescript-eslint! 🤖

🔒 This issue is being locked to prevent further unnecessary discussions. Thank you! 👋

@palantir palantir locked and limited conversation to collaborators Mar 20, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants