From 847a95b68283ee682eca9d01067ae9620d1bd296 Mon Sep 17 00:00:00 2001 From: dswij Date: Mon, 25 Oct 2021 11:59:18 +0800 Subject: [PATCH 1/3] Refactor utils on checking attribute Moved out reusable pieces from `is_automatically_derived` and `any_parent_is_automatically_derived`. --- clippy_utils/src/lib.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index e4e6c5ddbb23..95642adeff6c 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1402,7 +1402,7 @@ pub fn recurse_or_patterns<'tcx, F: FnMut(&'tcx Pat<'tcx>)>(pat: &'tcx Pat<'tcx> /// Checks for the `#[automatically_derived]` attribute all `#[derive]`d /// implementations have. pub fn is_automatically_derived(attrs: &[ast::Attribute]) -> bool { - attrs.iter().any(|attr| attr.has_name(sym::automatically_derived)) + has_attr(attrs, sym::automatically_derived) } /// Remove blocks around an expression. @@ -1524,20 +1524,29 @@ pub fn clip(tcx: TyCtxt<'_>, u: u128, ity: rustc_ty::UintTy) -> u128 { (u << amt) >> amt } -pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_>, node: HirId) -> bool { +pub fn has_attr(attrs: &[ast::Attribute], symbol: Symbol) -> bool { + attrs.iter().any(|attr| attr.has_name(symbol)) +} + +pub fn any_parent_has_attr(tcx: TyCtxt<'_>, node: HirId, symbol: Symbol) -> bool { let map = &tcx.hir(); let mut prev_enclosing_node = None; let mut enclosing_node = node; while Some(enclosing_node) != prev_enclosing_node { - if is_automatically_derived(map.attrs(enclosing_node)) { + if has_attr(map.attrs(enclosing_node), symbol) { return true; } prev_enclosing_node = Some(enclosing_node); enclosing_node = map.get_parent_item(enclosing_node); } + false } +pub fn any_parent_is_automatically_derived(tcx: TyCtxt<'_>, node: HirId) -> bool { + any_parent_has_attr(tcx, node, sym::automatically_derived) +} + /// Matches a function call with the given path and returns the arguments. /// /// Usage: From 2d037aa4a66edb601fb65fb3efdc573771cba3e2 Mon Sep 17 00:00:00 2001 From: dswij Date: Mon, 25 Oct 2021 12:29:05 +0800 Subject: [PATCH 2/3] Check for no_std and no_core attribute in `swap` lint This commit adds a `no_std` and `no_core` check on `swap` lint and additionally suggest `core::mem::swap` whenever possible. Remove warning if both `std` and `core` is not present. --- clippy_lints/src/lib.rs | 1 + clippy_lints/src/swap.rs | 48 ++++++++++++++++++++++------------------ clippy_utils/src/lib.rs | 20 +++++++++++++++++ 3 files changed, 48 insertions(+), 21 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d91b2e1f448d..02fb6b79dc47 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -8,6 +8,7 @@ #![feature(rustc_private)] #![feature(stmt_expr_attributes)] #![feature(control_flow_enum)] +#![feature(let_else)] #![recursion_limit = "512"] #![cfg_attr(feature = "deny-warnings", deny(warnings))] #![allow(clippy::missing_docs_in_private_items, clippy::must_use_candidate)] diff --git a/clippy_lints/src/swap.rs b/clippy_lints/src/swap.rs index ef26de5b6b93..bd779124deee 100644 --- a/clippy_lints/src/swap.rs +++ b/clippy_lints/src/swap.rs @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet_with_applicability; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{can_mut_borrow_both, differing_macro_contexts, eq_expr_value}; +use clippy_utils::{can_mut_borrow_both, differing_macro_contexts, eq_expr_value, std_or_core}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Block, Expr, ExprKind, PatKind, QPath, Stmt, StmtKind}; @@ -113,6 +113,8 @@ fn generate_swap_warning(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>, spa let first = Sugg::hir_with_applicability(cx, e1, "..", &mut applicability); let second = Sugg::hir_with_applicability(cx, e2, "..", &mut applicability); + let Some(sugg) = std_or_core(cx) else { return }; + span_lint_and_then( cx, MANUAL_SWAP, @@ -122,11 +124,11 @@ fn generate_swap_warning(cx: &LateContext<'_>, e1: &Expr<'_>, e2: &Expr<'_>, spa diag.span_suggestion( span, "try", - format!("std::mem::swap({}, {})", first.mut_addr(), second.mut_addr()), + format!("{}::mem::swap({}, {})", sugg, first.mut_addr(), second.mut_addr()), applicability, ); if !is_xor_based { - diag.note("or maybe you should use `std::mem::replace`?"); + diag.note(&format!("or maybe you should use `{}::mem::replace`?", sugg)); } }, ); @@ -187,26 +189,30 @@ fn check_suspicious_swap(cx: &LateContext<'_>, block: &Block<'_>) { }; let span = first.span.to(second.span); + let Some(sugg) = std_or_core(cx) else { return }; span_lint_and_then(cx, - ALMOST_SWAPPED, - span, - &format!("this looks like you are trying to swap{}", what), - |diag| { - if !what.is_empty() { - diag.span_suggestion( - span, - "try", - format!( - "std::mem::swap({}, {})", - lhs, - rhs, - ), - Applicability::MaybeIncorrect, - ); - diag.note("or maybe you should use `std::mem::replace`?"); - } - }); + ALMOST_SWAPPED, + span, + &format!("this looks like you are trying to swap{}", what), + |diag| { + if !what.is_empty() { + diag.span_suggestion( + span, + "try", + format!( + "{}::mem::swap({}, {})", + sugg, + lhs, + rhs, + ), + Applicability::MaybeIncorrect, + ); + diag.note( + &format!("or maybe you should use `{}::mem::replace`?", sugg) + ); + } + }); } } } diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 95642adeff6c..3fdea55aaa1b 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -1809,6 +1809,16 @@ pub fn is_expr_final_block_expr(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> bool { matches!(get_parent_node(tcx, expr.hir_id), Some(Node::Block(..))) } +pub fn std_or_core(cx: &LateContext<'_>) -> Option<&'static str> { + if !is_no_std_crate(cx) { + Some("std") + } else if !is_no_core_crate(cx) { + Some("core") + } else { + None + } +} + pub fn is_no_std_crate(cx: &LateContext<'_>) -> bool { cx.tcx.hir().attrs(hir::CRATE_HIR_ID).iter().any(|attr| { if let ast::AttrKind::Normal(ref attr, _) = attr.kind { @@ -1819,6 +1829,16 @@ pub fn is_no_std_crate(cx: &LateContext<'_>) -> bool { }) } +pub fn is_no_core_crate(cx: &LateContext<'_>) -> bool { + cx.tcx.hir().attrs(hir::CRATE_HIR_ID).iter().any(|attr| { + if let ast::AttrKind::Normal(ref attr, _) = attr.kind { + attr.path == sym::no_core + } else { + false + } + }) +} + /// Check if parent of a hir node is a trait implementation block. /// For example, `f` in /// ```rust,ignore From dcd1a16dd059fdc2c2678c51777f8af8a3a6b394 Mon Sep 17 00:00:00 2001 From: dswij Date: Mon, 25 Oct 2021 12:32:53 +0800 Subject: [PATCH 3/3] Add test for `swap` lint when `no_std` is present Adds additional test to check for `swap` suggestion when `no_std` is present --- tests/ui/crate_level_checks/no_std_swap.rs | 15 +++++++++++++++ tests/ui/crate_level_checks/no_std_swap.stderr | 12 ++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 tests/ui/crate_level_checks/no_std_swap.rs create mode 100644 tests/ui/crate_level_checks/no_std_swap.stderr diff --git a/tests/ui/crate_level_checks/no_std_swap.rs b/tests/ui/crate_level_checks/no_std_swap.rs new file mode 100644 index 000000000000..f5ff9eed7a1a --- /dev/null +++ b/tests/ui/crate_level_checks/no_std_swap.rs @@ -0,0 +1,15 @@ +#![no_std] +#![feature(lang_items, start, libc)] +#![crate_type = "lib"] + +use core::panic::PanicInfo; + +#[warn(clippy::all)] +fn main() { + // TODO: do somethjing with swap + let mut a = 42; + let mut b = 1337; + + a = b; + b = a; +} diff --git a/tests/ui/crate_level_checks/no_std_swap.stderr b/tests/ui/crate_level_checks/no_std_swap.stderr new file mode 100644 index 000000000000..079f1828cf93 --- /dev/null +++ b/tests/ui/crate_level_checks/no_std_swap.stderr @@ -0,0 +1,12 @@ +error: this looks like you are trying to swap `a` and `b` + --> $DIR/no_std_swap.rs:13:5 + | +LL | / a = b; +LL | | b = a; + | |_________^ help: try: `core::mem::swap(&mut a, &mut b)` + | + = note: `-D clippy::almost-swapped` implied by `-D warnings` + = note: or maybe you should use `core::mem::replace`? + +error: aborting due to previous error +