From 78f7e3745fe59db1e6309b1e42ea25df3317ee36 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 24 May 2022 11:50:12 -0400 Subject: [PATCH 1/2] Fix `manual_range_contains` with equal precedence --- clippy_lints/src/ranges.rs | 13 +++++++++++-- tests/ui/range_contains.fixed | 5 +++++ tests/ui/range_contains.rs | 5 +++++ tests/ui/range_contains.stderr | 26 +++++++++++++++++++++++++- 4 files changed, 46 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index a47dc26f6..416d6de55 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -194,7 +194,7 @@ impl<'tcx> LateLintPass<'tcx> for Ranges { }, ExprKind::Binary(ref op, l, r) => { if meets_msrv(self.msrv, msrvs::RANGE_CONTAINS) { - check_possible_range_contains(cx, op.node, l, r, expr); + check_possible_range_contains(cx, op.node, l, r, expr, expr.span); } }, _ => {}, @@ -213,12 +213,12 @@ fn check_possible_range_contains( left: &Expr<'_>, right: &Expr<'_>, expr: &Expr<'_>, + span: Span, ) { if in_constant(cx, expr.hir_id) { return; } - let span = expr.span; let combine_and = match op { BinOpKind::And | BinOpKind::BitAnd => true, BinOpKind::Or | BinOpKind::BitOr => false, @@ -294,6 +294,15 @@ fn check_possible_range_contains( ); } } + + // If the LHS is the same operator, we have to recurse to get the "real" RHS, since they have + // the same operator precedence. + if let ExprKind::Binary(ref lhs_op, _left, new_lhs) = left.kind { + if op == lhs_op.node { + let new_span = Span::new(new_lhs.span.lo(), right.span.hi(), expr.span.ctxt(), expr.span.parent()); + check_possible_range_contains(cx, op, new_lhs, right, expr, new_span); + } + } } struct RangeBounds<'a> { diff --git a/tests/ui/range_contains.fixed b/tests/ui/range_contains.fixed index f49771997..fd4dcf39f 100644 --- a/tests/ui/range_contains.fixed +++ b/tests/ui/range_contains.fixed @@ -49,6 +49,11 @@ fn main() { x >= 10 && x <= -10; (-3. ..=3.).contains(&y); y >= 3. && y <= -3.; + + // Fix #8745 + let z = 42; + (0..=10).contains(&x) && (0..=10).contains(&z); + !(0..10).contains(&x) || !(0..10).contains(&z); } // Fix #6373 diff --git a/tests/ui/range_contains.rs b/tests/ui/range_contains.rs index 9e2180b0c..b848e2e16 100644 --- a/tests/ui/range_contains.rs +++ b/tests/ui/range_contains.rs @@ -49,6 +49,11 @@ fn main() { x >= 10 && x <= -10; y >= -3. && y <= 3.; y >= 3. && y <= -3.; + + // Fix #8745 + let z = 42; + (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); + (x < 0) || (x >= 10) || (z < 0) || (z >= 10); } // Fix #6373 diff --git a/tests/ui/range_contains.stderr b/tests/ui/range_contains.stderr index 1817ee171..936859db5 100644 --- a/tests/ui/range_contains.stderr +++ b/tests/ui/range_contains.stderr @@ -96,5 +96,29 @@ error: manual `RangeInclusive::contains` implementation LL | y >= -3. && y <= 3.; | ^^^^^^^^^^^^^^^^^^^ help: use: `(-3. ..=3.).contains(&y)` -error: aborting due to 16 previous errors +error: manual `RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:55:30 + | +LL | (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); + | ^^^^^^^^^^^^^^^^^^^^^ help: use: `(0..=10).contains(&z)` + +error: manual `RangeInclusive::contains` implementation + --> $DIR/range_contains.rs:55:5 + | +LL | (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); + | ^^^^^^^^^^^^^^^^^^^^^ help: use: `(0..=10).contains(&x)` + +error: manual `!Range::contains` implementation + --> $DIR/range_contains.rs:56:29 + | +LL | (x < 0) || (x >= 10) || (z < 0) || (z >= 10); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `!(0..10).contains(&z)` + +error: manual `!Range::contains` implementation + --> $DIR/range_contains.rs:56:5 + | +LL | (x < 0) || (x >= 10) || (z < 0) || (z >= 10); + | ^^^^^^^^^^^^^^^^^^^^ help: use: `!(0..10).contains(&x)` + +error: aborting due to 20 previous errors From 257f09776a9c68ac903d1f23b80c414b70de1185 Mon Sep 17 00:00:00 2001 From: Evan Typanski Date: Tue, 24 May 2022 14:09:56 -0400 Subject: [PATCH 2/2] Fix issue with mismatched parens in suggestion --- clippy_lints/src/ranges.rs | 13 +++++++++---- tests/ui/range_contains.fixed | 2 ++ tests/ui/range_contains.rs | 2 ++ 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/ranges.rs b/clippy_lints/src/ranges.rs index 416d6de55..584b561dc 100644 --- a/clippy_lints/src/ranges.rs +++ b/clippy_lints/src/ranges.rs @@ -296,10 +296,15 @@ fn check_possible_range_contains( } // If the LHS is the same operator, we have to recurse to get the "real" RHS, since they have - // the same operator precedence. - if let ExprKind::Binary(ref lhs_op, _left, new_lhs) = left.kind { - if op == lhs_op.node { - let new_span = Span::new(new_lhs.span.lo(), right.span.hi(), expr.span.ctxt(), expr.span.parent()); + // the same operator precedence + if_chain! { + if let ExprKind::Binary(ref lhs_op, _left, new_lhs) = left.kind; + if op == lhs_op.node; + let new_span = Span::new(new_lhs.span.lo(), right.span.hi(), expr.span.ctxt(), expr.span.parent()); + if let Some(snip) = &snippet_opt(cx, new_span); + // Do not continue if we have mismatched number of parens, otherwise the suggestion is wrong + if snip.matches('(').count() == snip.matches(')').count(); + then { check_possible_range_contains(cx, op, new_lhs, right, expr, new_span); } } diff --git a/tests/ui/range_contains.fixed b/tests/ui/range_contains.fixed index fd4dcf39f..85d021b2f 100644 --- a/tests/ui/range_contains.fixed +++ b/tests/ui/range_contains.fixed @@ -54,6 +54,8 @@ fn main() { let z = 42; (0..=10).contains(&x) && (0..=10).contains(&z); !(0..10).contains(&x) || !(0..10).contains(&z); + // Make sure operators in parens don't give a breaking suggestion + ((x % 2 == 0) || (x < 0)) || (x >= 10); } // Fix #6373 diff --git a/tests/ui/range_contains.rs b/tests/ui/range_contains.rs index b848e2e16..9a7a75dc1 100644 --- a/tests/ui/range_contains.rs +++ b/tests/ui/range_contains.rs @@ -54,6 +54,8 @@ fn main() { let z = 42; (x >= 0) && (x <= 10) && (z >= 0) && (z <= 10); (x < 0) || (x >= 10) || (z < 0) || (z >= 10); + // Make sure operators in parens don't give a breaking suggestion + ((x % 2 == 0) || (x < 0)) || (x >= 10); } // Fix #6373