mathieu/granite-rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Rollup merge of #5871 - wiomoc:feature/methodcall-minmax, r=flip1995

Browse source 
Lint .min(x).max(y) with x < y

Fixes  #5854

changelog: Also lint `ord.min(a).max(b)`, where `a < b` in [`min_max`] lint
This commit is contained in:
Philipp Krones 2020-08-10 14:56:29 +02:00 • committed by GitHub
commit ee8db50e13
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 115 additions and 24 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

55
clippy_lints/src/minmax.rs
View file

@ -1,5 +1,6 @@
use crate::consts::{constant_simple, Constant};
use crate::utils::{match_def_path, paths, span_lint};
use crate::utils::{match_def_path, match_trait_method, paths, span_lint};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
@ -18,6 +19,10 @@ declare_clippy_lint! {
/// ```ignore
/// min(0, max(100, x))
/// ```
/// or
/// ```ignore
/// x.max(100).min(0)
/// ```
/// It will always be equal to `0`. Probably the author meant to clamp the value
/// between 0 and 100, but has erroneously swapped `min` and `max`.
pub MIN_MAX,
@ -60,25 +65,43 @@ enum MinMax {
}
fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Constant, &'a Expr<'a>)> {
if let ExprKind::Call(ref path, ref args) = expr.kind {
if let ExprKind::Path(ref qpath) = path.kind {
cx.typeck_results()
.qpath_res(qpath, path.hir_id)
.opt_def_id()
.and_then(|def_id| {
if match_def_path(cx, def_id, &paths::CMP_MIN) {
fetch_const(cx, args, MinMax::Min)
} else if match_def_path(cx, def_id, &paths::CMP_MAX) {
match expr.kind {
ExprKind::Call(ref path, ref args) => {
if let ExprKind::Path(ref qpath) = path.kind {
cx.typeck_results()
.qpath_res(qpath, path.hir_id)
.opt_def_id()
.and_then(|def_id| {
if match_def_path(cx, def_id, &paths::CMP_MIN) {
fetch_const(cx, args, MinMax::Min)
} else if match_def_path(cx, def_id, &paths::CMP_MAX) {
fetch_const(cx, args, MinMax::Max)
} else {
None
}
})
} else {
None
}
},
ExprKind::MethodCall(ref path, _, ref args, _) => {
if_chain! {
if let [obj, _] = args;
if cx.typeck_results().expr_ty(obj).is_floating_point() || match_trait_method(cx, expr, &paths::ORD);
then {
if path.ident.as_str() == sym!(max).as_str() {
fetch_const(cx, args, MinMax::Max)
} else if path.ident.as_str() == sym!(min).as_str() {
fetch_const(cx, args, MinMax::Min)
} else {
None
}
})
} else {
None
}
} else {
None
} else {
None
}
}
},
_ => None,
}
}

32
tests/ui/min_max.rs
View file

@ -6,6 +6,18 @@ use std::cmp::{max, min};
const LARGE: usize = 3;
struct NotOrd(u64);
impl NotOrd {
fn min(self, x: u64) -> NotOrd {
NotOrd(x)
}
fn max(self, x: u64) -> NotOrd {
NotOrd(x)
}
}
fn main() {
let x;
x = 2usize;
@ -30,4 +42,24 @@ fn main() {
max(min(s, "Apple"), "Zoo");
max("Apple", min(s, "Zoo")); // ok
let f = 3f32;
x.min(1).max(3);
x.max(3).min(1);
f.max(3f32).min(1f32);
x.max(1).min(3); // ok
x.min(3).max(1); // ok
f.min(3f32).max(1f32); // ok
max(x.min(1), 3);
min(x.max(1), 3); // ok
s.max("Zoo").min("Apple");
s.min("Apple").max("Zoo");
s.min("Zoo").max("Apple"); // ok
let not_ord = NotOrd(1);
not_ord.min(1).max(3); // ok
}

52
tests/ui/min_max.stderr
View file

@ -1,5 +1,5 @@
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:12:5
--> $DIR/min_max.rs:24:5
|
LL | min(1, max(3, x));
| ^^^^^^^^^^^^^^^^^
@ -7,40 +7,76 @@ LL | min(1, max(3, x));
= note: `-D clippy::min-max` implied by `-D warnings`
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:13:5
--> $DIR/min_max.rs:25:5
|
LL | min(max(3, x), 1);
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:14:5
--> $DIR/min_max.rs:26:5
|
LL | max(min(x, 1), 3);
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:15:5
--> $DIR/min_max.rs:27:5
|
LL | max(3, min(x, 1));
| ^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:17:5
--> $DIR/min_max.rs:29:5
|
LL | my_max(3, my_min(x, 1));
| ^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:29:5
--> $DIR/min_max.rs:41:5
|
LL | min("Apple", max("Zoo", s));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:30:5
--> $DIR/min_max.rs:42:5
|
LL | max(min(s, "Apple"), "Zoo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 7 previous errors
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:47:5
|
LL | x.min(1).max(3);
| ^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:48:5
|
LL | x.max(3).min(1);
| ^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:49:5
|
LL | f.max(3f32).min(1f32);
| ^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:55:5
|
LL | max(x.min(1), 3);
| ^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:58:5
|
LL | s.max("Zoo").min("Apple");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: this `min`/`max` combination leads to constant result
--> $DIR/min_max.rs:59:5
|
LL | s.min("Apple").max("Zoo");
| ^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 13 previous errors