coderaiser / putout

🐊 Pluggable and configurable JavaScript Linter, code transformer and formatter, drop-in ESLint superpower replacement 💪 with built-in support for js, jsx, typescript, flow, markdown, yaml and json. Write declarative codemods in a simplest possible way 😏
https://putout.cloudcmd.io/
MIT License
705 stars 40 forks source link

Package: conditions/convert-equal-to-strict-equal - Comparisons to null not safe to transform #139

Closed epreston closed 1 year ago

epreston commented 1 year ago

Package: conditions/convert-equal-to-strict-equal Issue: Comparisons to null not safe to transform

Example:

if (test != null) {
  // do something
}

This is not safe to transform into strict compare with !== when null is the right hand side of the expression. Often, the "loose equality comparison that performs type coercion if necessary" is desired to catch both null and undefined.

Detailed reference on the issue: https://stackoverflow.com/a/15992131/900263


Update:

Version: Version: 29.3

Description: yoda expressions are reversed but continue to be evaluated in the original order. This causes rule exclusions to be bypassed.

coderaiser commented 1 year ago

Are you up for a PR?

epreston commented 1 year ago

Almost, 11 more hours of testing. So many new goodies.

coderaiser commented 1 year ago

Wait a minute, as I see this case already handled:

https://github.com/coderaiser/putout/blob/ac566372d30937a7dba1e9282dd013e4478034f8/packages/plugin-conditions/lib/convert-equal-to-strict-equal/index.js#L5-L13

And it has tests:

https://github.com/coderaiser/putout/blob/ac566372d30937a7dba1e9282dd013e4478034f8/packages/plugin-conditions/lib/convert-equal-to-strict-equal/index.spec.js#L20-L28

How did you trap on it?

epreston commented 1 year ago

I was getting conversions in 2 cases. The normal one and when the operands reversed.

Will update with test cases.

epreston commented 1 year ago

I may have started with null != test that got corrected to test != null but then the transform was applied.

I have this habit of doing that because assigning to null by mistake triggers instant error. It also ensures that I can not assign null to the variable by missing a character. It's a habit from another language at the cost of readability.

coderaiser commented 1 year ago

May the yoda rule of ESLint help you with this.

epreston commented 1 year ago

Confirmed, I had statements like null != test that got reversed then snuck through.

The updated issue description would be:

yoda expressions are reversed but continue to be evaluated in the original order. This causes rule exclusions to be bypassed.

epreston commented 1 year ago

Retesting for 29.4.0 update

coderaiser commented 1 year ago

Retesting for 29.4.0 update

What purpose of this retest? I don't think it will be fixed if you don't send PR.

epreston commented 1 year ago

Oh.. ok. This is just a quick note to say that I'm not leaving old issues open from previous versions.
I'll get to work on it.

coderaiser commented 1 year ago

Closed due to a long time of inactivity 🏝. Fee free to reopen when you have chance to work on this.