-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add lint for debug_assert_with_mut_call
This lint will complain when you put a mutable function/method call inside a `debug_assert` macro, because it will not be executed in release mode, therefore it will change the execution flow, which is not wanted.
- Loading branch information
Showing
7 changed files
with
394 additions
and
2 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| use crate::utils::{is_direct_expn_of, span_lint}; | ||
| use if_chain::if_chain; | ||
| use matches::matches; | ||
| use rustc::hir::{Expr, ExprKind, Mutability, StmtKind, UnOp}; | ||
| use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass}; | ||
| use rustc::{declare_lint_pass, declare_tool_lint, ty}; | ||
| use syntax_pos::Span; | ||
|
|
||
| declare_clippy_lint! { | ||
| /// **What it does:** Checks for function/method calls with a mutable | ||
| /// parameter in `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!` macros. | ||
| /// | ||
| /// **Why is this bad?** In release builds `debug_assert!` macros are optimized out by the | ||
| /// compiler. | ||
| /// Therefore mutating something in a `debug_assert!` macro results in different behaviour | ||
| /// between a release and debug build. | ||
| /// | ||
| /// **Known problems:** None | ||
| /// | ||
| /// **Example:** | ||
| /// ```rust,ignore | ||
| /// debug_assert_eq!(vec![3].pop(), Some(3)); | ||
| /// // or | ||
| /// fn take_a_mut_parameter(_: &mut u32) -> bool { unimplemented!() } | ||
| /// debug_assert!(take_a_mut_parameter(&mut 5)); | ||
| /// ``` | ||
| pub DEBUG_ASSERT_WITH_MUT_CALL, | ||
| correctness, | ||
| "mutable arguments in `debug_assert{,_ne,_eq}!`" | ||
| } | ||
|
|
||
| declare_lint_pass!(DebugAssertWithMutCall => [DEBUG_ASSERT_WITH_MUT_CALL]); | ||
|
|
||
| fn check_for_mutable_call(cx: &LateContext<'_, '_>, e: &Expr) -> bool { | ||
| 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::Path(_) => cx.tables.adjustments().get(e.hir_id).map_or(false, |adj| { | ||
| adj.iter() | ||
| .any(|a| matches!(a.target.kind, ty::Ref(_, _, Mutability::MutMutable))) | ||
| }), | ||
| _ => false, | ||
| } | ||
| } | ||
|
|
||
| #[allow(clippy::cognitive_complexity)] | ||
| fn extract_call(cx: &LateContext<'_, '_>, e: &Expr) -> Option<Span> { | ||
| if_chain! { | ||
| if let ExprKind::Block(ref block, _) = e.kind; | ||
| if block.stmts.len() == 1; | ||
| if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind; | ||
| then { | ||
| 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 | ||
| if check_for_mutable_call(cx, condition) { | ||
| return Some(condition.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(_, ref lhs) = conditions[0].kind { | ||
| if check_for_mutable_call(cx, lhs) { | ||
| return Some(lhs.span); | ||
| } | ||
| } | ||
| if let ExprKind::AddrOf(_, ref rhs) = conditions[1].kind { | ||
| if check_for_mutable_call(cx, rhs) { | ||
| return Some(rhs.span); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| None | ||
| } | ||
| impl<'a, 'tcx> LateLintPass<'a, 'tcx> for DebugAssertWithMutCall { | ||
| fn check_expr(&mut self, cx: &LateContext<'a, 'tcx>, e: &'tcx Expr) { | ||
| if ["debug_assert", "debug_assert_eq", "debug_assert_ne"] | ||
| .iter() | ||
| .any(|name| is_direct_expn_of(e.span, name).is_some()) | ||
| { | ||
| if let Some(span) = extract_call(cx, e) { | ||
| span_lint( | ||
| cx, | ||
| DEBUG_ASSERT_WITH_MUT_CALL, | ||
| span, | ||
| "do not call functions with mutable arguments inside of a `debug_assert!`", | ||
| ); | ||
| } | ||
| } | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,110 @@ | ||
| #![feature(custom_inner_attributes)] | ||
| #![rustfmt::skip] | ||
| #![allow(clippy::trivially_copy_pass_by_ref, clippy::cognitive_complexity)] | ||
|
|
||
| struct S; | ||
|
|
||
| impl S { | ||
| fn bool_self_ref(&self) -> bool { false } | ||
| fn bool_self_mut(&mut self) -> bool { false } | ||
| fn bool_self_ref_arg_ref(&self, _: &u32) -> bool { false } | ||
| fn bool_self_ref_arg_mut(&self, _: &mut u32) -> bool { false } | ||
| fn bool_self_mut_arg_ref(&mut self, _: &u32) -> bool { false } | ||
| fn bool_self_mut_arg_mut(&mut self, _: &mut u32) -> bool { false } | ||
|
|
||
| fn u32_self_ref(&self) -> u32 { 0 } | ||
| fn u32_self_mut(&mut self) -> u32 { 0 } | ||
| fn u32_self_ref_arg_ref(&self, _: &u32) -> u32 { 0 } | ||
| fn u32_self_ref_arg_mut(&self, _: &mut u32) -> u32 { 0 } | ||
| fn u32_self_mut_arg_ref(&mut self, _: &u32) -> u32 { 0 } | ||
| fn u32_self_mut_arg_mut(&mut self, _: &mut u32) -> u32 { 0 } | ||
| } | ||
|
|
||
| fn bool_ref(_: &u32) -> bool { false } | ||
| fn bool_mut(_: &mut u32) -> bool { false } | ||
| fn u32_ref(_: &u32) -> u32 { 0 } | ||
| fn u32_mut(_: &mut u32) -> u32 { 0 } | ||
|
|
||
| fn func_non_mutable() { | ||
| debug_assert!(bool_ref(&3)); | ||
| debug_assert!(!bool_ref(&3)); | ||
|
|
||
| debug_assert_eq!(0, u32_ref(&3)); | ||
| debug_assert_eq!(u32_ref(&3), 0); | ||
|
|
||
| debug_assert_ne!(1, u32_ref(&3)); | ||
| debug_assert_ne!(u32_ref(&3), 1); | ||
| } | ||
|
|
||
| fn func_mutable() { | ||
| debug_assert!(bool_mut(&mut 3)); | ||
| debug_assert!(!bool_mut(&mut 3)); | ||
|
|
||
| debug_assert_eq!(0, u32_mut(&mut 3)); | ||
| debug_assert_eq!(u32_mut(&mut 3), 0); | ||
|
|
||
| debug_assert_ne!(1, u32_mut(&mut 3)); | ||
| debug_assert_ne!(u32_mut(&mut 3), 1); | ||
| } | ||
|
|
||
| fn method_non_mutable() { | ||
| debug_assert!(S.bool_self_ref()); | ||
| debug_assert!(S.bool_self_ref_arg_ref(&3)); | ||
|
|
||
| debug_assert_eq!(S.u32_self_ref(), 0); | ||
| debug_assert_eq!(S.u32_self_ref_arg_ref(&3), 0); | ||
|
|
||
| debug_assert_ne!(S.u32_self_ref(), 1); | ||
| debug_assert_ne!(S.u32_self_ref_arg_ref(&3), 1); | ||
| } | ||
|
|
||
| fn method_mutable() { | ||
| debug_assert!(S.bool_self_mut()); | ||
| debug_assert!(!S.bool_self_mut()); | ||
| debug_assert!(S.bool_self_ref_arg_mut(&mut 3)); | ||
| debug_assert!(S.bool_self_mut_arg_ref(&3)); | ||
| debug_assert!(S.bool_self_mut_arg_mut(&mut 3)); | ||
|
|
||
| debug_assert_eq!(S.u32_self_mut(), 0); | ||
| debug_assert_eq!(S.u32_self_mut_arg_ref(&3), 0); | ||
| debug_assert_eq!(S.u32_self_ref_arg_mut(&mut 3), 0); | ||
| debug_assert_eq!(S.u32_self_mut_arg_mut(&mut 3), 0); | ||
|
|
||
| debug_assert_ne!(S.u32_self_mut(), 1); | ||
| debug_assert_ne!(S.u32_self_mut_arg_ref(&3), 1); | ||
| debug_assert_ne!(S.u32_self_ref_arg_mut(&mut 3), 1); | ||
| debug_assert_ne!(S.u32_self_mut_arg_mut(&mut 3), 1); | ||
| } | ||
|
|
||
| fn misc() { | ||
| // with variable | ||
| let mut v: Vec<u32> = vec![1, 2, 3, 4]; | ||
| debug_assert_eq!(v.get(0), Some(&1)); | ||
| debug_assert_ne!(v[0], 2); | ||
| debug_assert_eq!(v.pop(), Some(1)); | ||
| debug_assert_ne!(Some(3), v.pop()); | ||
|
|
||
| let a = &mut 3; | ||
| debug_assert!(bool_mut(a)); | ||
|
|
||
| // nested | ||
| debug_assert!(!(bool_ref(&u32_mut(&mut 3)))); | ||
|
|
||
| // chained | ||
| debug_assert_eq!(v.pop().unwrap(), 3); | ||
|
|
||
| // format args | ||
| debug_assert!(bool_ref(&3), "w/o format"); | ||
| debug_assert!(bool_mut(&mut 3), "w/o format"); | ||
| debug_assert!(bool_ref(&3), "{} format", "w/"); | ||
| debug_assert!(bool_mut(&mut 3), "{} format", "w/"); | ||
| } | ||
|
|
||
| fn main() { | ||
| func_non_mutable(); | ||
| func_mutable(); | ||
| method_non_mutable(); | ||
| method_mutable(); | ||
|
|
||
| misc(); | ||
| } |
Oops, something went wrong.