Skip to content

Commit

Permalink
Assert macro args extractor as a common function in higher
Browse files Browse the repository at this point in the history
  • Loading branch information
@ThibsG
ThibsG committed Oct 16, 2020
1 parent 71c29b5 commit f95b2ee
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 63 deletions.
12 changes: 4 additions & 8 deletions clippy_lints/src/eq_op.rs
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::utils::{
eq_expr_value, implements_trait, in_macro, is_copy, is_expn_of, multispan_sugg, snippet, span_lint,
eq_expr_value, higher, implements_trait, in_macro, is_copy, is_expn_of, multispan_sugg, snippet, span_lint,
span_lint_and_then,
};
use if_chain::if_chain;
Expand Down Expand Up @@ -71,13 +71,9 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
if_chain! {
if is_expn_of(stmt.span, amn).is_some();
if let StmtKind::Semi(ref matchexpr) = stmt.kind;
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;
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref lhs) = conditions[0].kind;
if let ExprKind::AddrOf(BorrowKind::Ref, _, ref rhs) = conditions[1].kind;
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
if macro_args.len() == 2;
let (lhs, rhs) = (macro_args[0], macro_args[1]);
if eq_expr_value(cx, lhs, rhs);

then {
Expand Down
66 changes: 11 additions & 55 deletions clippy_lints/src/mutable_debug_assertion.rs
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use crate::utils::{is_direct_expn_of, span_lint};
use if_chain::if_chain;
use crate::utils::{higher, is_direct_expn_of, span_lint};
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability, StmtKind, UnOp};
use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::ty;
Expand Down Expand Up @@ -39,66 +38,23 @@ impl<'tcx> LateLintPass<'tcx> for DebugAssertWithMutCall {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
for dmn in &DEBUG_MACRO_NAMES {
if is_direct_expn_of(e.span, dmn).is_some() {
if let Some(span) = extract_call(cx, e) {
span_lint(
cx,
DEBUG_ASSERT_WITH_MUT_CALL,
span,
&format!("do not call a function with mutable arguments inside of `{}!`", dmn),
);
}
}
}
}
}

//HACK(hellow554): remove this when #4694 is implemented
fn extract_call<'tcx>(cx: &LateContext<'tcx>, e: &'tcx 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 {
// 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 {
let mut visitor = MutArgVisitor::new(cx);
visitor.visit_expr(condition);
return visitor.expr_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 {
if let Some(macro_args) = higher::extract_assert_macro_args(e) {
for arg in macro_args {
let mut visitor = MutArgVisitor::new(cx);
visitor.visit_expr(lhs);
visitor.visit_expr(arg);
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);
span_lint(
cx,
DEBUG_ASSERT_WITH_MUT_CALL,
span,
&format!("do not call a function with mutable arguments inside of `{}!`", dmn),
);
}
}
}
}
}
}

None
}

struct MutArgVisitor<'a, 'tcx> {
Expand Down
54 changes: 54 additions & 0 deletions clippy_lints/src/utils/higher.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::utils::{is_expn_of, match_def_path, paths};
use if_chain::if_chain;
use rustc_ast::ast;
use rustc_hir as hir;
use rustc_hir::{BorrowKind, Expr, ExprKind, StmtKind, UnOp};
use rustc_lint::LateContext;

/// Converts a hir binary operator to the corresponding `ast` type.
Expand Down Expand Up @@ -241,3 +242,56 @@ pub fn vec_macro<'e>(cx: &LateContext<'_>, expr: &'e hir::Expr<'_>) -> Option<Ve

None
}

/// Extract args from an assert-like macro.
/// Currently working with:
/// - `assert!`, `assert_eq!` and `assert_ne!`
/// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!`
/// For example:
/// `assert!(expr)` will return Some([expr])
/// `debug_assert_eq!(a, b)` will return Some([a, b])
pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option<Vec<&'tcx Expr<'tcx>>> {
/// Try to lint a block that contains a match, for example if two args are compared
fn report_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option<Vec<&Expr<'_>>> {
if_chain! {
if let ExprKind::Match(ref headerexpr, _, _) = &matchblock_expr.kind;
if let ExprKind::Tup(ref conditions) = headerexpr.kind;
if conditions.len() == 2;
if let ExprKind::AddrOf(BorrowKind::Ref, _, lhs) = conditions[0].kind;
if let ExprKind::AddrOf(BorrowKind::Ref, _, rhs) = conditions[1].kind;
then {
return Some(vec![lhs, rhs]);
}
}
None
}

if let ExprKind::Block(ref block, _) = e.kind {
if block.stmts.len() == 1 {
if let StmtKind::Semi(ref matchexpr) = block.stmts[0].kind {
// macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`)
if_chain! {
if let ExprKind::Match(ref ifclause, _, _) = matchexpr.kind;
if let ExprKind::DropTemps(ref droptmp) = ifclause.kind;
if let ExprKind::Unary(UnOp::UnNot, condition) = droptmp.kind;
then {
return Some(vec![condition]);
}
}

// debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
if_chain! {
if let ExprKind::Block(ref matchblock,_) = matchexpr.kind;
if let Some(ref matchblock_expr) = matchblock.expr;
then {
return report_matchblock(matchblock_expr);
}
}
}
} else if let Some(matchblock_expr) = block.expr {
// macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
return report_matchblock(&matchblock_expr);
}
}
None
}

0 comments on commit f95b2ee

Please sign in to comment.