hrzndhrn / recode

A linter with autocorrection and a refactoring tool.
MIT License
281 stars 15 forks source link

Unnecessary If/Unless #89

Closed sodapopcan closed 1 day ago

sodapopcan commented 1 week ago

Here's my first pass at collapsing what I'm calling "redundant booleans," ie:

if expr, do: true, else: false

I don't quite like that name and am still thinking on it but I needed something! It only takes case of do: true, else: false because I realized it could be taken further but I decided that isn't worth it. do: false, else: true could protentially be re-written with the if expression negated. You have to chose between ! and not (possibly making it configurable) and it seems like one of those things that should just be a credo warning.

I only dealt with leading comments on the if line though I can be more thorough. Again, it just didn't seem worth it since people usually write these "redundant booleans" pretty absent-mindedly and can't imagine they'd be full of comments in the do/else bodies. I'm also not exactly sure how to keep comments on the same line. "Leading comments" are comments both above and after a line. I'll study Sourceror a little more but I just wanted to get this up for review.

I'm certainly open to different ideas on the name! I was even thinking something as dumb as NoDoTrueElseFalse πŸ˜… πŸ˜… πŸ˜…

Thanks!

NickNeck commented 1 week ago

Thanks a lot. I will take a look at the weekend.

NickNeck commented 1 week ago

Already not looked at the code, but my suggestion for the name would be UnnecessaryIfUnless. As the name suggests, the implementation should also include unless.

To rewrite if expr, do: false, else: true to not expr would be nice, but we can leave this for later (or never).

sodapopcan commented 1 week ago

All feedback has been addressed including rewriting the corresponding unlesses.

I'm happy to squash a succinct commit if/when you're happy with it!

coveralls commented 1 week ago

Pull Request Test Coverage Report for Build 298454e56b33de8ccb84c5e6d60c1c5841f3358b-PR-89

Details


Changes Missing Coverage Covered Lines Changed/Added Lines %
lib/recode/task/unnecessary_if_unless.ex 20 21 95.24%
<!-- Total: 20 21 95.24% -->
Totals Coverage Status
Change from base Build a97ec42c4b51b478e237027cea09efa4e41f3539: 0.08%
Covered Lines: 958
Relevant Lines: 1047

πŸ’› - Coveralls
sodapopcan commented 1 week ago

Do you want me to squash this or does your CI handle that?

NickNeck commented 1 week ago

Do you want me to squash this or does your CI handle that?

I will handle this with GitHub.