github / codeql

CodeQL: the libraries and queries that power security researchers around the world, as well as code scanning in GitHub Advanced Security
https://codeql.github.com
MIT License
7.69k stars 1.54k forks source link

LGTM.com - false positive #2596

Closed brettz9 closed 4 years ago

brettz9 commented 4 years ago

Description of the false positive

This is not compare typeof s to null but s to null, so I don't see any reason this should be giving an issue here. (I know the code is currently redundant as far as the undefined check, but that is not the same.)

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/dragfyre/bahai-date-api/alerts?mode=list

max-schaefer commented 4 years ago

Can you explain a bit more why you consider this a false positive? The alert says that s cannot be of type null, which is true, since it has just been ==-compared to undefined, so it can be neither null nor undefined. I don't see any reference to typeof s in the alert message.

Or do you mean that the alert message is confusing? Could we perhaps improve it to clarify what it means?

brettz9 commented 4 years ago

Oh, silly me. Yes, I see s == undefined will be true for null (I think I'm more used to seeing and converting == null checks to ... === null || ... === undefined that I had a mental block thinking == undefined was, on the other hand, ok to only check undefined. Thanks! And my apologies... No, I think the message is clear, I just had forgotten that when comparing with undefined.