fix(integer_division_remainder_used): also detect inherent division/r… - #17618
Conversation
…emainder methods fixes rust-lang#17603
rustbot
added
the
S-waiting-on-community-reviews
|
Thanks for the pull request. A reviewer will take a look after it receives 2 community reviews. In the meantime, we would highly appreciate if you could try to review any of PRs waiting on community reviews. |
rustbot
added
the
S-waiting-on-review
|
|
@rustbot ready |
|
Lintcheck changes for 91d00cf
This comment will be updated if you push new changes |
There was a problem hiding this comment.
community review: needs a bit of cleanup, but generally looks like the right path
@rustbot author
| "checked_div" | ||
| | "checked_div_euclid" | ||
| | "checked_div_exact" | ||
| | "checked_rem" | ||
| | "checked_rem_euclid" | ||
| | "div_ceil" | ||
| | "div_euclid" | ||
| | "div_exact" | ||
| | "overflowing_div" | ||
| | "overflowing_div_euclid" | ||
| | "overflowing_rem" | ||
| | "overflowing_rem_euclid" | ||
| | "rem_euclid" | ||
| | "strict_div" | ||
| | "strict_div_euclid" | ||
| | "saturating_div" | ||
| | "strict_rem" | ||
| | "strict_rem_euclid" | ||
| | "wrapping_div" | ||
| | "wrapping_div_euclid" | ||
| | "wrapping_rem" | ||
| | "wrapping_rem_euclid" |
There was a problem hiding this comment.
please check using sym::* instead, since more efficient (?), better typed, ... and generally superiour
| } | ||
| // check method call is present in specific list if yes also lint it | ||
| pub(super) fn check_method_call(cx: &LateContext<'_>, method_name: &str, receiver: &Expr<'_>, span: Span) { | ||
| let instance_ty = cx.typeck_results().expr_ty(receiver); |
There was a problem hiding this comment.
please change the order here that we first check the name, then cx.typeck_results... since performance wise this is the better order
| let x: u16 = 7; | ||
| let _ = x.div_ceil(3); | ||
| //~^ integer_division_remainder_used | ||
|
|
||
| let _ = x.wrapping_div(3); | ||
| //~^ integer_division_remainder_used | ||
|
|
||
| let _ = x.checked_rem(3); | ||
| //~^ integer_division_remainder_used | ||
|
|
||
| let f: f64 = 7.0; | ||
| let _ = f.div_euclid(3.0); | ||
| } |
There was a problem hiding this comment.
for each of the methods noted above, I would expect one testcase, so please copy paste the list and add a testcase for each 😉
Since div_euclid does not lint here, also keep the cases where it lints and does not for each.
| // check method call is present in specific list if yes also lint it | ||
| pub(super) fn check_method_call(cx: &LateContext<'_>, method_name: &str, receiver: &Expr<'_>, span: Span) { |
There was a problem hiding this comment.
Does not seem like very usefull docs.. I can see this by glossing over the fn..
Also not a doc comment
| // check method call is present in specific list if yes also lint it | |
| pub(super) fn check_method_call(cx: &LateContext<'_>, method_name: &str, receiver: &Expr<'_>, span: Span) { | |
| pub(super) fn check_method_call(cx: &LateContext<'_>, method_name: &str, receiver: &Expr<'_>, span: Span) { |
rustbot
removed
the
S-waiting-on-review
|
Reminder, once the PR becomes ready for a review, use |
rustbot
added
the
S-waiting-on-author
3 participants
integer_division_remainder_usedonly caught the/and%operators, missing equivalent inherent methods like.div_ceil(),.wrapping_div(),.checked_rem(), etc. Now checks method calls too, matching against the method name and confirming the receiver is actually an integer type (so it doesn't fire on things likef64::div_euclid, which shares a name but isn't the same concern).fixes #17603
changelog: [
integer_division_remainder_used]: also detect inherent integer division/remainder methods, not just the/and%operators