Add {debug_,}assert{,-eq,-ne} to utils/higher.rs

[hellow554]

utils/higher.rs already contains an extraction for vec![] arguments.
It would be nice to add more macros

• assert!
• assert_eq!
• assert_ne!
• debug_assert!
• debug_assert_eq!
• debug_assert_ne!

[rustbot]

Error: Label good-first-issue can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[rustbot]

Error: Label T-macros can only be set by Rust team members

Please let @rust-lang/release know if you're having trouble with this bot.

[hellow554]

Error: Label good-first-issue can only be set by Rust team members

How am I supposed to add good first issues, which contains whitespaces, to rustbot?

Error: Label T-macros can only be set by Rust team members

True. Intentional?

[vss96]

@hellow554 I would like to pick this up if no one else is currently working on it.

[hellow554]

Sure, go ahead. If you like take a look at #4680 where this issue comes from.
If you have questions ask them.

[hellow554]

@vss96 did you have time to look into it? How are you going? Any progression so far? Any questions? Something you need a second opinion on?

[vss96]

@vss96 Sorry, I was caught up in other things for the past week and couldn't put in the time that i wanted to. I was trying to make sense of this code block in higher.rs. I was also trying to execute dummy code with vec! to see how these cases were being handled. If you take something like assert!, it can either have a single argument (the value is either true or false and you expect it to be true) or two arguments where you equate lhs and rhs. I think the part that I find difficult is the syntax around this code (need some time to get used Expr). If you could give a high level overview on what's happening here, that would be great.

pub fn vec_macro<'e>(cx: &LateContext<'_, '_>, expr: &'e hir::Expr) -> Option<VecArgs<'e>> {
    if_chain! {
        if let hir::ExprKind::Call(ref fun, ref args) = expr.kind;
        if let hir::ExprKind::Path(ref qpath) = fun.kind;
        if is_expn_of(fun.span, "vec").is_some();
        if let Some(fun_def_id) = cx.tables.qpath_res(qpath, fun.hir_id).opt_def_id();
        then {
            return if match_def_path(cx, fun_def_id, &paths::VEC_FROM_ELEM) && args.len() == 2 {
                // `vec![elem; size]` case
                Some(VecArgs::Repeat(&args[0], &args[1]))
            }
            else if match_def_path(cx, fun_def_id, &paths::SLICE_INTO_VEC) && args.len() == 1 {
                // `vec![a, b, c]` case
                if_chain! {
                    if let hir::ExprKind::Box(ref boxed) = args[0].kind;
                    if let hir::ExprKind::Array(ref args) = boxed.kind;
                    then {
                        return Some(VecArgs::Vec(&*args));
                    }
                }

                None
            }
            else {
                None
            };
        }
    }

    None
}```

[flip1995]

I think looking at the vec_macro function does not really make sense. But what it does is, that it matches the expanded code, that results from writing vec![_; _]. You have to do the same for assert!. This has already been done for a specific case in

rust-clippy/clippy_lints/src/mutable_debug_assertion.rs

Lines 56 to 102 in e2104d1

	fn extract_call<'a, 'tcx>(cx: &'a LateContext<'a, '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 {
	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(_, 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(_, 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);
	}
	}
	}
	}
	}
	}
	}
	}
	None
	}

This function now has to be generalized, so that it returns the array of expressions. So for assert!(expr) it would return [expr] and for assert_eq!(expr1, expr2) it would return [expr1, expr2].

A great extension of this would be to also return the format_arg expressions. So for assert!(expr, "some {} things", fmt_expr) it would return [expr, fmt_expr], but we can do this in a second iteration.

[vss96]

@flip1995 That helps. Thank you :)

[basil-cow]

@vss96 would you mind me snatching the issue or are you planning to work on it?

[basil-cow]

I am a little bit stuck and would appreciate some help. As a part of solving this, I want to add an utils function that checks if the expr/stmt is the exact macro expansion (that is, the macro expanded precisely to this expr), so we can match on the contents fearlessly. My approach right now is to compare outer ExpnId's of the expr and its parent. If they match, then the expansion is bigger than my expr. The problem is that a built-in macro's (format_args) expansion has exprs whose outer expansion is not format_args (its the calling macro, assert_eq). Their parent node's outer expansion, on the other hand, is format_args, I compare them and get a false positive. Is there a way to fix this or maybe there is a better way to do this (or this function already exists and I can't search).

[flip1995]

I don't think you have to check the IDs of the expansions, but just match on the exact expansion structure. The hir expansion has an ExprKind::DropTemps in the expansion, which is pretty much impossible to produce with non-macro code.

Also when matching the exact expansion your matching on code that probably no one would write that way in real code.