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
This commit is contained in:
bors 2024-03-09 17:01:03 +00:00
commit c173ea64b7
15 changed files with 136 additions and 54 deletions

View file

@ -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| {

View file

@ -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",
);

View file

@ -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,36 +77,52 @@ 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(
cx,
READ_ZERO_BYTE_VEC,
expr.span,
"reading zero byte data to `Vec`",
"try",
format!("{}.resize({len}, 0); {}", ident.as_str(), snippet(cx, expr.span, "..")),
applicability,
);
},
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_and_sugg(
span_lint_hir_and_then(
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,
|diag| {
diag.span_suggestion(
expr.span,
"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`");
span_lint_hir(
cx,
READ_ZERO_BYTE_VEC,
expr.hir_id,
expr.span,
"reading zero byte data to `Vec`",
);
},
}
}

View file

@ -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",
);

View file

@ -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 {

View file

@ -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);
}

View file

@ -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`");
},
);
}
}

View file

@ -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();

View file

@ -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 | |

View file

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

View file

@ -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() {}

View file

@ -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() {}

View file

@ -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() {}

View file

@ -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() {}

View file

@ -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;
}