Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New macro utils #8219

Closed
wants to merge 18 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 25 additions & 95 deletions clippy_lints/src/assertions_on_constants.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::span_lint_and_help;
use clippy_utils::higher;
use clippy_utils::source::snippet_opt;
use clippy_utils::{is_direct_expn_of, is_expn_of, match_panic_call, peel_blocks};
use if_chain::if_chain;
use rustc_hir::{Expr, ExprKind, UnOp};
use clippy_utils::macros::{find_assert_args, root_macro_call_first_node, PanicExpn};
use rustc_hir::Expr;
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::sym;

declare_clippy_lint! {
/// ### What it does
Expand Down Expand Up @@ -36,107 +34,39 @@ declare_lint_pass!(AssertionsOnConstants => [ASSERTIONS_ON_CONSTANTS]);

impl<'tcx> LateLintPass<'tcx> for AssertionsOnConstants {
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
let lint_true = |is_debug: bool| {
span_lint_and_help(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
if is_debug {
"`debug_assert!(true)` will be optimized out by the compiler"
} else {
"`assert!(true)` will be optimized out by the compiler"
},
None,
"remove it",
);
let Some(macro_call) = root_macro_call_first_node(cx, e) else { return };
let is_debug = match cx.tcx.get_diagnostic_name(macro_call.def_id) {
Some(sym::debug_assert_macro) => true,
Some(sym::assert_macro) => false,
_ => return,
};
let lint_false_without_message = || {
let Some((condition, panic_expn)) = find_assert_args(cx, e, macro_call.expn) else { return };
let Some((Constant::Bool(val), _)) = constant(cx, cx.typeck_results(), condition) else { return };
if val {
span_lint_and_help(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
"`assert!(false)` should probably be replaced",
macro_call.span,
&format!(
"`{}!(true)` will be optimized out by the compiler",
cx.tcx.item_name(macro_call.def_id)
),
None,
"use `panic!()` or `unreachable!()`",
"remove it",
);
};
let lint_false_with_message = |panic_message: String| {
} else if !is_debug {
let (assert_arg, panic_arg) = match panic_expn {
PanicExpn::Empty => ("", ""),
_ => (", ..", ".."),
};
span_lint_and_help(
cx,
ASSERTIONS_ON_CONSTANTS,
e.span,
&format!("`assert!(false, {})` should probably be replaced", panic_message),
macro_call.span,
&format!("`assert!(false{})` should probably be replaced", assert_arg),
None,
&format!("use `panic!({})` or `unreachable!({})`", panic_message, panic_message),
&format!("use `panic!({})` or `unreachable!({0})`", panic_arg),
);
};

if let Some(debug_assert_span) = is_expn_of(e.span, "debug_assert") {
if debug_assert_span.from_expansion() {
return;
}
if_chain! {
if let ExprKind::Unary(_, lit) = e.kind;
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), lit);
if is_true;
then {
lint_true(true);
}
};
} else if let Some(assert_span) = is_direct_expn_of(e.span, "assert") {
if assert_span.from_expansion() {
return;
}
if let Some(assert_match) = match_assert_with_message(cx, e) {
match assert_match {
// matched assert but not message
AssertKind::WithoutMessage(false) => lint_false_without_message(),
AssertKind::WithoutMessage(true) | AssertKind::WithMessage(_, true) => lint_true(false),
AssertKind::WithMessage(panic_message, false) => lint_false_with_message(panic_message),
};
}
}
}
}

/// Result of calling `match_assert_with_message`.
enum AssertKind {
WithMessage(String, bool),
WithoutMessage(bool),
}

/// Check if the expression matches
///
/// ```rust,ignore
/// if !c {
/// {
/// ::std::rt::begin_panic(message, _)
/// }
/// }
/// ```
///
/// where `message` is any expression and `c` is a constant bool.
fn match_assert_with_message<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<AssertKind> {
if_chain! {
if let Some(higher::If { cond, then, .. }) = higher::If::hir(expr);
if let ExprKind::Unary(UnOp::Not, expr) = cond.kind;
// bind the first argument of the `assert!` macro
if let Some((Constant::Bool(is_true), _)) = constant(cx, cx.typeck_results(), expr);
let begin_panic_call = peel_blocks(then);
// function call
if let Some(arg) = match_panic_call(cx, begin_panic_call);
// bind the second argument of the `assert!` macro if it exists
if let panic_message = snippet_opt(cx, arg.span);
// second argument of begin_panic is irrelevant
// as is the second match arm
then {
// an empty message occurs when it was generated by the macro
// (and not passed by the user)
return panic_message
.filter(|msg| !msg.is_empty())
.map(|msg| AssertKind::WithMessage(msg, is_true))
.or(Some(AssertKind::WithoutMessage(is_true)));
}
}
None
}
18 changes: 7 additions & 11 deletions clippy_lints/src/attrs.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
//! checks for attributes

use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then};
use clippy_utils::macros::{is_panic, macro_backtrace};
use clippy_utils::msrvs;
use clippy_utils::source::{first_line_of_span, is_present_in_source, snippet_opt, without_block_comments};
use clippy_utils::{extract_msrv_attr, match_panic_def_id, meets_msrv};
use clippy_utils::{extract_msrv_attr, meets_msrv};
use if_chain::if_chain;
use rustc_ast::{AttrKind, AttrStyle, Attribute, Lit, LitKind, MetaItemKind, NestedMetaItem};
use rustc_errors::Applicability;
Expand Down Expand Up @@ -443,20 +444,15 @@ fn is_relevant_block(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_
}

fn is_relevant_expr(cx: &LateContext<'_>, typeck_results: &ty::TypeckResults<'_>, expr: &Expr<'_>) -> bool {
if macro_backtrace(expr.span).last().map_or(false, |macro_call| {
is_panic(cx, macro_call.def_id) || cx.tcx.item_name(macro_call.def_id) == sym::unreachable
}) {
return false;
}
match &expr.kind {
ExprKind::Block(block, _) => is_relevant_block(cx, typeck_results, block),
ExprKind::Ret(Some(e)) => is_relevant_expr(cx, typeck_results, e),
ExprKind::Ret(None) | ExprKind::Break(_, None) => false,
ExprKind::Call(path_expr, _) => {
if let ExprKind::Path(qpath) = &path_expr.kind {
typeck_results
.qpath_res(qpath, path_expr.hir_id)
.opt_def_id()
.map_or(true, |fun_id| !match_panic_def_id(cx, fun_id))
} else {
true
}
},
_ => true,
}
}
Expand Down
73 changes: 35 additions & 38 deletions clippy_lints/src/bool_assert_comparison.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::{diagnostics::span_lint_and_sugg, higher, is_direct_expn_of, ty::implements_trait};
use clippy_utils::macros::{find_assert_eq_args, root_macro_call_first_node};
use clippy_utils::{diagnostics::span_lint_and_sugg, ty::implements_trait};
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{Expr, ExprKind, Lit};
Expand Down Expand Up @@ -66,44 +67,40 @@ fn is_impl_not_trait_with_bool_out(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) ->

impl<'tcx> LateLintPass<'tcx> for BoolAssertComparison {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
let macros = ["assert_eq", "debug_assert_eq"];
let inverted_macros = ["assert_ne", "debug_assert_ne"];

for mac in macros.iter().chain(inverted_macros.iter()) {
if let Some(span) = is_direct_expn_of(expr.span, mac) {
if let Some(args) = higher::extract_assert_macro_args(expr) {
if let [a, b, ..] = args[..] {
let nb_bool_args = usize::from(is_bool_lit(a)) + usize::from(is_bool_lit(b));

if nb_bool_args != 1 {
// If there are two boolean arguments, we definitely don't understand
// what's going on, so better leave things as is...
//
// Or there is simply no boolean and then we can leave things as is!
return;
}

if !is_impl_not_trait_with_bool_out(cx, a) || !is_impl_not_trait_with_bool_out(cx, b) {
// At this point the expression which is not a boolean
// literal does not implement Not trait with a bool output,
// so we cannot suggest to rewrite our code
return;
}
let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return };
let macro_name = cx.tcx.item_name(macro_call.def_id);
if !matches!(
macro_name.as_str(),
"assert_eq" | "debug_assert_eq" | "assert_ne" | "debug_assert_ne"
) {
return;
}
let Some ((a, b, _)) = find_assert_eq_args(cx, expr, macro_call.expn) else { return };
if !(is_bool_lit(a) ^ is_bool_lit(b)) {
// If there are two boolean arguments, we definitely don't understand
// what's going on, so better leave things as is...
//
// Or there is simply no boolean and then we can leave things as is!
return;
}

let non_eq_mac = &mac[..mac.len() - 3];
span_lint_and_sugg(
cx,
BOOL_ASSERT_COMPARISON,
span,
&format!("used `{}!` with a literal bool", mac),
"replace it with",
format!("{}!(..)", non_eq_mac),
Applicability::MaybeIncorrect,
);
return;
}
}
}
if !is_impl_not_trait_with_bool_out(cx, a) || !is_impl_not_trait_with_bool_out(cx, b) {
// At this point the expression which is not a boolean
// literal does not implement Not trait with a bool output,
// so we cannot suggest to rewrite our code
return;
}

let macro_name = macro_name.as_str();
let non_eq_mac = &macro_name[..macro_name.len() - 3];
span_lint_and_sugg(
cx,
BOOL_ASSERT_COMPARISON,
macro_call.span,
&format!("used `{}!` with a literal bool", macro_name),
"replace it with",
format!("{}!(..)", non_eq_mac),
Applicability::MaybeIncorrect,
);
}
}
33 changes: 11 additions & 22 deletions clippy_lints/src/doc.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
use clippy_utils::attrs::is_doc_hidden;
use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_note, span_lint_and_then};
use clippy_utils::macros::{is_panic, root_macro_call_first_node};
use clippy_utils::source::{first_line_of_span, snippet_with_applicability};
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
use clippy_utils::{is_entrypoint_fn, is_expn_of, match_panic_def_id, method_chain_args, return_ty};
use clippy_utils::{is_entrypoint_fn, method_chain_args, return_ty};
use if_chain::if_chain;
use itertools::Itertools;
use rustc_ast::ast::{Async, AttrKind, Attribute, Fn, FnRetTy, ItemKind};
Expand All @@ -13,7 +14,7 @@ use rustc_errors::emitter::EmitterWriter;
use rustc_errors::{Applicability, Handler, SuggestionStyle};
use rustc_hir as hir;
use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor};
use rustc_hir::{AnonConst, Expr, ExprKind, QPath};
use rustc_hir::{AnonConst, Expr};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::hir::map::Map;
use rustc_middle::lint::in_external_macro;
Expand Down Expand Up @@ -805,24 +806,17 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> {
return;
}

// check for `begin_panic`
if_chain! {
if let ExprKind::Call(func_expr, _) = expr.kind;
if let ExprKind::Path(QPath::Resolved(_, path)) = func_expr.kind;
if let Some(path_def_id) = path.res.opt_def_id();
if match_panic_def_id(self.cx, path_def_id);
if is_expn_of(expr.span, "unreachable").is_none();
if !is_expn_of_debug_assertions(expr.span);
then {
self.panic_span = Some(expr.span);
if let Some(macro_call) = root_macro_call_first_node(self.cx, expr) {
if is_panic(self.cx, macro_call.def_id)
|| matches!(
self.cx.tcx.item_name(macro_call.def_id).as_str(),
"assert" | "assert_eq" | "assert_ne" | "todo"
)
{
self.panic_span = Some(macro_call.span);
}
}

// check for `assert_eq` or `assert_ne`
if is_expn_of(expr.span, "assert_eq").is_some() || is_expn_of(expr.span, "assert_ne").is_some() {
self.panic_span = Some(expr.span);
}

// check for `unwrap`
if let Some(arglists) = method_chain_args(expr, &["unwrap"]) {
let receiver_ty = self.typeck_results.expr_ty(&arglists[0][0]).peel_refs();
Expand All @@ -844,8 +838,3 @@ impl<'a, 'tcx> Visitor<'tcx> for FindPanicUnwrap<'a, 'tcx> {
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
}
}

fn is_expn_of_debug_assertions(span: Span) -> bool {
const MACRO_NAMES: &[&str] = &["debug_assert", "debug_assert_eq", "debug_assert_ne"];
MACRO_NAMES.iter().any(|name| is_expn_of(span, name).is_some())
}
45 changes: 20 additions & 25 deletions clippy_lints/src/eq_op.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use clippy_utils::diagnostics::{multispan_sugg, span_lint, span_lint_and_then};
use clippy_utils::macros::{find_assert_eq_args, first_node_macro_backtrace};
use clippy_utils::source::snippet;
use clippy_utils::ty::{implements_trait, is_copy};
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, higher, is_expn_of, is_in_test_function};
use clippy_utils::{ast_utils::is_useless_with_eq_exprs, eq_expr_value, is_in_test_function};
use if_chain::if_chain;
use rustc_errors::Applicability;
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind, StmtKind};
use rustc_hir::{BinOpKind, BorrowKind, Expr, ExprKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};

Expand Down Expand Up @@ -68,32 +69,26 @@ declare_clippy_lint! {

declare_lint_pass!(EqOp => [EQ_OP, OP_REF]);

const ASSERT_MACRO_NAMES: [&str; 4] = ["assert_eq", "assert_ne", "debug_assert_eq", "debug_assert_ne"];

impl<'tcx> LateLintPass<'tcx> for EqOp {
#[allow(clippy::similar_names, clippy::too_many_lines)]
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
if let ExprKind::Block(block, _) = e.kind {
for stmt in block.stmts {
for amn in &ASSERT_MACRO_NAMES {
if_chain! {
if is_expn_of(stmt.span, amn).is_some();
if let StmtKind::Semi(matchexpr) = stmt.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);
if !is_in_test_function(cx.tcx, e.hir_id);
then {
span_lint(
cx,
EQ_OP,
lhs.span.to(rhs.span),
&format!("identical args used in this `{}!` macro call", amn),
);
}
}
}
if_chain! {
if let Some((macro_call, macro_name)) = first_node_macro_backtrace(cx, e).find_map(|macro_call| {
let name = cx.tcx.item_name(macro_call.def_id);
matches!(name.as_str(), "assert_eq" | "assert_ne" | "debug_assert_eq" | "debug_assert_ne")
.then(|| (macro_call, name))
});
if let Some((lhs, rhs, _)) = find_assert_eq_args(cx, e, macro_call.expn);
if eq_expr_value(cx, lhs, rhs);
if macro_call.is_local();
if !is_in_test_function(cx.tcx, e.hir_id);
then {
span_lint(
cx,
EQ_OP,
lhs.span.to(rhs.span),
&format!("identical args used in this `{}!` macro call", macro_name),
);
}
}
if let ExprKind::Binary(op, left, right) = e.kind {
Expand Down
Loading