From 7e638e1482c05c3924e74e70d2411faa91134cd2 Mon Sep 17 00:00:00 2001 From: Marcel Hellwig Date: Mon, 21 Oct 2019 15:21:02 +0200 Subject: [PATCH] add more expressions to check --- clippy_lints/src/mutable_debug_assertion.rs | 81 ++++++++++++++++----- tests/ui/debug_assert_with_mut_call.rs | 7 ++ tests/ui/debug_assert_with_mut_call.stderr | 20 +++-- 3 files changed, 84 insertions(+), 24 deletions(-) diff --git a/clippy_lints/src/mutable_debug_assertion.rs b/clippy_lints/src/mutable_debug_assertion.rs index 493f31df6e0d..0eba6b860f4f 100644 --- a/clippy_lints/src/mutable_debug_assertion.rs +++ b/clippy_lints/src/mutable_debug_assertion.rs @@ -1,7 +1,8 @@ use crate::utils::{is_direct_expn_of, span_lint}; +use core::ops::Deref; use if_chain::if_chain; use matches::matches; -use rustc::hir::{Expr, ExprKind, Mutability, StmtKind, UnOp}; +use rustc::hir::{Expr, ExprKind, Guard, Mutability, StmtKind, UnOp}; use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; use rustc::{declare_lint_pass, declare_tool_lint, ty}; use syntax_pos::Span; @@ -31,19 +32,67 @@ declare_clippy_lint! { declare_lint_pass!(DebugAssertWithMutCall => [DEBUG_ASSERT_WITH_MUT_CALL]); -fn check_for_mutable_call(cx: &LateContext<'_, '_>, e: &Expr) -> bool { +fn check_for_mutable_call(cx: &LateContext<'_, '_>, span: Span, e: &Expr) -> Option { match &e.kind { - ExprKind::AddrOf(Mutability::MutMutable, _) => true, - ExprKind::AddrOf(Mutability::MutImmutable, expr) | ExprKind::Unary(_, expr) => check_for_mutable_call(cx, expr), - ExprKind::Binary(_, lhs, rhs) => check_for_mutable_call(cx, lhs) | check_for_mutable_call(cx, rhs), - ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args) => { - (*args).iter().any(|a| check_for_mutable_call(cx, a)) + ExprKind::AddrOf(Mutability::MutMutable, _) => Some(span), + ExprKind::AddrOf(Mutability::MutImmutable, expr) + | ExprKind::Assign(_, expr) + | ExprKind::AssignOp(_, _, expr) + | ExprKind::Box(expr) + | ExprKind::Break(_, Some(expr)) + | ExprKind::Cast(expr, _) + | ExprKind::DropTemps(expr) + | ExprKind::Field(expr, _) + | ExprKind::Repeat(expr, _) + | ExprKind::Ret(Some(expr)) + | ExprKind::Type(expr, _) + | ExprKind::Unary(_, expr) + | ExprKind::Yield(expr, _) => check_for_mutable_call(cx, expr.span, expr), + ExprKind::Binary(_, lhs, rhs) | ExprKind::Index(lhs, rhs) => { + check_for_mutable_call(cx, lhs.span, lhs).or_else(|| check_for_mutable_call(cx, rhs.span, rhs)) }, - ExprKind::Path(_) => cx.tables.adjustments().get(e.hir_id).map_or(false, |adj| { - adj.iter() + ExprKind::Array(args) | ExprKind::Call(_, args) | ExprKind::MethodCall(_, _, args) | ExprKind::Tup(args) => { + (*args).iter().find_map(|a| check_for_mutable_call(cx, span, a)) + }, + ExprKind::Path(_) => cx.tables.adjustments().get(e.hir_id).and_then(|adj| { + if adj + .iter() .any(|a| matches!(a.target.kind, ty::Ref(_, _, Mutability::MutMutable))) + { + Some(span) + } else { + None + } + }), + ExprKind::Match(header, arm, _) => check_for_mutable_call(cx, header.span, header).or_else(|| { + (*arm).iter().find_map(|a| { + check_for_mutable_call(cx, a.body.span, &a.body).or_else(|| { + a.guard.as_ref().and_then(|g| { + let Guard::If(e) = g; + check_for_mutable_call(cx, e.span, &e) + }) + }) + }) }), - _ => false, + ExprKind::Block(block, _) | ExprKind::Loop(block, _, _) => block + .stmts + .iter() + .filter_map(|s| match &s.kind { + StmtKind::Local(l) => l.init.as_ref().map(|x| x.deref()), + StmtKind::Expr(e) => Some(e), + StmtKind::Semi(e) => Some(e), + StmtKind::Item(_) => None, + }) + .chain(block.expr.as_ref().map(|x| x.deref())) + .find_map(|a| check_for_mutable_call(cx, a.span, a)), + ExprKind::Err + | ExprKind::Lit(_) + | ExprKind::Continue(_) + | ExprKind::InlineAsm(_, _, _) + | ExprKind::Struct(_, _, _) + | ExprKind::Closure(_, _, _, _, _) + | ExprKind::Break(_, None) + | ExprKind::Ret(None) => None, } } @@ -60,9 +109,7 @@ fn extract_call(cx: &LateContext<'_, '_>, e: &Expr) -> Option { if let ExprKind::Unary(UnOp::UnNot, ref condition) = droptmp.kind; then { // debug_assert - if check_for_mutable_call(cx, condition) { - return Some(condition.span); - } + return check_for_mutable_call(cx, condition.span, condition) } else { // debug_assert_{eq,ne} if_chain! { @@ -73,13 +120,13 @@ fn extract_call(cx: &LateContext<'_, '_>, e: &Expr) -> Option { if conditions.len() == 2; then { if let ExprKind::AddrOf(_, ref lhs) = conditions[0].kind { - if check_for_mutable_call(cx, lhs) { - return Some(lhs.span); + if let Some(span) = check_for_mutable_call(cx, lhs.span, lhs) { + return Some(span); } } if let ExprKind::AddrOf(_, ref rhs) = conditions[1].kind { - if check_for_mutable_call(cx, rhs) { - return Some(rhs.span); + if let Some(span) = check_for_mutable_call(cx, rhs.span, rhs) { + return Some(span); } } } diff --git a/tests/ui/debug_assert_with_mut_call.rs b/tests/ui/debug_assert_with_mut_call.rs index 0858e218124a..1461f8cfa990 100644 --- a/tests/ui/debug_assert_with_mut_call.rs +++ b/tests/ui/debug_assert_with_mut_call.rs @@ -98,6 +98,13 @@ fn misc() { debug_assert!(bool_mut(&mut 3), "w/o format"); debug_assert!(bool_ref(&3), "{} format", "w/"); debug_assert!(bool_mut(&mut 3), "{} format", "w/"); + + // sub block + let mut x = 42_u32; + debug_assert!({ + bool_mut(&mut x); + x > 10 + }); } fn main() { diff --git a/tests/ui/debug_assert_with_mut_call.stderr b/tests/ui/debug_assert_with_mut_call.stderr index de4601063e99..dad89047f49b 100644 --- a/tests/ui/debug_assert_with_mut_call.stderr +++ b/tests/ui/debug_assert_with_mut_call.stderr @@ -7,10 +7,10 @@ LL | debug_assert!(bool_mut(&mut 3)); = note: `#[deny(clippy::debug_assert_with_mut_call)]` on by default error: do not call functions with mutable arguments inside of a `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:41:19 + --> $DIR/debug_assert_with_mut_call.rs:41:20 | LL | debug_assert!(!bool_mut(&mut 3)); - | ^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^ error: do not call functions with mutable arguments inside of a `debug_assert!` --> $DIR/debug_assert_with_mut_call.rs:43:25 @@ -43,10 +43,10 @@ LL | debug_assert!(S.bool_self_mut()); | ^^^^^^^^^^^^^^^^^ error: do not call functions with mutable arguments inside of a `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:63:19 + --> $DIR/debug_assert_with_mut_call.rs:63:20 | LL | debug_assert!(!S.bool_self_mut()); - | ^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^^^ error: do not call functions with mutable arguments inside of a `debug_assert!` --> $DIR/debug_assert_with_mut_call.rs:64:19 @@ -133,10 +133,10 @@ LL | debug_assert!(bool_mut(a)); | ^^^^^^^^^^^ error: do not call functions with mutable arguments inside of a `debug_assert!` - --> $DIR/debug_assert_with_mut_call.rs:91:19 + --> $DIR/debug_assert_with_mut_call.rs:91:31 | LL | debug_assert!(!(bool_ref(&u32_mut(&mut 3)))); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^^^ error: do not call functions with mutable arguments inside of a `debug_assert!` --> $DIR/debug_assert_with_mut_call.rs:94:22 @@ -156,5 +156,11 @@ error: do not call functions with mutable arguments inside of a `debug_assert!` LL | debug_assert!(bool_mut(&mut 3), "{} format", "w/"); | ^^^^^^^^^^^^^^^^ -error: aborting due to 26 previous errors +error: do not call functions with mutable arguments inside of a `debug_assert!` + --> $DIR/debug_assert_with_mut_call.rs:105:9 + | +LL | bool_mut(&mut x); + | ^^^^^^^^^^^^^^^^ + +error: aborting due to 27 previous errors