From b31a3dff480ffad248fa5b2c66fbe9b9d6726314 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 29 Jan 2020 15:45:42 +0100 Subject: [PATCH 1/4] Small refactor of mutable_debug_assertions --- clippy_lints/src/mutable_debug_assertion.rs | 48 ++++++++++----------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index 71a93071e..94ef61dcf 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -61,38 +61,38 @@ fn extract_call<'a, 'tcx>(cx: &'a LateContext<'a, 'tcx>, e: &'tcx Expr<'_>) -> O if block.stmts.len() == 1; if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind; then { + // debug_assert if_chain! { if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind; if let ExprKind::DropTemps(ref droptmp) = ifclause.kind; if let ExprKind::Unary(UnOp::UnNot, ref condition) = droptmp.kind; then { - // debug_assert let mut visitor = MutArgVisitor::new(cx); visitor.visit_expr(condition); return visitor.expr_span(); - } else { - // debug_assert_{eq,ne} - if_chain! { - if let ExprKind::Block(ref matchblock, _) = matchexpr.kind; - if let Some(ref matchheader) = matchblock.expr; - if let ExprKind::Match(ref headerexpr, _, _) = matchheader.kind; - if let ExprKind::Tup(ref conditions) = headerexpr.kind; - if conditions.len() == 2; - then { - if let ExprKind::AddrOf(BorrowKind::Ref, _, ref lhs) = conditions[0].kind { - let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(lhs); - if let Some(span) = visitor.expr_span() { - return Some(span); - } - } - if let ExprKind::AddrOf(BorrowKind::Ref, _, ref rhs) = conditions[1].kind { - let mut visitor = MutArgVisitor::new(cx); - visitor.visit_expr(rhs); - if let Some(span) = visitor.expr_span() { - return Some(span); - } - } + } + } + + // debug_assert_{eq,ne} + if_chain! { + if let ExprKind::Block(ref matchblock, _) = matchexpr.kind; + if let Some(ref matchheader) = matchblock.expr; + if let ExprKind::Match(ref headerexpr, _, _) = matchheader.kind; + if let ExprKind::Tup(ref conditions) = headerexpr.kind; + if conditions.len() == 2; + then { + if let ExprKind::AddrOf(BorrowKind::Ref, _, ref lhs) = conditions[0].kind { + let mut visitor = MutArgVisitor::new(cx); + visitor.visit_expr(lhs); + if let Some(span) = visitor.expr_span() { + return Some(span); + } + } + if let ExprKind::AddrOf(BorrowKind::Ref, _, ref rhs) = conditions[1].kind { + let mut visitor = MutArgVisitor::new(cx); + visitor.visit_expr(rhs); + if let Some(span) = visitor.expr_span() { + return Some(span); } } } From 9fbd7f478e59cca00011b6ac0ed89bada8823e2a Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 29 Jan 2020 15:46:38 +0100 Subject: [PATCH 2/4] Don't trigger `debug_assert_with_mut_call` on `.await` --- clippy_lints/src/mutable_debug_assertion.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index 94ef61dcf..881f59bba 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -4,7 +4,7 @@ use matches::matches; use rustc::hir::map::Map; use rustc::ty; use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; -use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability, StmtKind, UnOp}; +use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability, StmtKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::Span; @@ -147,6 +147,8 @@ impl<'a, 'tcx> Visitor<'tcx> for MutArgVisitor<'a, 'tcx> { } } }, + // Don't check await desugars + ExprKind::Match(_, _, MatchSource::AwaitDesugar) => return, _ if !self.found => self.expr_span = Some(expr.span), _ => return, } From 006f07f090894f09910fe6f15e5a670a955f8166 Mon Sep 17 00:00:00 2001 From: flip1995 Date: Wed, 29 Jan 2020 15:47:22 +0100 Subject: [PATCH 3/4] Add test for `await` in `debug_assert!(..)` --- tests/ui/debug_assert_with_mut_call.rs | 9 ++++ tests/ui/debug_assert_with_mut_call.stderr | 58 +++++++++++----------- 2 files changed, 38 insertions(+), 29 deletions(-) diff --git a/tests/ui/debug_assert_with_mut_call.rs b/tests/ui/debug_assert_with_mut_call.rs index a588547b9..3db7e0164 100644 --- a/tests/ui/debug_assert_with_mut_call.rs +++ b/tests/ui/debug_assert_with_mut_call.rs @@ -1,5 +1,7 @@ +// compile-flags: --edition=2018 #![feature(custom_inner_attributes)] #![rustfmt::skip] +#![warn(clippy::debug_assert_with_mut_call)] #![allow(clippy::trivially_copy_pass_by_ref, clippy::cognitive_complexity, clippy::redundant_closure_call)] struct S; @@ -114,6 +116,12 @@ fn misc() { })()); } +async fn debug_await() { + debug_assert!(async { + true + }.await); +} + fn main() { func_non_mutable(); func_mutable(); @@ -121,4 +129,5 @@ fn main() { method_mutable(); misc(); + debug_await(); } diff --git a/tests/ui/debug_assert_with_mut_call.stderr b/tests/ui/debug_assert_with_mut_call.stderr index 48c7f4ea8..a2ca71b57 100644 --- a/tests/ui/debug_assert_with_mut_call.stderr +++ b/tests/ui/debug_assert_with_mut_call.stderr @@ -1,169 +1,169 @@ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:40:19 + --> $DIR/debug_assert_with_mut_call.rs:42:19 | LL | debug_assert!(bool_mut(&mut 3)); | ^^^^^^^^^^^^^^^^ | - = note: `#[deny(clippy::debug_assert_with_mut_call)]` on by default + = note: `-D clippy::debug-assert-with-mut-call` implied by `-D warnings` error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:41:20 + --> $DIR/debug_assert_with_mut_call.rs:43:20 | LL | debug_assert!(!bool_mut(&mut 3)); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:43:25 + --> $DIR/debug_assert_with_mut_call.rs:45:25 | LL | debug_assert_eq!(0, u32_mut(&mut 3)); | ^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:44:22 + --> $DIR/debug_assert_with_mut_call.rs:46:22 | LL | debug_assert_eq!(u32_mut(&mut 3), 0); | ^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:46:25 + --> $DIR/debug_assert_with_mut_call.rs:48:25 | LL | debug_assert_ne!(1, u32_mut(&mut 3)); | ^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:47:22 + --> $DIR/debug_assert_with_mut_call.rs:49:22 | LL | debug_assert_ne!(u32_mut(&mut 3), 1); | ^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:62:19 + --> $DIR/debug_assert_with_mut_call.rs:64:19 | LL | debug_assert!(S.bool_self_mut()); | ^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:63:20 + --> $DIR/debug_assert_with_mut_call.rs:65:20 | LL | debug_assert!(!S.bool_self_mut()); | ^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:64:19 + --> $DIR/debug_assert_with_mut_call.rs:66:19 | LL | debug_assert!(S.bool_self_ref_arg_mut(&mut 3)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:65:19 + --> $DIR/debug_assert_with_mut_call.rs:67:19 | LL | debug_assert!(S.bool_self_mut_arg_ref(&3)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:66:19 + --> $DIR/debug_assert_with_mut_call.rs:68:19 | LL | debug_assert!(S.bool_self_mut_arg_mut(&mut 3)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:68:22 + --> $DIR/debug_assert_with_mut_call.rs:70:22 | LL | debug_assert_eq!(S.u32_self_mut(), 0); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:69:22 + --> $DIR/debug_assert_with_mut_call.rs:71:22 | LL | debug_assert_eq!(S.u32_self_mut_arg_ref(&3), 0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:70:22 + --> $DIR/debug_assert_with_mut_call.rs:72:22 | LL | debug_assert_eq!(S.u32_self_ref_arg_mut(&mut 3), 0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:71:22 + --> $DIR/debug_assert_with_mut_call.rs:73:22 | LL | debug_assert_eq!(S.u32_self_mut_arg_mut(&mut 3), 0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:73:22 + --> $DIR/debug_assert_with_mut_call.rs:75:22 | LL | debug_assert_ne!(S.u32_self_mut(), 1); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:74:22 + --> $DIR/debug_assert_with_mut_call.rs:76:22 | LL | debug_assert_ne!(S.u32_self_mut_arg_ref(&3), 1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:75:22 + --> $DIR/debug_assert_with_mut_call.rs:77:22 | LL | debug_assert_ne!(S.u32_self_ref_arg_mut(&mut 3), 1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:76:22 + --> $DIR/debug_assert_with_mut_call.rs:78:22 | LL | debug_assert_ne!(S.u32_self_mut_arg_mut(&mut 3), 1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:84:22 + --> $DIR/debug_assert_with_mut_call.rs:86:22 | LL | debug_assert_eq!(v.pop(), Some(1)); | ^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_ne!` - --> $DIR/debug_assert_with_mut_call.rs:85:31 + --> $DIR/debug_assert_with_mut_call.rs:87:31 | LL | debug_assert_ne!(Some(3), v.pop()); | ^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:88:19 + --> $DIR/debug_assert_with_mut_call.rs:90:19 | LL | debug_assert!(bool_mut(a)); | ^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:91:31 + --> $DIR/debug_assert_with_mut_call.rs:93:31 | LL | debug_assert!(!(bool_ref(&u32_mut(&mut 3)))); | ^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert_eq!` - --> $DIR/debug_assert_with_mut_call.rs:94:22 + --> $DIR/debug_assert_with_mut_call.rs:96:22 | LL | debug_assert_eq!(v.pop().unwrap(), 3); | ^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:98:19 + --> $DIR/debug_assert_with_mut_call.rs:100:19 | LL | debug_assert!(bool_mut(&mut 3), "w/o format"); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:100:19 + --> $DIR/debug_assert_with_mut_call.rs:102:19 | LL | debug_assert!(bool_mut(&mut 3), "{} format", "w/"); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:105:9 + --> $DIR/debug_assert_with_mut_call.rs:107:9 | LL | bool_mut(&mut x); | ^^^^^^^^^^^^^^^^ error: do not call a function with mutable arguments inside of `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:112:9 + --> $DIR/debug_assert_with_mut_call.rs:114:9 | LL | bool_mut(&mut x); | ^^^^^^^^^^^^^^^^ From 5ba4aa8ebab79ffff4cb621fa22a07d32ea41b1f Mon Sep 17 00:00:00 2001 From: flip1995 Date: Fri, 31 Jan 2020 10:42:31 +0100 Subject: [PATCH 4/4] Move debug_assertions_with_mut_call to nursery --- clippy_lints/src/lib.rs | 3 +-- clippy_lints/src/mutable_debug_assertion.rs | 2 +- src/lintlist/mod.rs | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 443a9c7e9..04239b797 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -1261,7 +1261,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&misc_early::ZERO_PREFIXED_LITERAL), LintId::of(&mut_key::MUTABLE_KEY_TYPE), LintId::of(&mut_reference::UNNECESSARY_MUT_PASSED), - LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&mutex_atomic::MUTEX_ATOMIC), LintId::of(&needless_bool::BOOL_COMPARISON), LintId::of(&needless_bool::NEEDLESS_BOOL), @@ -1578,7 +1577,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&misc::FLOAT_CMP), LintId::of(&misc::MODULO_ONE), LintId::of(&mut_key::MUTABLE_KEY_TYPE), - LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&non_copy_const::BORROW_INTERIOR_MUTABLE_CONST), LintId::of(&non_copy_const::DECLARE_INTERIOR_MUTABLE_CONST), LintId::of(&open_options::NONSENSICAL_OPEN_OPTIONS), @@ -1632,6 +1630,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: LintId::of(&fallible_impl_from::FALLIBLE_IMPL_FROM), LintId::of(&missing_const_for_fn::MISSING_CONST_FOR_FN), LintId::of(&mul_add::MANUAL_MUL_ADD), + LintId::of(&mutable_debug_assertion::DEBUG_ASSERT_WITH_MUT_CALL), LintId::of(&mutex_atomic::MUTEX_INTEGER), LintId::of(&needless_borrow::NEEDLESS_BORROW), LintId::of(&path_buf_push_overwrite::PATH_BUF_PUSH_OVERWRITE), diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index 881f59bba..f0c5c95b1 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -28,7 +28,7 @@ declare_clippy_lint! { /// debug_assert!(take_a_mut_parameter(&mut 5)); /// ``` pub DEBUG_ASSERT_WITH_MUT_CALL, - correctness, + nursery, "mutable arguments in `debug_assert{,_ne,_eq}!`" } diff --git a/src/lintlist/mod.rs b/src/lintlist/mod.rs index 3edde70a2..eaada9961 100644 --- a/src/lintlist/mod.rs +++ b/src/lintlist/mod.rs @@ -289,7 +289,7 @@ pub const ALL_LINTS: [Lint; 351] = [ }, Lint { name: "debug_assert_with_mut_call", - group: "correctness", + group: "nursery", desc: "mutable arguments in `debug_assert{,_ne,_eq}!`", deprecation: None, module: "mutable_debug_assertion",