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

no-negated-condition rule could detect and fix (not (eq... and others #2655

Closed
godric3 opened this issue Oct 23, 2022 · 5 comments · Fixed by #2653
Closed

no-negated-condition rule could detect and fix (not (eq... and others #2655

godric3 opened this issue Oct 23, 2022 · 5 comments · Fixed by #2653

Comments

@godric3
Copy link
Contributor

godric3 commented Oct 23, 2022

Negation connected with some other helpers could be simplified:

  • (not (eq ...)) -> (not-eq ...)
  • (not (not-eq ...)) -> (eq ...)
  • (not (gt ...)) -> (lte ...)
  • (not (gte ...)) -> (lt ...)
  • (not (lt ...)) -> (gte ...)
  • (not (lte ...)) -> (gt ...)

Cases with eq and not-eq looks harmless to autofix, but rest may not always be desirable so it could be enabled with some setting.

@bmish bmish added the breaking label Oct 23, 2022
@bmish
Copy link
Member

bmish commented Oct 23, 2022

Why wouldn't they all be safe and desirable?

I'm a big fan of boolean simplification rules. Such rules exist in other plugins, e.g.

@bmish
Copy link
Member

bmish commented Oct 23, 2022

This could be part of the next major release #2319 as a breaking change if the PR is ready.

Could also put it all behind a new option on the rule (off by default) to avoid a breaking change and release it sooner (with the option to be enabled by default in next major release). This is the my preferred option.

@godric3
Copy link
Contributor Author

godric3 commented Oct 23, 2022

Why wouldn't they all be safe and desirable?

That was just a remark as I encountered situation where some one wanted to have comparisons always done in one direction even if that would require negation. But objectively looking it won't break anything and code will be simplified :)

@bmish
Copy link
Member

bmish commented Oct 23, 2022

If someone has a personal preference on that, they can disable the rule :) but boolean simplification is a good idea for a rule.

@godric3
Copy link
Contributor Author

godric3 commented Oct 23, 2022

Hmm as some code for that would be based on code from #2653 maybe we will make both helpers mentioned here and double negation hidden behind the option simplifyHelpers(?) that will be false on default and changed to true on next major release?
If yes then I would rework existing PR to include this issue there

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

Successfully merging a pull request may close this issue.

2 participants