diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index 32c8731c53743..a9aafe7ed56cb 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -1,4 +1,4 @@ -use clippy_utils::diagnostics::{span_lint, span_lint_and_then, span_lint_hir_and_then}; +use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir, span_lint_hir_and_then}; use clippy_utils::source::{snippet, snippet_with_context}; use clippy_utils::sugg::Sugg; use clippy_utils::{ @@ -114,40 +114,35 @@ impl<'tcx> LateLintPass<'tcx> for LintPass { k: FnKind<'tcx>, decl: &'tcx FnDecl<'_>, body: &'tcx Body<'_>, - span: Span, + _: Span, _: LocalDefId, ) { - if let FnKind::Closure = k { - // Does not apply to closures - return; - } - if in_external_macro(cx.tcx.sess, span) { - return; - } - for arg in iter_input_pats(decl, body) { - // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. - if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) { - return; - } - if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind { - span_lint( - cx, - TOPLEVEL_REF_ARG, - arg.pat.span, - "`ref` directly on a function argument is ignored. \ - Consider using a reference type instead", - ); + if !matches!(k, FnKind::Closure) { + for arg in iter_input_pats(decl, body) { + if let PatKind::Binding(BindingMode(ByRef::Yes(_), _), ..) = arg.pat.kind + && is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) + && !in_external_macro(cx.tcx.sess, arg.span) + { + span_lint_hir( + cx, + TOPLEVEL_REF_ARG, + arg.hir_id, + arg.pat.span, + "`ref` directly on a function argument is ignored. \ + Consider using a reference type instead", + ); + } } } } fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { - if !in_external_macro(cx.tcx.sess, stmt.span) - && let StmtKind::Let(local) = stmt.kind + if let StmtKind::Let(local) = stmt.kind && let PatKind::Binding(BindingMode(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind && let Some(init) = local.init // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id) + && !in_external_macro(cx.tcx.sess, stmt.span) { let ctxt = local.span.ctxt(); let mut app = Applicability::MachineApplicable; diff --git a/clippy_lints/src/mismatching_type_param_order.rs b/clippy_lints/src/mismatching_type_param_order.rs index 934b9f490addf..748289454bea5 100644 --- a/clippy_lints/src/mismatching_type_param_order.rs +++ b/clippy_lints/src/mismatching_type_param_order.rs @@ -49,12 +49,12 @@ declare_lint_pass!(TypeParamMismatch => [MISMATCHING_TYPE_PARAM_ORDER]); impl<'tcx> LateLintPass<'tcx> for TypeParamMismatch { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - if !item.span.from_expansion() - && let ItemKind::Impl(imp) = &item.kind + if let ItemKind::Impl(imp) = &item.kind && let TyKind::Path(QPath::Resolved(_, path)) = &imp.self_ty.kind - && let Some(segment) = path.segments.iter().next() + && let [segment, ..] = path.segments && let Some(generic_args) = segment.args && !generic_args.args.is_empty() + && !item.span.from_expansion() { // get the name and span of the generic parameters in the Impl let mut impl_params = Vec::new(); diff --git a/clippy_lints/src/mut_mut.rs b/clippy_lints/src/mut_mut.rs index bc7b2c6b7c1b8..60372121a7a4c 100644 --- a/clippy_lints/src/mut_mut.rs +++ b/clippy_lints/src/mut_mut.rs @@ -35,33 +35,18 @@ impl<'tcx> LateLintPass<'tcx> for MutMut { } fn check_ty(&mut self, cx: &LateContext<'tcx>, ty: &'tcx hir::Ty<'_>) { - if in_external_macro(cx.sess(), ty.span) { - return; - } - - if let hir::TyKind::Ref( - _, - hir::MutTy { - ty: pty, - mutbl: hir::Mutability::Mut, - }, - ) = ty.kind + if let hir::TyKind::Ref(_, mty) = ty.kind + && mty.mutbl == hir::Mutability::Mut + && let hir::TyKind::Ref(_, mty) = mty.ty.kind + && mty.mutbl == hir::Mutability::Mut + && !in_external_macro(cx.sess(), ty.span) { - if let hir::TyKind::Ref( - _, - hir::MutTy { - mutbl: hir::Mutability::Mut, - .. - }, - ) = pty.kind - { - span_lint( - cx, - MUT_MUT, - ty.span, - "generally you want to avoid `&mut &mut _` if possible", - ); - } + span_lint( + cx, + MUT_MUT, + ty.span, + "generally you want to avoid `&mut &mut _` if possible", + ); } } } diff --git a/clippy_lints/src/needless_borrowed_ref.rs b/clippy_lints/src/needless_borrowed_ref.rs index fb02f24c9dc67..098098718af39 100644 --- a/clippy_lints/src/needless_borrowed_ref.rs +++ b/clippy_lints/src/needless_borrowed_ref.rs @@ -37,83 +37,75 @@ declare_lint_pass!(NeedlessBorrowedRef => [NEEDLESS_BORROWED_REFERENCE]); impl<'tcx> LateLintPass<'tcx> for NeedlessBorrowedRef { fn check_pat(&mut self, cx: &LateContext<'tcx>, ref_pat: &'tcx Pat<'_>) { - if ref_pat.span.from_expansion() { - // OK, simple enough, lints doesn't check in macro. - return; - } - - // 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(ref_pat.hir_id) { - let Node::Pat(pat) = node else { break }; - - if matches!(pat.kind, PatKind::Or(_)) { - return; + if let PatKind::Ref(pat, Mutability::Not) = ref_pat.kind + && !ref_pat.span.from_expansion() + && cx + .tcx + .hir() + .parent_iter(ref_pat.hir_id) + .map_while(|(_, parent)| if let Node::Pat(pat) = parent { Some(pat) } else { None }) + // Do not lint patterns that are part of an OR `|` pattern, the binding mode must match in all arms + .all(|pat| !matches!(pat.kind, PatKind::Or(_))) + { + match pat.kind { + // Check sub_pat got a `ref` keyword (excluding `ref mut`). + PatKind::Binding(BindingMode::REF, _, ident, None) => { + span_lint_and_then( + cx, + NEEDLESS_BORROWED_REFERENCE, + ref_pat.span, + "this pattern takes a reference on something that is being dereferenced", + |diag| { + // `&ref ident` + // ^^^^^ + let span = ref_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, + ) => { + check_subpatterns( + cx, + "dereferencing a slice pattern where every element takes a reference", + ref_pat, + pat, + itertools::chain(before, after), + ); + }, + PatKind::Tuple(subpatterns, _) | PatKind::TupleStruct(_, subpatterns, _) => { + check_subpatterns( + cx, + "dereferencing a tuple pattern where every element takes a reference", + ref_pat, + pat, + subpatterns, + ); + }, + PatKind::Struct(_, fields, _) => { + check_subpatterns( + cx, + "dereferencing a struct pattern where every field's pattern takes a reference", + ref_pat, + pat, + fields.iter().map(|field| field.pat), + ); + }, + _ => {}, } } - - // Only lint immutable refs, because `&mut ref T` may be useful. - let PatKind::Ref(pat, Mutability::Not) = ref_pat.kind else { - return; - }; - - match pat.kind { - // Check sub_pat got a `ref` keyword (excluding `ref mut`). - PatKind::Binding(BindingMode::REF, _, ident, None) => { - span_lint_and_then( - cx, - NEEDLESS_BORROWED_REFERENCE, - ref_pat.span, - "this pattern takes a reference on something that is being dereferenced", - |diag| { - // `&ref ident` - // ^^^^^ - let span = ref_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, - ) => { - check_subpatterns( - cx, - "dereferencing a slice pattern where every element takes a reference", - ref_pat, - pat, - itertools::chain(before, after), - ); - }, - PatKind::Tuple(subpatterns, _) | PatKind::TupleStruct(_, subpatterns, _) => { - check_subpatterns( - cx, - "dereferencing a tuple pattern where every element takes a reference", - ref_pat, - pat, - subpatterns, - ); - }, - PatKind::Struct(_, fields, _) => { - check_subpatterns( - cx, - "dereferencing a struct pattern where every field's pattern takes a reference", - ref_pat, - pat, - fields.iter().map(|field| field.pat), - ); - }, - _ => {}, - } } } diff --git a/clippy_lints/src/needless_for_each.rs b/clippy_lints/src/needless_for_each.rs index 143acc2b1cb76..6390e51f916b2 100644 --- a/clippy_lints/src/needless_for_each.rs +++ b/clippy_lints/src/needless_for_each.rs @@ -3,7 +3,7 @@ use rustc_hir::intravisit::{walk_expr, Visitor}; use rustc_hir::{Block, BlockCheckMode, Closure, Expr, ExprKind, Stmt, StmtKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::{sym, Span, Symbol}; +use rustc_span::{sym, Span}; use clippy_utils::diagnostics::span_lint_and_then; use clippy_utils::is_trait_method; @@ -55,16 +55,8 @@ declare_lint_pass!(NeedlessForEach => [NEEDLESS_FOR_EACH]); impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { - let (StmtKind::Expr(expr) | StmtKind::Semi(expr)) = stmt.kind else { - return; - }; - - if let ExprKind::MethodCall(method_name, for_each_recv, [for_each_arg], _) = expr.kind - // Check the method name is `for_each`. - && method_name.ident.name == Symbol::intern("for_each") - // Check `for_each` is an associated function of `Iterator`. - && is_trait_method(cx, expr, sym::Iterator) - // Checks the receiver of `for_each` is also a method call. + if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = stmt.kind + && let ExprKind::MethodCall(method_name, for_each_recv, [for_each_arg], _) = expr.kind && let ExprKind::MethodCall(_, iter_recv, [], _) = for_each_recv.kind // Skip the lint if the call chain is too long. e.g. `v.field.iter().for_each()` or // `v.foo().iter().for_each()` must be skipped. @@ -72,6 +64,8 @@ impl<'tcx> LateLintPass<'tcx> for NeedlessForEach { iter_recv.kind, ExprKind::Array(..) | ExprKind::Call(..) | ExprKind::Path(..) ) + && method_name.ident.name.as_str() == "for_each" + && is_trait_method(cx, expr, sym::Iterator) // Checks the type of the `iter` method receiver is NOT a user defined type. && has_iter_method(cx, cx.typeck_results().expr_ty(iter_recv)).is_some() // Skip the lint if the body is not block because this is simpler than `for` loop. diff --git a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs index f7621822b66c4..fa90ee606121b 100644 --- a/clippy_lints/src/neg_cmp_op_on_partial_ord.rs +++ b/clippy_lints/src/neg_cmp_op_on_partial_ord.rs @@ -45,10 +45,10 @@ declare_lint_pass!(NoNegCompOpForPartialOrd => [NEG_CMP_OP_ON_PARTIAL_ORD]); impl<'tcx> LateLintPass<'tcx> for NoNegCompOpForPartialOrd { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { - if !in_external_macro(cx.sess(), expr.span) - && let ExprKind::Unary(UnOp::Not, inner) = expr.kind + if let ExprKind::Unary(UnOp::Not, inner) = expr.kind && let ExprKind::Binary(ref op, left, _) = inner.kind && let BinOpKind::Le | BinOpKind::Ge | BinOpKind::Lt | BinOpKind::Gt = op.node + && !in_external_macro(cx.sess(), expr.span) { let ty = cx.typeck_results().expr_ty(left); diff --git a/clippy_lints/src/option_env_unwrap.rs b/clippy_lints/src/option_env_unwrap.rs index 4bfb26209d21d..d16f5f8e112c3 100644 --- a/clippy_lints/src/option_env_unwrap.rs +++ b/clippy_lints/src/option_env_unwrap.rs @@ -3,7 +3,7 @@ use clippy_utils::is_direct_expn_of; use rustc_ast::ast::{Expr, ExprKind, MethodCall}; use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_session::declare_lint_pass; -use rustc_span::{sym, Span}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -35,30 +35,18 @@ declare_lint_pass!(OptionEnvUnwrap => [OPTION_ENV_UNWRAP]); impl EarlyLintPass for OptionEnvUnwrap { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - fn lint(cx: &EarlyContext<'_>, span: Span) { + if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind + && matches!(seg.ident.name, sym::expect | sym::unwrap) + && is_direct_expn_of(receiver.span, "option_env").is_some() + { span_lint_and_help( cx, OPTION_ENV_UNWRAP, - span, + expr.span, "this will panic at run-time if the environment variable doesn't exist at compile-time", None, "consider using the `env!` macro instead", ); } - - if let ExprKind::MethodCall(box MethodCall { seg, receiver, .. }) = &expr.kind - && matches!(seg.ident.name, sym::expect | sym::unwrap) - { - if let ExprKind::Call(caller, _) = &receiver.kind && - // If it exists, it will be ::core::option::Option::Some("").unwrap() (A method call in the HIR) - is_direct_expn_of(caller.span, "option_env").is_some() - { - lint(cx, expr.span); - } else if let ExprKind::Path(_, caller) = &receiver.kind && // If it doesn't exist, it will be ::core::option::Option::None::<&'static str>.unwrap() (A path in the HIR) - is_direct_expn_of(caller.span, "option_env").is_some() - { - lint(cx, expr.span); - } - } } } diff --git a/clippy_lints/src/permissions_set_readonly_false.rs b/clippy_lints/src/permissions_set_readonly_false.rs index 704acdc103e84..dc142b6e15771 100644 --- a/clippy_lints/src/permissions_set_readonly_false.rs +++ b/clippy_lints/src/permissions_set_readonly_false.rs @@ -31,10 +31,10 @@ declare_lint_pass!(PermissionsSetReadonlyFalse => [PERMISSIONS_SET_READONLY_FALS impl<'tcx> LateLintPass<'tcx> for PermissionsSetReadonlyFalse { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { if let ExprKind::MethodCall(path, receiver, [arg], _) = &expr.kind - && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::FsPermissions) - && path.ident.name == sym!(set_readonly) && let ExprKind::Lit(lit) = &arg.kind && LitKind::Bool(false) == lit.node + && path.ident.name.as_str() == "set_readonly" + && is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(receiver), sym::FsPermissions) { span_lint_and_then( cx, diff --git a/clippy_lints/src/raw_strings.rs b/clippy_lints/src/raw_strings.rs index 9564fdbeae13a..3c19ee3522d08 100644 --- a/clippy_lints/src/raw_strings.rs +++ b/clippy_lints/src/raw_strings.rs @@ -1,6 +1,6 @@ use clippy_config::Conf; use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet; +use clippy_utils::source::SpanRangeExt; use rustc_ast::ast::{Expr, ExprKind}; use rustc_ast::token::LitKind; use rustc_errors::Applicability; @@ -71,20 +71,17 @@ impl RawStrings { impl EarlyLintPass for RawStrings { fn check_expr(&mut self, cx: &EarlyContext<'_>, expr: &Expr) { - if !in_external_macro(cx.sess(), expr.span) - && let ExprKind::Lit(lit) = expr.kind - && let LitKind::StrRaw(max) | LitKind::ByteStrRaw(max) | LitKind::CStrRaw(max) = lit.kind + if let ExprKind::Lit(lit) = expr.kind + && let (prefix, max) = match lit.kind { + LitKind::StrRaw(max) => ("r", max), + LitKind::ByteStrRaw(max) => ("br", max), + LitKind::CStrRaw(max) => ("cr", max), + _ => return, + } + && !in_external_macro(cx.sess(), expr.span) + && expr.span.check_source_text(cx, |src| src.starts_with(prefix)) { let str = lit.symbol.as_str(); - let prefix = match lit.kind { - LitKind::StrRaw(..) => "r", - LitKind::ByteStrRaw(..) => "br", - LitKind::CStrRaw(..) => "cr", - _ => unreachable!(), - }; - if !snippet(cx, expr.span, prefix).trim().starts_with(prefix) { - return; - } let descr = lit.kind.descr(); if !str.contains(['\\', '"']) { @@ -94,7 +91,7 @@ impl EarlyLintPass for RawStrings { expr.span, "unnecessary raw string literal", |diag| { - let (start, end) = hash_spans(expr.span, prefix, 0, max); + let (start, end) = hash_spans(expr.span, prefix.len(), 0, max); // BytePos: skip over the `b` in `br`, we checked the prefix appears in the source text let r_pos = expr.span.lo() + BytePos::from_usize(prefix.len() - 1); @@ -156,7 +153,7 @@ impl EarlyLintPass for RawStrings { expr.span, "unnecessary hashes around raw string literal", |diag| { - let (start, end) = hash_spans(expr.span, prefix, req, max); + let (start, end) = hash_spans(expr.span, prefix.len(), req, max); let message = match max - req { _ if req == 0 => format!("remove all the hashes around the {descr} literal"), @@ -182,11 +179,11 @@ impl EarlyLintPass for RawStrings { /// r###".."### /// ^^ ^^ /// ``` -fn hash_spans(literal_span: Span, prefix: &str, req: u8, max: u8) -> (Span, Span) { +fn hash_spans(literal_span: Span, prefix_len: usize, req: u8, max: u8) -> (Span, Span) { let literal_span = literal_span.data(); // BytePos: we checked prefix appears literally in the source text - let hash_start = literal_span.lo + BytePos::from_usize(prefix.len()); + let hash_start = literal_span.lo + BytePos::from_usize(prefix_len); let hash_end = literal_span.hi; // BytePos: req/max are counts of the ASCII character # diff --git a/tests/ui/toplevel_ref_arg.fixed b/tests/ui/toplevel_ref_arg.fixed index ff5cd7abbb69e..3eb47a5b5fd65 100644 --- a/tests/ui/toplevel_ref_arg.fixed +++ b/tests/ui/toplevel_ref_arg.fixed @@ -36,4 +36,6 @@ fn main() { // do not lint in external macro external!(let ref _y = 42;); + + fn f(#[allow(clippy::toplevel_ref_arg)] ref x: i32) {} } diff --git a/tests/ui/toplevel_ref_arg.rs b/tests/ui/toplevel_ref_arg.rs index ab79b8959605f..cd731387de976 100644 --- a/tests/ui/toplevel_ref_arg.rs +++ b/tests/ui/toplevel_ref_arg.rs @@ -36,4 +36,6 @@ fn main() { // do not lint in external macro external!(let ref _y = 42;); + + fn f(#[allow(clippy::toplevel_ref_arg)] ref x: i32) {} }