Auto merge of #12446 - y21:check_fn_span_lint, r=xFrednet

use `span_lint_hir` instead of `span_lint` in more lints

Decided to grep for `check_(fn|block)` and look where `span_lint` is used, since some lints lint will then emit a lint on a statement or expression in that function, which would use the wrong lint level attributes

The `LintContext` keeps track of the last entered HIR node that had any attributes, and uses those (and its parents) for figuring out the lint level when a lint is emitted

However, this only works when we actually emit a lint at the same node as the `check_*` function we are in.
If we're in `check_fn` and we emit a lint on a statement within that function, then there is no way to allow the lint only for that one statement (if `span_lint` is used). It would only count allow attributes on the function

changelog: [`needless_return`]: [`useless_let_if_seq`]: [`mut_mut`]: [`read_zero_byte_vec`]: [`unused_io_amount`]: [`unused_peekable`]: now respects `#[allow]` attributes on the affected statement instead of only on the enclosing block or function

Author: bors

clippy_lints/src/let_if_seq.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::path_to_local_id;
 use clippy_utils::source::snippet;
 use clippy_utils::visitors::is_local_used;
@@ -122,9 +122,10 @@ impl<'tcx> LateLintPass<'tcx> for LetIfSeq {
                     value=snippet(cx, value.span, "<value>"),
                     default=snippet(cx, default.span, "<default>"),
                 );
-                span_lint_and_then(
+                span_lint_hir_and_then(
                     cx,
                     USELESS_LET_IF_SEQ,
+                    local.hir_id,
                     span,
                     "`if _ { .. } else { .. }` is an expression",
                     |diag| {

clippy_lints/src/mut_mut.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint;
+use clippy_utils::diagnostics::{span_lint, span_lint_hir};
 use clippy_utils::higher;
 use rustc_hir as hir;
 use rustc_hir::intravisit;
@@ -87,17 +87,19 @@ impl<'a, 'tcx> intravisit::Visitor<'tcx> for MutVisitor<'a, 'tcx> {
             intravisit::walk_expr(self, body);
         } else if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Mut, e) = expr.kind {
             if let hir::ExprKind::AddrOf(hir::BorrowKind::Ref, hir::Mutability::Mut, _) = e.kind {
-                span_lint(
+                span_lint_hir(
                     self.cx,
                     MUT_MUT,
+                    expr.hir_id,
                     expr.span,
                     "generally you want to avoid `&mut &mut _` if possible",
                 );
             } else if let ty::Ref(_, ty, hir::Mutability::Mut) = self.cx.typeck_results().expr_ty(e).kind() {
                 if ty.peel_refs().is_sized(self.cx.tcx, self.cx.param_env) {
-                    span_lint(
+                    span_lint_hir(
                         self.cx,
                         MUT_MUT,
+                        expr.hir_id,
                         expr.span,
                         "this expression mutably borrows a mutable reference. Consider reborrowing",
                     );

clippy_lints/src/read_zero_byte_vec.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg};
+use clippy_utils::diagnostics::{span_lint_hir, span_lint_hir_and_then};
 use clippy_utils::get_enclosing_block;
 use clippy_utils::higher::{get_vec_init_kind, VecInitKind};
 use clippy_utils::source::snippet;
@@ -77,37 +77,53 @@ impl<'tcx> LateLintPass<'tcx> for ReadZeroByteVec {
                 if let Some(expr) = visitor.read_zero_expr {
                     let applicability = Applicability::MaybeIncorrect;
                     match vec_init_kind {
-                        VecInitKind::WithConstCapacity(len) => {
-                            span_lint_and_sugg(
+                        VecInitKind::WithConstCapacity(len) => span_lint_hir_and_then(
+                            cx,
+                            READ_ZERO_BYTE_VEC,
+                            expr.hir_id,
+                            expr.span,
+                            "reading zero byte data to `Vec`",
+                            |diag| {
+                                diag.span_suggestion(
+                                    expr.span,
+                                    "try",
+                                    format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")),
+                                    applicability,
+                                );
+                            },
+                        ),
+                        VecInitKind::WithExprCapacity(hir_id) => {
+                            let e = cx.tcx.hir().expect_expr(hir_id);
+                            span_lint_hir_and_then(
                                 cx,
                                 READ_ZERO_BYTE_VEC,
+                                expr.hir_id,
                                 expr.span,
                                 "reading zero byte data to `Vec`",
-                                "try",
-                                format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")),
-                                applicability,
+                                |diag| {
+                                    diag.span_suggestion(
+                                        expr.span,
+                                        "try",
+                                        format!(
+                                            "{}.resize({}, 0); {}",
+                                            ident.as_str(),
+                                            snippet(cx, e.span, ".."),
+                                            snippet(cx, expr.span, "..")
+                                        ),
+                                        applicability,
+                                    );
+                                },
                             );
                         },
-                        VecInitKind::WithExprCapacity(hir_id) => {
-                            let e = cx.tcx.hir().expect_expr(hir_id);
-                            span_lint_and_sugg(
+                        _ => {
+                            span_lint_hir(
                                 cx,
                                 READ_ZERO_BYTE_VEC,
+                                expr.hir_id,
                                 expr.span,
                                 "reading zero byte data to `Vec`",
-                                "try",
-                                format!(
-                                    "{}.resize({}, 0); {}",
-                                    ident.as_str(),
-                                    snippet(cx, e.span, ".."),
-                                    snippet(cx, expr.span, "..")
-                                ),
-                                applicability,
                             );
                         },
-                        _ => {
-                            span_lint(cx, READ_ZERO_BYTE_VEC, expr.span, "reading zero byte data to `Vec`");
-                        },
                     }
                 }
             }

clippy_lints/src/redundant_closure_call.rs

@@ -1,5 +1,5 @@
 use crate::rustc_lint::LintContext;
-use clippy_utils::diagnostics::{span_lint, span_lint_and_then};
+use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir};
 use clippy_utils::get_parent_expr;
 use clippy_utils::sugg::Sugg;
 use hir::Param;
@@ -273,9 +273,10 @@ impl<'tcx> LateLintPass<'tcx> for RedundantClosureCall {
                 && ident == path.segments[0].ident
                 && count_closure_usage(cx, block, path) == 1
             {
-                span_lint(
+                span_lint_hir(
                     cx,
                     REDUNDANT_CLOSURE_CALL,
+                    second.hir_id,
                     second.span,
                     "closure called just once immediately after it was declared",
                 );

clippy_lints/src/returns.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
+use clippy_utils::diagnostics::{span_lint_and_sugg, span_lint_hir_and_then};
 use clippy_utils::source::{snippet_opt, snippet_with_context};
 use clippy_utils::sugg::has_enclosing_paren;
 use clippy_utils::visitors::{for_each_expr_with_closures, Descend};
@@ -380,7 +380,7 @@ fn check_final_expr<'tcx>(
                 return;
             }
 
-            emit_return_lint(cx, ret_span, semi_spans, &replacement);
+            emit_return_lint(cx, ret_span, semi_spans, &replacement, expr.hir_id);
         },
         ExprKind::If(_, then, else_clause_opt) => {
             check_block_return(cx, &then.kind, peeled_drop_expr.span, semi_spans.clone());
@@ -415,18 +415,31 @@ fn expr_contains_conjunctive_ifs<'tcx>(expr: &'tcx Expr<'tcx>) -> bool {
     contains_if(expr, false)
 }
 
-fn emit_return_lint(cx: &LateContext<'_>, ret_span: Span, semi_spans: Vec<Span>, replacement: &RetReplacement<'_>) {
+fn emit_return_lint(
+    cx: &LateContext<'_>,
+    ret_span: Span,
+    semi_spans: Vec<Span>,
+    replacement: &RetReplacement<'_>,
+    at: HirId,
+) {
     if ret_span.from_expansion() {
         return;
     }
 
-    span_lint_and_then(cx, NEEDLESS_RETURN, ret_span, "unneeded `return` statement", |diag| {
-        let suggestions = std::iter::once((ret_span, replacement.to_string()))
-            .chain(semi_spans.into_iter().map(|span| (span, String::new())))
-            .collect();
+    span_lint_hir_and_then(
+        cx,
+        NEEDLESS_RETURN,
+        at,
+        ret_span,
+        "unneeded `return` statement",
+        |diag| {
+            let suggestions = std::iter::once((ret_span, replacement.to_string()))
+                .chain(semi_spans.into_iter().map(|span| (span, String::new())))
+                .collect();
 
-        diag.multipart_suggestion_verbose(replacement.sugg_help(), suggestions, replacement.applicability());
-    });
+            diag.multipart_suggestion_verbose(replacement.sugg_help(), suggestions, replacement.applicability());
+        },
+    );
 }
 
 fn last_statement_borrows<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {

clippy_lints/src/unused_io_amount.rs

@@ -1,7 +1,7 @@
-use clippy_utils::diagnostics::span_lint_and_then;
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::macros::{is_panic, root_macro_call_first_node};
 use clippy_utils::{is_res_lang_ctor, is_trait_method, match_trait_method, paths, peel_blocks};
-use hir::{ExprKind, PatKind};
+use hir::{ExprKind, HirId, PatKind};
 use rustc_hir as hir;
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_session::declare_lint_pass;
@@ -135,22 +135,22 @@ fn check_expr<'a>(cx: &LateContext<'a>, expr: &'a hir::Expr<'a>) {
                 && is_ok_wild_or_dotdot_pattern(cx, pat)
                 && let Some(op) = should_lint(cx, init) =>
         {
-            emit_lint(cx, cond.span, op, &[pat.span]);
+            emit_lint(cx, cond.span, cond.hir_id, op, &[pat.span]);
         },
         // we will capture only the case where the match is Ok( ) or Err( )
         // prefer to match the minimum possible, and expand later if needed
         // to avoid false positives on something as used as this
         hir::ExprKind::Match(expr, [arm1, arm2], hir::MatchSource::Normal) if let Some(op) = should_lint(cx, expr) => {
             if non_consuming_ok_arm(cx, arm1) && non_consuming_err_arm(cx, arm2) {
-                emit_lint(cx, expr.span, op, &[arm1.pat.span]);
+                emit_lint(cx, expr.span, expr.hir_id, op, &[arm1.pat.span]);
             }
             if non_consuming_ok_arm(cx, arm2) && non_consuming_err_arm(cx, arm1) {
-                emit_lint(cx, expr.span, op, &[arm2.pat.span]);
+                emit_lint(cx, expr.span, expr.hir_id, op, &[arm2.pat.span]);
             }
         },
         hir::ExprKind::Match(_, _, hir::MatchSource::Normal) => {},
         _ if let Some(op) = should_lint(cx, expr) => {
-            emit_lint(cx, expr.span, op, &[]);
+            emit_lint(cx, expr.span, expr.hir_id, op, &[]);
         },
         _ => {},
     };
@@ -279,7 +279,7 @@ fn check_io_mode(cx: &LateContext<'_>, call: &hir::Expr<'_>) -> Option<IoOp> {
     }
 }
 
-fn emit_lint(cx: &LateContext<'_>, span: Span, op: IoOp, wild_cards: &[Span]) {
+fn emit_lint(cx: &LateContext<'_>, span: Span, at: HirId, op: IoOp, wild_cards: &[Span]) {
     let (msg, help) = match op {
         IoOp::AsyncRead(false) => (
             "read amount is not handled",
@@ -301,7 +301,7 @@ fn emit_lint(cx: &LateContext<'_>, span: Span, op: IoOp, wild_cards: &[Span]) {
         IoOp::SyncWrite(true) | IoOp::AsyncWrite(true) => ("written amount is not handled", None),
     };
 
-    span_lint_and_then(cx, UNUSED_IO_AMOUNT, span, msg, |diag| {
+    span_lint_hir_and_then(cx, UNUSED_IO_AMOUNT, at, span, msg, |diag| {
         if let Some(help_str) = help {
             diag.help(help_str);
         }

clippy_lints/src/unused_peekable.rs

@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_help;
+use clippy_utils::diagnostics::span_lint_hir_and_then;
 use clippy_utils::ty::{is_type_diagnostic_item, peel_mid_ty_refs_is_mutable};
 use clippy_utils::{fn_def_id, is_trait_method, path_to_local_id, peel_ref_operators};
 use rustc_ast::Mutability;
@@ -79,13 +79,15 @@ impl<'tcx> LateLintPass<'tcx> for UnusedPeekable {
                 }
 
                 if !vis.found_peek_call {
-                    span_lint_and_help(
+                    span_lint_hir_and_then(
                         cx,
                         UNUSED_PEEKABLE,
+                        local.hir_id,
                         ident.span,
                         "`peek` never called on `Peekable` iterator",
-                        None,
-                        "consider removing the call to `peekable`",
+                        |diag| {
+                            diag.help("consider removing the call to `peekable`");
+                        },
                     );
                 }
             }

tests/ui/let_if_seq.rs

@@ -57,6 +57,17 @@ fn early_return() -> u8 {
     foo
 }
 
+fn allow_works() -> i32 {
+    #[allow(clippy::useless_let_if_seq)]
+    let x;
+    if true {
+        x = 1;
+    } else {
+        x = 2;
+    }
+    x
+}
+
 fn main() {
     early_return();
     issue975();

tests/ui/let_if_seq.stderr

@@ -1,5 +1,5 @@
 error: `if _ { .. } else { .. }` is an expression
-  --> tests/ui/let_if_seq.rs:66:5
+  --> tests/ui/let_if_seq.rs:77:5
    |
 LL | /     let mut foo = 0;
 LL | |
@@ -14,7 +14,7 @@ LL | |     }
    = help: to override `-D warnings` add `#[allow(clippy::useless_let_if_seq)]`
 
 error: `if _ { .. } else { .. }` is an expression
-  --> tests/ui/let_if_seq.rs:73:5
+  --> tests/ui/let_if_seq.rs:84:5
    |
 LL | /     let mut bar = 0;
 LL | |
@@ -28,7 +28,7 @@ LL | |     }
    = note: you might not need `mut` at all
 
 error: `if _ { .. } else { .. }` is an expression
-  --> tests/ui/let_if_seq.rs:83:5
+  --> tests/ui/let_if_seq.rs:94:5
    |
 LL | /     let quz;
 LL | |
@@ -40,7 +40,7 @@ LL | |     }
    | |_____^ help: it is more idiomatic to write: `let quz = if f() { 42 } else { 0 };`
 
 error: `if _ { .. } else { .. }` is an expression
-  --> tests/ui/let_if_seq.rs:113:5
+  --> tests/ui/let_if_seq.rs:124:5
    |
 LL | /     let mut baz = 0;
 LL | |

tests/ui/mut_mut.rs

@@ -81,3 +81,8 @@ mod issue9035 {
 
     fn bar(_: &mut impl Display) {}
 }
+
+fn allow_works() {
+    #[allow(clippy::mut_mut)]
+    let _ = &mut &mut 1;
+}

tests/ui/needless_return.fixed

@@ -316,4 +316,11 @@ fn test_match_as_stmt() {
     };
 }
 
+fn allow_works() -> i32 {
+    #[allow(clippy::needless_return, clippy::match_single_binding)]
+    match () {
+        () => return 42,
+    }
+}
+
 fn main() {}

tests/ui/needless_return.rs

@@ -326,4 +326,11 @@ fn test_match_as_stmt() {
     };
 }
 
+fn allow_works() -> i32 {
+    #[allow(clippy::needless_return, clippy::match_single_binding)]
+    match () {
+        () => return 42,
+    }
+}
+
 fn main() {}

tests/ui/read_zero_byte_vec.rs

@@ -112,4 +112,10 @@ async fn test_tokio<R: TokioAsyncRead + Unpin>(r: &mut R) {
     //~^ ERROR: reading zero byte data to `Vec`
 }
 
+fn allow_works<F: std::io::Read>(mut f: F) {
+    let mut data = Vec::with_capacity(100);
+    #[allow(clippy::read_zero_byte_vec)]
+    f.read(&mut data).unwrap();
+}
+
 fn main() {}

tests/ui/unused_io_amount.rs

@@ -271,5 +271,10 @@ pub fn wildcards(rdr: &mut dyn std::io::Read) {
         }
     }
 }
+fn allow_works<F: std::io::Read>(mut f: F) {
+    let mut data = Vec::with_capacity(100);
+    #[allow(clippy::unused_io_amount)]
+    f.read(&mut data).unwrap();
+}
 
 fn main() {}

tests/ui/unused_peekable.rs

@@ -174,3 +174,9 @@ fn valid() {
     let mut peekable = std::iter::empty::<u32>().peekable();
     takes_dyn(&mut peekable);
 }
+
+fn allow_works() {
+    #[allow(clippy::unused_peekable)]
+    let iter = [1, 2, 3].iter().peekable();
+    iter;
+}