diff --git a/clippy_dev/src/update_lints.rs b/clippy_dev/src/update_lints.rs index 93955bee3f4d..0eb443167ecf 100644 --- a/clippy_dev/src/update_lints.rs +++ b/clippy_dev/src/update_lints.rs @@ -45,9 +45,8 @@ fn generate_lint_files( renamed_lints: &[RenamedLint], ) { let internal_lints = Lint::internal_lints(lints); - let usable_lints = Lint::usable_lints(lints); - let mut sorted_usable_lints = usable_lints.clone(); - sorted_usable_lints.sort_by_key(|lint| lint.name.clone()); + let mut usable_lints = Lint::usable_lints(lints); + usable_lints.sort_by_key(|lint| lint.name.clone()); replace_region_in_file( update_mode, diff --git a/clippy_lints/src/asm_syntax.rs b/clippy_lints/src/asm_syntax.rs index ad31d708f64d..9717aa9e981f 100644 --- a/clippy_lints/src/asm_syntax.rs +++ b/clippy_lints/src/asm_syntax.rs @@ -64,6 +64,7 @@ declare_clippy_lint! { /// /// ```rust,no_run /// # #![feature(asm)] + /// # #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] /// # unsafe { let ptr = "".as_ptr(); /// # use std::arch::asm; /// asm!("lea {}, [{}]", lateout(reg) _, in(reg) ptr); @@ -72,6 +73,7 @@ declare_clippy_lint! { /// Use instead: /// ```rust,no_run /// # #![feature(asm)] + /// # #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] /// # unsafe { let ptr = "".as_ptr(); /// # use std::arch::asm; /// asm!("lea ({}), {}", in(reg) ptr, lateout(reg) _, options(att_syntax)); @@ -103,6 +105,7 @@ declare_clippy_lint! { /// /// ```rust,no_run /// # #![feature(asm)] + /// # #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] /// # unsafe { let ptr = "".as_ptr(); /// # use std::arch::asm; /// asm!("lea ({}), {}", in(reg) ptr, lateout(reg) _, options(att_syntax)); @@ -111,6 +114,7 @@ declare_clippy_lint! { /// Use instead: /// ```rust,no_run /// # #![feature(asm)] + /// # #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] /// # unsafe { let ptr = "".as_ptr(); /// # use std::arch::asm; /// asm!("lea {}, [{}]", lateout(reg) _, in(reg) ptr); diff --git a/clippy_lints/src/await_holding_invalid.rs b/clippy_lints/src/await_holding_invalid.rs index 1761360fb281..24a3588ecf16 100644 --- a/clippy_lints/src/await_holding_invalid.rs +++ b/clippy_lints/src/await_holding_invalid.rs @@ -6,7 +6,7 @@ use rustc_hir::{def::Res, AsyncGeneratorKind, Body, BodyId, GeneratorKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::GeneratorInteriorTypeCause; use rustc_session::{declare_tool_lint, impl_lint_pass}; -use rustc_span::Span; +use rustc_span::{sym, Span}; use crate::utils::conf::DisallowedType; @@ -276,9 +276,9 @@ fn emit_invalid_type(cx: &LateContext<'_>, span: Span, disallowed: &DisallowedTy } fn is_mutex_guard(cx: &LateContext<'_>, def_id: DefId) -> bool { - match_def_path(cx, def_id, &paths::MUTEX_GUARD) - || match_def_path(cx, def_id, &paths::RWLOCK_READ_GUARD) - || match_def_path(cx, def_id, &paths::RWLOCK_WRITE_GUARD) + cx.tcx.is_diagnostic_item(sym::MutexGuard, def_id) + || cx.tcx.is_diagnostic_item(sym::RwLockReadGuard, def_id) + || cx.tcx.is_diagnostic_item(sym::RwLockWriteGuard, def_id) || match_def_path(cx, def_id, &paths::PARKING_LOT_MUTEX_GUARD) || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_READ_GUARD) || match_def_path(cx, def_id, &paths::PARKING_LOT_RWLOCK_WRITE_GUARD) diff --git a/clippy_lints/src/blocks_in_if_conditions.rs b/clippy_lints/src/blocks_in_if_conditions.rs index 4df4d6ddf416..569bf27c3e71 100644 --- a/clippy_lints/src/blocks_in_if_conditions.rs +++ b/clippy_lints/src/blocks_in_if_conditions.rs @@ -3,10 +3,11 @@ use clippy_utils::get_parent_expr; use clippy_utils::higher; use clippy_utils::source::snippet_block_with_applicability; use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::{for_each_expr, Descend}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{BlockCheckMode, Closure, Expr, ExprKind}; +use rustc_hir::{BlockCheckMode, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -44,39 +45,6 @@ declare_clippy_lint! { declare_lint_pass!(BlocksInIfConditions => [BLOCKS_IN_IF_CONDITIONS]); -struct ExVisitor<'a, 'tcx> { - found_block: Option<&'tcx Expr<'tcx>>, - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> Visitor<'tcx> for ExVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if let ExprKind::Closure(&Closure { body, .. }) = expr.kind { - // do not lint if the closure is called using an iterator (see #1141) - if_chain! { - if let Some(parent) = get_parent_expr(self.cx, expr); - if let ExprKind::MethodCall(_, self_arg, ..) = &parent.kind; - let caller = self.cx.typeck_results().expr_ty(self_arg); - if let Some(iter_id) = self.cx.tcx.get_diagnostic_item(sym::Iterator); - if implements_trait(self.cx, caller, iter_id, &[]); - then { - return; - } - } - - let body = self.cx.tcx.hir().body(body); - let ex = &body.value; - if let ExprKind::Block(block, _) = ex.kind { - if !body.value.span.from_expansion() && !block.stmts.is_empty() { - self.found_block = Some(ex); - return; - } - } - } - walk_expr(self, expr); - } -} - const BRACED_EXPR_MESSAGE: &str = "omit braces around single expression condition"; const COMPLEX_BLOCK_MESSAGE: &str = "in an `if` condition, avoid complex blocks or closures with blocks; \ instead, move the block or closure higher and bind it with a `let`"; @@ -144,11 +112,31 @@ impl<'tcx> LateLintPass<'tcx> for BlocksInIfConditions { } } } else { - let mut visitor = ExVisitor { found_block: None, cx }; - walk_expr(&mut visitor, cond); - if let Some(block) = visitor.found_block { - span_lint(cx, BLOCKS_IN_IF_CONDITIONS, block.span, COMPLEX_BLOCK_MESSAGE); - } + let _: Option = for_each_expr(cond, |e| { + if let ExprKind::Closure(closure) = e.kind { + // do not lint if the closure is called using an iterator (see #1141) + if_chain! { + if let Some(parent) = get_parent_expr(cx, e); + if let ExprKind::MethodCall(_, self_arg, _, _) = &parent.kind; + let caller = cx.typeck_results().expr_ty(self_arg); + if let Some(iter_id) = cx.tcx.get_diagnostic_item(sym::Iterator); + if implements_trait(cx, caller, iter_id, &[]); + then { + return ControlFlow::Continue(Descend::No); + } + } + + let body = cx.tcx.hir().body(closure.body); + let ex = &body.value; + if let ExprKind::Block(block, _) = ex.kind { + if !body.value.span.from_expansion() && !block.stmts.is_empty() { + span_lint(cx, BLOCKS_IN_IF_CONDITIONS, ex.span, COMPLEX_BLOCK_MESSAGE); + return ControlFlow::Continue(Descend::No); + } + } + } + ControlFlow::Continue(Descend::Yes) + }); } } } diff --git a/clippy_lints/src/bool_to_int_with_if.rs b/clippy_lints/src/bool_to_int_with_if.rs index 51e98cda8451..001d74c26054 100644 --- a/clippy_lints/src/bool_to_int_with_if.rs +++ b/clippy_lints/src/bool_to_int_with_if.rs @@ -3,7 +3,7 @@ use rustc_hir::{Block, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, sugg::Sugg}; +use clippy_utils::{diagnostics::span_lint_and_then, is_else_clause, is_integer_literal, sugg::Sugg}; use rustc_errors::Applicability; declare_clippy_lint! { @@ -56,13 +56,9 @@ fn check_if_else<'tcx>(ctx: &LateContext<'tcx>, expr: &'tcx rustc_hir::Expr<'tcx && let Some(then_lit) = int_literal(then) && let Some(else_lit) = int_literal(else_) { - let inverted = if - check_int_literal_equals_val(then_lit, 1) - && check_int_literal_equals_val(else_lit, 0) { + let inverted = if is_integer_literal(then_lit, 1) && is_integer_literal(else_lit, 0) { false - } else if - check_int_literal_equals_val(then_lit, 0) - && check_int_literal_equals_val(else_lit, 1) { + } else if is_integer_literal(then_lit, 0) && is_integer_literal(else_lit, 1) { true } else { // Expression isn't boolean, exit @@ -123,14 +119,3 @@ fn int_literal<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>) -> Option<&'tcx rustc_hi None } } - -fn check_int_literal_equals_val<'tcx>(expr: &'tcx rustc_hir::Expr<'tcx>, expected_value: u128) -> bool { - if let ExprKind::Lit(lit) = &expr.kind - && let LitKind::Int(val, _) = lit.node - && val == expected_value - { - true - } else { - false - } -} diff --git a/clippy_lints/src/casts/unnecessary_cast.rs b/clippy_lints/src/casts/unnecessary_cast.rs index ea29f5d12c67..21ed7f4844cc 100644 --- a/clippy_lints/src/casts/unnecessary_cast.rs +++ b/clippy_lints/src/casts/unnecessary_cast.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::get_parent_expr; use clippy_utils::numeric_literal::NumericLiteral; use clippy_utils::source::snippet_opt; use if_chain::if_chain; @@ -30,8 +31,10 @@ pub(super) fn check<'tcx>( } } + let cast_str = snippet_opt(cx, cast_expr.span).unwrap_or_default(); + if let Some(lit) = get_numeric_literal(cast_expr) { - let literal_str = snippet_opt(cx, cast_expr.span).unwrap_or_default(); + let literal_str = &cast_str; if_chain! { if let LitKind::Int(n, _) = lit.node; @@ -49,12 +52,16 @@ pub(super) fn check<'tcx>( match lit.node { LitKind::Int(_, LitIntType::Unsuffixed) if cast_to.is_integral() => { - lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + lint_unnecessary_cast(cx, expr, literal_str, cast_from, cast_to); + return false; }, LitKind::Float(_, LitFloatType::Unsuffixed) if cast_to.is_floating_point() => { - lint_unnecessary_cast(cx, expr, &literal_str, cast_from, cast_to); + lint_unnecessary_cast(cx, expr, literal_str, cast_from, cast_to); + return false; + }, + LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => { + return false; }, - LitKind::Int(_, LitIntType::Unsuffixed) | LitKind::Float(_, LitFloatType::Unsuffixed) => {}, LitKind::Int(_, LitIntType::Signed(_) | LitIntType::Unsigned(_)) | LitKind::Float(_, LitFloatType::Suffixed(_)) if cast_from.kind() == cast_to.kind() => @@ -62,45 +69,62 @@ pub(super) fn check<'tcx>( if let Some(src) = snippet_opt(cx, cast_expr.span) { if let Some(num_lit) = NumericLiteral::from_lit_kind(&src, &lit.node) { lint_unnecessary_cast(cx, expr, num_lit.integer, cast_from, cast_to); + return true; } } }, - _ => { - if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { - span_lint_and_sugg( - cx, - UNNECESSARY_CAST, - expr.span, - &format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"), - "try", - literal_str, - Applicability::MachineApplicable, - ); - return true; - } - }, + _ => {}, } } + if cast_from.kind() == cast_to.kind() && !in_external_macro(cx.sess(), expr.span) { + span_lint_and_sugg( + cx, + UNNECESSARY_CAST, + expr.span, + &format!("casting to the same type is unnecessary (`{cast_from}` -> `{cast_to}`)"), + "try", + cast_str, + Applicability::MachineApplicable, + ); + return true; + } + false } -fn lint_unnecessary_cast(cx: &LateContext<'_>, expr: &Expr<'_>, literal_str: &str, cast_from: Ty<'_>, cast_to: Ty<'_>) { +fn lint_unnecessary_cast( + cx: &LateContext<'_>, + expr: &Expr<'_>, + raw_literal_str: &str, + cast_from: Ty<'_>, + cast_to: Ty<'_>, +) { let literal_kind_name = if cast_from.is_integral() { "integer" } else { "float" }; - let replaced_literal; - let matchless = if literal_str.contains(['(', ')']) { - replaced_literal = literal_str.replace(['(', ')'], ""); - &replaced_literal - } else { - literal_str + // first we remove all matches so `-(1)` become `-1`, and remove trailing dots, so `1.` become `1` + let literal_str = raw_literal_str + .replace(['(', ')'], "") + .trim_end_matches('.') + .to_string(); + // we know need to check if the parent is a method call, to add parenthesis accordingly (eg: + // (-1).foo() instead of -1.foo()) + let sugg = if let Some(parent_expr) = get_parent_expr(cx, expr) + && let ExprKind::MethodCall(..) = parent_expr.kind + && literal_str.starts_with('-') + { + format!("({literal_str}_{cast_to})") + + } else { + format!("{literal_str}_{cast_to}") }; + span_lint_and_sugg( cx, UNNECESSARY_CAST, expr.span, &format!("casting {literal_kind_name} literal to `{cast_to}` is unnecessary"), "try", - format!("{}_{cast_to}", matchless.trim_end_matches('.')), + sugg, Applicability::MachineApplicable, ); } diff --git a/clippy_lints/src/checked_conversions.rs b/clippy_lints/src/checked_conversions.rs index 1d113c7cbee6..78e9921f036f 100644 --- a/clippy_lints/src/checked_conversions.rs +++ b/clippy_lints/src/checked_conversions.rs @@ -2,9 +2,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{in_constant, meets_msrv, msrvs, SpanlessEq}; +use clippy_utils::{in_constant, is_integer_literal, meets_msrv, msrvs, SpanlessEq}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::{BinOp, BinOpKind, Expr, ExprKind, QPath, TyKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -223,16 +222,7 @@ fn check_lower_bound<'tcx>(expr: &'tcx Expr<'tcx>) -> Option> { /// Check for `expr >= 0` fn check_lower_bound_zero<'a>(candidate: &'a Expr<'_>, check: &'a Expr<'_>) -> Option> { - if_chain! { - if let ExprKind::Lit(ref lit) = &check.kind; - if let LitKind::Int(0, _) = &lit.node; - - then { - Some(Conversion::new_any(candidate)) - } else { - None - } - } + is_integer_literal(check, 0).then(|| Conversion::new_any(candidate)) } /// Check for `expr >= (to_type::MIN as from_type)` diff --git a/clippy_lints/src/cognitive_complexity.rs b/clippy_lints/src/cognitive_complexity.rs index fed04ae7f3d5..77af3b53d633 100644 --- a/clippy_lints/src/cognitive_complexity.rs +++ b/clippy_lints/src/cognitive_complexity.rs @@ -3,10 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::for_each_expr; use clippy_utils::LimitStack; +use core::ops::ControlFlow; use rustc_ast::ast::Attribute; -use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; -use rustc_hir::{Body, Expr, ExprKind, FnDecl, HirId}; +use rustc_hir::intravisit::FnKind; +use rustc_hir::{Body, ExprKind, FnDecl, HirId}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; @@ -61,11 +63,27 @@ impl CognitiveComplexity { return; } - let expr = &body.value; + let expr = body.value; + + let mut cc = 1u64; + let mut returns = 0u64; + let _: Option = for_each_expr(expr, |e| { + match e.kind { + ExprKind::If(_, _, _) => { + cc += 1; + }, + ExprKind::Match(_, arms, _) => { + if arms.len() > 1 { + cc += 1; + } + cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64; + }, + ExprKind::Ret(_) => returns += 1, + _ => {}, + } + ControlFlow::Continue(()) + }); - let mut helper = CcHelper { cc: 1, returns: 0 }; - helper.visit_expr(expr); - let CcHelper { cc, returns } = helper; let ret_ty = cx.typeck_results().node_type(expr.hir_id); let ret_adjust = if is_type_diagnostic_item(cx, ret_ty, sym::Result) { returns @@ -74,13 +92,12 @@ impl CognitiveComplexity { (returns / 2) }; - let mut rust_cc = cc; // prevent degenerate cases where unreachable code contains `return` statements - if rust_cc >= ret_adjust { - rust_cc -= ret_adjust; + if cc >= ret_adjust { + cc -= ret_adjust; } - if rust_cc > self.limit.limit() { + if cc > self.limit.limit() { let fn_span = match kind { FnKind::ItemFn(ident, _, _) | FnKind::Method(ident, _) => ident.span, FnKind::Closure => { @@ -107,7 +124,7 @@ impl CognitiveComplexity { COGNITIVE_COMPLEXITY, fn_span, &format!( - "the function has a cognitive complexity of ({rust_cc}/{})", + "the function has a cognitive complexity of ({cc}/{})", self.limit.limit() ), None, @@ -140,27 +157,3 @@ impl<'tcx> LateLintPass<'tcx> for CognitiveComplexity { self.limit.pop_attrs(cx.sess(), attrs, "cognitive_complexity"); } } - -struct CcHelper { - cc: u64, - returns: u64, -} - -impl<'tcx> Visitor<'tcx> for CcHelper { - fn visit_expr(&mut self, e: &'tcx Expr<'_>) { - walk_expr(self, e); - match e.kind { - ExprKind::If(_, _, _) => { - self.cc += 1; - }, - ExprKind::Match(_, arms, _) => { - if arms.len() > 1 { - self.cc += 1; - } - self.cc += arms.iter().filter(|arm| arm.guard.is_some()).count() as u64; - }, - ExprKind::Ret(_) => self.returns += 1, - _ => {}, - } - } -} diff --git a/clippy_lints/src/from_str_radix_10.rs b/clippy_lints/src/from_str_radix_10.rs index 2a82473be8c5..cf8b7acd66d2 100644 --- a/clippy_lints/src/from_str_radix_10.rs +++ b/clippy_lints/src/from_str_radix_10.rs @@ -1,4 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_integer_literal; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; @@ -60,8 +61,7 @@ impl<'tcx> LateLintPass<'tcx> for FromStrRadix10 { if pathseg.ident.name.as_str() == "from_str_radix"; // check if the second argument is a primitive `10` - if let ExprKind::Lit(lit) = &radix.kind; - if let rustc_ast::ast::LitKind::Int(10, _) = lit.node; + if is_integer_literal(radix, 10); then { let expr = if let ExprKind::AddrOf(_, _, expr) = &src.kind { diff --git a/clippy_lints/src/functions/must_use.rs b/clippy_lints/src/functions/must_use.rs index 977c8ee594f4..d263804f32cf 100644 --- a/clippy_lints/src/functions/must_use.rs +++ b/clippy_lints/src/functions/must_use.rs @@ -1,7 +1,7 @@ use rustc_ast::ast::Attribute; use rustc_errors::Applicability; use rustc_hir::def_id::{DefIdSet, LocalDefId}; -use rustc_hir::{self as hir, def::Res, intravisit, QPath}; +use rustc_hir::{self as hir, def::Res, QPath}; use rustc_lint::{LateContext, LintContext}; use rustc_middle::{ lint::in_external_macro, @@ -13,8 +13,11 @@ use clippy_utils::attrs::is_proc_macro; use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_then}; use clippy_utils::source::snippet_opt; use clippy_utils::ty::is_must_use_ty; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{match_def_path, return_ty, trait_ref_of_method}; +use core::ops::ControlFlow; + use super::{DOUBLE_MUST_USE, MUST_USE_CANDIDATE, MUST_USE_UNIT}; pub(super) fn check_item<'tcx>(cx: &LateContext<'tcx>, item: &'tcx hir::Item<'_>) { @@ -200,63 +203,6 @@ fn is_mutable_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, span: Span, tys: &m } } -struct StaticMutVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - mutates_static: bool, -} - -impl<'a, 'tcx> intravisit::Visitor<'tcx> for StaticMutVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall}; - - if self.mutates_static { - return; - } - match expr.kind { - Call(_, args) => { - let mut tys = DefIdSet::default(); - for arg in args { - if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) - && is_mutable_ty( - self.cx, - self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), - arg.span, - &mut tys, - ) - && is_mutated_static(arg) - { - self.mutates_static = true; - return; - } - tys.clear(); - } - }, - MethodCall(_, receiver, args, _) => { - let mut tys = DefIdSet::default(); - for arg in std::iter::once(receiver).chain(args.iter()) { - if self.cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) - && is_mutable_ty( - self.cx, - self.cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), - arg.span, - &mut tys, - ) - && is_mutated_static(arg) - { - self.mutates_static = true; - return; - } - tys.clear(); - } - }, - Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) => { - self.mutates_static |= is_mutated_static(target); - }, - _ => {}, - } - } -} - fn is_mutated_static(e: &hir::Expr<'_>) -> bool { use hir::ExprKind::{Field, Index, Path}; @@ -269,10 +215,53 @@ fn is_mutated_static(e: &hir::Expr<'_>) -> bool { } fn mutates_static<'tcx>(cx: &LateContext<'tcx>, body: &'tcx hir::Body<'_>) -> bool { - let mut v = StaticMutVisitor { - cx, - mutates_static: false, - }; - intravisit::walk_expr(&mut v, body.value); - v.mutates_static + for_each_expr(body.value, |e| { + use hir::ExprKind::{AddrOf, Assign, AssignOp, Call, MethodCall}; + + match e.kind { + Call(_, args) => { + let mut tys = DefIdSet::default(); + for arg in args { + if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) + && is_mutable_ty( + cx, + cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), + arg.span, + &mut tys, + ) + && is_mutated_static(arg) + { + return ControlFlow::Break(()); + } + tys.clear(); + } + ControlFlow::Continue(()) + }, + MethodCall(_, receiver, args, _) => { + let mut tys = DefIdSet::default(); + for arg in std::iter::once(receiver).chain(args.iter()) { + if cx.tcx.has_typeck_results(arg.hir_id.owner.to_def_id()) + && is_mutable_ty( + cx, + cx.tcx.typeck(arg.hir_id.owner.def_id).expr_ty(arg), + arg.span, + &mut tys, + ) + && is_mutated_static(arg) + { + return ControlFlow::Break(()); + } + tys.clear(); + } + ControlFlow::Continue(()) + }, + Assign(target, ..) | AssignOp(_, target, _) | AddrOf(_, hir::Mutability::Mut, target) + if is_mutated_static(target) => + { + ControlFlow::Break(()) + }, + _ => ControlFlow::Continue(()), + } + }) + .is_some() } diff --git a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs index 0b50431fbaab..b7595d101e0f 100644 --- a/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs +++ b/clippy_lints/src/functions/not_unsafe_ptr_arg_deref.rs @@ -5,8 +5,11 @@ use rustc_span::def_id::LocalDefId; use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::type_is_unsafe_function; +use clippy_utils::visitors::for_each_expr_with_closures; use clippy_utils::{iter_input_pats, path_to_local}; +use core::ops::ControlFlow; + use super::NOT_UNSAFE_PTR_ARG_DEREF; pub(super) fn check_fn<'tcx>( @@ -39,21 +42,34 @@ fn check_raw_ptr<'tcx>( body: &'tcx hir::Body<'tcx>, def_id: LocalDefId, ) { - let expr = &body.value; if unsafety == hir::Unsafety::Normal && cx.access_levels.is_exported(def_id) { let raw_ptrs = iter_input_pats(decl, body) .filter_map(|arg| raw_ptr_arg(cx, arg)) .collect::(); if !raw_ptrs.is_empty() { - let typeck_results = cx.tcx.typeck_body(body.id()); - let mut v = DerefVisitor { - cx, - ptrs: raw_ptrs, - typeck_results, - }; - - intravisit::walk_expr(&mut v, expr); + let typeck = cx.tcx.typeck_body(body.id()); + let _: Option = for_each_expr_with_closures(cx, body.value, |e| { + match e.kind { + hir::ExprKind::Call(f, args) if type_is_unsafe_function(cx, typeck.expr_ty(f)) => { + for arg in args { + check_arg(cx, &raw_ptrs, arg); + } + }, + hir::ExprKind::MethodCall(_, recv, args, _) => { + let def_id = typeck.type_dependent_def_id(e.hir_id).unwrap(); + if cx.tcx.fn_sig(def_id).skip_binder().unsafety == hir::Unsafety::Unsafe { + check_arg(cx, &raw_ptrs, recv); + for arg in args { + check_arg(cx, &raw_ptrs, arg); + } + } + }, + hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => check_arg(cx, &raw_ptrs, ptr), + _ => (), + } + ControlFlow::Continue(()) + }); } } } @@ -70,54 +86,13 @@ fn raw_ptr_arg(cx: &LateContext<'_>, arg: &hir::Param<'_>) -> Option } } -struct DerefVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - ptrs: HirIdSet, - typeck_results: &'a ty::TypeckResults<'tcx>, -} - -impl<'a, 'tcx> intravisit::Visitor<'tcx> for DerefVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - match expr.kind { - hir::ExprKind::Call(f, args) => { - let ty = self.typeck_results.expr_ty(f); - - if type_is_unsafe_function(self.cx, ty) { - for arg in args { - self.check_arg(arg); - } - } - }, - hir::ExprKind::MethodCall(_, receiver, args, _) => { - let def_id = self.typeck_results.type_dependent_def_id(expr.hir_id).unwrap(); - let base_type = self.cx.tcx.type_of(def_id); - - if type_is_unsafe_function(self.cx, base_type) { - self.check_arg(receiver); - for arg in args { - self.check_arg(arg); - } - } - }, - hir::ExprKind::Unary(hir::UnOp::Deref, ptr) => self.check_arg(ptr), - _ => (), - } - - intravisit::walk_expr(self, expr); - } -} - -impl<'a, 'tcx> DerefVisitor<'a, 'tcx> { - fn check_arg(&self, ptr: &hir::Expr<'_>) { - if let Some(id) = path_to_local(ptr) { - if self.ptrs.contains(&id) { - span_lint( - self.cx, - NOT_UNSAFE_PTR_ARG_DEREF, - ptr.span, - "this public function might dereference a raw pointer but is not marked `unsafe`", - ); - } - } +fn check_arg(cx: &LateContext<'_>, raw_ptrs: &HirIdSet, arg: &hir::Expr<'_>) { + if path_to_local(arg).map_or(false, |id| raw_ptrs.contains(&id)) { + span_lint( + cx, + NOT_UNSAFE_PTR_ARG_DEREF, + arg.span, + "this public function might dereference a raw pointer but is not marked `unsafe`", + ); } } diff --git a/clippy_lints/src/if_then_some_else_none.rs b/clippy_lints/src/if_then_some_else_none.rs index 0800e0644f7f..0d6718c168a5 100644 --- a/clippy_lints/src/if_then_some_else_none.rs +++ b/clippy_lints/src/if_then_some_else_none.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::eager_or_lazy::switch_to_eager_eval; use clippy_utils::source::snippet_with_macro_callsite; -use clippy_utils::{contains_return, higher, is_else_clause, is_lang_ctor, meets_msrv, msrvs, peel_blocks}; +use clippy_utils::{ + contains_return, higher, is_else_clause, is_res_lang_ctor, meets_msrv, msrvs, path_res, peel_blocks, +}; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_hir::{Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; @@ -76,10 +78,8 @@ impl<'tcx> LateLintPass<'tcx> for IfThenSomeElseNone { && let ExprKind::Block(then_block, _) = then.kind && let Some(then_expr) = then_block.expr && let ExprKind::Call(then_call, [then_arg]) = then_expr.kind - && let ExprKind::Path(ref then_call_qpath) = then_call.kind - && is_lang_ctor(cx, then_call_qpath, OptionSome) - && let ExprKind::Path(ref qpath) = peel_blocks(els).kind - && is_lang_ctor(cx, qpath, OptionNone) + && is_res_lang_ctor(cx, path_res(cx, then_call), OptionSome) + && is_res_lang_ctor(cx, path_res(cx, peel_blocks(els)), OptionNone) && !stmts_contains_early_return(then_block.stmts) { let cond_snip = snippet_with_macro_callsite(cx, cond.span, "[condition]"); diff --git a/clippy_lints/src/implicit_return.rs b/clippy_lints/src/implicit_return.rs index cfc988da2335..946d04eff6f9 100644 --- a/clippy_lints/src/implicit_return.rs +++ b/clippy_lints/src/implicit_return.rs @@ -2,10 +2,11 @@ use clippy_utils::{ diagnostics::span_lint_hir_and_then, get_async_fn_body, is_async_fn, source::{snippet_with_applicability, snippet_with_context, walk_span_to_context}, - visitors::expr_visitor_no_bodies, + visitors::for_each_expr, }; +use core::ops::ControlFlow; use rustc_errors::Applicability; -use rustc_hir::intravisit::{FnKind, Visitor}; +use rustc_hir::intravisit::FnKind; use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, FnRetTy, HirId}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; @@ -152,7 +153,7 @@ fn lint_implicit_returns( ExprKind::Loop(block, ..) => { let mut add_return = false; - expr_visitor_no_bodies(|e| { + let _: Option = for_each_expr(block, |e| { if let ExprKind::Break(dest, sub_expr) = e.kind { if dest.target_id.ok() == Some(expr.hir_id) { if call_site_span.is_none() && e.span.ctxt() == ctxt { @@ -167,9 +168,8 @@ fn lint_implicit_returns( } } } - true - }) - .visit_block(block); + ControlFlow::Continue(()) + }); if add_return { #[expect(clippy::option_if_let_else)] if let Some(span) = call_site_span { diff --git a/clippy_lints/src/implicit_saturating_sub.rs b/clippy_lints/src/implicit_saturating_sub.rs index f0dbe17d83a5..48edbf6ae576 100644 --- a/clippy_lints/src/implicit_saturating_sub.rs +++ b/clippy_lints/src/implicit_saturating_sub.rs @@ -1,5 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::{higher, peel_blocks_with_stmt, SpanlessEq}; +use clippy_utils::{higher, is_integer_literal, peel_blocks_with_stmt, SpanlessEq}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; @@ -131,17 +131,8 @@ impl<'tcx> LateLintPass<'tcx> for ImplicitSaturatingSub { fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Expr<'a>> { match peel_blocks_with_stmt(expr).kind { ExprKind::AssignOp(ref op1, target, value) => { - if_chain! { - if BinOpKind::Sub == op1.node; - // Check if literal being subtracted is one - if let ExprKind::Lit(ref lit1) = value.kind; - if let LitKind::Int(1, _) = lit1.node; - then { - Some(target) - } else { - None - } - } + // Check if literal being subtracted is one + (BinOpKind::Sub == op1.node && is_integer_literal(value, 1)).then_some(target) }, ExprKind::Assign(target, value, _) => { if_chain! { @@ -150,8 +141,7 @@ fn subtracts_one<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<&'a Exp if SpanlessEq::new(cx).eq_expr(left1, target); - if let ExprKind::Lit(ref lit1) = right1.kind; - if let LitKind::Int(1, _) = lit1.node; + if is_integer_literal(right1, 1); then { Some(target) } else { diff --git a/clippy_lints/src/infinite_iter.rs b/clippy_lints/src/infinite_iter.rs index 8c2c96fa105a..d1d2db27c6fc 100644 --- a/clippy_lints/src/infinite_iter.rs +++ b/clippy_lints/src/infinite_iter.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::higher; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; -use clippy_utils::{higher, match_def_path, path_def_id, paths}; use rustc_hir::{BorrowKind, Closure, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -168,9 +168,16 @@ fn is_infinite(cx: &LateContext<'_>, expr: &Expr<'_>) -> Finiteness { }, ExprKind::Block(block, _) => block.expr.as_ref().map_or(Finite, |e| is_infinite(cx, e)), ExprKind::Box(e) | ExprKind::AddrOf(BorrowKind::Ref, _, e) => is_infinite(cx, e), - ExprKind::Call(path, _) => path_def_id(cx, path) - .map_or(false, |id| match_def_path(cx, id, &paths::ITER_REPEAT)) - .into(), + ExprKind::Call(path, _) => { + if let ExprKind::Path(ref qpath) = path.kind { + cx.qpath_res(qpath, path.hir_id) + .opt_def_id() + .map_or(false, |id| cx.tcx.is_diagnostic_item(sym::iter_repeat, id)) + .into() + } else { + Finite + } + }, ExprKind::Struct(..) => higher::Range::hir(expr).map_or(false, |r| r.end.is_none()).into(), _ => Finite, } diff --git a/clippy_lints/src/inherent_to_string.rs b/clippy_lints/src/inherent_to_string.rs index d0e603dcf4ef..676136df572b 100644 --- a/clippy_lints/src/inherent_to_string.rs +++ b/clippy_lints/src/inherent_to_string.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_help; use clippy_utils::ty::{implements_trait, is_type_diagnostic_item}; -use clippy_utils::{get_trait_def_id, paths, return_ty, trait_ref_of_method}; +use clippy_utils::{return_ty, trait_ref_of_method}; use if_chain::if_chain; use rustc_hir::{GenericParamKind, ImplItem, ImplItemKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -118,7 +118,10 @@ impl<'tcx> LateLintPass<'tcx> for InherentToString { } fn show_lint(cx: &LateContext<'_>, item: &ImplItem<'_>) { - let display_trait_id = get_trait_def_id(cx, &paths::DISPLAY_TRAIT).expect("Failed to get trait ID of `Display`!"); + let display_trait_id = cx + .tcx + .get_diagnostic_item(sym::Display) + .expect("Failed to get trait ID of `Display`!"); // Get the real type of 'self' let self_type = cx.tcx.fn_sig(item.def_id).input(0); diff --git a/clippy_lints/src/lib.register_internal.rs b/clippy_lints/src/lib.register_internal.rs index be63646a12f5..71dfdab369b9 100644 --- a/clippy_lints/src/lib.register_internal.rs +++ b/clippy_lints/src/lib.register_internal.rs @@ -13,10 +13,10 @@ store.register_group(true, "clippy::internal", Some("clippy_internal"), vec![ LintId::of(utils::internal_lints::INVALID_CLIPPY_VERSION_ATTRIBUTE), LintId::of(utils::internal_lints::INVALID_PATHS), LintId::of(utils::internal_lints::LINT_WITHOUT_LINT_PASS), - LintId::of(utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM), LintId::of(utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE), LintId::of(utils::internal_lints::MISSING_MSRV_ATTR_IMPL), LintId::of(utils::internal_lints::OUTER_EXPN_EXPN_DATA), LintId::of(utils::internal_lints::PRODUCE_ICE), + LintId::of(utils::internal_lints::UNNECESSARY_DEF_PATH), LintId::of(utils::internal_lints::UNNECESSARY_SYMBOL_STR), ]) diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index b8a20de9768d..307ec40f40b3 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -24,8 +24,6 @@ store.register_lints(&[ #[cfg(feature = "internal")] utils::internal_lints::LINT_WITHOUT_LINT_PASS, #[cfg(feature = "internal")] - utils::internal_lints::MATCH_TYPE_ON_DIAGNOSTIC_ITEM, - #[cfg(feature = "internal")] utils::internal_lints::MISSING_CLIPPY_VERSION_ATTRIBUTE, #[cfg(feature = "internal")] utils::internal_lints::MISSING_MSRV_ATTR_IMPL, @@ -34,6 +32,8 @@ store.register_lints(&[ #[cfg(feature = "internal")] utils::internal_lints::PRODUCE_ICE, #[cfg(feature = "internal")] + utils::internal_lints::UNNECESSARY_DEF_PATH, + #[cfg(feature = "internal")] utils::internal_lints::UNNECESSARY_SYMBOL_STR, almost_complete_letter_range::ALMOST_COMPLETE_LETTER_RANGE, approx_const::APPROX_CONSTANT, diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 05a9e13c9fb6..3b78e492baa4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -535,7 +535,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|_| Box::new(utils::internal_lints::InvalidPaths)); store.register_late_pass(|_| Box::::default()); store.register_late_pass(|_| Box::::default()); - store.register_late_pass(|_| Box::new(utils::internal_lints::MatchTypeOnDiagItem)); + store.register_late_pass(|_| Box::new(utils::internal_lints::UnnecessaryDefPath)); store.register_late_pass(|_| Box::new(utils::internal_lints::OuterExpnDataPass)); store.register_late_pass(|_| Box::new(utils::internal_lints::MsrvAttrImpl)); } diff --git a/clippy_lints/src/loops/manual_find.rs b/clippy_lints/src/loops/manual_find.rs index 09b2376d5c04..4bb9936e9cde 100644 --- a/clippy_lints/src/loops/manual_find.rs +++ b/clippy_lints/src/loops/manual_find.rs @@ -1,7 +1,7 @@ use super::utils::make_iterator_snippet; use super::MANUAL_FIND; use clippy_utils::{ - diagnostics::span_lint_and_then, higher, is_lang_ctor, path_res, peel_blocks_with_stmt, + diagnostics::span_lint_and_then, higher, is_res_lang_ctor, path_res, peel_blocks_with_stmt, source::snippet_with_applicability, ty::implements_trait, }; use if_chain::if_chain; @@ -30,8 +30,8 @@ pub(super) fn check<'tcx>( if let [stmt] = block.stmts; if let StmtKind::Semi(semi) = stmt.kind; if let ExprKind::Ret(Some(ret_value)) = semi.kind; - if let ExprKind::Call(Expr { kind: ExprKind::Path(ctor), .. }, [inner_ret]) = ret_value.kind; - if is_lang_ctor(cx, ctor, LangItem::OptionSome); + if let ExprKind::Call(ctor, [inner_ret]) = ret_value.kind; + if is_res_lang_ctor(cx, path_res(cx, ctor), LangItem::OptionSome); if path_res(cx, inner_ret) == Res::Local(binding_id); if let Some((last_stmt, last_ret)) = last_stmt_and_ret(cx, expr); then { @@ -143,8 +143,7 @@ fn last_stmt_and_ret<'tcx>( if let Some((_, Node::Block(block))) = parent_iter.next(); if let Some((last_stmt, last_ret)) = extract(block); if last_stmt.hir_id == node_hir; - if let ExprKind::Path(path) = &last_ret.kind; - if is_lang_ctor(cx, path, LangItem::OptionNone); + if is_res_lang_ctor(cx, path_res(cx, last_ret), LangItem::OptionNone); if let Some((_, Node::Expr(_block))) = parent_iter.next(); // This includes the function header if let Some((_, func)) = parent_iter.next(); diff --git a/clippy_lints/src/loops/manual_flatten.rs b/clippy_lints/src/loops/manual_flatten.rs index 1b36d452647e..8c27c09404b1 100644 --- a/clippy_lints/src/loops/manual_flatten.rs +++ b/clippy_lints/src/loops/manual_flatten.rs @@ -3,13 +3,13 @@ use super::MANUAL_FLATTEN; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher; use clippy_utils::visitors::is_local_used; -use clippy_utils::{is_lang_ctor, path_to_local_id, peel_blocks_with_stmt}; +use clippy_utils::{path_to_local_id, peel_blocks_with_stmt}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionSome, ResultOk}; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::{Expr, Pat, PatKind}; use rustc_lint::LateContext; -use rustc_middle::ty; +use rustc_middle::ty::{self, DefIdTree}; use rustc_span::source_map::Span; /// Check for unnecessary `if let` usage in a for loop where only the `Some` or `Ok` variant of the @@ -30,8 +30,10 @@ pub(super) fn check<'tcx>( if path_to_local_id(let_expr, pat_hir_id); // Ensure the `if let` statement is for the `Some` variant of `Option` or the `Ok` variant of `Result` if let PatKind::TupleStruct(ref qpath, _, _) = let_pat.kind; - let some_ctor = is_lang_ctor(cx, qpath, OptionSome); - let ok_ctor = is_lang_ctor(cx, qpath, ResultOk); + if let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, let_pat.hir_id); + if let Some(variant_id) = cx.tcx.opt_parent(ctor_id); + let some_ctor = cx.tcx.lang_items().option_some_variant() == Some(variant_id); + let ok_ctor = cx.tcx.lang_items().result_ok_variant() == Some(variant_id); if some_ctor || ok_ctor; // Ensure expr in `if let` is not used afterwards if !is_local_used(cx, if_then, pat_hir_id); diff --git a/clippy_lints/src/loops/while_let_on_iterator.rs b/clippy_lints/src/loops/while_let_on_iterator.rs index 1c6f0264cb54..153f97e4e66c 100644 --- a/clippy_lints/src/loops/while_let_on_iterator.rs +++ b/clippy_lints/src/loops/while_let_on_iterator.rs @@ -3,13 +3,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::higher; use clippy_utils::source::snippet_with_applicability; use clippy_utils::{ - get_enclosing_loop_or_multi_call_closure, is_refutable, is_trait_method, match_def_path, paths, - visitors::is_res_used, + get_enclosing_loop_or_multi_call_closure, is_refutable, is_res_lang_ctor, is_trait_method, visitors::is_res_used, }; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_expr, Visitor}; -use rustc_hir::{def::Res, Closure, Expr, ExprKind, HirId, Local, Mutability, PatKind, QPath, UnOp}; +use rustc_hir::{def::Res, Closure, Expr, ExprKind, HirId, LangItem, Local, Mutability, PatKind, UnOp}; use rustc_lint::LateContext; use rustc_middle::hir::nested_filter::OnlyBodies; use rustc_middle::ty::adjustment::Adjust; @@ -19,9 +18,8 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { let (scrutinee_expr, iter_expr_struct, iter_expr, some_pat, loop_expr) = if_chain! { if let Some(higher::WhileLet { if_then, let_pat, let_expr }) = higher::WhileLet::hir(expr); // check for `Some(..)` pattern - if let PatKind::TupleStruct(QPath::Resolved(None, pat_path), some_pat, _) = let_pat.kind; - if let Res::Def(_, pat_did) = pat_path.res; - if match_def_path(cx, pat_did, &paths::OPTION_SOME); + if let PatKind::TupleStruct(ref pat_path, some_pat, _) = let_pat.kind; + if is_res_lang_ctor(cx, cx.qpath_res(pat_path, let_pat.hir_id), LangItem::OptionSome); // check for call to `Iterator::next` if let ExprKind::MethodCall(method_name, iter_expr, [], _) = let_expr.kind; if method_name.ident.name == sym::next; diff --git a/clippy_lints/src/manual_assert.rs b/clippy_lints/src/manual_assert.rs index 26b53ab5d683..825ec84b4a81 100644 --- a/clippy_lints/src/manual_assert.rs +++ b/clippy_lints/src/manual_assert.rs @@ -1,7 +1,8 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; +use crate::rustc_lint::LintContext; +use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::{root_macro_call, FormatArgsExpn}; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{peel_blocks_with_stmt, sugg}; +use clippy_utils::{peel_blocks_with_stmt, span_extract_comment, sugg}; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind, UnOp}; use rustc_lint::{LateContext, LateLintPass}; @@ -50,20 +51,36 @@ impl<'tcx> LateLintPass<'tcx> for ManualAssert { let mut applicability = Applicability::MachineApplicable; let format_args_snip = snippet_with_applicability(cx, format_args.inputs_span(), "..", &mut applicability); let cond = cond.peel_drop_temps(); + let mut comments = span_extract_comment(cx.sess().source_map(), expr.span); + if !comments.is_empty() { + comments += "\n"; + } let (cond, not) = match cond.kind { ExprKind::Unary(UnOp::Not, e) => (e, ""), _ => (cond, "!"), }; let cond_sugg = sugg::Sugg::hir_with_applicability(cx, cond, "..", &mut applicability).maybe_par(); let sugg = format!("assert!({not}{cond_sugg}, {format_args_snip});"); - span_lint_and_sugg( + // we show to the user the suggestion without the comments, but when applicating the fix, include the comments in the block + span_lint_and_then( cx, MANUAL_ASSERT, expr.span, "only a `panic!` in `if`-then statement", - "try", - sugg, - Applicability::MachineApplicable, + |diag| { + // comments can be noisy, do not show them to the user + diag.tool_only_span_suggestion( + expr.span.shrink_to_lo(), + "add comments back", + comments, + applicability); + diag.span_suggestion( + expr.span, + "try instead", + sugg, + applicability); + } + ); } } diff --git a/clippy_lints/src/manual_clamp.rs b/clippy_lints/src/manual_clamp.rs index ac5c24ee604b..ece4df95505c 100644 --- a/clippy_lints/src/manual_clamp.rs +++ b/clippy_lints/src/manual_clamp.rs @@ -324,7 +324,7 @@ fn is_call_max_min_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) outer_arg: &'tcx Expr<'tcx>, span: Span, ) -> Option> { - if let ExprKind::Call(inner_fn, &[ref first, ref second]) = &inner_call.kind + if let ExprKind::Call(inner_fn, [first, second]) = &inner_call.kind && let Some(inner_seg) = segment(cx, inner_fn) && let Some(outer_seg) = segment(cx, outer_fn) { @@ -377,9 +377,7 @@ fn is_call_max_min_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) /// # ; /// ``` fn is_match_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option> { - if let ExprKind::Match(value, &[ref first_arm, ref second_arm, ref last_arm], rustc_hir::MatchSource::Normal) = - &expr.kind - { + if let ExprKind::Match(value, [first_arm, second_arm, last_arm], rustc_hir::MatchSource::Normal) = &expr.kind { // Find possible min/max branches let minmax_values = |a: &'tcx Arm<'tcx>| { if let PatKind::Binding(_, var_hir_id, _, None) = &a.pat.kind diff --git a/clippy_lints/src/manual_retain.rs b/clippy_lints/src/manual_retain.rs index 570fe7368183..3181bc86d179 100644 --- a/clippy_lints/src/manual_retain.rs +++ b/clippy_lints/src/manual_retain.rs @@ -92,7 +92,7 @@ fn check_into_iter( && match_def_path(cx, filter_def_id, &paths::CORE_ITER_FILTER) && let hir::ExprKind::MethodCall(_, struct_expr, [], _) = &into_iter_expr.kind && let Some(into_iter_def_id) = cx.typeck_results().type_dependent_def_id(into_iter_expr.hir_id) - && match_def_path(cx, into_iter_def_id, &paths::CORE_ITER_INTO_ITER) + && cx.tcx.lang_items().require(hir::LangItem::IntoIterIntoIter).ok() == Some(into_iter_def_id) && match_acceptable_type(cx, left_expr, msrv) && SpanlessEq::new(cx).eq_expr(left_expr, struct_expr) { suggest(cx, parent_expr, left_expr, target_expr); diff --git a/clippy_lints/src/map_unit_fn.rs b/clippy_lints/src/map_unit_fn.rs index df5684541e90..32da37a862d8 100644 --- a/clippy_lints/src/map_unit_fn.rs +++ b/clippy_lints/src/map_unit_fn.rs @@ -131,12 +131,12 @@ fn reduce_unit_expression<'a>(cx: &LateContext<'_>, expr: &'a hir::Expr<'_>) -> }, hir::ExprKind::Block(block, _) => { match (block.stmts, block.expr.as_ref()) { - (&[], Some(inner_expr)) => { + ([], Some(inner_expr)) => { // If block only contains an expression, // reduce `{ X }` to `X` reduce_unit_expression(cx, inner_expr) }, - (&[ref inner_stmt], None) => { + ([inner_stmt], None) => { // If block only contains statements, // reduce `{ X; }` to `X` or `X;` match inner_stmt.kind { diff --git a/clippy_lints/src/matches/collapsible_match.rs b/clippy_lints/src/matches/collapsible_match.rs index 07021f1bcad8..fd14d868df34 100644 --- a/clippy_lints/src/matches/collapsible_match.rs +++ b/clippy_lints/src/matches/collapsible_match.rs @@ -1,7 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::higher::IfLetOrMatch; use clippy_utils::visitors::is_local_used; -use clippy_utils::{is_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators, SpanlessEq}; +use clippy_utils::{ + is_res_lang_ctor, is_unit_expr, path_to_local, peel_blocks_with_stmt, peel_ref_operators, SpanlessEq, +}; use if_chain::if_chain; use rustc_errors::MultiSpan; use rustc_hir::LangItem::OptionNone; @@ -110,7 +112,7 @@ fn arm_is_wild_like(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { } match arm.pat.kind { PatKind::Binding(..) | PatKind::Wild => true, - PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), + PatKind::Path(ref qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), OptionNone), _ => false, } } diff --git a/clippy_lints/src/matches/manual_map.rs b/clippy_lints/src/matches/manual_map.rs index 96b8339550ce..76f5e1c941c7 100644 --- a/clippy_lints/src/matches/manual_map.rs +++ b/clippy_lints/src/matches/manual_map.rs @@ -3,8 +3,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable, type_is_unsafe_function}; use clippy_utils::{ - can_move_expr_to_closure, is_else_clause, is_lang_ctor, is_lint_allowed, path_to_local_id, peel_blocks, - peel_hir_expr_refs, peel_hir_expr_while, CaptureKind, + can_move_expr_to_closure, is_else_clause, is_lint_allowed, is_res_lang_ctor, path_res, path_to_local_id, + peel_blocks, peel_hir_expr_refs, peel_hir_expr_while, CaptureKind, }; use rustc_ast::util::parser::PREC_POSTFIX; use rustc_errors::Applicability; @@ -251,9 +251,11 @@ fn try_parse_pattern<'tcx>(cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>, ctxt: Syn match pat.kind { PatKind::Wild => Some(OptionPat::Wild), PatKind::Ref(pat, _) => f(cx, pat, ref_count + 1, ctxt), - PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone) => Some(OptionPat::None), + PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), OptionNone) => { + Some(OptionPat::None) + }, PatKind::TupleStruct(ref qpath, [pattern], _) - if is_lang_ctor(cx, qpath, OptionSome) && pat.span.ctxt() == ctxt => + if is_res_lang_ctor(cx, cx.qpath_res(qpath, pat.hir_id), OptionSome) && pat.span.ctxt() == ctxt => { Some(OptionPat::Some { pattern, ref_count }) }, @@ -272,16 +274,14 @@ fn get_some_expr<'tcx>( ) -> Option> { // TODO: Allow more complex expressions. match expr.kind { - ExprKind::Call( - Expr { - kind: ExprKind::Path(ref qpath), - .. - }, - [arg], - ) if ctxt == expr.span.ctxt() && is_lang_ctor(cx, qpath, OptionSome) => Some(SomeExpr { - expr: arg, - needs_unsafe_block, - }), + ExprKind::Call(callee, [arg]) + if ctxt == expr.span.ctxt() && is_res_lang_ctor(cx, path_res(cx, callee), OptionSome) => + { + Some(SomeExpr { + expr: arg, + needs_unsafe_block, + }) + }, ExprKind::Block( Block { stmts: [], @@ -302,5 +302,5 @@ fn get_some_expr<'tcx>( // Checks for the `None` value. fn is_none_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool { - matches!(peel_blocks(expr).kind, ExprKind::Path(ref qpath) if is_lang_ctor(cx, qpath, OptionNone)) + is_res_lang_ctor(cx, path_res(cx, peel_blocks(expr)), OptionNone) } diff --git a/clippy_lints/src/matches/manual_unwrap_or.rs b/clippy_lints/src/matches/manual_unwrap_or.rs index 2fe7fe98a2e8..587c926dc01c 100644 --- a/clippy_lints/src/matches/manual_unwrap_or.rs +++ b/clippy_lints/src/matches/manual_unwrap_or.rs @@ -3,12 +3,14 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::usage::contains_return_break_continue_macro; -use clippy_utils::{is_lang_ctor, path_to_local_id, sugg}; +use clippy_utils::{is_res_lang_ctor, path_to_local_id, sugg}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; +use rustc_hir::def::{DefKind, Res}; +use rustc_hir::LangItem::{OptionNone, ResultErr}; use rustc_hir::{Arm, Expr, PatKind}; use rustc_lint::LateContext; +use rustc_middle::ty::DefIdTree; use rustc_span::sym; use super::MANUAL_UNWRAP_OR; @@ -59,15 +61,19 @@ fn applicable_or_arm<'a>(cx: &LateContext<'_>, arms: &'a [Arm<'a>]) -> Option<&' if arms.iter().all(|arm| arm.guard.is_none()); if let Some((idx, or_arm)) = arms.iter().enumerate().find(|(_, arm)| { match arm.pat.kind { - PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), + PatKind::Path(ref qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), OptionNone), PatKind::TupleStruct(ref qpath, [pat], _) => - matches!(pat.kind, PatKind::Wild) && is_lang_ctor(cx, qpath, ResultErr), + matches!(pat.kind, PatKind::Wild) + && is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), ResultErr), _ => false, } }); let unwrap_arm = &arms[1 - idx]; if let PatKind::TupleStruct(ref qpath, [unwrap_pat], _) = unwrap_arm.pat.kind; - if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk); + if let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(qpath, unwrap_arm.pat.hir_id); + if let Some(variant_id) = cx.tcx.opt_parent(ctor_id); + if cx.tcx.lang_items().option_some_variant() == Some(variant_id) + || cx.tcx.lang_items().result_ok_variant() == Some(variant_id); if let PatKind::Binding(_, binding_hir_id, ..) = unwrap_pat.kind; if path_to_local_id(unwrap_arm.body, binding_hir_id); if cx.typeck_results().expr_adjustments(unwrap_arm.body).is_empty(); diff --git a/clippy_lints/src/matches/match_as_ref.rs b/clippy_lints/src/matches/match_as_ref.rs index 39d30212f36a..2818f030b7a6 100644 --- a/clippy_lints/src/matches/match_as_ref.rs +++ b/clippy_lints/src/matches/match_as_ref.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::{is_lang_ctor, peel_blocks}; +use clippy_utils::{is_res_lang_ctor, path_res, peel_blocks}; use rustc_errors::Applicability; use rustc_hir::{Arm, BindingAnnotation, ByRef, Expr, ExprKind, LangItem, Mutability, PatKind, QPath}; use rustc_lint::LateContext; @@ -59,18 +59,20 @@ pub(crate) fn check(cx: &LateContext<'_>, ex: &Expr<'_>, arms: &[Arm<'_>], expr: // Checks if arm has the form `None => None` fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { - matches!(arm.pat.kind, PatKind::Path(ref qpath) if is_lang_ctor(cx, qpath, LangItem::OptionNone)) + matches!( + arm.pat.kind, + PatKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), LangItem::OptionNone) + ) } // Checks if arm has the form `Some(ref v) => Some(v)` (checks for `ref` and `ref mut`) fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option { if_chain! { if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind; - if is_lang_ctor(cx, qpath, LangItem::OptionSome); + if is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), LangItem::OptionSome); if let PatKind::Binding(BindingAnnotation(ByRef::Yes, mutabl), .., ident, _) = first_pat.kind; if let ExprKind::Call(e, [arg]) = peel_blocks(arm.body).kind; - if let ExprKind::Path(ref some_path) = e.kind; - if is_lang_ctor(cx, some_path, LangItem::OptionSome); + if is_res_lang_ctor(cx, path_res(cx, e), LangItem::OptionSome); if let ExprKind::Path(QPath::Resolved(_, path2)) = arg.kind; if path2.segments.len() == 1 && ident.name == path2.segments[0].ident.name; then { diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index 9cbffbe61f15..c4f6852aedc3 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -3,7 +3,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; use clippy_utils::{ - eq_expr_value, get_parent_expr_for_hir, get_parent_node, higher, is_else_clause, is_lang_ctor, over, + eq_expr_value, get_parent_expr_for_hir, get_parent_node, higher, is_else_clause, is_res_lang_ctor, over, path_res, peel_blocks_with_stmt, }; use rustc_errors::Applicability; @@ -112,10 +112,7 @@ fn check_if_let_inner(cx: &LateContext<'_>, if_let: &higher::IfLet<'_>) -> bool let ret = strip_return(else_expr); let let_expr_ty = cx.typeck_results().expr_ty(if_let.let_expr); if is_type_diagnostic_item(cx, let_expr_ty, sym::Option) { - if let ExprKind::Path(ref qpath) = ret.kind { - return is_lang_ctor(cx, qpath, OptionNone) || eq_expr_value(cx, if_let.let_expr, ret); - } - return false; + return is_res_lang_ctor(cx, path_res(cx, ret), OptionNone) || eq_expr_value(cx, if_let.let_expr, ret); } return eq_expr_value(cx, if_let.let_expr, ret); } diff --git a/clippy_lints/src/matches/redundant_pattern_match.rs b/clippy_lints/src/matches/redundant_pattern_match.rs index 6d6aa43df2a7..81bebff34c82 100644 --- a/clippy_lints/src/matches/redundant_pattern_match.rs +++ b/clippy_lints/src/matches/redundant_pattern_match.rs @@ -4,10 +4,11 @@ use clippy_utils::source::snippet; use clippy_utils::sugg::Sugg; use clippy_utils::ty::{is_type_diagnostic_item, needs_ordered_drop}; use clippy_utils::visitors::any_temporaries_need_ordered_drop; -use clippy_utils::{higher, is_lang_ctor, is_trait_method}; +use clippy_utils::{higher, is_trait_method}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::LangItem::{self, OptionNone, OptionSome, PollPending, PollReady, ResultErr, ResultOk}; use rustc_hir::{Arm, Expr, ExprKind, Node, Pat, PatKind, QPath, UnOp}; use rustc_lint::LateContext; @@ -87,15 +88,21 @@ fn find_sugg_for_if_let<'tcx>( } }, PatKind::Path(ref path) => { - let method = if is_lang_ctor(cx, path, OptionNone) { - "is_none()" - } else if is_lang_ctor(cx, path, PollPending) { - "is_pending()" + if let Res::Def(DefKind::Ctor(..), ctor_id) = cx.qpath_res(path, check_pat.hir_id) + && let Some(variant_id) = cx.tcx.opt_parent(ctor_id) + { + let method = if cx.tcx.lang_items().option_none_variant() == Some(variant_id) { + "is_none()" + } else if cx.tcx.lang_items().poll_pending_variant() == Some(variant_id) { + "is_pending()" + } else { + return; + }; + // `None` and `Pending` don't have an inner type. + (method, cx.tcx.types.unit) } else { return; - }; - // `None` and `Pending` don't have an inner type. - (method, cx.tcx.types.unit) + } }, _ => return, }; diff --git a/clippy_lints/src/matches/try_err.rs b/clippy_lints/src/matches/try_err.rs index a3ec1ff24820..c6cba81d8718 100644 --- a/clippy_lints/src/matches/try_err.rs +++ b/clippy_lints/src/matches/try_err.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{get_parent_expr, is_lang_ctor, match_def_path, paths}; +use clippy_utils::{get_parent_expr, is_res_lang_ctor, match_def_path, path_res, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::ResultErr; @@ -27,8 +27,7 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, scrutine if let ExprKind::Path(ref match_fun_path) = match_fun.kind; if matches!(match_fun_path, QPath::LangItem(LangItem::TryTraitBranch, ..)); if let ExprKind::Call(err_fun, [err_arg, ..]) = try_arg.kind; - if let ExprKind::Path(ref err_fun_path) = err_fun.kind; - if is_lang_ctor(cx, err_fun_path, ResultErr); + if is_res_lang_ctor(cx, path_res(cx, err_fun), ResultErr); if let Some(return_ty) = find_return_type(cx, &expr.kind); then { let prefix; diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs index cad3ea2a176c..0c4d9f100f7a 100644 --- a/clippy_lints/src/mem_replace.rs +++ b/clippy_lints/src/mem_replace.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::is_non_aggregate_primitive_type; -use clippy_utils::{is_default_equivalent, is_lang_ctor, meets_msrv, msrvs}; +use clippy_utils::{is_default_equivalent, is_res_lang_ctor, meets_msrv, msrvs, path_res}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::OptionNone; @@ -102,40 +102,38 @@ impl_lint_pass!(MemReplace => [MEM_REPLACE_OPTION_WITH_NONE, MEM_REPLACE_WITH_UNINIT, MEM_REPLACE_WITH_DEFAULT]); fn check_replace_option_with_none(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) { - if let ExprKind::Path(ref replacement_qpath) = src.kind { - // Check that second argument is `Option::None` - if is_lang_ctor(cx, replacement_qpath, OptionNone) { - // Since this is a late pass (already type-checked), - // and we already know that the second argument is an - // `Option`, we do not need to check the first - // argument's type. All that's left is to get - // replacee's path. - let replaced_path = match dest.kind { - ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, replaced) => { - if let ExprKind::Path(QPath::Resolved(None, replaced_path)) = replaced.kind { - replaced_path - } else { - return; - } - }, - ExprKind::Path(QPath::Resolved(None, replaced_path)) => replaced_path, - _ => return, - }; + // Check that second argument is `Option::None` + if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) { + // Since this is a late pass (already type-checked), + // and we already know that the second argument is an + // `Option`, we do not need to check the first + // argument's type. All that's left is to get + // replacee's path. + let replaced_path = match dest.kind { + ExprKind::AddrOf(BorrowKind::Ref, Mutability::Mut, replaced) => { + if let ExprKind::Path(QPath::Resolved(None, replaced_path)) = replaced.kind { + replaced_path + } else { + return; + } + }, + ExprKind::Path(QPath::Resolved(None, replaced_path)) => replaced_path, + _ => return, + }; - let mut applicability = Applicability::MachineApplicable; - span_lint_and_sugg( - cx, - MEM_REPLACE_OPTION_WITH_NONE, - expr_span, - "replacing an `Option` with `None`", - "consider `Option::take()` instead", - format!( - "{}.take()", - snippet_with_applicability(cx, replaced_path.span, "", &mut applicability) - ), - applicability, - ); - } + let mut applicability = Applicability::MachineApplicable; + span_lint_and_sugg( + cx, + MEM_REPLACE_OPTION_WITH_NONE, + expr_span, + "replacing an `Option` with `None`", + "consider `Option::take()` instead", + format!( + "{}.take()", + snippet_with_applicability(cx, replaced_path.span, "", &mut applicability) + ), + applicability, + ); } } @@ -203,10 +201,8 @@ fn check_replace_with_default(cx: &LateContext<'_>, src: &Expr<'_>, dest: &Expr< return; } // disable lint for Option since it is covered in another lint - if let ExprKind::Path(q) = &src.kind { - if is_lang_ctor(cx, q, OptionNone) { - return; - } + if is_res_lang_ctor(cx, path_res(cx, src), OptionNone) { + return; } if is_default_equivalent(cx, src) && !in_external_macro(cx.tcx.sess, expr_span) { span_lint_and_then( diff --git a/clippy_lints/src/methods/filetype_is_file.rs b/clippy_lints/src/methods/filetype_is_file.rs index 60f8283c3e09..3fef53739fbd 100644 --- a/clippy_lints/src/methods/filetype_is_file.rs +++ b/clippy_lints/src/methods/filetype_is_file.rs @@ -1,17 +1,18 @@ use clippy_utils::diagnostics::span_lint_and_help; -use clippy_utils::ty::match_type; -use clippy_utils::{get_parent_expr, paths}; +use clippy_utils::get_parent_expr; +use clippy_utils::ty::is_type_diagnostic_item; use if_chain::if_chain; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::source_map::Span; +use rustc_span::sym; use super::FILETYPE_IS_FILE; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>) { let ty = cx.typeck_results().expr_ty(recv); - if !match_type(cx, ty, &paths::FILE_TYPE) { + if !is_type_diagnostic_item(cx, ty, sym::FileType) { return; } diff --git a/clippy_lints/src/methods/get_last_with_len.rs b/clippy_lints/src/methods/get_last_with_len.rs index 02aada87202c..3bdc154df049 100644 --- a/clippy_lints/src/methods/get_last_with_len.rs +++ b/clippy_lints/src/methods/get_last_with_len.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; -use clippy_utils::SpanlessEq; -use rustc_ast::LitKind; +use clippy_utils::{is_integer_literal, SpanlessEq}; use rustc_errors::Applicability; use rustc_hir::{BinOpKind, Expr, ExprKind}; use rustc_lint::LateContext; @@ -26,8 +25,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &Expr<'_>, recv: &Expr<'_>, arg: && lhs_path.ident.name == sym::len // RHS of subtraction is 1 - && let ExprKind::Lit(rhs_lit) = &rhs.kind - && let LitKind::Int(1, ..) = rhs_lit.node + && is_integer_literal(rhs, 1) // check that recv == lhs_recv `recv.get(lhs_recv.len() - 1)` && SpanlessEq::new(cx).eq_expr(recv, lhs_recv) diff --git a/clippy_lints/src/methods/inefficient_to_string.rs b/clippy_lints/src/methods/inefficient_to_string.rs index e5dc3711b0b4..ede3b8bb74e9 100644 --- a/clippy_lints/src/methods/inefficient_to_string.rs +++ b/clippy_lints/src/methods/inefficient_to_string.rs @@ -65,7 +65,7 @@ fn specializes_tostring(cx: &LateContext<'_>, ty: Ty<'_>) -> bool { } if let ty::Adt(adt, substs) = ty.kind() { - match_def_path(cx, adt.did(), &paths::COW) && substs.type_at(1).is_str() + cx.tcx.is_diagnostic_item(sym::Cow, adt.did()) && substs.type_at(1).is_str() } else { false } diff --git a/clippy_lints/src/methods/iter_on_single_or_empty_collections.rs b/clippy_lints/src/methods/iter_on_single_or_empty_collections.rs index cea7b0d82ff3..4f73b3ec4224 100644 --- a/clippy_lints/src/methods/iter_on_single_or_empty_collections.rs +++ b/clippy_lints/src/methods/iter_on_single_or_empty_collections.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; -use clippy_utils::{get_expr_use_or_unification_node, is_lang_ctor, is_no_std_crate}; +use clippy_utils::{get_expr_use_or_unification_node, is_no_std_crate, is_res_lang_ctor, path_res}; use rustc_errors::Applicability; use rustc_hir::LangItem::{OptionNone, OptionSome}; @@ -26,26 +26,11 @@ impl IterType { } pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &Expr<'_>, method_name: &str, recv: &Expr<'_>) { - let item = match &recv.kind { - ExprKind::Array(v) if v.len() <= 1 => v.first(), - ExprKind::Path(p) => { - if is_lang_ctor(cx, p, OptionNone) { - None - } else { - return; - } - }, - ExprKind::Call(f, some_args) if some_args.len() == 1 => { - if let ExprKind::Path(p) = &f.kind { - if is_lang_ctor(cx, p, OptionSome) { - Some(&some_args[0]) - } else { - return; - } - } else { - return; - } - }, + let item = match recv.kind { + ExprKind::Array([]) => None, + ExprKind::Array([e]) => Some(e), + ExprKind::Path(ref p) if is_res_lang_ctor(cx, cx.qpath_res(p, recv.hir_id), OptionNone) => None, + ExprKind::Call(f, [arg]) if is_res_lang_ctor(cx, path_res(cx, f), OptionSome) => Some(arg), _ => return, }; let iter_type = match method_name { diff --git a/clippy_lints/src/methods/manual_ok_or.rs b/clippy_lints/src/methods/manual_ok_or.rs index c5c0ace7729c..5b758f1e6547 100644 --- a/clippy_lints/src/methods/manual_ok_or.rs +++ b/clippy_lints/src/methods/manual_ok_or.rs @@ -1,11 +1,11 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{indent_of, reindent_multiline, snippet_opt}; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_lang_ctor, path_to_local_id}; +use clippy_utils::{is_res_lang_ctor, path_res, path_to_local_id}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::LangItem::{ResultErr, ResultOk}; -use rustc_hir::{Closure, Expr, ExprKind, PatKind}; +use rustc_hir::{Expr, ExprKind, PatKind}; use rustc_lint::LateContext; use rustc_span::symbol::sym; @@ -22,8 +22,8 @@ pub(super) fn check<'tcx>( if let Some(method_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id); if let Some(impl_id) = cx.tcx.impl_of_method(method_id); if is_type_diagnostic_item(cx, cx.tcx.type_of(impl_id), sym::Option); - if let ExprKind::Call(Expr { kind: ExprKind::Path(err_path), .. }, [err_arg]) = or_expr.kind; - if is_lang_ctor(cx, err_path, ResultErr); + if let ExprKind::Call(err_path, [err_arg]) = or_expr.kind; + if is_res_lang_ctor(cx, path_res(cx, err_path), ResultErr); if is_ok_wrapping(cx, map_expr); if let Some(recv_snippet) = snippet_opt(cx, recv.span); if let Some(err_arg_snippet) = snippet_opt(cx, err_arg.span); @@ -46,17 +46,19 @@ pub(super) fn check<'tcx>( } fn is_ok_wrapping(cx: &LateContext<'_>, map_expr: &Expr<'_>) -> bool { - if let ExprKind::Path(ref qpath) = map_expr.kind { - if is_lang_ctor(cx, qpath, ResultOk) { - return true; - } - } - if_chain! { - if let ExprKind::Closure(&Closure { body, .. }) = map_expr.kind; - let body = cx.tcx.hir().body(body); - if let PatKind::Binding(_, param_id, ..) = body.params[0].pat.kind; - if let ExprKind::Call(Expr { kind: ExprKind::Path(ok_path), .. }, &[ref ok_arg]) = body.value.kind; - if is_lang_ctor(cx, ok_path, ResultOk); - then { path_to_local_id(ok_arg, param_id) } else { false } + match map_expr.kind { + ExprKind::Path(ref qpath) if is_res_lang_ctor(cx, cx.qpath_res(qpath, map_expr.hir_id), ResultOk) => true, + ExprKind::Closure(closure) => { + let body = cx.tcx.hir().body(closure.body); + if let PatKind::Binding(_, param_id, ..) = body.params[0].pat.kind + && let ExprKind::Call(callee, [ok_arg]) = body.value.kind + && is_res_lang_ctor(cx, path_res(cx, callee), ResultOk) + { + path_to_local_id(ok_arg, param_id) + } else { + false + } + }, + _ => false, } } diff --git a/clippy_lints/src/methods/manual_str_repeat.rs b/clippy_lints/src/methods/manual_str_repeat.rs index 67e504af161c..8b6b8f1bf16c 100644 --- a/clippy_lints/src/methods/manual_str_repeat.rs +++ b/clippy_lints/src/methods/manual_str_repeat.rs @@ -1,8 +1,8 @@ use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::is_path_diagnostic_item; use clippy_utils::source::{snippet_with_applicability, snippet_with_context}; use clippy_utils::sugg::Sugg; -use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; -use clippy_utils::{is_expr_path_def_path, paths}; +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item}; use if_chain::if_chain; use rustc_ast::LitKind; use rustc_errors::Applicability; @@ -38,7 +38,7 @@ fn parse_repeat_arg(cx: &LateContext<'_>, e: &Expr<'_>) -> Option { let ty = cx.typeck_results().expr_ty(e); if is_type_diagnostic_item(cx, ty, sym::String) || (is_type_lang_item(cx, ty, LangItem::OwnedBox) && get_ty_param(ty).map_or(false, Ty::is_str)) - || (match_type(cx, ty, &paths::COW) && get_ty_param(ty).map_or(false, Ty::is_str)) + || (is_type_diagnostic_item(cx, ty, sym::Cow) && get_ty_param(ty).map_or(false, Ty::is_str)) { Some(RepeatKind::String) } else { @@ -57,7 +57,7 @@ pub(super) fn check( ) { if_chain! { if let ExprKind::Call(repeat_fn, [repeat_arg]) = take_self_arg.kind; - if is_expr_path_def_path(cx, repeat_fn, &paths::ITER_REPEAT); + if is_path_diagnostic_item(cx, repeat_fn, sym::iter_repeat); if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(collect_expr), sym::String); if let Some(collect_id) = cx.typeck_results().type_dependent_def_id(collect_expr.hir_id); if let Some(take_id) = cx.typeck_results().type_dependent_def_id(take_expr.hir_id); diff --git a/clippy_lints/src/methods/option_map_or_none.rs b/clippy_lints/src/methods/option_map_or_none.rs index 76572425346b..3a23ecc50dc1 100644 --- a/clippy_lints/src/methods/option_map_or_none.rs +++ b/clippy_lints/src/methods/option_map_or_none.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{is_lang_ctor, path_def_id}; +use clippy_utils::{is_res_lang_ctor, path_def_id, path_res}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_hir::LangItem::{OptionNone, OptionSome}; @@ -51,22 +51,12 @@ pub(super) fn check<'tcx>( return; } - let default_arg_is_none = if let hir::ExprKind::Path(ref qpath) = def_arg.kind { - is_lang_ctor(cx, qpath, OptionNone) - } else { - return; - }; - - if !default_arg_is_none { + if !is_res_lang_ctor(cx, path_res(cx, def_arg), OptionNone) { // nothing to lint! return; } - let f_arg_is_some = if let hir::ExprKind::Path(ref qpath) = map_arg.kind { - is_lang_ctor(cx, qpath, OptionSome) - } else { - false - }; + let f_arg_is_some = is_res_lang_ctor(cx, path_res(cx, map_arg), OptionSome); if is_option { let self_snippet = snippet(cx, recv.span, ".."); diff --git a/clippy_lints/src/methods/or_then_unwrap.rs b/clippy_lints/src/methods/or_then_unwrap.rs index be5768c35450..55ba6e262df7 100644 --- a/clippy_lints/src/methods/or_then_unwrap.rs +++ b/clippy_lints/src/methods/or_then_unwrap.rs @@ -1,6 +1,6 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{diagnostics::span_lint_and_sugg, is_lang_ctor}; +use clippy_utils::{diagnostics::span_lint_and_sugg, is_res_lang_ctor, path_res}; use rustc_errors::Applicability; use rustc_hir::{lang_items::LangItem, Expr, ExprKind}; use rustc_lint::LateContext; @@ -58,8 +58,7 @@ pub(super) fn check<'tcx>( fn get_content_if_ctor_matches(cx: &LateContext<'_>, expr: &Expr<'_>, item: LangItem) -> Option { if let ExprKind::Call(some_expr, [arg]) = expr.kind - && let ExprKind::Path(qpath) = &some_expr.kind - && is_lang_ctor(cx, qpath, item) + && is_res_lang_ctor(cx, path_res(cx, some_expr), item) { Some(arg.span) } else { diff --git a/clippy_lints/src/methods/str_splitn.rs b/clippy_lints/src/methods/str_splitn.rs index 9ca4d65550d3..ae3594bd36c3 100644 --- a/clippy_lints/src/methods/str_splitn.rs +++ b/clippy_lints/src/methods/str_splitn.rs @@ -2,11 +2,11 @@ use clippy_utils::consts::{constant, Constant}; use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then}; use clippy_utils::source::snippet_with_context; use clippy_utils::usage::local_used_after_expr; -use clippy_utils::visitors::expr_visitor; +use clippy_utils::visitors::{for_each_expr_with_closures, Descend}; use clippy_utils::{is_diag_item_method, match_def_path, meets_msrv, msrvs, path_to_local_id, paths}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::Visitor; use rustc_hir::{ BindingAnnotation, Expr, ExprKind, HirId, LangItem, Local, MatchSource, Node, Pat, PatKind, QPath, Stmt, StmtKind, }; @@ -211,7 +211,7 @@ fn indirect_usage<'tcx>( binding: HirId, ctxt: SyntaxContext, ) -> Option> { - if let StmtKind::Local(Local { + if let StmtKind::Local(&Local { pat: Pat { kind: PatKind::Binding(BindingAnnotation::NONE, _, ident, None), .. @@ -222,14 +222,12 @@ fn indirect_usage<'tcx>( }) = stmt.kind { let mut path_to_binding = None; - expr_visitor(cx, |expr| { - if path_to_local_id(expr, binding) { - path_to_binding = Some(expr); + let _: Option = for_each_expr_with_closures(cx, init_expr, |e| { + if path_to_local_id(e, binding) { + path_to_binding = Some(e); } - - path_to_binding.is_none() - }) - .visit_expr(init_expr); + ControlFlow::Continue(Descend::from(path_to_binding.is_none())) + }); let mut parents = cx.tcx.hir().parent_iter(path_to_binding?.hir_id); let iter_usage = parse_iter_usage(cx, ctxt, &mut parents)?; @@ -250,7 +248,7 @@ fn indirect_usage<'tcx>( .. } = iter_usage { - if parent_id == *local_hir_id { + if parent_id == local_hir_id { return Some(IndirectUsage { name: ident.name, span: stmt.span, diff --git a/clippy_lints/src/methods/unnecessary_filter_map.rs b/clippy_lints/src/methods/unnecessary_filter_map.rs index ee16982d5248..1cef6226ad4f 100644 --- a/clippy_lints/src/methods/unnecessary_filter_map.rs +++ b/clippy_lints/src/methods/unnecessary_filter_map.rs @@ -2,9 +2,10 @@ use super::utils::clone_or_copy_needed; use clippy_utils::diagnostics::span_lint; use clippy_utils::ty::is_copy; use clippy_utils::usage::mutated_variables; -use clippy_utils::{is_lang_ctor, is_trait_method, path_to_local_id}; +use clippy_utils::visitors::{for_each_expr, Descend}; +use clippy_utils::{is_res_lang_ctor, is_trait_method, path_res, path_to_local_id}; +use core::ops::ControlFlow; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::LangItem::{OptionNone, OptionSome}; use rustc_lint::LateContext; use rustc_middle::ty; @@ -13,7 +14,7 @@ use rustc_span::sym; use super::UNNECESSARY_FILTER_MAP; use super::UNNECESSARY_FIND_MAP; -pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr<'_>, name: &str) { +pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'tcx>, arg: &'tcx hir::Expr<'tcx>, name: &str) { if !is_trait_method(cx, expr, sym::Iterator) { return; } @@ -26,10 +27,16 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< let (mut found_mapping, mut found_filtering) = check_expression(cx, arg_id, body.value); - let mut return_visitor = ReturnVisitor::new(cx, arg_id); - return_visitor.visit_expr(body.value); - found_mapping |= return_visitor.found_mapping; - found_filtering |= return_visitor.found_filtering; + let _: Option = for_each_expr(body.value, |e| { + if let hir::ExprKind::Ret(Some(e)) = &e.kind { + let (found_mapping_res, found_filtering_res) = check_expression(cx, arg_id, e); + found_mapping |= found_mapping_res; + found_filtering |= found_filtering_res; + ControlFlow::Continue(Descend::No) + } else { + ControlFlow::Continue(Descend::Yes) + } + }); let in_ty = cx.typeck_results().node_type(body.params[0].hir_id); let sugg = if !found_filtering { @@ -61,15 +68,13 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, arg: &hir::Expr< // returns (found_mapping, found_filtering) fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tcx hir::Expr<'_>) -> (bool, bool) { - match &expr.kind { + match expr.kind { hir::ExprKind::Call(func, args) => { - if let hir::ExprKind::Path(ref path) = func.kind { - if is_lang_ctor(cx, path, OptionSome) { - if path_to_local_id(&args[0], arg_id) { - return (false, false); - } - return (true, false); + if is_res_lang_ctor(cx, path_res(cx, func), OptionSome) { + if path_to_local_id(&args[0], arg_id) { + return (false, false); } + return (true, false); } (true, true) }, @@ -80,7 +85,7 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc hir::ExprKind::Match(_, arms, _) => { let mut found_mapping = false; let mut found_filtering = false; - for arm in *arms { + for arm in arms { let (m, f) = check_expression(cx, arg_id, arm.body); found_mapping |= m; found_filtering |= f; @@ -93,39 +98,9 @@ fn check_expression<'tcx>(cx: &LateContext<'tcx>, arg_id: hir::HirId, expr: &'tc let else_check = check_expression(cx, arg_id, else_arm); (if_check.0 | else_check.0, if_check.1 | else_check.1) }, - hir::ExprKind::Path(path) if is_lang_ctor(cx, path, OptionNone) => (false, true), + hir::ExprKind::Path(ref path) if is_res_lang_ctor(cx, cx.qpath_res(path, expr.hir_id), OptionNone) => { + (false, true) + }, _ => (true, true), } } - -struct ReturnVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - arg_id: hir::HirId, - // Found a non-None return that isn't Some(input) - found_mapping: bool, - // Found a return that isn't Some - found_filtering: bool, -} - -impl<'a, 'tcx> ReturnVisitor<'a, 'tcx> { - fn new(cx: &'a LateContext<'tcx>, arg_id: hir::HirId) -> ReturnVisitor<'a, 'tcx> { - ReturnVisitor { - cx, - arg_id, - found_mapping: false, - found_filtering: false, - } - } -} - -impl<'a, 'tcx> Visitor<'tcx> for ReturnVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - if let hir::ExprKind::Ret(Some(expr)) = &expr.kind { - let (found_mapping, found_filtering) = check_expression(self.cx, self.arg_id, expr); - self.found_mapping |= found_mapping; - self.found_filtering |= found_filtering; - } else { - walk_expr(self, expr); - } - } -} diff --git a/clippy_lints/src/methods/useless_asref.rs b/clippy_lints/src/methods/useless_asref.rs index 0380a82411ae..c1139d84e2f4 100644 --- a/clippy_lints/src/methods/useless_asref.rs +++ b/clippy_lints/src/methods/useless_asref.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::walk_ptrs_ty_depth; -use clippy_utils::{get_parent_expr, match_trait_method, paths}; +use clippy_utils::{get_parent_expr, is_trait_method}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::LateContext; +use rustc_span::sym; use super::USELESS_ASREF; @@ -13,7 +14,7 @@ use super::USELESS_ASREF; pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, call_name: &str, recvr: &hir::Expr<'_>) { // when we get here, we've already checked that the call name is "as_ref" or "as_mut" // check if the call is to the actual `AsRef` or `AsMut` trait - if match_trait_method(cx, expr, &paths::ASREF_TRAIT) || match_trait_method(cx, expr, &paths::ASMUT_TRAIT) { + if is_trait_method(cx, expr, sym::AsRef) || is_trait_method(cx, expr, sym::AsMut) { // check if the type after `as_ref` or `as_mut` is the same as before let rcv_ty = cx.typeck_results().expr_ty(recvr); let res_ty = cx.typeck_results().expr_ty(expr); diff --git a/clippy_lints/src/minmax.rs b/clippy_lints/src/minmax.rs index 4d8579135fc0..4f967755bfa1 100644 --- a/clippy_lints/src/minmax.rs +++ b/clippy_lints/src/minmax.rs @@ -1,6 +1,6 @@ use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::{match_trait_method, paths}; +use clippy_utils::is_trait_method; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -83,7 +83,7 @@ fn min_max<'a>(cx: &LateContext<'_>, expr: &'a Expr<'a>) -> Option<(MinMax, Cons } }, ExprKind::MethodCall(path, receiver, args @ [_], _) => { - if cx.typeck_results().expr_ty(receiver).is_floating_point() || match_trait_method(cx, expr, &paths::ORD) { + if cx.typeck_results().expr_ty(receiver).is_floating_point() || is_trait_method(cx, expr, sym::Ord) { if path.ident.name == sym!(max) { fetch_const(cx, Some(receiver), args, MinMax::Max) } else if path.ident.name == sym!(min) { diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 381458b91e67..516dee20f8b1 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -1,7 +1,6 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_hir_and_then}; use clippy_utils::source::{snippet, snippet_opt}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; use rustc_hir::{ @@ -15,7 +14,7 @@ use rustc_span::hygiene::DesugaringKind; use rustc_span::source_map::{ExpnKind, Span}; use clippy_utils::sugg::Sugg; -use clippy_utils::{get_parent_expr, in_constant, iter_input_pats, last_path_segment, SpanlessEq}; +use clippy_utils::{get_parent_expr, in_constant, is_integer_literal, iter_input_pats, last_path_segment, SpanlessEq}; declare_clippy_lint! { /// ### What it does @@ -314,8 +313,7 @@ fn non_macro_local(cx: &LateContext<'_>, res: def::Res) -> bool { fn check_cast(cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) { if_chain! { if let TyKind::Ptr(ref mut_ty) = ty.kind; - if let ExprKind::Lit(ref lit) = e.kind; - if let LitKind::Int(0, _) = lit.node; + if is_integer_literal(e, 0); if !in_constant(cx, e.hir_id); then { let (msg, sugg_fn) = match mut_ty.mutbl { diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index b8855e5adbff..10c3ff026b6d 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -1,6 +1,4 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_with_applicability; -use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BindingAnnotation, Mutability, Node, Pat, PatKind}; use rustc_lint::{LateContext, LateLintPass}; @@ -8,36 +6,26 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// Checks for bindings that destructure a reference and borrow the inner + /// Checks for bindings that needlessly destructure a reference and borrow the inner /// value with `&ref`. /// /// ### Why is this bad? /// This pattern has no effect in almost all cases. /// - /// ### Known problems - /// In some cases, `&ref` is needed to avoid a lifetime mismatch error. - /// Example: - /// ```rust - /// fn foo(a: &Option, b: &Option) { - /// match (a, b) { - /// (None, &ref c) | (&ref c, None) => (), - /// (&Some(ref c), _) => (), - /// }; - /// } - /// ``` - /// /// ### Example /// ```rust /// let mut v = Vec::::new(); - /// # #[allow(unused)] /// v.iter_mut().filter(|&ref a| a.is_empty()); + /// + /// if let &[ref first, ref second] = v.as_slice() {} /// ``` /// /// Use instead: /// ```rust /// let mut v = Vec::::new(); - /// # #[allow(unused)] /// v.iter_mut().filter(|a| a.is_empty()); + /// + /// if let [first, second] = v.as_slice() {} /// ``` #[clippy::version = "pre 1.29.0"] pub NEEDLESS_BORROWED_REFERENCE, @@ -54,34 +42,83 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef { return; } - if_chain! { - // Only lint immutable refs, because `&mut ref T` may be useful. - if let PatKind::Ref(sub_pat, Mutability::Not) = pat.kind; + // Do not lint patterns that are part of an OR `|` pattern, the binding mode must match in all arms + for (_, node) in cx.tcx.hir().parent_iter(pat.hir_id) { + let Node::Pat(pat) = node else { break }; - // Check sub_pat got a `ref` keyword (excluding `ref mut`). - if let PatKind::Binding(BindingAnnotation::REF, .., spanned_name, _) = sub_pat.kind; - let parent_id = cx.tcx.hir().get_parent_node(pat.hir_id); - if let Some(parent_node) = cx.tcx.hir().find(parent_id); - then { - // do not recurse within patterns, as they may have other references - // XXXManishearth we can relax this constraint if we only check patterns - // with a single ref pattern inside them - if let Node::Pat(_) = parent_node { - return; - } - let mut applicability = Applicability::MachineApplicable; - span_lint_and_then(cx, NEEDLESS_BORROWED_REFERENCE, pat.span, - "this pattern takes a reference on something that is being de-referenced", - |diag| { - let hint = snippet_with_applicability(cx, spanned_name.span, "..", &mut applicability).into_owned(); - diag.span_suggestion( - pat.span, - "try removing the `&ref` part and just keep", - hint, - applicability, - ); - }); + if matches!(pat.kind, PatKind::Or(_)) { + return; } } + + // Only lint immutable refs, because `&mut ref T` may be useful. + let PatKind::Ref(sub_pat, Mutability::Not) = pat.kind else { return }; + + match sub_pat.kind { + // Check sub_pat got a `ref` keyword (excluding `ref mut`). + PatKind::Binding(BindingAnnotation::REF, _, ident, None) => { + span_lint_and_then( + cx, + NEEDLESS_BORROWED_REFERENCE, + pat.span, + "this pattern takes a reference on something that is being dereferenced", + |diag| { + // `&ref ident` + // ^^^^^ + let span = pat.span.until(ident.span); + diag.span_suggestion_verbose( + span, + "try removing the `&ref` part", + String::new(), + Applicability::MachineApplicable, + ); + }, + ); + }, + // Slices where each element is `ref`: `&[ref a, ref b, ..., ref z]` + PatKind::Slice( + before, + None + | Some(Pat { + kind: PatKind::Wild, .. + }), + after, + ) => { + let mut suggestions = Vec::new(); + + for element_pat in itertools::chain(before, after) { + if let PatKind::Binding(BindingAnnotation::REF, _, ident, None) = element_pat.kind { + // `&[..., ref ident, ...]` + // ^^^^ + let span = element_pat.span.until(ident.span); + suggestions.push((span, String::new())); + } else { + return; + } + } + + if !suggestions.is_empty() { + span_lint_and_then( + cx, + NEEDLESS_BORROWED_REFERENCE, + pat.span, + "dereferencing a slice pattern where every element takes a reference", + |diag| { + // `&[...]` + // ^ + let span = pat.span.until(sub_pat.span); + suggestions.push((span, String::new())); + + diag.multipart_suggestion( + "try removing the `&` and `ref` parts", + suggestions, + Applicability::MachineApplicable, + ); + }, + ); + } + }, + _ => {}, + } } } diff --git a/clippy_lints/src/needless_late_init.rs b/clippy_lints/src/needless_late_init.rs index cbad53f4450b..9d26e5900866 100644 --- a/clippy_lints/src/needless_late_init.rs +++ b/clippy_lints/src/needless_late_init.rs @@ -2,9 +2,9 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::path_to_local; use clippy_utils::source::snippet_opt; use clippy_utils::ty::needs_ordered_drop; -use clippy_utils::visitors::{expr_visitor, expr_visitor_no_bodies, is_local_used}; +use clippy_utils::visitors::{for_each_expr, for_each_expr_with_closures, is_local_used}; +use core::ops::ControlFlow; use rustc_errors::{Applicability, MultiSpan}; -use rustc_hir::intravisit::Visitor; use rustc_hir::{ BindingAnnotation, Block, Expr, ExprKind, HirId, Local, LocalSource, MatchSource, Node, Pat, PatKind, Stmt, StmtKind, @@ -64,31 +64,25 @@ declare_clippy_lint! { declare_lint_pass!(NeedlessLateInit => [NEEDLESS_LATE_INIT]); fn contains_assign_expr<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> bool { - let mut seen = false; - expr_visitor(cx, |expr| { - if let ExprKind::Assign(..) = expr.kind { - seen = true; + for_each_expr_with_closures(cx, stmt, |e| { + if matches!(e.kind, ExprKind::Assign(..)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - - !seen }) - .visit_stmt(stmt); - - seen + .is_some() } fn contains_let(cond: &Expr<'_>) -> bool { - let mut seen = false; - expr_visitor_no_bodies(|expr| { - if let ExprKind::Let(_) = expr.kind { - seen = true; + for_each_expr(cond, |e| { + if matches!(e.kind, ExprKind::Let(_)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - - !seen }) - .visit_expr(cond); - - seen + .is_some() } fn stmt_needs_ordered_drop(cx: &LateContext<'_>, stmt: &Stmt<'_>) -> bool { diff --git a/clippy_lints/src/needless_question_mark.rs b/clippy_lints/src/needless_question_mark.rs index 59b6492e112c..97c8cfbd3eb7 100644 --- a/clippy_lints/src/needless_question_mark.rs +++ b/clippy_lints/src/needless_question_mark.rs @@ -1,11 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::is_lang_ctor; +use clippy_utils::path_res; use clippy_utils::source::snippet; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::LangItem::{OptionSome, ResultOk}; +use rustc_hir::def::{DefKind, Res}; use rustc_hir::{AsyncGeneratorKind, Block, Body, Expr, ExprKind, GeneratorKind, LangItem, MatchSource, QPath}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::DefIdTree; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -112,11 +113,12 @@ impl LateLintPass<'_> for NeedlessQuestionMark { fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { if_chain! { - if let ExprKind::Call(path, [arg]) = &expr.kind; - if let ExprKind::Path(ref qpath) = &path.kind; - let sugg_remove = if is_lang_ctor(cx, qpath, OptionSome) { + if let ExprKind::Call(path, [arg]) = expr.kind; + if let Res::Def(DefKind::Ctor(..), ctor_id) = path_res(cx, path); + if let Some(variant_id) = cx.tcx.opt_parent(ctor_id); + let sugg_remove = if cx.tcx.lang_items().option_some_variant() == Some(variant_id) { "Some()" - } else if is_lang_ctor(cx, qpath, ResultOk) { + } else if cx.tcx.lang_items().result_ok_variant() == Some(variant_id) { "Ok()" } else { return; diff --git a/clippy_lints/src/non_octal_unix_permissions.rs b/clippy_lints/src/non_octal_unix_permissions.rs index 25fb4f0f4cff..1a765b14892f 100644 --- a/clippy_lints/src/non_octal_unix_permissions.rs +++ b/clippy_lints/src/non_octal_unix_permissions.rs @@ -1,12 +1,13 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::{snippet_opt, snippet_with_applicability}; -use clippy_utils::ty::match_type; +use clippy_utils::ty::{is_type_diagnostic_item, match_type}; use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -49,7 +50,7 @@ impl<'tcx> LateLintPass<'tcx> for NonOctalUnixPermissions { if_chain! { if (path.ident.name == sym!(mode) && (match_type(cx, obj_ty, &paths::OPEN_OPTIONS) - || match_type(cx, obj_ty, &paths::DIR_BUILDER))) + || is_type_diagnostic_item(cx, obj_ty, sym::DirBuilder))) || (path.ident.name == sym!(set_mode) && match_type(cx, obj_ty, &paths::PERMISSIONS)); if let ExprKind::Lit(_) = param.kind; diff --git a/clippy_lints/src/operators/arithmetic_side_effects.rs b/clippy_lints/src/operators/arithmetic_side_effects.rs index c8a374d90b55..1f22fbd53872 100644 --- a/clippy_lints/src/operators/arithmetic_side_effects.rs +++ b/clippy_lints/src/operators/arithmetic_side_effects.rs @@ -44,25 +44,19 @@ impl ArithmeticSideEffects { /// Assuming that `expr` is a literal integer, checks operators (+=, -=, *, /) in a /// non-constant environment that won't overflow. fn has_valid_op(op: &Spanned, expr: &hir::Expr<'_>) -> bool { - if let hir::BinOpKind::Add | hir::BinOpKind::Sub = op.node - && let hir::ExprKind::Lit(ref lit) = expr.kind - && let ast::LitKind::Int(0, _) = lit.node + if let hir::ExprKind::Lit(ref lit) = expr.kind && + let ast::LitKind::Int(value, _) = lit.node { - return true; + match (&op.node, value) { + (hir::BinOpKind::Add | hir::BinOpKind::Sub, 0) | + (hir::BinOpKind::Mul, 0 | 1) => true, + (hir::BinOpKind::Div | hir::BinOpKind::Rem, 0) => false, + (hir::BinOpKind::Div | hir::BinOpKind::Rem, _) => true, + _ => false, + } + } else { + false } - if let hir::BinOpKind::Div | hir::BinOpKind::Rem = op.node - && let hir::ExprKind::Lit(ref lit) = expr.kind - && !matches!(lit.node, ast::LitKind::Int(0, _)) - { - return true; - } - if let hir::BinOpKind::Mul = op.node - && let hir::ExprKind::Lit(ref lit) = expr.kind - && let ast::LitKind::Int(0 | 1, _) = lit.node - { - return true; - } - false } /// Checks if the given `expr` has any of the inner `allowed` elements. diff --git a/clippy_lints/src/operators/assign_op_pattern.rs b/clippy_lints/src/operators/assign_op_pattern.rs index 2d5d5d143ffa..26bca7c306a8 100644 --- a/clippy_lints/src/operators/assign_op_pattern.rs +++ b/clippy_lints/src/operators/assign_op_pattern.rs @@ -2,11 +2,12 @@ use clippy_utils::binop_traits; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet_opt; use clippy_utils::ty::implements_trait; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{eq_expr_value, trait_ref_of_method}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir_analysis::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId}; use rustc_lint::LateContext; use rustc_middle::mir::FakeReadCause; @@ -65,15 +66,19 @@ pub(super) fn check<'tcx>( } }; - let mut visitor = ExprVisitor { - assignee, - counter: 0, - cx, - }; + let mut found = false; + let found_multiple = for_each_expr(e, |e| { + if eq_expr_value(cx, assignee, e) { + if found { + return ControlFlow::Break(()); + } + found = true; + } + ControlFlow::Continue(()) + }) + .is_some(); - walk_expr(&mut visitor, e); - - if visitor.counter == 1 { + if found && !found_multiple { // a = a op b if eq_expr_value(cx, assignee, l) { lint(assignee, r); @@ -98,22 +103,6 @@ pub(super) fn check<'tcx>( } } -struct ExprVisitor<'a, 'tcx> { - assignee: &'a hir::Expr<'a>, - counter: u8, - cx: &'a LateContext<'tcx>, -} - -impl<'a, 'tcx> Visitor<'tcx> for ExprVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - if eq_expr_value(self.cx, self.assignee, expr) { - self.counter += 1; - } - - walk_expr(self, expr); - } -} - fn imm_borrows_in_expr(cx: &LateContext<'_>, e: &hir::Expr<'_>) -> hir::HirIdSet { struct S(hir::HirIdSet); impl Delegate<'_> for S { diff --git a/clippy_lints/src/operators/numeric_arithmetic.rs b/clippy_lints/src/operators/numeric_arithmetic.rs index b6097710dc68..0830a106f556 100644 --- a/clippy_lints/src/operators/numeric_arithmetic.rs +++ b/clippy_lints/src/operators/numeric_arithmetic.rs @@ -1,5 +1,6 @@ use clippy_utils::consts::constant_simple; use clippy_utils::diagnostics::span_lint; +use clippy_utils::is_integer_literal; use rustc_hir as hir; use rustc_lint::LateContext; use rustc_span::source_map::Span; @@ -50,11 +51,9 @@ impl Context { hir::BinOpKind::Div | hir::BinOpKind::Rem => match &r.kind { hir::ExprKind::Lit(_lit) => (), hir::ExprKind::Unary(hir::UnOp::Neg, expr) => { - if let hir::ExprKind::Lit(lit) = &expr.kind { - if let rustc_ast::ast::LitKind::Int(1, _) = lit.node { - span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); - self.expr_id = Some(expr.hir_id); - } + if is_integer_literal(expr, 1) { + span_lint(cx, INTEGER_ARITHMETIC, expr.span, "integer arithmetic detected"); + self.expr_id = Some(expr.hir_id); } }, _ => { diff --git a/clippy_lints/src/option_if_let_else.rs b/clippy_lints/src/option_if_let_else.rs index 2c1e3b855d52..4eb42da1fed0 100644 --- a/clippy_lints/src/option_if_let_else.rs +++ b/clippy_lints/src/option_if_let_else.rs @@ -1,7 +1,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::sugg::Sugg; use clippy_utils::{ - can_move_expr_to_closure, eager_or_lazy, higher, in_constant, is_else_clause, is_lang_ctor, peel_blocks, + can_move_expr_to_closure, eager_or_lazy, higher, in_constant, is_else_clause, is_res_lang_ctor, peel_blocks, peel_hir_expr_while, CaptureKind, }; use if_chain::if_chain; @@ -174,7 +174,8 @@ fn try_get_option_occurrence<'tcx>( fn try_get_inner_pat<'tcx>(cx: &LateContext<'tcx>, pat: &Pat<'tcx>) -> Option<&'tcx Pat<'tcx>> { if let PatKind::TupleStruct(ref qpath, [inner_pat], ..) = pat.kind { - if is_lang_ctor(cx, qpath, OptionSome) || is_lang_ctor(cx, qpath, ResultOk) { + let res = cx.qpath_res(qpath, pat.hir_id); + if is_res_lang_ctor(cx, res, OptionSome) || is_res_lang_ctor(cx, res, ResultOk) { return Some(inner_pat); } } @@ -226,9 +227,10 @@ fn try_convert_match<'tcx>( fn is_none_or_err_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { match arm.pat.kind { - PatKind::Path(ref qpath) => is_lang_ctor(cx, qpath, OptionNone), + PatKind::Path(ref qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), OptionNone), PatKind::TupleStruct(ref qpath, [first_pat], _) => { - is_lang_ctor(cx, qpath, ResultErr) && matches!(first_pat.kind, PatKind::Wild) + is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), ResultErr) + && matches!(first_pat.kind, PatKind::Wild) }, PatKind::Wild => true, _ => false, diff --git a/clippy_lints/src/panic_in_result_fn.rs b/clippy_lints/src/panic_in_result_fn.rs index 4aa0d9227aba..efec12489a9b 100644 --- a/clippy_lints/src/panic_in_result_fn.rs +++ b/clippy_lints/src/panic_in_result_fn.rs @@ -2,9 +2,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::macros::root_macro_call_first_node; use clippy_utils::return_ty; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::visitors::expr_visitor_no_bodies; +use clippy_utils::visitors::{for_each_expr, Descend}; +use core::ops::ControlFlow; use rustc_hir as hir; -use rustc_hir::intravisit::{FnKind, Visitor}; +use rustc_hir::intravisit::FnKind; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; @@ -58,18 +59,20 @@ impl<'tcx> LateLintPass<'tcx> for PanicInResultFn { fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, body: &'tcx hir::Body<'tcx>) { let mut panics = Vec::new(); - expr_visitor_no_bodies(|expr| { - let Some(macro_call) = root_macro_call_first_node(cx, expr) else { return true }; + let _: Option = for_each_expr(body.value, |e| { + let Some(macro_call) = root_macro_call_first_node(cx, e) else { + return ControlFlow::Continue(Descend::Yes); + }; if matches!( cx.tcx.item_name(macro_call.def_id).as_str(), "unimplemented" | "unreachable" | "panic" | "todo" | "assert" | "assert_eq" | "assert_ne" ) { panics.push(macro_call.span); - return false; + ControlFlow::Continue(Descend::No) + } else { + ControlFlow::Continue(Descend::Yes) } - true - }) - .visit_expr(body.value); + }); if !panics.is_empty() { span_lint_and_then( cx, diff --git a/clippy_lints/src/partialeq_to_none.rs b/clippy_lints/src/partialeq_to_none.rs index 000b0ba7a148..6810a2431758 100644 --- a/clippy_lints/src/partialeq_to_none.rs +++ b/clippy_lints/src/partialeq_to_none.rs @@ -1,5 +1,5 @@ use clippy_utils::{ - diagnostics::span_lint_and_sugg, is_lang_ctor, peel_hir_expr_refs, peel_ref_operators, sugg, + diagnostics::span_lint_and_sugg, is_res_lang_ctor, path_res, peel_hir_expr_refs, peel_ref_operators, sugg, ty::is_type_diagnostic_item, }; use rustc_errors::Applicability; @@ -54,8 +54,7 @@ impl<'tcx> LateLintPass<'tcx> for PartialeqToNone { // If the expression is a literal `Option::None` let is_none_ctor = |expr: &Expr<'_>| { !expr.span.from_expansion() - && matches!(&peel_hir_expr_refs(expr).0.kind, - ExprKind::Path(p) if is_lang_ctor(cx, p, LangItem::OptionNone)) + && is_res_lang_ctor(cx, path_res(cx, peel_hir_expr_refs(expr).0), LangItem::OptionNone) }; let mut applicability = Applicability::MachineApplicable; diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index 0b3d5d174804..328371fd602f 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -3,11 +3,12 @@ use clippy_utils::higher; use clippy_utils::source::snippet_with_applicability; use clippy_utils::ty::is_type_diagnostic_item; use clippy_utils::{ - eq_expr_value, get_parent_node, in_constant, is_else_clause, is_lang_ctor, path_to_local, path_to_local_id, + eq_expr_value, get_parent_node, in_constant, is_else_clause, is_res_lang_ctor, path_to_local, path_to_local_id, peel_blocks, peel_blocks_with_stmt, }; use if_chain::if_chain; use rustc_errors::Applicability; +use rustc_hir::def::Res; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::{BindingAnnotation, ByRef, Expr, ExprKind, Node, PatKind, PathSegment, QPath}; use rustc_lint::{LateContext, LateLintPass}; @@ -58,7 +59,7 @@ enum IfBlockType<'hir> { /// Contains: let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c), /// if_else (d) IfLet( - &'hir QPath<'hir>, + Res, Ty<'hir>, Symbol, &'hir Expr<'hir>, @@ -126,7 +127,14 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: if ddpos.as_opt_usize().is_none(); if let PatKind::Binding(BindingAnnotation(by_ref, _), bind_id, ident, None) = field.kind; let caller_ty = cx.typeck_results().expr_ty(let_expr); - let if_block = IfBlockType::IfLet(path1, caller_ty, ident.name, let_expr, if_then, if_else); + let if_block = IfBlockType::IfLet( + cx.qpath_res(path1, let_pat.hir_id), + caller_ty, + ident.name, + let_expr, + if_then, + if_else + ); if (is_early_return(sym::Option, cx, &if_block) && path_to_local_id(peel_blocks(if_then), bind_id)) || is_early_return(sym::Result, cx, &if_block); if if_else.map(|e| eq_expr_value(cx, let_expr, peel_blocks(e))).filter(|e| *e).is_none(); @@ -165,21 +173,21 @@ fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_ _ => false, } }, - IfBlockType::IfLet(qpath, let_expr_ty, let_pat_sym, let_expr, if_then, if_else) => { + IfBlockType::IfLet(res, let_expr_ty, let_pat_sym, let_expr, if_then, if_else) => { is_type_diagnostic_item(cx, let_expr_ty, smbl) && match smbl { sym::Option => { // We only need to check `if let Some(x) = option` not `if let None = option`, // because the later one will be suggested as `if option.is_none()` thus causing conflict. - is_lang_ctor(cx, qpath, OptionSome) + is_res_lang_ctor(cx, res, OptionSome) && if_else.is_some() && expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, None) }, sym::Result => { - (is_lang_ctor(cx, qpath, ResultOk) + (is_res_lang_ctor(cx, res, ResultOk) && if_else.is_some() && expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, Some(let_pat_sym))) - || is_lang_ctor(cx, qpath, ResultErr) + || is_res_lang_ctor(cx, res, ResultErr) && expr_return_none_or_err(smbl, cx, if_then, let_expr, Some(let_pat_sym)) }, _ => false, @@ -198,7 +206,7 @@ fn expr_return_none_or_err( match peel_blocks_with_stmt(expr).kind { ExprKind::Ret(Some(ret_expr)) => expr_return_none_or_err(smbl, cx, ret_expr, cond_expr, err_sym), ExprKind::Path(ref qpath) => match smbl { - sym::Option => is_lang_ctor(cx, qpath, OptionNone), + sym::Option => is_res_lang_ctor(cx, cx.qpath_res(qpath, expr.hir_id), OptionNone), sym::Result => path_to_local(expr).is_some() && path_to_local(expr) == path_to_local(cond_expr), _ => false, }, diff --git a/clippy_lints/src/read_zero_byte_vec.rs b/clippy_lints/src/read_zero_byte_vec.rs index ae80b6f12691..fa107858863a 100644 --- a/clippy_lints/src/read_zero_byte_vec.rs +++ b/clippy_lints/src/read_zero_byte_vec.rs @@ -2,9 +2,10 @@ use clippy_utils::{ diagnostics::{span_lint, span_lint_and_sugg}, higher::{get_vec_init_kind, VecInitKind}, source::snippet, - visitors::expr_visitor_no_bodies, + visitors::for_each_expr, }; -use hir::{intravisit::Visitor, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; +use core::ops::ControlFlow; +use hir::{Expr, ExprKind, Local, PatKind, PathSegment, QPath, StmtKind}; use rustc_errors::Applicability; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; @@ -58,10 +59,8 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { && let PatKind::Binding(_, _, ident, _) = pat.kind && let Some(vec_init_kind) = get_vec_init_kind(cx, init) { - // finds use of `_.read(&mut v)` - let mut read_found = false; - let mut visitor = expr_visitor_no_bodies(|expr| { - if let ExprKind::MethodCall(path, _self, [arg], _) = expr.kind + let visitor = |expr: &Expr<'_>| { + if let ExprKind::MethodCall(path, _, [arg], _) = expr.kind && let PathSegment { ident: read_or_read_exact, .. } = *path && matches!(read_or_read_exact.as_str(), "read" | "read_exact") && let ExprKind::AddrOf(_, hir::Mutability::Mut, inner) = arg.kind @@ -69,27 +68,22 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec { && let [inner_seg] = inner_path.segments && ident.name == inner_seg.ident.name { - read_found = true; - } - !read_found - }); - - let next_stmt_span; - if idx == block.stmts.len() - 1 { - // case { .. stmt; expr } - if let Some(e) = block.expr { - visitor.visit_expr(e); - next_stmt_span = e.span; + ControlFlow::Break(()) } else { - return; + ControlFlow::Continue(()) } - } else { + }; + + let (read_found, next_stmt_span) = + if let Some(next_stmt) = block.stmts.get(idx + 1) { // case { .. stmt; stmt; .. } - let next_stmt = &block.stmts[idx + 1]; - visitor.visit_stmt(next_stmt); - next_stmt_span = next_stmt.span; - } - drop(visitor); + (for_each_expr(next_stmt, visitor).is_some(), next_stmt.span) + } else if let Some(e) = block.expr { + // case { .. stmt; expr } + (for_each_expr(e, visitor).is_some(), e.span) + } else { + return + }; if read_found && !next_stmt_span.from_expansion() { let applicability = Applicability::MaybeIncorrect; diff --git a/clippy_lints/src/returns.rs b/clippy_lints/src/returns.rs index f758f4cff8ba..2b2a41d16011 100644 --- a/clippy_lints/src/returns.rs +++ b/clippy_lints/src/returns.rs @@ -1,9 +1,11 @@ use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then}; use clippy_utils::source::{snippet_opt, snippet_with_context}; +use clippy_utils::visitors::{for_each_expr, Descend}; use clippy_utils::{fn_def_id, path_to_local_id}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; +use rustc_hir::intravisit::FnKind; use rustc_hir::{Block, Body, Expr, ExprKind, FnDecl, HirId, MatchSource, PatKind, StmtKind}; use rustc_lint::{LateContext, LateLintPass, LintContext}; use rustc_middle::lint::in_external_macro; @@ -270,33 +272,20 @@ fn emit_return_lint( } fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool { - let mut visitor = BorrowVisitor { cx, borrows: false }; - walk_expr(&mut visitor, expr); - visitor.borrows -} - -struct BorrowVisitor<'a, 'tcx> { - cx: &'a LateContext<'tcx>, - borrows: bool, -} - -impl<'tcx> Visitor<'tcx> for BorrowVisitor<'_, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - if self.borrows || expr.span.from_expansion() { - return; - } - - if let Some(def_id) = fn_def_id(self.cx, expr) { - self.borrows = self - .cx + for_each_expr(expr, |e| { + if let Some(def_id) = fn_def_id(cx, e) + && cx .tcx .fn_sig(def_id) - .output() .skip_binder() + .output() .walk() - .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))); + .any(|arg| matches!(arg.unpack(), GenericArgKind::Lifetime(_))) + { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(Descend::from(!expr.span.from_expansion())) } - - walk_expr(self, expr); - } + }) + .is_some() } diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index e57ab8cd7a3a..760399231513 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -1,9 +1,10 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::sugg::Sugg; use clippy_utils::ty::is_type_diagnostic_item; -use clippy_utils::{get_enclosing_block, is_expr_path_def_path, path_to_local, path_to_local_id, paths, SpanlessEq}; +use clippy_utils::{ + get_enclosing_block, is_integer_literal, is_path_diagnostic_item, path_to_local, path_to_local_id, SpanlessEq, +}; use if_chain::if_chain; -use rustc_ast::ast::LitKind; use rustc_errors::Applicability; use rustc_hir::intravisit::{walk_block, walk_expr, walk_stmt, Visitor}; use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, PatKind, QPath, Stmt, StmtKind}; @@ -219,8 +220,7 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> { && path_to_local_id(self_arg, self.vec_alloc.local_id) && path.ident.name == sym!(resize) // Check that is filled with 0 - && let ExprKind::Lit(ref lit) = fill_arg.kind - && let LitKind::Int(0, _) = lit.node { + && is_integer_literal(fill_arg, 0) { // Check that len expression is equals to `with_capacity` expression if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr) { self.slow_expression = Some(InitializationType::Resize(expr)); @@ -254,10 +254,8 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> { fn is_repeat_zero(&self, expr: &Expr<'_>) -> bool { if_chain! { if let ExprKind::Call(fn_expr, [repeat_arg]) = expr.kind; - if is_expr_path_def_path(self.cx, fn_expr, &paths::ITER_REPEAT); - if let ExprKind::Lit(ref lit) = repeat_arg.kind; - if let LitKind::Int(0, _) = lit.node; - + if is_path_diagnostic_item(self.cx, fn_expr, sym::iter_repeat); + if is_integer_literal(repeat_arg, 0); then { true } else { diff --git a/clippy_lints/src/suspicious_trait_impl.rs b/clippy_lints/src/suspicious_trait_impl.rs index d47ed459387e..b57b484bdc89 100644 --- a/clippy_lints/src/suspicious_trait_impl.rs +++ b/clippy_lints/src/suspicious_trait_impl.rs @@ -1,8 +1,9 @@ use clippy_utils::diagnostics::span_lint; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{binop_traits, trait_ref_of_method, BINOP_TRAITS, OP_ASSIGN_TRAITS}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_hir as hir; -use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -92,25 +93,17 @@ impl<'tcx> LateLintPass<'tcx> for SuspiciousImpl { } fn count_binops(expr: &hir::Expr<'_>) -> u32 { - let mut visitor = BinaryExprVisitor::default(); - visitor.visit_expr(expr); - visitor.nb_binops -} - -#[derive(Default)] -struct BinaryExprVisitor { - nb_binops: u32, -} - -impl<'tcx> Visitor<'tcx> for BinaryExprVisitor { - fn visit_expr(&mut self, expr: &'tcx hir::Expr<'_>) { - match expr.kind { + let mut count = 0u32; + let _: Option = for_each_expr(expr, |e| { + if matches!( + e.kind, hir::ExprKind::Binary(..) - | hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _) - | hir::ExprKind::AssignOp(..) => self.nb_binops += 1, - _ => {}, + | hir::ExprKind::Unary(hir::UnOp::Not | hir::UnOp::Neg, _) + | hir::ExprKind::AssignOp(..) + ) { + count += 1; } - - walk_expr(self, expr); - } + ControlFlow::Continue(()) + }); + count } diff --git a/clippy_lints/src/transmute/transmuting_null.rs b/clippy_lints/src/transmute/transmuting_null.rs index d8e349af7af8..19ce5ae72c24 100644 --- a/clippy_lints/src/transmute/transmuting_null.rs +++ b/clippy_lints/src/transmute/transmuting_null.rs @@ -1,8 +1,6 @@ use clippy_utils::consts::{constant_context, Constant}; use clippy_utils::diagnostics::span_lint; -use clippy_utils::is_path_diagnostic_item; -use if_chain::if_chain; -use rustc_ast::LitKind; +use clippy_utils::{is_integer_literal, is_path_diagnostic_item}; use rustc_hir::{Expr, ExprKind}; use rustc_lint::LateContext; use rustc_middle::ty::Ty; @@ -19,37 +17,28 @@ pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, arg: &'t // Catching transmute over constants that resolve to `null`. let mut const_eval_context = constant_context(cx, cx.typeck_results()); - if_chain! { - if let ExprKind::Path(ref _qpath) = arg.kind; - if let Some(Constant::RawPtr(x)) = const_eval_context.expr(arg); - if x == 0; - then { - span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); - return true; - } + if let ExprKind::Path(ref _qpath) = arg.kind && + let Some(Constant::RawPtr(x)) = const_eval_context.expr(arg) && + x == 0 + { + span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); + return true; } // Catching: // `std::mem::transmute(0 as *const i32)` - if_chain! { - if let ExprKind::Cast(inner_expr, _cast_ty) = arg.kind; - if let ExprKind::Lit(ref lit) = inner_expr.kind; - if let LitKind::Int(0, _) = lit.node; - then { - span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); - return true; - } + if let ExprKind::Cast(inner_expr, _cast_ty) = arg.kind && is_integer_literal(inner_expr, 0) { + span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); + return true; } // Catching: // `std::mem::transmute(std::ptr::null::())` - if_chain! { - if let ExprKind::Call(func1, []) = arg.kind; - if is_path_diagnostic_item(cx, func1, sym::ptr_null); - then { - span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); - return true; - } + if let ExprKind::Call(func1, []) = arg.kind && + is_path_diagnostic_item(cx, func1, sym::ptr_null) + { + span_lint(cx, TRANSMUTING_NULL, expr.span, LINT_MSG); + return true; } // FIXME: diff --git a/clippy_lints/src/uninit_vec.rs b/clippy_lints/src/uninit_vec.rs index bde7c318f448..1ab0162a8813 100644 --- a/clippy_lints/src/uninit_vec.rs +++ b/clippy_lints/src/uninit_vec.rs @@ -1,8 +1,7 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_then}; use clippy_utils::higher::{get_vec_init_kind, VecInitKind}; use clippy_utils::ty::{is_type_diagnostic_item, is_uninit_value_valid_for_ty}; -use clippy_utils::{is_lint_allowed, path_to_local_id, peel_hir_expr_while, SpanlessEq}; -use rustc_ast::ast::LitKind; +use clippy_utils::{is_integer_literal, is_lint_allowed, path_to_local_id, peel_hir_expr_while, SpanlessEq}; use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, PathSegment, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::lint::in_external_macro; @@ -216,7 +215,7 @@ fn extract_set_len_self<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Opt let self_type = cx.typeck_results().expr_ty(self_expr).peel_refs(); if is_type_diagnostic_item(cx, self_type, sym::Vec) && path.ident.name.as_str() == "set_len" - && !is_literal_zero(arg) + && !is_integer_literal(arg, 0) { Some((self_expr, expr.span)) } else { @@ -226,13 +225,3 @@ fn extract_set_len_self<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Opt _ => None, } } - -fn is_literal_zero(arg: &Expr<'_>) -> bool { - if let ExprKind::Lit(lit) = &arg.kind - && let LitKind::Int(0, _) = lit.node - { - true - } else { - false - } -} diff --git a/clippy_lints/src/unnecessary_owned_empty_strings.rs b/clippy_lints/src/unnecessary_owned_empty_strings.rs index 8a4f4c0ad971..016aacbf9da3 100644 --- a/clippy_lints/src/unnecessary_owned_empty_strings.rs +++ b/clippy_lints/src/unnecessary_owned_empty_strings.rs @@ -3,7 +3,7 @@ use clippy_utils::{match_def_path, paths}; use if_chain::if_chain; use rustc_ast::ast::LitKind; use rustc_errors::Applicability; -use rustc_hir::{BorrowKind, Expr, ExprKind, Mutability}; +use rustc_hir::{BorrowKind, Expr, ExprKind, LangItem, Mutability}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -55,7 +55,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryOwnedEmptyStrings { ); } else { if_chain! { - if match_def_path(cx, fun_def_id, &paths::FROM_FROM); + if cx.tcx.lang_items().require(LangItem::FromFrom).ok() == Some(fun_def_id); if let [.., last_arg] = args; if let ExprKind::Lit(spanned) = &last_arg.kind; if let LitKind::Str(symbol, _) = spanned.node; diff --git a/clippy_lints/src/unnecessary_wraps.rs b/clippy_lints/src/unnecessary_wraps.rs index 83ef3b0fac87..7211e6864f3a 100644 --- a/clippy_lints/src/unnecessary_wraps.rs +++ b/clippy_lints/src/unnecessary_wraps.rs @@ -1,6 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::source::snippet; -use clippy_utils::{contains_return, is_lang_ctor, return_ty, visitors::find_all_ret_expressions}; +use clippy_utils::{contains_return, is_res_lang_ctor, path_res, return_ty, visitors::find_all_ret_expressions}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::intravisit::FnKind; @@ -120,9 +120,7 @@ impl<'tcx> LateLintPass<'tcx> for UnnecessaryWraps { if !ret_expr.span.from_expansion(); // Check if a function call. if let ExprKind::Call(func, [arg]) = ret_expr.kind; - // Check if OPTION_SOME or RESULT_OK, depending on return type. - if let ExprKind::Path(qpath) = &func.kind; - if is_lang_ctor(cx, qpath, lang_item); + if is_res_lang_ctor(cx, path_res(cx, func), lang_item); // Make sure the function argument does not contain a return expression. if !contains_return(arg); then { diff --git a/clippy_lints/src/unused_io_amount.rs b/clippy_lints/src/unused_io_amount.rs index b38d71784fcf..8bcdff66331d 100644 --- a/clippy_lints/src/unused_io_amount.rs +++ b/clippy_lints/src/unused_io_amount.rs @@ -1,8 +1,9 @@ use clippy_utils::diagnostics::{span_lint, span_lint_and_help}; -use clippy_utils::{is_try, match_trait_method, paths}; +use clippy_utils::{is_trait_method, is_try, match_trait_method, paths}; use rustc_hir as hir; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -116,13 +117,13 @@ fn check_method_call(cx: &LateContext<'_>, call: &hir::Expr<'_>, expr: &hir::Exp match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCREADEXT) || match_trait_method(cx, call, &paths::TOKIO_IO_ASYNCREADEXT) } else { - match_trait_method(cx, call, &paths::IO_READ) + is_trait_method(cx, call, sym::IoRead) }; let write_trait = if is_await { match_trait_method(cx, call, &paths::FUTURES_IO_ASYNCWRITEEXT) || match_trait_method(cx, call, &paths::TOKIO_IO_ASYNCWRITEEXT) } else { - match_trait_method(cx, call, &paths::IO_WRITE) + is_trait_method(cx, call, sym::IoWrite) }; match (read_trait, write_trait, symbol, is_await) { diff --git a/clippy_lints/src/unwrap_in_result.rs b/clippy_lints/src/unwrap_in_result.rs index baa53ba664f6..a69719b127b2 100644 --- a/clippy_lints/src/unwrap_in_result.rs +++ b/clippy_lints/src/unwrap_in_result.rs @@ -1,12 +1,12 @@ use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::ty::is_type_diagnostic_item; +use clippy_utils::visitors::for_each_expr; use clippy_utils::{method_chain_args, return_ty}; +use core::ops::ControlFlow; use if_chain::if_chain; use rustc_hir as hir; -use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::{Expr, ImplItemKind}; +use rustc_hir::ImplItemKind; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::{sym, Span}; @@ -73,51 +73,37 @@ impl<'tcx> LateLintPass<'tcx> for UnwrapInResult { } } -struct FindExpectUnwrap<'a, 'tcx> { - lcx: &'a LateContext<'tcx>, - typeck_results: &'tcx ty::TypeckResults<'tcx>, - result: Vec, -} - -impl<'a, 'tcx> Visitor<'tcx> for FindExpectUnwrap<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr<'_>) { - // check for `expect` - if let Some(arglists) = method_chain_args(expr, &["expect"]) { - let receiver_ty = self.typeck_results.expr_ty(arglists[0].0).peel_refs(); - if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option) - || is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result) - { - self.result.push(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(); - if is_type_diagnostic_item(self.lcx, receiver_ty, sym::Option) - || is_type_diagnostic_item(self.lcx, receiver_ty, sym::Result) - { - self.result.push(expr.span); - } - } - - // and check sub-expressions - intravisit::walk_expr(self, expr); - } -} - fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tcx hir::ImplItem<'_>) { if let ImplItemKind::Fn(_, body_id) = impl_item.kind { let body = cx.tcx.hir().body(body_id); - let mut fpu = FindExpectUnwrap { - lcx: cx, - typeck_results: cx.tcx.typeck(impl_item.def_id.def_id), - result: Vec::new(), - }; - fpu.visit_expr(body.value); + let typeck = cx.tcx.typeck(impl_item.def_id.def_id); + let mut result = Vec::new(); + let _: Option = for_each_expr(body.value, |e| { + // check for `expect` + if let Some(arglists) = method_chain_args(e, &["expect"]) { + let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs(); + if is_type_diagnostic_item(cx, receiver_ty, sym::Option) + || is_type_diagnostic_item(cx, receiver_ty, sym::Result) + { + result.push(e.span); + } + } + + // check for `unwrap` + if let Some(arglists) = method_chain_args(e, &["unwrap"]) { + let receiver_ty = typeck.expr_ty(arglists[0].0).peel_refs(); + if is_type_diagnostic_item(cx, receiver_ty, sym::Option) + || is_type_diagnostic_item(cx, receiver_ty, sym::Result) + { + result.push(e.span); + } + } + + ControlFlow::Continue(()) + }); // if we've found one, lint - if !fpu.result.is_empty() { + if !result.is_empty() { span_lint_and_then( cx, UNWRAP_IN_RESULT, @@ -125,7 +111,7 @@ fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_item: &'tc "used unwrap or expect in a function that returns result or option", move |diag| { diag.help("unwrap and expect should not be used in a function that returns result or option"); - diag.span_note(fpu.result, "potential non-recoverable error(s)"); + diag.span_note(result, "potential non-recoverable error(s)"); }, ); } diff --git a/clippy_lints/src/useless_conversion.rs b/clippy_lints/src/useless_conversion.rs index 50c5a832430a..a82643a59f97 100644 --- a/clippy_lints/src/useless_conversion.rs +++ b/clippy_lints/src/useless_conversion.rs @@ -5,7 +5,7 @@ use clippy_utils::ty::{is_type_diagnostic_item, same_type_and_consts}; use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, paths}; use if_chain::if_chain; use rustc_errors::Applicability; -use rustc_hir::{Expr, ExprKind, HirId, MatchSource}; +use rustc_hir::{Expr, ExprKind, HirId, LangItem, MatchSource}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty; use rustc_session::{declare_tool_lint, impl_lint_pass}; @@ -154,7 +154,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion { } if_chain! { - if match_def_path(cx, def_id, &paths::FROM_FROM); + if cx.tcx.lang_items().require(LangItem::FromFrom).ok() == Some(def_id); if same_type_and_consts(a, b); then { diff --git a/clippy_lints/src/utils/internal_lints.rs b/clippy_lints/src/utils/internal_lints.rs index bc3f920a087a..6309a04c73d5 100644 --- a/clippy_lints/src/utils/internal_lints.rs +++ b/clippy_lints/src/utils/internal_lints.rs @@ -2,11 +2,11 @@ use crate::utils::internal_lints::metadata_collector::is_deprecated_lint; use clippy_utils::consts::{constant_simple, Constant}; use clippy_utils::diagnostics::{span_lint, span_lint_and_help, span_lint_and_sugg, span_lint_and_then}; use clippy_utils::macros::root_macro_call_first_node; -use clippy_utils::source::snippet; +use clippy_utils::source::{snippet, snippet_with_applicability}; use clippy_utils::ty::match_type; use clippy_utils::{ - def_path_res, higher, is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_def_path, - method_calls, paths, peel_blocks_with_stmt, SpanlessEq, + def_path_res, higher, is_else_clause, is_expn_of, is_expr_path_def_path, is_lint_allowed, match_any_def_paths, + match_def_path, method_calls, paths, peel_blocks_with_stmt, peel_hir_expr_refs, SpanlessEq, }; use if_chain::if_chain; use rustc_ast as ast; @@ -20,21 +20,24 @@ use rustc_hir::def_id::DefId; use rustc_hir::hir_id::CRATE_HIR_ID; use rustc_hir::intravisit::Visitor; use rustc_hir::{ - BinOpKind, Block, Closure, Expr, ExprKind, HirId, Item, Local, MutTy, Mutability, Node, Path, Stmt, StmtKind, Ty, + BinOpKind, Block, Closure, Expr, ExprKind, HirId, Item, Local, MutTy, Mutability, Node, Path, Stmt, StmtKind, TyKind, UnOp, }; use rustc_hir_analysis::hir_ty_to_ty; use rustc_lint::{EarlyContext, EarlyLintPass, LateContext, LateLintPass, LintContext}; use rustc_middle::hir::nested_filter; -use rustc_middle::mir::interpret::ConstValue; -use rustc_middle::ty::{self, fast_reject::SimplifiedTypeGen, subst::GenericArgKind, FloatTy}; +use rustc_middle::mir::interpret::{Allocation, ConstValue, GlobalAlloc}; +use rustc_middle::ty::{ + self, fast_reject::SimplifiedTypeGen, subst::GenericArgKind, AssocKind, DefIdTree, FloatTy, Ty, +}; use rustc_semver::RustcVersion; use rustc_session::{declare_lint_pass, declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Spanned; -use rustc_span::symbol::Symbol; +use rustc_span::symbol::{Ident, Symbol}; use rustc_span::{sym, BytePos, Span}; use std::borrow::{Borrow, Cow}; +use std::str; #[cfg(feature = "internal")] pub mod metadata_collector; @@ -226,11 +229,11 @@ declare_clippy_lint! { declare_clippy_lint! { /// ### What it does - /// Checks for calls to `utils::match_type()` on a type diagnostic item - /// and suggests to use `utils::is_type_diagnostic_item()` instead. + /// Checks for usages of def paths when a diagnostic item or a `LangItem` could be used. /// /// ### Why is this bad? - /// `utils::is_type_diagnostic_item()` does not require hardcoded paths. + /// The path for an item is subject to change and is less efficient to look up than a + /// diagnostic item or a `LangItem`. /// /// ### Example /// ```rust,ignore @@ -241,9 +244,9 @@ declare_clippy_lint! { /// ```rust,ignore /// utils::is_type_diagnostic_item(cx, ty, sym::Vec) /// ``` - pub MATCH_TYPE_ON_DIAGNOSTIC_ITEM, + pub UNNECESSARY_DEF_PATH, internal, - "using `utils::match_type()` instead of `utils::is_type_diagnostic_item()`" + "using a def path when a diagnostic item or a `LangItem` is available" } declare_clippy_lint! { @@ -537,7 +540,7 @@ impl<'tcx> LateLintPass<'tcx> for LintWithoutLintPass { } } -fn is_lint_ref_type<'tcx>(cx: &LateContext<'tcx>, ty: &Ty<'_>) -> bool { +fn is_lint_ref_type<'tcx>(cx: &LateContext<'tcx>, ty: &hir::Ty<'_>) -> bool { if let TyKind::Rptr( _, MutTy { @@ -888,89 +891,225 @@ fn suggest_note( ); } -declare_lint_pass!(MatchTypeOnDiagItem => [MATCH_TYPE_ON_DIAGNOSTIC_ITEM]); +declare_lint_pass!(UnnecessaryDefPath => [UNNECESSARY_DEF_PATH]); -impl<'tcx> LateLintPass<'tcx> for MatchTypeOnDiagItem { +#[allow(clippy::too_many_lines)] +impl<'tcx> LateLintPass<'tcx> for UnnecessaryDefPath { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx hir::Expr<'_>) { - if is_lint_allowed(cx, MATCH_TYPE_ON_DIAGNOSTIC_ITEM, expr.hir_id) { + enum Item { + LangItem(Symbol), + DiagnosticItem(Symbol), + } + static PATHS: &[&[&str]] = &[ + &["clippy_utils", "match_def_path"], + &["clippy_utils", "match_trait_method"], + &["clippy_utils", "ty", "match_type"], + &["clippy_utils", "is_expr_path_def_path"], + ]; + + if is_lint_allowed(cx, UNNECESSARY_DEF_PATH, expr.hir_id) { return; } if_chain! { - // Check if this is a call to utils::match_type() - if let ExprKind::Call(fn_path, [context, ty, ty_path]) = expr.kind; - if is_expr_path_def_path(cx, fn_path, &["clippy_utils", "ty", "match_type"]); + if let ExprKind::Call(func, [cx_arg, def_arg, args@..]) = expr.kind; + if let ExprKind::Path(path) = &func.kind; + if let Some(id) = cx.qpath_res(path, func.hir_id).opt_def_id(); + if let Some(which_path) = match_any_def_paths(cx, id, PATHS); + let item_arg = if which_path == 4 { &args[1] } else { &args[0] }; // Extract the path to the matched type - if let Some(segments) = path_to_matched_type(cx, ty_path); - let segments: Vec<&str> = segments.iter().map(Symbol::as_str).collect(); - if let Some(ty_did) = def_path_res(cx, &segments[..]).opt_def_id(); - // Check if the matched type is a diagnostic item - if let Some(item_name) = cx.tcx.get_diagnostic_name(ty_did); + if let Some(segments) = path_to_matched_type(cx, item_arg); + let segments: Vec<&str> = segments.iter().map(|sym| &**sym).collect(); + if let Some(def_id) = def_path_res(cx, &segments[..]).opt_def_id(); then { - // TODO: check paths constants from external crates. - let cx_snippet = snippet(cx, context.span, "_"); - let ty_snippet = snippet(cx, ty.span, "_"); + // def_path_res will match field names before anything else, but for this we want to match + // inherent functions first. + let def_id = if cx.tcx.def_kind(def_id) == DefKind::Field { + let method_name = *segments.last().unwrap(); + cx.tcx.def_key(def_id).parent + .and_then(|parent_idx| + cx.tcx.inherent_impls(DefId { index: parent_idx, krate: def_id.krate }).iter() + .find_map(|impl_id| cx.tcx.associated_items(*impl_id) + .find_by_name_and_kind( + cx.tcx, + Ident::from_str(method_name), + AssocKind::Fn, + *impl_id, + ) + ) + ) + .map_or(def_id, |item| item.def_id) + } else { + def_id + }; - span_lint_and_sugg( + // Check if the target item is a diagnostic item or LangItem. + let (msg, item) = if let Some(item_name) + = cx.tcx.diagnostic_items(def_id.krate).id_to_name.get(&def_id) + { + ( + "use of a def path to a diagnostic item", + Item::DiagnosticItem(*item_name), + ) + } else if let Some(lang_item) = cx.tcx.lang_items().items().iter().position(|id| *id == Some(def_id)) { + let lang_items = def_path_res(cx, &["rustc_hir", "lang_items", "LangItem"]).def_id(); + let item_name = cx.tcx.adt_def(lang_items).variants().iter().nth(lang_item).unwrap().name; + ( + "use of a def path to a `LangItem`", + Item::LangItem(item_name), + ) + } else { + return; + }; + + let has_ctor = match cx.tcx.def_kind(def_id) { + DefKind::Struct => { + let variant = cx.tcx.adt_def(def_id).non_enum_variant(); + variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public()) + } + DefKind::Variant => { + let variant = cx.tcx.adt_def(cx.tcx.parent(def_id)).variant_with_id(def_id); + variant.ctor_def_id.is_some() && variant.fields.iter().all(|f| f.vis.is_public()) + } + _ => false, + }; + + let mut app = Applicability::MachineApplicable; + let cx_snip = snippet_with_applicability(cx, cx_arg.span, "..", &mut app); + let def_snip = snippet_with_applicability(cx, def_arg.span, "..", &mut app); + let (sugg, with_note) = match (which_path, item) { + // match_def_path + (0, Item::DiagnosticItem(item)) => + (format!("{cx_snip}.tcx.is_diagnostic_item(sym::{item}, {def_snip})"), has_ctor), + (0, Item::LangItem(item)) => ( + format!("{cx_snip}.tcx.lang_items().require(LangItem::{item}).ok() == Some({def_snip})"), + has_ctor + ), + // match_trait_method + (1, Item::DiagnosticItem(item)) => + (format!("is_trait_method({cx_snip}, {def_snip}, sym::{item})"), false), + // match_type + (2, Item::DiagnosticItem(item)) => + (format!("is_type_diagnostic_item({cx_snip}, {def_snip}, sym::{item})"), false), + (2, Item::LangItem(item)) => + (format!("is_type_lang_item({cx_snip}, {def_snip}, LangItem::{item})"), false), + // is_expr_path_def_path + (3, Item::DiagnosticItem(item)) if has_ctor => ( + format!( + "is_res_diag_ctor({cx_snip}, path_res({cx_snip}, {def_snip}), sym::{item})", + ), + false, + ), + (3, Item::LangItem(item)) if has_ctor => ( + format!( + "is_res_lang_ctor({cx_snip}, path_res({cx_snip}, {def_snip}), LangItem::{item})", + ), + false, + ), + (3, Item::DiagnosticItem(item)) => + (format!("is_path_diagnostic_item({cx_snip}, {def_snip}, sym::{item})"), false), + (3, Item::LangItem(item)) => ( + format!( + "path_res({cx_snip}, {def_snip}).opt_def_id()\ + .map_or(false, |id| {cx_snip}.tcx.lang_items().require(LangItem::{item}).ok() == Some(id))", + ), + false, + ), + _ => return, + }; + + span_lint_and_then( cx, - MATCH_TYPE_ON_DIAGNOSTIC_ITEM, + UNNECESSARY_DEF_PATH, expr.span, - "usage of `clippy_utils::ty::match_type()` on a type diagnostic item", - "try", - format!("clippy_utils::ty::is_type_diagnostic_item({cx_snippet}, {ty_snippet}, sym::{item_name})"), - Applicability::MaybeIncorrect, + msg, + |diag| { + diag.span_suggestion(expr.span, "try", sugg, app); + if with_note { + diag.help( + "if this `DefId` came from a constructor expression or pattern then the \ + parent `DefId` should be used instead" + ); + } + }, ); } } } } -fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option> { - use rustc_hir::ItemKind; - - match &expr.kind { - ExprKind::AddrOf(.., expr) => return path_to_matched_type(cx, expr), - ExprKind::Path(qpath) => match cx.qpath_res(qpath, expr.hir_id) { +fn path_to_matched_type(cx: &LateContext<'_>, expr: &hir::Expr<'_>) -> Option> { + match peel_hir_expr_refs(expr).0.kind { + ExprKind::Path(ref qpath) => match cx.qpath_res(qpath, expr.hir_id) { Res::Local(hir_id) => { let parent_id = cx.tcx.hir().get_parent_node(hir_id); - if let Some(Node::Local(local)) = cx.tcx.hir().find(parent_id) { - if let Some(init) = local.init { - return path_to_matched_type(cx, init); - } - } - }, - Res::Def(DefKind::Const | DefKind::Static(..), def_id) => { - if let Some(Node::Item(item)) = cx.tcx.hir().get_if_local(def_id) { - if let ItemKind::Const(.., body_id) | ItemKind::Static(.., body_id) = item.kind { - let body = cx.tcx.hir().body(body_id); - return path_to_matched_type(cx, body.value); - } - } - }, - _ => {}, - }, - ExprKind::Array(exprs) => { - let segments: Vec = exprs - .iter() - .filter_map(|expr| { - if let ExprKind::Lit(lit) = &expr.kind { - if let LitKind::Str(sym, _) = lit.node { - return Some(sym); - } - } - + if let Some(Node::Local(Local { init: Some(init), .. })) = cx.tcx.hir().find(parent_id) { + path_to_matched_type(cx, init) + } else { None - }) - .collect(); - - if segments.len() == exprs.len() { - return Some(segments); - } + } + }, + Res::Def(DefKind::Static(_), def_id) => read_mir_alloc_def_path( + cx, + cx.tcx.eval_static_initializer(def_id).ok()?.inner(), + cx.tcx.type_of(def_id), + ), + Res::Def(DefKind::Const, def_id) => match cx.tcx.const_eval_poly(def_id).ok()? { + ConstValue::ByRef { alloc, offset } if offset.bytes() == 0 => { + read_mir_alloc_def_path(cx, alloc.inner(), cx.tcx.type_of(def_id)) + }, + _ => None, + }, + _ => None, }, - _ => {}, - } + ExprKind::Array(exprs) => exprs + .iter() + .map(|expr| { + if let ExprKind::Lit(lit) = &expr.kind { + if let LitKind::Str(sym, _) = lit.node { + return Some((*sym.as_str()).to_owned()); + } + } - None + None + }) + .collect(), + _ => None, + } +} + +fn read_mir_alloc_def_path<'tcx>(cx: &LateContext<'tcx>, alloc: &'tcx Allocation, ty: Ty<'_>) -> Option> { + let (alloc, ty) = if let ty::Ref(_, ty, Mutability::Not) = *ty.kind() { + let &alloc = alloc.provenance().values().next()?; + if let GlobalAlloc::Memory(alloc) = cx.tcx.global_alloc(alloc) { + (alloc.inner(), ty) + } else { + return None; + } + } else { + (alloc, ty) + }; + + if let ty::Array(ty, _) | ty::Slice(ty) = *ty.kind() + && let ty::Ref(_, ty, Mutability::Not) = *ty.kind() + && ty.is_str() + { + alloc + .provenance() + .values() + .map(|&alloc| { + if let GlobalAlloc::Memory(alloc) = cx.tcx.global_alloc(alloc) { + let alloc = alloc.inner(); + str::from_utf8(alloc.inspect_with_uninit_and_ptr_outside_interpreter(0..alloc.len())) + .ok().map(ToOwned::to_owned) + } else { + None + } + }) + .collect() + } else { + None + } } // This is not a complete resolver for paths. It works on all the paths currently used in the paths diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index c2c52d08a3c1..3caa6380e098 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -3,6 +3,7 @@ #![feature(control_flow_enum)] #![feature(let_chains)] #![feature(lint_reasons)] +#![feature(never_type)] #![feature(once_cell)] #![feature(rustc_private)] #![recursion_limit = "512"] @@ -65,6 +66,7 @@ pub use self::hir_utils::{ both, count_eq, eq_expr_value, hash_expr, hash_stmt, over, HirEqInterExpr, SpanlessEq, SpanlessHash, }; +use core::ops::ControlFlow; use std::collections::hash_map::Entry; use std::hash::BuildHasherDefault; use std::sync::OnceLock; @@ -113,7 +115,7 @@ use rustc_target::abi::Integer; use crate::consts::{constant, Constant}; use crate::ty::{can_partially_move_ty, expr_sig, is_copy, is_recursively_primitive_type, ty_is_fn_once_param}; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::for_each_expr; pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option) -> Option { if let Ok(version) = RustcVersion::parse(msrv) { @@ -238,19 +240,69 @@ pub fn in_constant(cx: &LateContext<'_>, id: HirId) -> bool { } } -/// Checks if a `QPath` resolves to a constructor of a `LangItem`. +/// Checks if a `Res` refers to a constructor of a `LangItem` /// For example, use this to check whether a function call or a pattern is `Some(..)`. -pub fn is_lang_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, lang_item: LangItem) -> bool { +pub fn is_res_lang_ctor(cx: &LateContext<'_>, res: Res, lang_item: LangItem) -> bool { + if let Res::Def(DefKind::Ctor(..), id) = res + && let Ok(lang_id) = cx.tcx.lang_items().require(lang_item) + && let Some(id) = cx.tcx.opt_parent(id) + { + id == lang_id + } else { + false + } +} + +pub fn is_res_diagnostic_ctor(cx: &LateContext<'_>, res: Res, diag_item: Symbol) -> bool { + if let Res::Def(DefKind::Ctor(..), id) = res + && let Some(id) = cx.tcx.opt_parent(id) + { + cx.tcx.is_diagnostic_item(diag_item, id) + } else { + false + } +} + +/// Checks if a `QPath` resolves to a constructor of a diagnostic item. +pub fn is_diagnostic_ctor(cx: &LateContext<'_>, qpath: &QPath<'_>, diagnostic_item: Symbol) -> bool { if let QPath::Resolved(_, path) = qpath { if let Res::Def(DefKind::Ctor(..), ctor_id) = path.res { - if let Ok(item_id) = cx.tcx.lang_items().require(lang_item) { - return cx.tcx.parent(ctor_id) == item_id; - } + return cx.tcx.is_diagnostic_item(diagnostic_item, cx.tcx.parent(ctor_id)); } } false } +/// Checks if the `DefId` matches the given diagnostic item or it's constructor. +pub fn is_diagnostic_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: Symbol) -> bool { + let did = match cx.tcx.def_kind(did) { + DefKind::Ctor(..) => cx.tcx.parent(did), + // Constructors for types in external crates seem to have `DefKind::Variant` + DefKind::Variant => match cx.tcx.opt_parent(did) { + Some(did) if matches!(cx.tcx.def_kind(did), DefKind::Variant) => did, + _ => did, + }, + _ => did, + }; + + cx.tcx.is_diagnostic_item(item, did) +} + +/// Checks if the `DefId` matches the given `LangItem` or it's constructor. +pub fn is_lang_item_or_ctor(cx: &LateContext<'_>, did: DefId, item: LangItem) -> bool { + let did = match cx.tcx.def_kind(did) { + DefKind::Ctor(..) => cx.tcx.parent(did), + // Constructors for types in external crates seem to have `DefKind::Variant` + DefKind::Variant => match cx.tcx.opt_parent(did) { + Some(did) if matches!(cx.tcx.def_kind(did), DefKind::Variant) => did, + _ => did, + }, + _ => did, + }; + + cx.tcx.lang_items().require(item).map_or(false, |id| id == did) +} + pub fn is_unit_expr(expr: &Expr<'_>) -> bool { matches!( expr.kind, @@ -486,6 +538,13 @@ pub fn def_path_res(cx: &LateContext<'_>, path: &[&str]) -> Res { .copied() .find(|assoc_def_id| tcx.item_name(*assoc_def_id).as_str() == name) .map(|assoc_def_id| Res::Def(tcx.def_kind(assoc_def_id), assoc_def_id)), + DefKind::Struct | DefKind::Union => tcx + .adt_def(def_id) + .non_enum_variant() + .fields + .iter() + .find(|f| f.name.as_str() == name) + .map(|f| Res::Def(DefKind::Field, f.did)), _ => None, } } @@ -738,7 +797,7 @@ pub fn is_default_equivalent(cx: &LateContext<'_>, e: &Expr<'_>) -> bool { } }, ExprKind::Call(repl_func, _) => is_default_equivalent_call(cx, repl_func), - ExprKind::Path(qpath) => is_lang_ctor(cx, qpath, OptionNone), + ExprKind::Path(qpath) => is_res_lang_ctor(cx, cx.qpath_res(qpath, e.hir_id), OptionNone), ExprKind::AddrOf(rustc_hir::BorrowKind::Ref, _, expr) => matches!(expr.kind, ExprKind::Array([])), _ => false, } @@ -1136,17 +1195,14 @@ pub fn contains_name(name: Symbol, expr: &Expr<'_>) -> bool { /// Returns `true` if `expr` contains a return expression pub fn contains_return(expr: &hir::Expr<'_>) -> bool { - let mut found = false; - expr_visitor_no_bodies(|expr| { - if !found { - if let hir::ExprKind::Ret(..) = &expr.kind { - found = true; - } + for_each_expr(expr, |e| { + if matches!(e.kind, hir::ExprKind::Ret(..)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !found }) - .visit_expr(expr); - found + .is_some() } /// Extends the span to the beginning of the spans line, incl. whitespaces. @@ -1553,7 +1609,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc if_chain! { if let PatKind::TupleStruct(ref path, pat, ddpos) = arm.pat.kind; if ddpos.as_opt_usize().is_none(); - if is_lang_ctor(cx, path, ResultOk); + if is_res_lang_ctor(cx, cx.qpath_res(path, arm.pat.hir_id), ResultOk); if let PatKind::Binding(_, hir_id, _, None) = pat[0].kind; if path_to_local_id(arm.body, hir_id); then { @@ -1565,7 +1621,7 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc fn is_err(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { if let PatKind::TupleStruct(ref path, _, _) = arm.pat.kind { - is_lang_ctor(cx, path, ResultErr) + is_res_lang_ctor(cx, cx.qpath_res(path, arm.pat.hir_id), ResultErr) } else { false } @@ -2295,6 +2351,29 @@ pub fn span_contains_comment(sm: &SourceMap, span: Span) -> bool { }); } +/// Return all the comments a given span contains +/// Comments are returned wrapped with their relevant delimiters +pub fn span_extract_comment(sm: &SourceMap, span: Span) -> String { + let snippet = sm.span_to_snippet(span).unwrap_or_default(); + let mut comments_buf: Vec = Vec::new(); + let mut index: usize = 0; + + for token in tokenize(&snippet) { + let token_range = index..(index + token.len as usize); + index += token.len as usize; + match token.kind { + TokenKind::BlockComment { .. } | TokenKind::LineComment { .. } => { + if let Some(comment) = snippet.get(token_range) { + comments_buf.push(comment.to_string()); + } + }, + _ => (), + } + } + + comments_buf.join("\n") +} + macro_rules! op_utils { ($($name:ident $assign:ident)*) => { /// Binary operation traits like `LangItem::Add` diff --git a/clippy_utils/src/macros.rs b/clippy_utils/src/macros.rs index 079c8f50f12a..dda21b903901 100644 --- a/clippy_utils/src/macros.rs +++ b/clippy_utils/src/macros.rs @@ -2,7 +2,7 @@ use crate::is_path_diagnostic_item; use crate::source::snippet_opt; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::{for_each_expr, Descend}; use arrayvec::ArrayVec; use itertools::{izip, Either, Itertools}; @@ -270,20 +270,19 @@ fn find_assert_args_inner<'a, const N: usize>( }; let mut args = ArrayVec::new(); let mut panic_expn = None; - expr_visitor_no_bodies(|e| { + let _: Option = for_each_expr(expr, |e| { if args.is_full() { if panic_expn.is_none() && e.span.ctxt() != expr.span.ctxt() { panic_expn = PanicExpn::parse(cx, e); } - panic_expn.is_none() + ControlFlow::Continue(Descend::from(panic_expn.is_none())) } else if is_assert_arg(cx, e, expn) { args.push(e); - false + ControlFlow::Continue(Descend::No) } else { - true + ControlFlow::Continue(Descend::Yes) } - }) - .visit_expr(expr); + }); let args = args.into_inner().ok()?; // if no `panic!(..)` is found, use `PanicExpn::Empty` // to indicate that the default assertion message is used @@ -297,22 +296,19 @@ fn find_assert_within_debug_assert<'a>( expn: ExpnId, assert_name: Symbol, ) -> Option<(&'a Expr<'a>, ExpnId)> { - let mut found = None; - expr_visitor_no_bodies(|e| { - if found.is_some() || !e.span.from_expansion() { - return false; + for_each_expr(expr, |e| { + if !e.span.from_expansion() { + return ControlFlow::Continue(Descend::No); } let e_expn = e.span.ctxt().outer_expn(); if e_expn == expn { - return true; + ControlFlow::Continue(Descend::Yes) + } else if e_expn.expn_data().macro_def_id.map(|id| cx.tcx.item_name(id)) == Some(assert_name) { + ControlFlow::Break((e, e_expn)) + } else { + ControlFlow::Continue(Descend::No) } - if e_expn.expn_data().macro_def_id.map(|id| cx.tcx.item_name(id)) == Some(assert_name) { - found = Some((e, e_expn)); - } - false }) - .visit_expr(expr); - found } fn is_assert_arg(cx: &LateContext<'_>, expr: &Expr<'_>, assert_expn: ExpnId) -> bool { @@ -396,16 +392,14 @@ impl FormatString { }); let mut parts = Vec::new(); - expr_visitor_no_bodies(|expr| { - if let ExprKind::Lit(lit) = &expr.kind { - if let LitKind::Str(symbol, _) = lit.node { - parts.push(symbol); - } + let _: Option = for_each_expr(pieces, |expr| { + if let ExprKind::Lit(lit) = &expr.kind + && let LitKind::Str(symbol, _) = lit.node + { + parts.push(symbol); } - - true - }) - .visit_expr(pieces); + ControlFlow::Continue(()) + }); Some(Self { span, @@ -431,7 +425,7 @@ impl<'tcx> FormatArgsValues<'tcx> { fn new(args: &'tcx Expr<'tcx>, format_string_span: SpanData) -> Self { let mut pos_to_value_index = Vec::new(); let mut value_args = Vec::new(); - expr_visitor_no_bodies(|expr| { + let _: Option = for_each_expr(args, |expr| { if expr.span.ctxt() == args.span.ctxt() { // ArgumentV1::new_() // ArgumentV1::from_usize() @@ -453,16 +447,13 @@ impl<'tcx> FormatArgsValues<'tcx> { pos_to_value_index.push(val_idx); } - - true + ControlFlow::Continue(Descend::Yes) } else { // assume that any expr with a differing span is a value value_args.push(expr); - - false + ControlFlow::Continue(Descend::No) } - }) - .visit_expr(args); + }); Self { value_args, @@ -866,22 +857,20 @@ impl<'tcx> FormatArgsExpn<'tcx> { } pub fn find_nested(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>, expn_id: ExpnId) -> Option { - let mut format_args = None; - expr_visitor_no_bodies(|e| { - if format_args.is_some() { - return false; - } + for_each_expr(expr, |e| { let e_ctxt = e.span.ctxt(); if e_ctxt == expr.span.ctxt() { - return true; + ControlFlow::Continue(Descend::Yes) + } else if e_ctxt.outer_expn().is_descendant_of(expn_id) { + if let Some(args) = FormatArgsExpn::parse(cx, e) { + ControlFlow::Break(args) + } else { + ControlFlow::Continue(Descend::No) + } + } else { + ControlFlow::Continue(Descend::No) } - if e_ctxt.outer_expn().is_descendant_of(expn_id) { - format_args = FormatArgsExpn::parse(cx, e); - } - false }) - .visit_expr(expr); - format_args } /// Source callsite span of all inputs diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs index 07170e2df12a..13938645fc3e 100644 --- a/clippy_utils/src/paths.rs +++ b/clippy_utils/src/paths.rs @@ -34,7 +34,6 @@ pub const DEFAULT_TRAIT_METHOD: [&str; 4] = ["core", "default", "Default", "defa pub const DEREF_MUT_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "DerefMut", "deref_mut"]; /// Preferably use the diagnostic item `sym::deref_method` where possible pub const DEREF_TRAIT_METHOD: [&str; 5] = ["core", "ops", "deref", "Deref", "deref"]; -pub const DIR_BUILDER: [&str; 3] = ["std", "fs", "DirBuilder"]; pub const DISPLAY_TRAIT: [&str; 3] = ["core", "fmt", "Display"]; #[cfg(feature = "internal")] pub const EARLY_CONTEXT: [&str; 2] = ["rustc_lint", "EarlyContext"]; @@ -64,8 +63,6 @@ pub const IDENT_AS_STR: [&str; 4] = ["rustc_span", "symbol", "Ident", "as_str"]; pub const INDEX: [&str; 3] = ["core", "ops", "Index"]; pub const INDEX_MUT: [&str; 3] = ["core", "ops", "IndexMut"]; pub const INSERT_STR: [&str; 4] = ["alloc", "string", "String", "insert_str"]; -pub const IO_READ: [&str; 3] = ["std", "io", "Read"]; -pub const IO_WRITE: [&str; 3] = ["std", "io", "Write"]; pub const ITER_COUNT: [&str; 6] = ["core", "iter", "traits", "iterator", "Iterator", "count"]; pub const ITER_EMPTY: [&str; 5] = ["core", "iter", "sources", "empty", "Empty"]; pub const ITER_REPEAT: [&str; 5] = ["core", "iter", "sources", "repeat", "repeat"]; diff --git a/clippy_utils/src/ptr.rs b/clippy_utils/src/ptr.rs index 0226f74906b5..88837d8a143e 100644 --- a/clippy_utils/src/ptr.rs +++ b/clippy_utils/src/ptr.rs @@ -1,7 +1,7 @@ use crate::source::snippet; -use crate::visitors::expr_visitor_no_bodies; +use crate::visitors::{for_each_expr, Descend}; use crate::{path_to_local_id, strip_pat_refs}; -use rustc_hir::intravisit::Visitor; +use core::ops::ControlFlow; use rustc_hir::{Body, BodyId, ExprKind, HirId, PatKind}; use rustc_lint::LateContext; use rustc_span::Span; @@ -30,28 +30,23 @@ fn extract_clone_suggestions<'tcx>( replace: &[(&'static str, &'static str)], body: &'tcx Body<'_>, ) -> Option)>> { - let mut abort = false; let mut spans = Vec::new(); - expr_visitor_no_bodies(|expr| { - if abort { - return false; - } - if let ExprKind::MethodCall(seg, recv, [], _) = expr.kind { - if path_to_local_id(recv, id) { - if seg.ident.name.as_str() == "capacity" { - abort = true; - return false; - } - for &(fn_name, suffix) in replace { - if seg.ident.name.as_str() == fn_name { - spans.push((expr.span, snippet(cx, recv.span, "_") + suffix)); - return false; - } + for_each_expr(body, |e| { + if let ExprKind::MethodCall(seg, recv, [], _) = e.kind + && path_to_local_id(recv, id) + { + if seg.ident.as_str() == "capacity" { + return ControlFlow::Break(()); + } + for &(fn_name, suffix) in replace { + if seg.ident.as_str() == fn_name { + spans.push((e.span, snippet(cx, recv.span, "_") + suffix)); + return ControlFlow::Continue(Descend::No); } } } - !abort + ControlFlow::Continue(Descend::Yes) }) - .visit_body(body); - if abort { None } else { Some(spans) } + .is_none() + .then_some(spans) } diff --git a/clippy_utils/src/usage.rs b/clippy_utils/src/usage.rs index 3221b82aed41..b5ec3fef3e0b 100644 --- a/clippy_utils/src/usage.rs +++ b/clippy_utils/src/usage.rs @@ -1,5 +1,6 @@ use crate as utils; -use crate::visitors::{expr_visitor, expr_visitor_no_bodies}; +use crate::visitors::{for_each_expr, for_each_expr_with_closures, Descend}; +use core::ops::ControlFlow; use rustc_hir as hir; use rustc_hir::intravisit::{self, Visitor}; use rustc_hir::HirIdSet; @@ -148,28 +149,17 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for BindingUsageFinder<'a, 'tcx> { } pub fn contains_return_break_continue_macro(expression: &Expr<'_>) -> bool { - let mut seen_return_break_continue = false; - expr_visitor_no_bodies(|ex| { - if seen_return_break_continue { - return false; - } - match &ex.kind { - ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => { - seen_return_break_continue = true; - }, + for_each_expr(expression, |e| { + match e.kind { + ExprKind::Ret(..) | ExprKind::Break(..) | ExprKind::Continue(..) => ControlFlow::Break(()), // Something special could be done here to handle while or for loop // desugaring, as this will detect a break if there's a while loop // or a for loop inside the expression. - _ => { - if ex.span.from_expansion() { - seen_return_break_continue = true; - } - }, + _ if e.span.from_expansion() => ControlFlow::Break(()), + _ => ControlFlow::Continue(()), } - !seen_return_break_continue }) - .visit_expr(expression); - seen_return_break_continue + .is_some() } pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr<'_>) -> bool { @@ -200,23 +190,16 @@ pub fn local_used_after_expr(cx: &LateContext<'_>, local_id: HirId, after: &Expr return true; } - let mut used_after_expr = false; let mut past_expr = false; - expr_visitor(cx, |expr| { - if used_after_expr { - return false; - } - - if expr.hir_id == after.hir_id { + for_each_expr_with_closures(cx, block, |e| { + if e.hir_id == after.hir_id { past_expr = true; - return false; + ControlFlow::Continue(Descend::No) + } else if past_expr && utils::path_to_local_id(e, local_id) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(Descend::Yes) } - - if past_expr && utils::path_to_local_id(expr, local_id) { - used_after_expr = true; - } - !used_after_expr }) - .visit_block(block); - used_after_expr + .is_some() } diff --git a/clippy_utils/src/visitors.rs b/clippy_utils/src/visitors.rs index 232d571902b6..d4294f18fd50 100644 --- a/clippy_utils/src/visitors.rs +++ b/clippy_utils/src/visitors.rs @@ -5,14 +5,13 @@ use rustc_hir as hir; use rustc_hir::def::{CtorKind, DefKind, Res}; use rustc_hir::intravisit::{self, walk_block, walk_expr, Visitor}; use rustc_hir::{ - Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, Pat, QPath, Stmt, UnOp, - UnsafeSource, Unsafety, + AnonConst, Arm, Block, BlockCheckMode, Body, BodyId, Expr, ExprKind, HirId, ItemId, ItemKind, Let, Pat, QPath, + Stmt, UnOp, UnsafeSource, Unsafety, }; use rustc_lint::LateContext; -use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; use rustc_middle::ty::adjustment::Adjust; -use rustc_middle::ty::{self, Ty, TypeckResults}; +use rustc_middle::ty::{self, Ty, TyCtxt, TypeckResults}; use rustc_span::Span; mod internal { @@ -48,6 +47,26 @@ impl Continue for Descend { } } +/// A type which can be visited. +pub trait Visitable<'tcx> { + /// Calls the corresponding `visit_*` function on the visitor. + fn visit>(self, visitor: &mut V); +} +macro_rules! visitable_ref { + ($t:ident, $f:ident) => { + impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> { + fn visit>(self, visitor: &mut V) { + visitor.$f(self); + } + } + }; +} +visitable_ref!(Arm, visit_arm); +visitable_ref!(Block, visit_block); +visitable_ref!(Body, visit_body); +visitable_ref!(Expr, visit_expr); +visitable_ref!(Stmt, visit_stmt); + /// Calls the given function once for each expression contained. This does not enter any bodies or /// nested items. pub fn for_each_expr<'tcx, B, C: Continue>( @@ -82,57 +101,63 @@ pub fn for_each_expr<'tcx, B, C: Continue>( v.res } -/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested -/// bodies (i.e. closures) are visited. -/// If the callback returns `true`, the expr just provided to the callback is walked. -#[must_use] -pub fn expr_visitor<'tcx>(cx: &LateContext<'tcx>, f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> { - struct V<'tcx, F> { - hir: Map<'tcx>, +/// Calls the given function once for each expression contained. This will enter bodies, but not +/// nested items. +pub fn for_each_expr_with_closures<'tcx, B, C: Continue>( + cx: &LateContext<'tcx>, + node: impl Visitable<'tcx>, + f: impl FnMut(&'tcx Expr<'tcx>) -> ControlFlow, +) -> Option { + struct V<'tcx, B, F> { + tcx: TyCtxt<'tcx>, f: F, + res: Option, } - impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V<'tcx, F> { + impl<'tcx, B, C: Continue, F: FnMut(&'tcx Expr<'tcx>) -> ControlFlow> Visitor<'tcx> for V<'tcx, B, F> { type NestedFilter = nested_filter::OnlyBodies; fn nested_visit_map(&mut self) -> Self::Map { - self.hir + self.tcx.hir() } - fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { - if (self.f)(expr) { - walk_expr(self, expr); + fn visit_expr(&mut self, e: &'tcx Expr<'tcx>) { + if self.res.is_some() { + return; + } + match (self.f)(e) { + ControlFlow::Continue(c) if c.descend() => walk_expr(self, e), + ControlFlow::Break(b) => self.res = Some(b), + ControlFlow::Continue(_) => (), } } - } - V { hir: cx.tcx.hir(), f } -} -/// Convenience method for creating a `Visitor` with just `visit_expr` overridden and nested -/// bodies (i.e. closures) are not visited. -/// If the callback returns `true`, the expr just provided to the callback is walked. -#[must_use] -pub fn expr_visitor_no_bodies<'tcx>(f: impl FnMut(&'tcx Expr<'tcx>) -> bool) -> impl Visitor<'tcx> { - struct V(F); - impl<'tcx, F: FnMut(&'tcx Expr<'tcx>) -> bool> Visitor<'tcx> for V { - fn visit_expr(&mut self, e: &'tcx Expr<'_>) { - if (self.0)(e) { - walk_expr(self, e); - } - } + // Only walk closures + fn visit_anon_const(&mut self, _: &'tcx AnonConst) {} + // Avoid unnecessary `walk_*` calls. + fn visit_ty(&mut self, _: &'tcx hir::Ty<'tcx>) {} + fn visit_pat(&mut self, _: &'tcx Pat<'tcx>) {} + fn visit_qpath(&mut self, _: &'tcx QPath<'tcx>, _: HirId, _: Span) {} + // Avoid monomorphising all `visit_*` functions. + fn visit_nested_item(&mut self, _: ItemId) {} } - V(f) + let mut v = V { + tcx: cx.tcx, + f, + res: None, + }; + node.visit(&mut v); + v.res } /// returns `true` if expr contains match expr desugared from try fn contains_try(expr: &hir::Expr<'_>) -> bool { - let mut found = false; - expr_visitor_no_bodies(|e| { - if !found { - found = matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)); + for_each_expr(expr, |e| { + if matches!(e.kind, hir::ExprKind::Match(_, _, hir::MatchSource::TryDesugar)) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !found }) - .visit_expr(expr); - found + .is_some() } pub fn find_all_ret_expressions<'hir, F>(_cx: &LateContext<'_>, expr: &'hir hir::Expr<'hir>, callback: F) -> bool @@ -228,68 +253,29 @@ where } } -/// A type which can be visited. -pub trait Visitable<'tcx> { - /// Calls the corresponding `visit_*` function on the visitor. - fn visit>(self, visitor: &mut V); -} -macro_rules! visitable_ref { - ($t:ident, $f:ident) => { - impl<'tcx> Visitable<'tcx> for &'tcx $t<'tcx> { - fn visit>(self, visitor: &mut V) { - visitor.$f(self); - } - } - }; -} -visitable_ref!(Arm, visit_arm); -visitable_ref!(Block, visit_block); -visitable_ref!(Body, visit_body); -visitable_ref!(Expr, visit_expr); -visitable_ref!(Stmt, visit_stmt); - -// impl<'tcx, I: IntoIterator> Visitable<'tcx> for I -// where -// I::Item: Visitable<'tcx>, -// { -// fn visit>(self, visitor: &mut V) { -// for x in self { -// x.visit(visitor); -// } -// } -// } - /// Checks if the given resolved path is used in the given body. pub fn is_res_used(cx: &LateContext<'_>, res: Res, body: BodyId) -> bool { - let mut found = false; - expr_visitor(cx, |e| { - if found { - return false; - } - + for_each_expr_with_closures(cx, cx.tcx.hir().body(body).value, |e| { if let ExprKind::Path(p) = &e.kind { if cx.qpath_res(p, e.hir_id) == res { - found = true; + return ControlFlow::Break(()); } } - !found + ControlFlow::Continue(()) }) - .visit_expr(cx.tcx.hir().body(body).value); - found + .is_some() } /// Checks if the given local is used. pub fn is_local_used<'tcx>(cx: &LateContext<'tcx>, visitable: impl Visitable<'tcx>, id: HirId) -> bool { - let mut is_used = false; - let mut visitor = expr_visitor(cx, |expr| { - if !is_used { - is_used = path_to_local_id(expr, id); + for_each_expr_with_closures(cx, visitable, |e| { + if path_to_local_id(e, id) { + ControlFlow::Break(()) + } else { + ControlFlow::Continue(()) } - !is_used - }); - visitable.visit(&mut visitor); - drop(visitor); - is_used + }) + .is_some() } /// Checks if the given expression is a constant. diff --git a/src/docs/needless_borrowed_reference.txt b/src/docs/needless_borrowed_reference.txt index 55faa0cf5719..152459ba1c9d 100644 --- a/src/docs/needless_borrowed_reference.txt +++ b/src/docs/needless_borrowed_reference.txt @@ -1,30 +1,22 @@ ### What it does -Checks for bindings that destructure a reference and borrow the inner +Checks for bindings that needlessly destructure a reference and borrow the inner value with `&ref`. ### Why is this bad? This pattern has no effect in almost all cases. -### Known problems -In some cases, `&ref` is needed to avoid a lifetime mismatch error. -Example: -``` -fn foo(a: &Option, b: &Option) { - match (a, b) { - (None, &ref c) | (&ref c, None) => (), - (&Some(ref c), _) => (), - }; -} -``` - ### Example ``` let mut v = Vec::::new(); v.iter_mut().filter(|&ref a| a.is_empty()); + +if let &[ref first, ref second] = v.as_slice() {} ``` Use instead: ``` let mut v = Vec::::new(); v.iter_mut().filter(|a| a.is_empty()); + +if let [first, second] = v.as_slice() {} ``` \ No newline at end of file diff --git a/tests/ui-internal/auxiliary/paths.rs b/tests/ui-internal/auxiliary/paths.rs new file mode 100644 index 000000000000..52fcaec4df32 --- /dev/null +++ b/tests/ui-internal/auxiliary/paths.rs @@ -0,0 +1,2 @@ +pub static OPTION: [&str; 3] = ["core", "option", "Option"]; +pub const RESULT: &[&str] = &["core", "result", "Result"]; diff --git a/tests/ui-internal/match_type_on_diag_item.rs b/tests/ui-internal/match_type_on_diag_item.rs deleted file mode 100644 index 4b41ff15e80f..000000000000 --- a/tests/ui-internal/match_type_on_diag_item.rs +++ /dev/null @@ -1,39 +0,0 @@ -#![deny(clippy::internal)] -#![allow(clippy::missing_clippy_version_attribute)] -#![feature(rustc_private)] - -extern crate clippy_utils; -extern crate rustc_hir; -extern crate rustc_lint; -extern crate rustc_middle; - -#[macro_use] -extern crate rustc_session; -use clippy_utils::{paths, ty::match_type}; -use rustc_hir::Expr; -use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::Ty; - -declare_lint! { - pub TEST_LINT, - Warn, - "" -} - -declare_lint_pass!(Pass => [TEST_LINT]); - -static OPTION: [&str; 3] = ["core", "option", "Option"]; - -impl<'tcx> LateLintPass<'tcx> for Pass { - fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr) { - let ty = cx.typeck_results().expr_ty(expr); - - let _ = match_type(cx, ty, &OPTION); - let _ = match_type(cx, ty, &["core", "result", "Result"]); - - let rc_path = &["alloc", "rc", "Rc"]; - let _ = clippy_utils::ty::match_type(cx, ty, rc_path); - } -} - -fn main() {} diff --git a/tests/ui-internal/match_type_on_diag_item.stderr b/tests/ui-internal/match_type_on_diag_item.stderr deleted file mode 100644 index e3cb6b6c22ea..000000000000 --- a/tests/ui-internal/match_type_on_diag_item.stderr +++ /dev/null @@ -1,27 +0,0 @@ -error: usage of `clippy_utils::ty::match_type()` on a type diagnostic item - --> $DIR/match_type_on_diag_item.rs:31:17 - | -LL | let _ = match_type(cx, ty, &OPTION); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clippy_utils::ty::is_type_diagnostic_item(cx, ty, sym::Option)` - | -note: the lint level is defined here - --> $DIR/match_type_on_diag_item.rs:1:9 - | -LL | #![deny(clippy::internal)] - | ^^^^^^^^^^^^^^^^ - = note: `#[deny(clippy::match_type_on_diagnostic_item)]` implied by `#[deny(clippy::internal)]` - -error: usage of `clippy_utils::ty::match_type()` on a type diagnostic item - --> $DIR/match_type_on_diag_item.rs:32:17 - | -LL | let _ = match_type(cx, ty, &["core", "result", "Result"]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clippy_utils::ty::is_type_diagnostic_item(cx, ty, sym::Result)` - -error: usage of `clippy_utils::ty::match_type()` on a type diagnostic item - --> $DIR/match_type_on_diag_item.rs:35:17 - | -LL | let _ = clippy_utils::ty::match_type(cx, ty, rc_path); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `clippy_utils::ty::is_type_diagnostic_item(cx, ty, sym::Rc)` - -error: aborting due to 3 previous errors - diff --git a/tests/ui-internal/unnecessary_def_path.fixed b/tests/ui-internal/unnecessary_def_path.fixed new file mode 100644 index 000000000000..4c050332f2cc --- /dev/null +++ b/tests/ui-internal/unnecessary_def_path.fixed @@ -0,0 +1,62 @@ +// run-rustfix +// aux-build:paths.rs +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate clippy_utils; +extern crate paths; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_span; + +#[allow(unused)] +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; +#[allow(unused)] +use clippy_utils::{ + is_expr_path_def_path, is_path_diagnostic_item, is_res_diagnostic_ctor, is_res_lang_ctor, is_trait_method, + match_def_path, match_trait_method, path_res, +}; + +#[allow(unused)] +use rustc_hir::LangItem; +#[allow(unused)] +use rustc_span::sym; + +use rustc_hir::def_id::DefId; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; + +#[allow(unused)] +static OPTION: [&str; 3] = ["core", "option", "Option"]; +#[allow(unused)] +const RESULT: &[&str] = &["core", "result", "Result"]; + +fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>) { + let _ = is_type_diagnostic_item(cx, ty, sym::Option); + let _ = is_type_diagnostic_item(cx, ty, sym::Result); + let _ = is_type_diagnostic_item(cx, ty, sym::Result); + + #[allow(unused)] + let rc_path = &["alloc", "rc", "Rc"]; + let _ = is_type_diagnostic_item(cx, ty, sym::Rc); + + let _ = is_type_diagnostic_item(cx, ty, sym::Option); + let _ = is_type_diagnostic_item(cx, ty, sym::Result); + + let _ = is_type_lang_item(cx, ty, LangItem::OwnedBox); + let _ = is_type_diagnostic_item(cx, ty, sym::maybe_uninit_uninit); + + let _ = cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did); + let _ = cx.tcx.is_diagnostic_item(sym::Option, did); + let _ = cx.tcx.lang_items().require(LangItem::OptionSome).ok() == Some(did); + + let _ = is_trait_method(cx, expr, sym::AsRef); + + let _ = is_path_diagnostic_item(cx, expr, sym::Option); + let _ = path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id)); + let _ = is_res_lang_ctor(cx, path_res(cx, expr), LangItem::OptionSome); +} + +fn main() {} diff --git a/tests/ui-internal/unnecessary_def_path.rs b/tests/ui-internal/unnecessary_def_path.rs new file mode 100644 index 000000000000..6506f1f164ac --- /dev/null +++ b/tests/ui-internal/unnecessary_def_path.rs @@ -0,0 +1,62 @@ +// run-rustfix +// aux-build:paths.rs +#![deny(clippy::internal)] +#![feature(rustc_private)] + +extern crate clippy_utils; +extern crate paths; +extern crate rustc_hir; +extern crate rustc_lint; +extern crate rustc_middle; +extern crate rustc_span; + +#[allow(unused)] +use clippy_utils::ty::{is_type_diagnostic_item, is_type_lang_item, match_type}; +#[allow(unused)] +use clippy_utils::{ + is_expr_path_def_path, is_path_diagnostic_item, is_res_diagnostic_ctor, is_res_lang_ctor, is_trait_method, + match_def_path, match_trait_method, path_res, +}; + +#[allow(unused)] +use rustc_hir::LangItem; +#[allow(unused)] +use rustc_span::sym; + +use rustc_hir::def_id::DefId; +use rustc_hir::Expr; +use rustc_lint::LateContext; +use rustc_middle::ty::Ty; + +#[allow(unused)] +static OPTION: [&str; 3] = ["core", "option", "Option"]; +#[allow(unused)] +const RESULT: &[&str] = &["core", "result", "Result"]; + +fn _f<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>, did: DefId, expr: &Expr<'_>) { + let _ = match_type(cx, ty, &OPTION); + let _ = match_type(cx, ty, RESULT); + let _ = match_type(cx, ty, &["core", "result", "Result"]); + + #[allow(unused)] + let rc_path = &["alloc", "rc", "Rc"]; + let _ = clippy_utils::ty::match_type(cx, ty, rc_path); + + let _ = match_type(cx, ty, &paths::OPTION); + let _ = match_type(cx, ty, paths::RESULT); + + let _ = match_type(cx, ty, &["alloc", "boxed", "Box"]); + let _ = match_type(cx, ty, &["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"]); + + let _ = match_def_path(cx, did, &["alloc", "boxed", "Box"]); + let _ = match_def_path(cx, did, &["core", "option", "Option"]); + let _ = match_def_path(cx, did, &["core", "option", "Option", "Some"]); + + let _ = match_trait_method(cx, expr, &["core", "convert", "AsRef"]); + + let _ = is_expr_path_def_path(cx, expr, &["core", "option", "Option"]); + let _ = is_expr_path_def_path(cx, expr, &["core", "iter", "traits", "Iterator", "next"]); + let _ = is_expr_path_def_path(cx, expr, &["core", "option", "Option", "Some"]); +} + +fn main() {} diff --git a/tests/ui-internal/unnecessary_def_path.stderr b/tests/ui-internal/unnecessary_def_path.stderr new file mode 100644 index 000000000000..a99a8f71fa6a --- /dev/null +++ b/tests/ui-internal/unnecessary_def_path.stderr @@ -0,0 +1,101 @@ +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:37:13 + | +LL | let _ = match_type(cx, ty, &OPTION); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Option)` + | +note: the lint level is defined here + --> $DIR/unnecessary_def_path.rs:3:9 + | +LL | #![deny(clippy::internal)] + | ^^^^^^^^^^^^^^^^ + = note: `#[deny(clippy::unnecessary_def_path)]` implied by `#[deny(clippy::internal)]` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:38:13 + | +LL | let _ = match_type(cx, ty, RESULT); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:39:13 + | +LL | let _ = match_type(cx, ty, &["core", "result", "Result"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:43:13 + | +LL | let _ = clippy_utils::ty::match_type(cx, ty, rc_path); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Rc)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:45:13 + | +LL | let _ = match_type(cx, ty, &paths::OPTION); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Option)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:46:13 + | +LL | let _ = match_type(cx, ty, paths::RESULT); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::Result)` + +error: use of a def path to a `LangItem` + --> $DIR/unnecessary_def_path.rs:48:13 + | +LL | let _ = match_type(cx, ty, &["alloc", "boxed", "Box"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_lang_item(cx, ty, LangItem::OwnedBox)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:49:13 + | +LL | let _ = match_type(cx, ty, &["core", "mem", "maybe_uninit", "MaybeUninit", "uninit"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_type_diagnostic_item(cx, ty, sym::maybe_uninit_uninit)` + +error: use of a def path to a `LangItem` + --> $DIR/unnecessary_def_path.rs:51:13 + | +LL | let _ = match_def_path(cx, did, &["alloc", "boxed", "Box"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().require(LangItem::OwnedBox).ok() == Some(did)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:52:13 + | +LL | let _ = match_def_path(cx, did, &["core", "option", "Option"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.is_diagnostic_item(sym::Option, did)` + +error: use of a def path to a `LangItem` + --> $DIR/unnecessary_def_path.rs:53:13 + | +LL | let _ = match_def_path(cx, did, &["core", "option", "Option", "Some"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `cx.tcx.lang_items().require(LangItem::OptionSome).ok() == Some(did)` + | + = help: if this `DefId` came from a constructor expression or pattern then the parent `DefId` should be used instead + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:55:13 + | +LL | let _ = match_trait_method(cx, expr, &["core", "convert", "AsRef"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_trait_method(cx, expr, sym::AsRef)` + +error: use of a def path to a diagnostic item + --> $DIR/unnecessary_def_path.rs:57:13 + | +LL | let _ = is_expr_path_def_path(cx, expr, &["core", "option", "Option"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_path_diagnostic_item(cx, expr, sym::Option)` + +error: use of a def path to a `LangItem` + --> $DIR/unnecessary_def_path.rs:58:13 + | +LL | let _ = is_expr_path_def_path(cx, expr, &["core", "iter", "traits", "Iterator", "next"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `path_res(cx, expr).opt_def_id().map_or(false, |id| cx.tcx.lang_items().require(LangItem::IteratorNext).ok() == Some(id))` + +error: use of a def path to a `LangItem` + --> $DIR/unnecessary_def_path.rs:59:13 + | +LL | let _ = is_expr_path_def_path(cx, expr, &["core", "option", "Option", "Some"]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `is_res_lang_ctor(cx, path_res(cx, expr), LangItem::OptionSome)` + +error: aborting due to 15 previous errors + diff --git a/tests/ui/floating_point_exp.fixed b/tests/ui/floating_point_exp.fixed index c86a502d15f0..b9e3d89c2b29 100644 --- a/tests/ui/floating_point_exp.fixed +++ b/tests/ui/floating_point_exp.fixed @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::imprecise_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 2f32; diff --git a/tests/ui/floating_point_exp.rs b/tests/ui/floating_point_exp.rs index e59589f912a2..ef008dd9be05 100644 --- a/tests/ui/floating_point_exp.rs +++ b/tests/ui/floating_point_exp.rs @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::imprecise_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 2f32; diff --git a/tests/ui/floating_point_exp.stderr b/tests/ui/floating_point_exp.stderr index f84eede19872..b92fae56e421 100644 --- a/tests/ui/floating_point_exp.stderr +++ b/tests/ui/floating_point_exp.stderr @@ -1,5 +1,5 @@ error: (e.pow(x) - 1) can be computed more accurately - --> $DIR/floating_point_exp.rs:6:13 + --> $DIR/floating_point_exp.rs:7:13 | LL | let _ = x.exp() - 1.0; | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` @@ -7,25 +7,25 @@ LL | let _ = x.exp() - 1.0; = note: `-D clippy::imprecise-flops` implied by `-D warnings` error: (e.pow(x) - 1) can be computed more accurately - --> $DIR/floating_point_exp.rs:7:13 + --> $DIR/floating_point_exp.rs:8:13 | LL | let _ = x.exp() - 1.0 + 2.0; | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` error: (e.pow(x) - 1) can be computed more accurately - --> $DIR/floating_point_exp.rs:8:13 + --> $DIR/floating_point_exp.rs:9:13 | LL | let _ = (x as f32).exp() - 1.0 + 2.0; | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x as f32).exp_m1()` error: (e.pow(x) - 1) can be computed more accurately - --> $DIR/floating_point_exp.rs:14:13 + --> $DIR/floating_point_exp.rs:15:13 | LL | let _ = x.exp() - 1.0; | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` error: (e.pow(x) - 1) can be computed more accurately - --> $DIR/floating_point_exp.rs:15:13 + --> $DIR/floating_point_exp.rs:16:13 | LL | let _ = x.exp() - 1.0 + 2.0; | ^^^^^^^^^^^^^ help: consider using: `x.exp_m1()` diff --git a/tests/ui/floating_point_log.fixed b/tests/ui/floating_point_log.fixed index 4def9300bb7d..ee5406461600 100644 --- a/tests/ui/floating_point_log.fixed +++ b/tests/ui/floating_point_log.fixed @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code, clippy::double_parens)] +#![allow(dead_code, clippy::double_parens, clippy::unnecessary_cast)] #![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] const TWO: f32 = 2.0; diff --git a/tests/ui/floating_point_log.rs b/tests/ui/floating_point_log.rs index 1e04caa7d2a8..0590670a50bc 100644 --- a/tests/ui/floating_point_log.rs +++ b/tests/ui/floating_point_log.rs @@ -1,5 +1,5 @@ // run-rustfix -#![allow(dead_code, clippy::double_parens)] +#![allow(dead_code, clippy::double_parens, clippy::unnecessary_cast)] #![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] const TWO: f32 = 2.0; diff --git a/tests/ui/floating_point_logbase.fixed b/tests/ui/floating_point_logbase.fixed index 936462f94066..7347bf72cbea 100644 --- a/tests/ui/floating_point_logbase.fixed +++ b/tests/ui/floating_point_logbase.fixed @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_logbase.rs b/tests/ui/floating_point_logbase.rs index 0b56fa8fa41f..ba5b8d406928 100644 --- a/tests/ui/floating_point_logbase.rs +++ b/tests/ui/floating_point_logbase.rs @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_logbase.stderr b/tests/ui/floating_point_logbase.stderr index 384e3554cbbe..9d736b5e1a27 100644 --- a/tests/ui/floating_point_logbase.stderr +++ b/tests/ui/floating_point_logbase.stderr @@ -1,5 +1,5 @@ error: log base can be expressed more clearly - --> $DIR/floating_point_logbase.rs:7:13 + --> $DIR/floating_point_logbase.rs:8:13 | LL | let _ = x.ln() / y.ln(); | ^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` @@ -7,25 +7,25 @@ LL | let _ = x.ln() / y.ln(); = note: `-D clippy::suboptimal-flops` implied by `-D warnings` error: log base can be expressed more clearly - --> $DIR/floating_point_logbase.rs:8:13 + --> $DIR/floating_point_logbase.rs:9:13 | LL | let _ = (x as f32).ln() / y.ln(); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x as f32).log(y)` error: log base can be expressed more clearly - --> $DIR/floating_point_logbase.rs:9:13 + --> $DIR/floating_point_logbase.rs:10:13 | LL | let _ = x.log2() / y.log2(); | ^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` error: log base can be expressed more clearly - --> $DIR/floating_point_logbase.rs:10:13 + --> $DIR/floating_point_logbase.rs:11:13 | LL | let _ = x.log10() / y.log10(); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` error: log base can be expressed more clearly - --> $DIR/floating_point_logbase.rs:11:13 + --> $DIR/floating_point_logbase.rs:12:13 | LL | let _ = x.log(5f32) / y.log(5f32); | ^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.log(y)` diff --git a/tests/ui/floating_point_powf.fixed b/tests/ui/floating_point_powf.fixed index e7ef45634dff..f7f93de29577 100644 --- a/tests/ui/floating_point_powf.fixed +++ b/tests/ui/floating_point_powf.fixed @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_powf.rs b/tests/ui/floating_point_powf.rs index d749aa2d48a4..499fc0e15e47 100644 --- a/tests/ui/floating_point_powf.rs +++ b/tests/ui/floating_point_powf.rs @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops, clippy::imprecise_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let x = 3f32; diff --git a/tests/ui/floating_point_powf.stderr b/tests/ui/floating_point_powf.stderr index e9693de8fc90..7c9d50db2f78 100644 --- a/tests/ui/floating_point_powf.stderr +++ b/tests/ui/floating_point_powf.stderr @@ -1,5 +1,5 @@ error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:6:13 + --> $DIR/floating_point_powf.rs:7:13 | LL | let _ = 2f32.powf(x); | ^^^^^^^^^^^^ help: consider using: `x.exp2()` @@ -7,43 +7,43 @@ LL | let _ = 2f32.powf(x); = note: `-D clippy::suboptimal-flops` implied by `-D warnings` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:7:13 + --> $DIR/floating_point_powf.rs:8:13 | LL | let _ = 2f32.powf(3.1); | ^^^^^^^^^^^^^^ help: consider using: `3.1f32.exp2()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:8:13 + --> $DIR/floating_point_powf.rs:9:13 | LL | let _ = 2f32.powf(-3.1); | ^^^^^^^^^^^^^^^ help: consider using: `(-3.1f32).exp2()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:9:13 + --> $DIR/floating_point_powf.rs:10:13 | LL | let _ = std::f32::consts::E.powf(x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.exp()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:10:13 + --> $DIR/floating_point_powf.rs:11:13 | LL | let _ = std::f32::consts::E.powf(3.1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `3.1f32.exp()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:11:13 + --> $DIR/floating_point_powf.rs:12:13 | LL | let _ = std::f32::consts::E.powf(-3.1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(-3.1f32).exp()` error: square-root of a number can be computed more efficiently and accurately - --> $DIR/floating_point_powf.rs:12:13 + --> $DIR/floating_point_powf.rs:13:13 | LL | let _ = x.powf(1.0 / 2.0); | ^^^^^^^^^^^^^^^^^ help: consider using: `x.sqrt()` error: cube-root of a number can be computed more accurately - --> $DIR/floating_point_powf.rs:13:13 + --> $DIR/floating_point_powf.rs:14:13 | LL | let _ = x.powf(1.0 / 3.0); | ^^^^^^^^^^^^^^^^^ help: consider using: `x.cbrt()` @@ -51,139 +51,139 @@ LL | let _ = x.powf(1.0 / 3.0); = note: `-D clippy::imprecise-flops` implied by `-D warnings` error: cube-root of a number can be computed more accurately - --> $DIR/floating_point_powf.rs:14:13 + --> $DIR/floating_point_powf.rs:15:13 | LL | let _ = (x as f32).powf(1.0 / 3.0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x as f32).cbrt()` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:15:13 + --> $DIR/floating_point_powf.rs:16:13 | LL | let _ = x.powf(3.0); | ^^^^^^^^^^^ help: consider using: `x.powi(3)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:16:13 + --> $DIR/floating_point_powf.rs:17:13 | LL | let _ = x.powf(-2.0); | ^^^^^^^^^^^^ help: consider using: `x.powi(-2)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:17:13 + --> $DIR/floating_point_powf.rs:18:13 | LL | let _ = x.powf(16_777_215.0); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(16_777_215)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:18:13 + --> $DIR/floating_point_powf.rs:19:13 | LL | let _ = x.powf(-16_777_215.0); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(-16_777_215)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:19:13 + --> $DIR/floating_point_powf.rs:20:13 | LL | let _ = (x as f32).powf(-16_777_215.0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x as f32).powi(-16_777_215)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:20:13 + --> $DIR/floating_point_powf.rs:21:13 | LL | let _ = (x as f32).powf(3.0); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `(x as f32).powi(3)` error: cube-root of a number can be computed more accurately - --> $DIR/floating_point_powf.rs:21:13 + --> $DIR/floating_point_powf.rs:22:13 | LL | let _ = (1.5_f32 + 1.0).powf(1.0 / 3.0); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(1.5_f32 + 1.0).cbrt()` error: cube-root of a number can be computed more accurately - --> $DIR/floating_point_powf.rs:22:13 + --> $DIR/floating_point_powf.rs:23:13 | LL | let _ = 1.5_f64.powf(1.0 / 3.0); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `1.5_f64.cbrt()` error: square-root of a number can be computed more efficiently and accurately - --> $DIR/floating_point_powf.rs:23:13 + --> $DIR/floating_point_powf.rs:24:13 | LL | let _ = 1.5_f64.powf(1.0 / 2.0); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `1.5_f64.sqrt()` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:24:13 + --> $DIR/floating_point_powf.rs:25:13 | LL | let _ = 1.5_f64.powf(3.0); | ^^^^^^^^^^^^^^^^^ help: consider using: `1.5_f64.powi(3)` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:33:13 + --> $DIR/floating_point_powf.rs:34:13 | LL | let _ = 2f64.powf(x); | ^^^^^^^^^^^^ help: consider using: `x.exp2()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:34:13 + --> $DIR/floating_point_powf.rs:35:13 | LL | let _ = 2f64.powf(3.1); | ^^^^^^^^^^^^^^ help: consider using: `3.1f64.exp2()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:35:13 + --> $DIR/floating_point_powf.rs:36:13 | LL | let _ = 2f64.powf(-3.1); | ^^^^^^^^^^^^^^^ help: consider using: `(-3.1f64).exp2()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:36:13 + --> $DIR/floating_point_powf.rs:37:13 | LL | let _ = std::f64::consts::E.powf(x); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.exp()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:37:13 + --> $DIR/floating_point_powf.rs:38:13 | LL | let _ = std::f64::consts::E.powf(3.1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `3.1f64.exp()` error: exponent for bases 2 and e can be computed more accurately - --> $DIR/floating_point_powf.rs:38:13 + --> $DIR/floating_point_powf.rs:39:13 | LL | let _ = std::f64::consts::E.powf(-3.1); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(-3.1f64).exp()` error: square-root of a number can be computed more efficiently and accurately - --> $DIR/floating_point_powf.rs:39:13 + --> $DIR/floating_point_powf.rs:40:13 | LL | let _ = x.powf(1.0 / 2.0); | ^^^^^^^^^^^^^^^^^ help: consider using: `x.sqrt()` error: cube-root of a number can be computed more accurately - --> $DIR/floating_point_powf.rs:40:13 + --> $DIR/floating_point_powf.rs:41:13 | LL | let _ = x.powf(1.0 / 3.0); | ^^^^^^^^^^^^^^^^^ help: consider using: `x.cbrt()` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:41:13 + --> $DIR/floating_point_powf.rs:42:13 | LL | let _ = x.powf(3.0); | ^^^^^^^^^^^ help: consider using: `x.powi(3)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:42:13 + --> $DIR/floating_point_powf.rs:43:13 | LL | let _ = x.powf(-2.0); | ^^^^^^^^^^^^ help: consider using: `x.powi(-2)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:43:13 + --> $DIR/floating_point_powf.rs:44:13 | LL | let _ = x.powf(-2_147_483_648.0); | ^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(-2_147_483_648)` error: exponentiation with integer powers can be computed more efficiently - --> $DIR/floating_point_powf.rs:44:13 + --> $DIR/floating_point_powf.rs:45:13 | LL | let _ = x.powf(2_147_483_647.0); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `x.powi(2_147_483_647)` diff --git a/tests/ui/floating_point_powi.fixed b/tests/ui/floating_point_powi.fixed index cc3370388885..884d05fed71b 100644 --- a/tests/ui/floating_point_powi.fixed +++ b/tests/ui/floating_point_powi.fixed @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let one = 1; diff --git a/tests/ui/floating_point_powi.rs b/tests/ui/floating_point_powi.rs index 5950902178cb..e6a1c895371b 100644 --- a/tests/ui/floating_point_powi.rs +++ b/tests/ui/floating_point_powi.rs @@ -1,5 +1,6 @@ // run-rustfix #![warn(clippy::suboptimal_flops)] +#![allow(clippy::unnecessary_cast)] fn main() { let one = 1; diff --git a/tests/ui/floating_point_powi.stderr b/tests/ui/floating_point_powi.stderr index 643b70739cdc..5df0de1fef22 100644 --- a/tests/ui/floating_point_powi.stderr +++ b/tests/ui/floating_point_powi.stderr @@ -1,5 +1,5 @@ error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:9:13 + --> $DIR/floating_point_powi.rs:10:13 | LL | let _ = x.powi(2) + y; | ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` @@ -7,37 +7,37 @@ LL | let _ = x.powi(2) + y; = note: `-D clippy::suboptimal-flops` implied by `-D warnings` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:10:13 + --> $DIR/floating_point_powi.rs:11:13 | LL | let _ = x.powi(2) - y; | ^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, -y)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:11:13 + --> $DIR/floating_point_powi.rs:12:13 | LL | let _ = x + y.powi(2); | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:12:13 + --> $DIR/floating_point_powi.rs:13:13 | LL | let _ = x - y.powi(2); | ^^^^^^^^^^^^^ help: consider using: `y.mul_add(-y, x)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:13:13 + --> $DIR/floating_point_powi.rs:14:13 | LL | let _ = x + (y as f32).powi(2); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(y as f32).mul_add(y as f32, x)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:14:13 + --> $DIR/floating_point_powi.rs:15:13 | LL | let _ = (x.powi(2) + y).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `x.mul_add(x, y)` error: multiply and add expressions can be calculated more efficiently and accurately - --> $DIR/floating_point_powi.rs:15:13 + --> $DIR/floating_point_powi.rs:16:13 | LL | let _ = (x + y.powi(2)).sqrt(); | ^^^^^^^^^^^^^^^ help: consider using: `y.mul_add(y, x)` diff --git a/tests/ui/manual_assert.edition2018.fixed b/tests/ui/manual_assert.edition2018.fixed index 65598f1eaccc..84f6855f3387 100644 --- a/tests/ui/manual_assert.edition2018.fixed +++ b/tests/ui/manual_assert.edition2018.fixed @@ -4,7 +4,7 @@ // run-rustfix #![warn(clippy::manual_assert)] -#![allow(clippy::nonminimal_bool)] +#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)] macro_rules! one { () => { @@ -50,3 +50,14 @@ fn main() { assert!(!(a.is_empty() || !b.is_empty()), "panic5"); assert!(!a.is_empty(), "with expansion {}", one!()); } + +fn issue7730(a: u8) { + // Suggestion should preserve comment + // comment +/* this is a + multiline + comment */ +/// Doc comment +// comment after `panic!` +assert!(!(a > 2), "panic with comment"); +} diff --git a/tests/ui/manual_assert.edition2018.stderr b/tests/ui/manual_assert.edition2018.stderr index a0f31afd6ebf..dbd21be2da9e 100644 --- a/tests/ui/manual_assert.edition2018.stderr +++ b/tests/ui/manual_assert.edition2018.stderr @@ -4,9 +4,13 @@ error: only a `panic!` in `if`-then statement LL | / if !a.is_empty() { LL | | panic!("qaqaq{:?}", a); LL | | } - | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);` + | |_____^ | = note: `-D clippy::manual-assert` implied by `-D warnings` +help: try instead + | +LL | assert!(a.is_empty(), "qaqaq{:?}", a); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:33:5 @@ -14,7 +18,12 @@ error: only a `panic!` in `if`-then statement LL | / if !a.is_empty() { LL | | panic!("qwqwq"); LL | | } - | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");` + | |_____^ + | +help: try instead + | +LL | assert!(a.is_empty(), "qwqwq"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:50:5 @@ -22,7 +31,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() { LL | | panic!("panic1"); LL | | } - | |_____^ help: try: `assert!(!b.is_empty(), "panic1");` + | |_____^ + | +help: try instead + | +LL | assert!(!b.is_empty(), "panic1"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:53:5 @@ -30,7 +44,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() && a.is_empty() { LL | | panic!("panic2"); LL | | } - | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");` + | |_____^ + | +help: try instead + | +LL | assert!(!(b.is_empty() && a.is_empty()), "panic2"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:56:5 @@ -38,7 +57,12 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() && !b.is_empty() { LL | | panic!("panic3"); LL | | } - | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");` + | |_____^ + | +help: try instead + | +LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:59:5 @@ -46,7 +70,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() || a.is_empty() { LL | | panic!("panic4"); LL | | } - | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");` + | |_____^ + | +help: try instead + | +LL | assert!(!(b.is_empty() || a.is_empty()), "panic4"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:62:5 @@ -54,7 +83,12 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() || !b.is_empty() { LL | | panic!("panic5"); LL | | } - | |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");` + | |_____^ + | +help: try instead + | +LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:65:5 @@ -62,7 +96,29 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() { LL | | panic!("with expansion {}", one!()) LL | | } - | |_____^ help: try: `assert!(!a.is_empty(), "with expansion {}", one!());` + | |_____^ + | +help: try instead + | +LL | assert!(!a.is_empty(), "with expansion {}", one!()); + | -error: aborting due to 8 previous errors +error: only a `panic!` in `if`-then statement + --> $DIR/manual_assert.rs:72:5 + | +LL | / if a > 2 { +LL | | // comment +LL | | /* this is a +LL | | multiline +... | +LL | | panic!("panic with comment") // comment after `panic!` +LL | | } + | |_____^ + | +help: try instead + | +LL | assert!(!(a > 2), "panic with comment"); + | + +error: aborting due to 9 previous errors diff --git a/tests/ui/manual_assert.edition2021.fixed b/tests/ui/manual_assert.edition2021.fixed index 65598f1eaccc..84f6855f3387 100644 --- a/tests/ui/manual_assert.edition2021.fixed +++ b/tests/ui/manual_assert.edition2021.fixed @@ -4,7 +4,7 @@ // run-rustfix #![warn(clippy::manual_assert)] -#![allow(clippy::nonminimal_bool)] +#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)] macro_rules! one { () => { @@ -50,3 +50,14 @@ fn main() { assert!(!(a.is_empty() || !b.is_empty()), "panic5"); assert!(!a.is_empty(), "with expansion {}", one!()); } + +fn issue7730(a: u8) { + // Suggestion should preserve comment + // comment +/* this is a + multiline + comment */ +/// Doc comment +// comment after `panic!` +assert!(!(a > 2), "panic with comment"); +} diff --git a/tests/ui/manual_assert.edition2021.stderr b/tests/ui/manual_assert.edition2021.stderr index a0f31afd6ebf..dbd21be2da9e 100644 --- a/tests/ui/manual_assert.edition2021.stderr +++ b/tests/ui/manual_assert.edition2021.stderr @@ -4,9 +4,13 @@ error: only a `panic!` in `if`-then statement LL | / if !a.is_empty() { LL | | panic!("qaqaq{:?}", a); LL | | } - | |_____^ help: try: `assert!(a.is_empty(), "qaqaq{:?}", a);` + | |_____^ | = note: `-D clippy::manual-assert` implied by `-D warnings` +help: try instead + | +LL | assert!(a.is_empty(), "qaqaq{:?}", a); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:33:5 @@ -14,7 +18,12 @@ error: only a `panic!` in `if`-then statement LL | / if !a.is_empty() { LL | | panic!("qwqwq"); LL | | } - | |_____^ help: try: `assert!(a.is_empty(), "qwqwq");` + | |_____^ + | +help: try instead + | +LL | assert!(a.is_empty(), "qwqwq"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:50:5 @@ -22,7 +31,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() { LL | | panic!("panic1"); LL | | } - | |_____^ help: try: `assert!(!b.is_empty(), "panic1");` + | |_____^ + | +help: try instead + | +LL | assert!(!b.is_empty(), "panic1"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:53:5 @@ -30,7 +44,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() && a.is_empty() { LL | | panic!("panic2"); LL | | } - | |_____^ help: try: `assert!(!(b.is_empty() && a.is_empty()), "panic2");` + | |_____^ + | +help: try instead + | +LL | assert!(!(b.is_empty() && a.is_empty()), "panic2"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:56:5 @@ -38,7 +57,12 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() && !b.is_empty() { LL | | panic!("panic3"); LL | | } - | |_____^ help: try: `assert!(!(a.is_empty() && !b.is_empty()), "panic3");` + | |_____^ + | +help: try instead + | +LL | assert!(!(a.is_empty() && !b.is_empty()), "panic3"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:59:5 @@ -46,7 +70,12 @@ error: only a `panic!` in `if`-then statement LL | / if b.is_empty() || a.is_empty() { LL | | panic!("panic4"); LL | | } - | |_____^ help: try: `assert!(!(b.is_empty() || a.is_empty()), "panic4");` + | |_____^ + | +help: try instead + | +LL | assert!(!(b.is_empty() || a.is_empty()), "panic4"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:62:5 @@ -54,7 +83,12 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() || !b.is_empty() { LL | | panic!("panic5"); LL | | } - | |_____^ help: try: `assert!(!(a.is_empty() || !b.is_empty()), "panic5");` + | |_____^ + | +help: try instead + | +LL | assert!(!(a.is_empty() || !b.is_empty()), "panic5"); + | error: only a `panic!` in `if`-then statement --> $DIR/manual_assert.rs:65:5 @@ -62,7 +96,29 @@ error: only a `panic!` in `if`-then statement LL | / if a.is_empty() { LL | | panic!("with expansion {}", one!()) LL | | } - | |_____^ help: try: `assert!(!a.is_empty(), "with expansion {}", one!());` + | |_____^ + | +help: try instead + | +LL | assert!(!a.is_empty(), "with expansion {}", one!()); + | -error: aborting due to 8 previous errors +error: only a `panic!` in `if`-then statement + --> $DIR/manual_assert.rs:72:5 + | +LL | / if a > 2 { +LL | | // comment +LL | | /* this is a +LL | | multiline +... | +LL | | panic!("panic with comment") // comment after `panic!` +LL | | } + | |_____^ + | +help: try instead + | +LL | assert!(!(a > 2), "panic with comment"); + | + +error: aborting due to 9 previous errors diff --git a/tests/ui/manual_assert.fixed b/tests/ui/manual_assert.fixed deleted file mode 100644 index a2393674fe61..000000000000 --- a/tests/ui/manual_assert.fixed +++ /dev/null @@ -1,45 +0,0 @@ -// revisions: edition2018 edition2021 -// [edition2018] edition:2018 -// [edition2021] edition:2021 -// run-rustfix - -#![warn(clippy::manual_assert)] -#![allow(clippy::nonminimal_bool)] - -fn main() { - let a = vec![1, 2, 3]; - let c = Some(2); - if !a.is_empty() - && a.len() == 3 - && c.is_some() - && !a.is_empty() - && a.len() == 3 - && !a.is_empty() - && a.len() == 3 - && !a.is_empty() - && a.len() == 3 - { - panic!("qaqaq{:?}", a); - } - assert!(a.is_empty(), "qaqaq{:?}", a); - assert!(a.is_empty(), "qwqwq"); - if a.len() == 3 { - println!("qwq"); - println!("qwq"); - println!("qwq"); - } - if let Some(b) = c { - panic!("orz {}", b); - } - if a.len() == 3 { - panic!("qaqaq"); - } else { - println!("qwq"); - } - let b = vec![1, 2, 3]; - assert!(!b.is_empty(), "panic1"); - assert!(!(b.is_empty() && a.is_empty()), "panic2"); - assert!(!(a.is_empty() && !b.is_empty()), "panic3"); - assert!(!(b.is_empty() || a.is_empty()), "panic4"); - assert!(!(a.is_empty() || !b.is_empty()), "panic5"); -} diff --git a/tests/ui/manual_assert.rs b/tests/ui/manual_assert.rs index 4d2706dd6211..14abf94965af 100644 --- a/tests/ui/manual_assert.rs +++ b/tests/ui/manual_assert.rs @@ -4,7 +4,7 @@ // run-rustfix #![warn(clippy::manual_assert)] -#![allow(clippy::nonminimal_bool)] +#![allow(dead_code, unused_doc_comments, clippy::nonminimal_bool)] macro_rules! one { () => { @@ -66,3 +66,15 @@ fn main() { panic!("with expansion {}", one!()) } } + +fn issue7730(a: u8) { + // Suggestion should preserve comment + if a > 2 { + // comment + /* this is a + multiline + comment */ + /// Doc comment + panic!("panic with comment") // comment after `panic!` + } +} diff --git a/tests/ui/manual_bits.fixed b/tests/ui/manual_bits.fixed index 386360dbdcdb..e7f8cd878ca7 100644 --- a/tests/ui/manual_bits.fixed +++ b/tests/ui/manual_bits.fixed @@ -6,7 +6,8 @@ clippy::useless_conversion, path_statements, unused_must_use, - clippy::unnecessary_operation + clippy::unnecessary_operation, + clippy::unnecessary_cast )] use std::mem::{size_of, size_of_val}; diff --git a/tests/ui/manual_bits.rs b/tests/ui/manual_bits.rs index 62638f047eb0..7b1d15495287 100644 --- a/tests/ui/manual_bits.rs +++ b/tests/ui/manual_bits.rs @@ -6,7 +6,8 @@ clippy::useless_conversion, path_statements, unused_must_use, - clippy::unnecessary_operation + clippy::unnecessary_operation, + clippy::unnecessary_cast )] use std::mem::{size_of, size_of_val}; diff --git a/tests/ui/manual_bits.stderr b/tests/ui/manual_bits.stderr index 69c591a203d3..652fafbc41d8 100644 --- a/tests/ui/manual_bits.stderr +++ b/tests/ui/manual_bits.stderr @@ -1,5 +1,5 @@ error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:15:5 + --> $DIR/manual_bits.rs:16:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `i8::BITS as usize` @@ -7,169 +7,169 @@ LL | size_of::() * 8; = note: `-D clippy::manual-bits` implied by `-D warnings` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:16:5 + --> $DIR/manual_bits.rs:17:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i16::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:17:5 + --> $DIR/manual_bits.rs:18:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i32::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:18:5 + --> $DIR/manual_bits.rs:19:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i64::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:19:5 + --> $DIR/manual_bits.rs:20:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `i128::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:20:5 + --> $DIR/manual_bits.rs:21:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `isize::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:22:5 + --> $DIR/manual_bits.rs:23:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^ help: consider using: `u8::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:23:5 + --> $DIR/manual_bits.rs:24:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u16::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:24:5 + --> $DIR/manual_bits.rs:25:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u32::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:25:5 + --> $DIR/manual_bits.rs:26:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u64::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:26:5 + --> $DIR/manual_bits.rs:27:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `u128::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:27:5 + --> $DIR/manual_bits.rs:28:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `usize::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:29:5 + --> $DIR/manual_bits.rs:30:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^ help: consider using: `i8::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:30:5 + --> $DIR/manual_bits.rs:31:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i16::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:31:5 + --> $DIR/manual_bits.rs:32:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i32::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:32:5 + --> $DIR/manual_bits.rs:33:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `i64::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:33:5 + --> $DIR/manual_bits.rs:34:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `i128::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:34:5 + --> $DIR/manual_bits.rs:35:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `isize::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:36:5 + --> $DIR/manual_bits.rs:37:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^ help: consider using: `u8::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:37:5 + --> $DIR/manual_bits.rs:38:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u16::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:38:5 + --> $DIR/manual_bits.rs:39:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u32::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:39:5 + --> $DIR/manual_bits.rs:40:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^ help: consider using: `u64::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:40:5 + --> $DIR/manual_bits.rs:41:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `u128::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:41:5 + --> $DIR/manual_bits.rs:42:5 | LL | 8 * size_of::(); | ^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `usize::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:51:5 + --> $DIR/manual_bits.rs:52:5 | LL | size_of::() * 8; | ^^^^^^^^^^^^^^^^^^^^^ help: consider using: `Word::BITS as usize` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:55:18 + --> $DIR/manual_bits.rs:56:18 | LL | let _: u32 = (size_of::() * 8) as u32; | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `u128::BITS` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:56:18 + --> $DIR/manual_bits.rs:57:18 | LL | let _: u32 = (size_of::() * 8).try_into().unwrap(); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `u128::BITS` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:57:13 + --> $DIR/manual_bits.rs:58:13 | LL | let _ = (size_of::() * 8).pow(5); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(u128::BITS as usize)` error: usage of `mem::size_of::()` to obtain the size of `T` in bits - --> $DIR/manual_bits.rs:58:14 + --> $DIR/manual_bits.rs:59:14 | LL | let _ = &(size_of::() * 8); | ^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `(u128::BITS as usize)` diff --git a/tests/ui/needless_borrowed_ref.fixed b/tests/ui/needless_borrowed_ref.fixed index a0937a2c5f62..bcb4eb2dd48a 100644 --- a/tests/ui/needless_borrowed_ref.fixed +++ b/tests/ui/needless_borrowed_ref.fixed @@ -1,17 +1,38 @@ // run-rustfix -#[warn(clippy::needless_borrowed_reference)] -#[allow(unused_variables)] -fn main() { +#![warn(clippy::needless_borrowed_reference)] +#![allow(unused, clippy::needless_borrow)] + +fn main() {} + +fn should_lint(array: [u8; 4], slice: &[u8], slice_of_refs: &[&u8], vec: Vec) { let mut v = Vec::::new(); let _ = v.iter_mut().filter(|a| a.is_empty()); - // ^ should be linted let var = 3; let thingy = Some(&var); - if let Some(&ref v) = thingy { - // ^ should be linted - } + if let Some(v) = thingy {} + + if let &[a, ref b] = slice_of_refs {} + + let [a, ..] = &array; + let [a, b, ..] = &array; + + if let [a, b] = slice {} + if let [a, b] = &vec[..] {} + + if let [a, b, ..] = slice {} + if let [a, .., b] = slice {} + if let [.., a, b] = slice {} +} + +fn should_not_lint(array: [u8; 4], slice: &[u8], slice_of_refs: &[&u8], vec: Vec) { + if let [ref a] = slice {} + if let &[ref a, b] = slice {} + if let &[ref a, .., b] = slice {} + + // must not be removed as variables must be bound consistently across | patterns + if let (&[ref a], _) | ([], ref a) = (slice_of_refs, &1u8) {} let mut var2 = 5; let thingy2 = Some(&mut var2); @@ -28,17 +49,15 @@ fn main() { } } -#[allow(dead_code)] enum Animal { Cat(u64), Dog(u64), } -#[allow(unused_variables)] -#[allow(dead_code)] fn foo(a: &Animal, b: &Animal) { match (a, b) { - (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref' + // lifetime mismatch error if there is no '&ref' before `feature(nll)` stabilization in 1.63 + (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // ^ and ^ should **not** be linted (&Animal::Dog(ref a), &Animal::Dog(_)) => (), // ^ should **not** be linted } diff --git a/tests/ui/needless_borrowed_ref.rs b/tests/ui/needless_borrowed_ref.rs index 500ac448f0d5..f6de1a6d83d1 100644 --- a/tests/ui/needless_borrowed_ref.rs +++ b/tests/ui/needless_borrowed_ref.rs @@ -1,17 +1,38 @@ // run-rustfix -#[warn(clippy::needless_borrowed_reference)] -#[allow(unused_variables)] -fn main() { +#![warn(clippy::needless_borrowed_reference)] +#![allow(unused, clippy::needless_borrow)] + +fn main() {} + +fn should_lint(array: [u8; 4], slice: &[u8], slice_of_refs: &[&u8], vec: Vec) { let mut v = Vec::::new(); let _ = v.iter_mut().filter(|&ref a| a.is_empty()); - // ^ should be linted let var = 3; let thingy = Some(&var); - if let Some(&ref v) = thingy { - // ^ should be linted - } + if let Some(&ref v) = thingy {} + + if let &[&ref a, ref b] = slice_of_refs {} + + let &[ref a, ..] = &array; + let &[ref a, ref b, ..] = &array; + + if let &[ref a, ref b] = slice {} + if let &[ref a, ref b] = &vec[..] {} + + if let &[ref a, ref b, ..] = slice {} + if let &[ref a, .., ref b] = slice {} + if let &[.., ref a, ref b] = slice {} +} + +fn should_not_lint(array: [u8; 4], slice: &[u8], slice_of_refs: &[&u8], vec: Vec) { + if let [ref a] = slice {} + if let &[ref a, b] = slice {} + if let &[ref a, .., b] = slice {} + + // must not be removed as variables must be bound consistently across | patterns + if let (&[ref a], _) | ([], ref a) = (slice_of_refs, &1u8) {} let mut var2 = 5; let thingy2 = Some(&mut var2); @@ -28,17 +49,15 @@ fn main() { } } -#[allow(dead_code)] enum Animal { Cat(u64), Dog(u64), } -#[allow(unused_variables)] -#[allow(dead_code)] fn foo(a: &Animal, b: &Animal) { match (a, b) { - (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // lifetime mismatch error if there is no '&ref' + // lifetime mismatch error if there is no '&ref' before `feature(nll)` stabilization in 1.63 + (&Animal::Cat(v), &ref k) | (&ref k, &Animal::Cat(v)) => (), // ^ and ^ should **not** be linted (&Animal::Dog(ref a), &Animal::Dog(_)) => (), // ^ should **not** be linted } diff --git a/tests/ui/needless_borrowed_ref.stderr b/tests/ui/needless_borrowed_ref.stderr index 0a5cfb3db0b1..7453542e673f 100644 --- a/tests/ui/needless_borrowed_ref.stderr +++ b/tests/ui/needless_borrowed_ref.stderr @@ -1,10 +1,123 @@ -error: this pattern takes a reference on something that is being de-referenced - --> $DIR/needless_borrowed_ref.rs:7:34 +error: this pattern takes a reference on something that is being dereferenced + --> $DIR/needless_borrowed_ref.rs:10:34 | LL | let _ = v.iter_mut().filter(|&ref a| a.is_empty()); - | ^^^^^^ help: try removing the `&ref` part and just keep: `a` + | ^^^^^^ | = note: `-D clippy::needless-borrowed-reference` implied by `-D warnings` +help: try removing the `&ref` part + | +LL - let _ = v.iter_mut().filter(|&ref a| a.is_empty()); +LL + let _ = v.iter_mut().filter(|a| a.is_empty()); + | -error: aborting due to previous error +error: this pattern takes a reference on something that is being dereferenced + --> $DIR/needless_borrowed_ref.rs:14:17 + | +LL | if let Some(&ref v) = thingy {} + | ^^^^^^ + | +help: try removing the `&ref` part + | +LL - if let Some(&ref v) = thingy {} +LL + if let Some(v) = thingy {} + | + +error: this pattern takes a reference on something that is being dereferenced + --> $DIR/needless_borrowed_ref.rs:16:14 + | +LL | if let &[&ref a, ref b] = slice_of_refs {} + | ^^^^^^ + | +help: try removing the `&ref` part + | +LL - if let &[&ref a, ref b] = slice_of_refs {} +LL + if let &[a, ref b] = slice_of_refs {} + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:18:9 + | +LL | let &[ref a, ..] = &array; + | ^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - let &[ref a, ..] = &array; +LL + let [a, ..] = &array; + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:19:9 + | +LL | let &[ref a, ref b, ..] = &array; + | ^^^^^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - let &[ref a, ref b, ..] = &array; +LL + let [a, b, ..] = &array; + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:21:12 + | +LL | if let &[ref a, ref b] = slice {} + | ^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - if let &[ref a, ref b] = slice {} +LL + if let [a, b] = slice {} + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:22:12 + | +LL | if let &[ref a, ref b] = &vec[..] {} + | ^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - if let &[ref a, ref b] = &vec[..] {} +LL + if let [a, b] = &vec[..] {} + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:24:12 + | +LL | if let &[ref a, ref b, ..] = slice {} + | ^^^^^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - if let &[ref a, ref b, ..] = slice {} +LL + if let [a, b, ..] = slice {} + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:25:12 + | +LL | if let &[ref a, .., ref b] = slice {} + | ^^^^^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - if let &[ref a, .., ref b] = slice {} +LL + if let [a, .., b] = slice {} + | + +error: dereferencing a slice pattern where every element takes a reference + --> $DIR/needless_borrowed_ref.rs:26:12 + | +LL | if let &[.., ref a, ref b] = slice {} + | ^^^^^^^^^^^^^^^^^^^ + | +help: try removing the `&` and `ref` parts + | +LL - if let &[.., ref a, ref b] = slice {} +LL + if let [.., a, b] = slice {} + | + +error: aborting due to 10 previous errors diff --git a/tests/ui/option_take_on_temporary.fixed b/tests/ui/option_take_on_temporary.fixed deleted file mode 100644 index 29691e81666f..000000000000 --- a/tests/ui/option_take_on_temporary.fixed +++ /dev/null @@ -1,15 +0,0 @@ -// run-rustfix - -fn main() { - println!("Testing non erroneous option_take_on_temporary"); - let mut option = Some(1); - let _ = Box::new(move || option.take().unwrap()); - - println!("Testing non erroneous option_take_on_temporary"); - let x = Some(3); - x.as_ref(); - - println!("Testing erroneous option_take_on_temporary"); - let x = Some(3); - x.as_ref(); -} diff --git a/tests/ui/ptr_offset_with_cast.fixed b/tests/ui/ptr_offset_with_cast.fixed index 718e391e8bf6..c57e2990fb95 100644 --- a/tests/ui/ptr_offset_with_cast.fixed +++ b/tests/ui/ptr_offset_with_cast.fixed @@ -1,4 +1,5 @@ // run-rustfix +#![allow(clippy::unnecessary_cast)] fn main() { let vec = vec![b'a', b'b', b'c']; diff --git a/tests/ui/ptr_offset_with_cast.rs b/tests/ui/ptr_offset_with_cast.rs index f613742c741e..3de7997acddd 100644 --- a/tests/ui/ptr_offset_with_cast.rs +++ b/tests/ui/ptr_offset_with_cast.rs @@ -1,4 +1,5 @@ // run-rustfix +#![allow(clippy::unnecessary_cast)] fn main() { let vec = vec![b'a', b'b', b'c']; diff --git a/tests/ui/ptr_offset_with_cast.stderr b/tests/ui/ptr_offset_with_cast.stderr index fd45224ca067..3ba40593d644 100644 --- a/tests/ui/ptr_offset_with_cast.stderr +++ b/tests/ui/ptr_offset_with_cast.stderr @@ -1,5 +1,5 @@ error: use of `offset` with a `usize` casted to an `isize` - --> $DIR/ptr_offset_with_cast.rs:12:17 + --> $DIR/ptr_offset_with_cast.rs:13:17 | LL | let _ = ptr.offset(offset_usize as isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.add(offset_usize)` @@ -7,7 +7,7 @@ LL | let _ = ptr.offset(offset_usize as isize); = note: `-D clippy::ptr-offset-with-cast` implied by `-D warnings` error: use of `wrapping_offset` with a `usize` casted to an `isize` - --> $DIR/ptr_offset_with_cast.rs:16:17 + --> $DIR/ptr_offset_with_cast.rs:17:17 | LL | let _ = ptr.wrapping_offset(offset_usize as isize); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `ptr.wrapping_add(offset_usize)` diff --git a/tests/ui/unnecessary_cast.fixed b/tests/ui/unnecessary_cast.fixed index ee9f157342d4..94dc96427263 100644 --- a/tests/ui/unnecessary_cast.fixed +++ b/tests/ui/unnecessary_cast.fixed @@ -97,4 +97,18 @@ mod fixable { let _ = -(1 + 1) as i64; } + + fn issue_9563() { + let _: f64 = (-8.0_f64).exp(); + #[allow(clippy::precedence)] + let _: f64 = -8.0_f64.exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior + } + + fn issue_9562_non_literal() { + fn foo() -> f32 { + 0. + } + + let _num = foo(); + } } diff --git a/tests/ui/unnecessary_cast.rs b/tests/ui/unnecessary_cast.rs index 5b70412424c0..e5150256f69a 100644 --- a/tests/ui/unnecessary_cast.rs +++ b/tests/ui/unnecessary_cast.rs @@ -97,4 +97,18 @@ mod fixable { let _ = -(1 + 1) as i64; } + + fn issue_9563() { + let _: f64 = (-8.0 as f64).exp(); + #[allow(clippy::precedence)] + let _: f64 = -(8.0 as f64).exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior + } + + fn issue_9562_non_literal() { + fn foo() -> f32 { + 0. + } + + let _num = foo() as f32; + } } diff --git a/tests/ui/unnecessary_cast.stderr b/tests/ui/unnecessary_cast.stderr index f7829ff3b0ef..e5c3dd5e53f8 100644 --- a/tests/ui/unnecessary_cast.stderr +++ b/tests/ui/unnecessary_cast.stderr @@ -162,5 +162,23 @@ error: casting integer literal to `i64` is unnecessary LL | let _: i64 = -(1) as i64; | ^^^^^^^^^^^ help: try: `-1_i64` -error: aborting due to 27 previous errors +error: casting float literal to `f64` is unnecessary + --> $DIR/unnecessary_cast.rs:102:22 + | +LL | let _: f64 = (-8.0 as f64).exp(); + | ^^^^^^^^^^^^^ help: try: `(-8.0_f64)` + +error: casting float literal to `f64` is unnecessary + --> $DIR/unnecessary_cast.rs:104:23 + | +LL | let _: f64 = -(8.0 as f64).exp(); // should suggest `-8.0_f64.exp()` here not to change code behavior + | ^^^^^^^^^^^^ help: try: `8.0_f64` + +error: casting to the same type is unnecessary (`f32` -> `f32`) + --> $DIR/unnecessary_cast.rs:112:20 + | +LL | let _num = foo() as f32; + | ^^^^^^^^^^^^ help: try: `foo()` + +error: aborting due to 30 previous errors