Skip to content

Commit cf20dbc

Browse files
committed
change to a structure for parsing assert macro
1 parent 9f2c2e5 commit cf20dbc

File tree

7 files changed

+305
-96
lines changed

7 files changed

+305
-96
lines changed

clippy_lints/src/bool_assert_comparison.rs

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, source, ty::implements_trait};
1+
use clippy_utils::{
2+
diagnostics::span_lint_and_sugg, higher::AssertExpn, is_direct_expn_of, source, sugg::Sugg, ty::implements_trait,
3+
};
24
use rustc_ast::ast::LitKind;
35
use rustc_errors::Applicability;
46
use rustc_hir::{Expr, ExprKind, Lit};
@@ -79,10 +81,8 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
7981
.chain(inverted_macros.iter().map(|el| (el, false)))
8082
{
8183
if let Some(span) = is_direct_expn_of(expr.span, mac) {
82-
if let Some(args) = higher::extract_assert_macro_args(expr) {
83-
if let [a, b, ..] = args[..] {
84-
//let nb_bool_args = is_bool_lit(a) as usize + is_bool_lit(b) as usize;
85-
84+
if let Some(args) = AssertExpn::parse(expr).map(|v| v.argument_vector()) {
85+
if let [a, b, ref fmt_args @ ..] = args[..] {
8686
let (lit_value, other_expr) = match (bool_lit(a), bool_lit(b)) {
8787
(Some(lit), None) => (lit, b),
8888
(None, Some(lit)) => (lit, a),
@@ -103,20 +103,37 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
103103
}
104104

105105
let non_eq_mac = &mac[..mac.len() - 3];
106+
let mut applicability = Applicability::MachineApplicable;
106107
let expr_string = if lit_value ^ is_eq {
107108
format!("!({})", source::snippet(cx, other_expr.span, ""))
108109
} else {
109110
source::snippet(cx, other_expr.span, "").to_string()
110111
};
111112

112-
let suggestion = if args.len() <= 2 {
113-
format!("{}!({})", non_eq_mac, expr_string)
113+
let arg_span = match fmt_args {
114+
[] => None,
115+
[a] => Some(format!(
116+
"{}",
117+
Sugg::hir_with_applicability(cx, a, "..", &mut applicability)
118+
)),
119+
_ => {
120+
let mut args = format!(
121+
"{}",
122+
Sugg::hir_with_applicability(cx, fmt_args[0], "..", &mut applicability)
123+
);
124+
for el in &fmt_args[1..] {
125+
args.push_str(&format!(
126+
", {}",
127+
Sugg::hir_with_applicability(cx, el, "..", &mut applicability)
128+
));
129+
}
130+
Some(args)
131+
},
132+
};
133+
let suggestion = if let Some(spans) = arg_span {
134+
format!("{}!({}, {})", non_eq_mac, expr_string, spans)
114135
} else {
115-
let mut str = format!("{}!({}", non_eq_mac, expr_string);
116-
for el in &args[2..] {
117-
str = format!("{}, {}", str, source::snippet(cx, el.span, ""));
118-
}
119-
format!("{})", str)
136+
format!("{}!({})", non_eq_mac, expr_string)
120137
};
121138
span_lint_and_sugg(
122139
cx,
@@ -125,7 +142,7 @@ impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
125142
&format!("used `{}!` with a literal bool", mac),
126143
"replace it with",
127144
suggestion,
128-
Applicability::MaybeIncorrect,
145+
applicability,
129146
);
130147
return;
131148
}

clippy_lints/src/eq_op.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then};
22
use clippy_utils::source::snippet;
33
use clippy_utils::ty::{implements_trait, is_copy};
44
use clippy_utils::{
5-
ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, in_macro, is_expn_of, is_in_test_function,
5+
ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher::AssertExpn, in_macro, is_expn_of, is_in_test_function,
66
};
77
use if_chain::if_chain;
88
use rustc_errors::Applicability;
@@ -79,7 +79,7 @@ impl<'tcx> LateLintPass<'tcx> for EqOp {
7979
if_chain! {
8080
if is_expn_of(stmt.span, amn).is_some();
8181
if let StmtKind::Semi(matchexpr) = stmt.kind;
82-
if let Some(macro_args) = higher::extract_assert_macro_args(matchexpr);
82+
if let Some(macro_args) = AssertExpn::parse(matchexpr).map(|v| v.argument_vector());
8383
if macro_args.len() == 2;
8484
let (lhs, rhs) = (macro_args[0], macro_args[1]);
8585
if eq_expr_value(cx, lhs, rhs);

clippy_lints/src/mutable_debug_assertion.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::{higher, is_direct_expn_of};
2+
use clippy_utils::{higher::AssertExpn, is_direct_expn_of};
33
use rustc_hir::intravisit::{walk_expr, NestedVisitorMap, Visitor};
44
use rustc_hir::{BorrowKind, Expr, ExprKind, MatchSource, Mutability};
55
use rustc_lint::{LateContext, LateLintPass};
@@ -39,7 +39,7 @@ impl<'tcx> LateLintPass<'tcx> for DebugAssertWithMutCall {
3939
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
4040
for dmn in &DEBUG_MACRO_NAMES {
4141
if is_direct_expn_of(e.span, dmn).is_some() {
42-
if let Some(macro_args) = higher::extract_assert_macro_args(e) {
42+
if let Some(macro_args) = AssertExpn::parse(e).map(|v| v.argument_vector()) {
4343
for arg in macro_args {
4444
let mut visitor = MutArgVisitor::new(cx);
4545
visitor.visit_expr(arg);

clippy_utils/src/higher.rs

Lines changed: 85 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -418,24 +418,73 @@ pub fn binop(op: hir::BinOpKind) -> ast::BinOpKind {
418418
}
419419
}
420420

421-
/// Extract args from an assert-like macro.
422-
/// Currently working with:
423-
/// - `assert!`, `assert_eq!` and `assert_ne!`
424-
/// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!`
425-
/// For example:
426-
/// `assert!(expr)` will return `Some([expr])`
427-
/// `debug_assert_eq!(a, b)` will return `Some([a, b])`
428-
pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option<Vec<&'tcx Expr<'tcx>>> {
421+
/// A parsed
422+
/// assert!`, `assert_eq!` or `assert_ne!`,
423+
/// debug_assert!`, `debug_assert_eq!` or `debug_assert_ne!`
424+
/// macro.
425+
pub struct AssertExpn<'tcx> {
426+
/// the first agrument of the assret e.g. `var` in element `assert!(var, ...)`
427+
pub first_assert_argument: &'tcx Expr<'tcx>,
428+
/// second argument of the asset for case `assert_eq!`,
429+
/// `assert_ne!` etc ... Eg var_2 in `debug_assert_eq!(x, var_2,..)`
430+
pub second_assert_argument: Option<&'tcx Expr<'tcx>>,
431+
/// The format argument passed at the end of the macro
432+
pub format_arg: Option<FormatArgsExpn<'tcx>>,
433+
}
434+
435+
impl<'tcx> AssertExpn<'tcx> {
436+
/// Extract args from an assert-like macro.
437+
/// Currently working with:
438+
/// - `assert!`, `assert_eq!` and `assert_ne!`
439+
/// - `debug_assert!`, `debug_assert_eq!` and `debug_assert_ne!`
440+
/// For example:
441+
/// `assert!(expr)` will return `Some(AssertExpn { first_assert_argument: expr,
442+
/// second_assert_argument: None, format_arg:None })` `debug_assert_eq!(a, b)` will return
443+
/// `Some(AssertExpn { first_assert_argument: a, second_assert_argument: Some(b),
444+
/// format_arg:None })`
445+
pub fn parse(e: &'tcx Expr<'tcx>) -> Option<Self> {
446+
if let ExprKind::Block(block, _) = e.kind {
447+
if block.stmts.len() == 1 {
448+
if let StmtKind::Semi(matchexpr) = block.stmts.get(0)?.kind {
449+
// macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`)
450+
if_chain! {
451+
if let Some(If { cond, .. }) = If::hir(matchexpr);
452+
if let ExprKind::Unary(UnOp::Not, condition) = cond.kind;
453+
then {
454+
return Some(Self {
455+
first_assert_argument: condition,
456+
second_assert_argument: None,
457+
format_arg: None, // FIXME actually parse the aguments
458+
});
459+
}
460+
}
461+
462+
// debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
463+
if_chain! {
464+
if let ExprKind::Block(matchblock,_) = matchexpr.kind;
465+
if let Some(matchblock_expr) = matchblock.expr;
466+
then {
467+
return Self::ast_matchblock(matchblock_expr);
468+
}
469+
}
470+
}
471+
} else if let Some(matchblock_expr) = block.expr {
472+
// macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
473+
return Self::ast_matchblock(matchblock_expr);
474+
}
475+
}
476+
None
477+
}
478+
429479
/// Try to match the AST for a pattern that contains a match, for example when two args are
430480
/// compared
431-
fn ast_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option<Vec<&Expr<'_>>> {
481+
fn ast_matchblock(matchblock_expr: &'tcx Expr<'tcx>) -> Option<Self> {
432482
if_chain! {
433483
if let ExprKind::Match(headerexpr, arms, _) = &matchblock_expr.kind;
434484
if let ExprKind::Tup([lhs, rhs]) = &headerexpr.kind;
435485
if let ExprKind::AddrOf(BorrowKind::Ref, _, lhs) = lhs.kind;
436486
if let ExprKind::AddrOf(BorrowKind::Ref, _, rhs) = rhs.kind;
437487
then {
438-
let mut vec_arg = vec![lhs, rhs];
439488
if_chain! {
440489
if !arms.is_empty();
441490
if let ExprKind::Block(Block{expr: Some(if_expr),..},_) = arms[0].body.kind;
@@ -446,58 +495,43 @@ pub fn extract_assert_macro_args<'tcx>(e: &'tcx Expr<'tcx>) -> Option<Vec<&'tcx
446495
| StmtKind::Semi(call_assert_failed) = stmts_if_block[1].kind;
447496
if let ExprKind::Call(_, args_assert_failed) = call_assert_failed.kind;
448497
if args_assert_failed.len() >= 4;
449-
if let ExprKind::Call(_, args) = args_assert_failed[3].kind;
450-
if !args.is_empty();
451-
if let ExprKind::Call(_, args_fmt) = args[0].kind;
452-
if !args_fmt.is_empty();
498+
if let ExprKind::Call(_, [arg, ..]) = args_assert_failed[3].kind;
499+
if let Some(format_arg_expn) = FormatArgsExpn::parse(&arg);
453500
then {
454-
vec_arg.push(&args_fmt[0]);
455-
if_chain! {
456-
if args_fmt.len() >= 2;
457-
if let ExprKind::AddrOf(_, _, expr_match) = args_fmt[1].kind;
458-
if let ExprKind::Match(tup_match, _, _) = expr_match.kind;
459-
if let ExprKind::Tup(tup_args_list) = tup_match.kind;
460-
then{
461-
for arg in tup_args_list {
462-
vec_arg.push(arg);
463-
}
464-
}
465-
}
501+
return Some(AssertExpn {
502+
first_assert_argument: lhs,
503+
second_assert_argument: Some(rhs),
504+
format_arg: Some(format_arg_expn)
505+
});
506+
}
507+
else {
508+
return Some(AssertExpn {
509+
first_assert_argument: lhs,
510+
second_assert_argument:
511+
Some(rhs),
512+
format_arg: None
513+
});
466514
}
467515
}
468-
return Some(vec_arg);
469516
}
470517
}
471518
None
472519
}
473520

474-
if let ExprKind::Block(block, _) = e.kind {
475-
if block.stmts.len() == 1 {
476-
if let StmtKind::Semi(matchexpr) = block.stmts.get(0)?.kind {
477-
// macros with unique arg: `{debug_}assert!` (e.g., `debug_assert!(some_condition)`)
478-
if_chain! {
479-
if let Some(If { cond, .. }) = If::hir(matchexpr);
480-
if let ExprKind::Unary(UnOp::Not, condition) = cond.kind;
481-
then {
482-
return Some(vec![condition]);
483-
}
484-
}
485-
486-
// debug macros with two args: `debug_assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
487-
if_chain! {
488-
if let ExprKind::Block(matchblock,_) = matchexpr.kind;
489-
if let Some(matchblock_expr) = matchblock.expr;
490-
then {
491-
return ast_matchblock(matchblock_expr);
492-
}
493-
}
521+
/// Gives the argument as a vector
522+
pub fn argument_vector(&self) -> Vec<&'tcx Expr<'tcx>> {
523+
let mut expr_vec = vec![self.first_assert_argument];
524+
if let Some(sec_agr) = self.second_assert_argument {
525+
expr_vec.push(sec_agr);
526+
}
527+
if let Some(ref format_arg) = self.format_arg {
528+
expr_vec.push(format_arg.format_string);
529+
for arg in &format_arg.value_args {
530+
expr_vec.push(arg)
494531
}
495-
} else if let Some(matchblock_expr) = block.expr {
496-
// macros with two args: `assert_{ne, eq}` (e.g., `assert_ne!(a, b)`)
497-
return ast_matchblock(matchblock_expr);
498532
}
533+
expr_vec
499534
}
500-
None
501535
}
502536

503537
/// A parsed `format!` expansion

0 commit comments

Comments
 (0)