Addresses #1955 - Suggests >= y + 1 become > y #2060
Conversation
This module handles the following cases: - `... >= ... + 1` and `... >= 1 + ...` - `... - 1 >= ...` and `-1 + ... >= ...` - `... + 1 <= ...` and `... + 1 <= ...` - `... <= ... - 1` and `... <= -1 + ...` Note: this only goes 1 level deep (i.e., does not constant-fold) and does not currently simplify expressions. Examples of these cases include: ```rust let x = 1; y >= y + x; // won't catch this case or any permutation x + 1 >= y + 2; // won't catch this case x + 1 - 1 >= y - 1 + 1; // WILL catch this case when it likely shouldn't ```
mrecachinas
changed the title
|
Yay! Thanks for starting this lint. The detection code is perfect. The actual lint call should use |
|
Sounds good -- I'll make those changes now. Do you think the message should be a bit more specific with respect to adding 1 or subtracting 1, or do you think the general message gets across? |
|
I think the lint docs are where such details belong. The lint message itself is good |
|
@oli-obk Sorry for the delay -- check out the latest commit and let me know your thoughts. I can change it to return |
clippy_lints/src/int_plus_one.rs
Outdated
| false | ||
| } | ||
|
|
||
| fn check_binop(&self, cx: &EarlyContext, block: &Expr, binop: BinOpKind, lhs: &Expr, rhs: &Expr) -> Option<(bool, Option<String>)> { |
There was a problem hiding this comment.
I'm very confused as to how this function successfully compiles ( the return type matches no returned value)
There was a problem hiding this comment.
Good catch -- I started moving toward returning the (bool, String) tuple I described above, but I forgot to remove the return value.
I'm not sure why the compiler didn't catch that though. AFAIK there is no implicit None returned.
|
You can remove the bool and inner option and just make the entire return type |
|
If you have any other recommendations to reduce the duplicated code, I'm happy to do it in this PR! Perhaps having the match statements produce the arguments to |
|
I think any further deduplication will only make the code less comprehensible. This looks great now. Let's activate it and see how it goes in the wild :D |
This is a first pass, and as such, doesn't address a few cases (illustrated below).
This module handles the following cases:
... >= ... + 1and... >= 1 + ...... - 1 >= ...and-1 + ... >= ...... + 1 <= ...and... + 1 <= ...... <= ... - 1and... <= -1 + ...Note: this only goes 1 level deep (i.e., does not constant-fold or
perform constant propagation) and does not currently simplify
expressions.
Examples of these cases include:
Interested in your thoughts and suggestions. :)