From 32d3387c80b59ad85756aa66237aa6d2a0eca884 Mon Sep 17 00:00:00 2001
From: Alex Macleod <alex@macleod.io>
Date: Sun, 17 Sep 2023 14:33:06 +0000
Subject: [PATCH] used_underscore_bindings: respect lint levels on the binding
definition
---
clippy_lints/src/misc.rs | 104 +++++++++++-------------
clippy_utils/src/lib.rs | 27 ++++++
tests/ui/used_underscore_binding.rs | 28 ++++++-
tests/ui/used_underscore_binding.stderr | 47 +++++++++--
4 files changed, 140 insertions(+), 66 deletions(-)
diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs
index 303f0125690..f8e76a5833e 100644
--- a/clippy_lints/src/misc.rs
+++ b/clippy_lints/src/misc.rs
@@ -1,24 +1,23 @@
-use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_hir_and_then};
+use clippy_utils::diagnostics::{span_lint, span_lint_and_sugg, span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::{snippet, snippet_opt, snippet_with_context};
+use clippy_utils::sugg::Sugg;
+use clippy_utils::{
+ any_parent_is_automatically_derived, fulfill_or_allowed, get_parent_expr, in_constant, is_integer_literal,
+ is_lint_allowed, is_no_std_crate, iter_input_pats, last_path_segment, SpanlessEq,
+};
use if_chain::if_chain;
use rustc_errors::Applicability;
+use rustc_hir::def::Res;
use rustc_hir::intravisit::FnKind;
use rustc_hir::{
- self as hir, def, BinOpKind, BindingAnnotation, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, Stmt,
- StmtKind, TyKind,
+ BinOpKind, BindingAnnotation, Body, ByRef, Expr, ExprKind, FnDecl, Mutability, PatKind, QPath, Stmt, StmtKind, Ty,
+ TyKind,
};
-use rustc_lint::{LateContext, LateLintPass};
+use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::def_id::LocalDefId;
-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, is_integer_literal, is_lint_allowed, is_no_std_crate, iter_input_pats,
- last_path_segment, SpanlessEq,
-};
+use rustc_span::source_map::Span;
use crate::ref_patterns::REF_PATTERNS;
@@ -257,46 +256,56 @@ impl<'tcx> LateLintPass<'tcx> for LintPass {
self.check_cast(cx, expr.span, e, ty);
return;
}
- if in_attributes_expansion(expr) || expr.span.is_desugaring(DesugaringKind::Await) {
- // Don't lint things expanded by #[derive(...)], etc or `await` desugaring
+ if in_external_macro(cx.sess(), expr.span)
+ || expr.span.desugaring_kind().is_some()
+ || any_parent_is_automatically_derived(cx.tcx, expr.hir_id)
+ {
return;
}
- let sym;
- let binding = match expr.kind {
- ExprKind::Path(ref qpath) if !matches!(qpath, hir::QPath::LangItem(..)) => {
- let binding = last_path_segment(qpath).ident.as_str();
- if binding.starts_with('_') &&
- !binding.starts_with("__") &&
- binding != "_result" && // FIXME: #944
- is_used(cx, expr) &&
- // don't lint if the declaration is in a macro
- non_macro_local(cx, cx.qpath_res(qpath, expr.hir_id))
+ let (definition_hir_id, ident) = match expr.kind {
+ ExprKind::Path(ref qpath) => {
+ if let QPath::Resolved(None, path) = qpath
+ && let Res::Local(id) = path.res
+ && is_used(cx, expr)
{
- Some(binding)
+ (id, last_path_segment(qpath).ident)
} else {
- None
+ return;
}
},
- ExprKind::Field(_, ident) => {
- sym = ident.name;
- let name = sym.as_str();
- if name.starts_with('_') && !name.starts_with("__") {
- Some(name)
+ ExprKind::Field(recv, ident) => {
+ if let Some(adt_def) = cx.typeck_results().expr_ty_adjusted(recv).ty_adt_def()
+ && let Some(field) = adt_def.all_fields().find(|field| field.name == ident.name)
+ && let Some(local_did) = field.did.as_local()
+ && let Some(hir_id) = cx.tcx.opt_local_def_id_to_hir_id(local_did)
+ && !cx.tcx.type_of(field.did).skip_binder().is_phantom_data()
+ {
+ (hir_id, ident)
} else {
- None
+ return;
}
},
- _ => None,
+ _ => return,
};
- if let Some(binding) = binding {
- span_lint(
+
+ let name = ident.name.as_str();
+ if name.starts_with('_')
+ && !name.starts_with("__")
+ && let definition_span = cx.tcx.hir().span(definition_hir_id)
+ && !definition_span.from_expansion()
+ && !fulfill_or_allowed(cx, USED_UNDERSCORE_BINDING, [expr.hir_id, definition_hir_id])
+ {
+ span_lint_and_then(
cx,
USED_UNDERSCORE_BINDING,
expr.span,
&format!(
- "used binding `{binding}` which is prefixed with an underscore. A leading \
+ "used binding `{name}` which is prefixed with an underscore. A leading \
underscore signals that a binding will not be used"
),
+ |diag| {
+ diag.span_note(definition_span, format!("`{name}` is defined here"));
+ }
);
}
}
@@ -312,29 +321,8 @@ fn is_used(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
})
}
-/// Tests whether an expression is in a macro expansion (e.g., something
-/// generated by `#[derive(...)]` or the like).
-fn in_attributes_expansion(expr: &Expr<'_>) -> bool {
- use rustc_span::hygiene::MacroKind;
- if expr.span.from_expansion() {
- let data = expr.span.ctxt().outer_expn_data();
- matches!(data.kind, ExpnKind::Macro(MacroKind::Attr | MacroKind::Derive, _))
- } else {
- false
- }
-}
-
-/// Tests whether `res` is a variable defined outside a macro.
-fn non_macro_local(cx: &LateContext<'_>, res: def::Res) -> bool {
- if let def::Res::Local(id) = res {
- !cx.tcx.hir().span(id).from_expansion()
- } else {
- false
- }
-}
-
impl LintPass {
- fn check_cast(&self, cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &hir::Ty<'_>) {
+ fn check_cast(&self, cx: &LateContext<'_>, span: Span, e: &Expr<'_>, ty: &Ty<'_>) {
if_chain! {
if let TyKind::Ptr(ref mut_ty) = ty.kind;
if is_integer_literal(e, 0);
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 4ef3ec19647..eb5baa98f62 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -1785,6 +1785,33 @@ pub fn is_try<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<&'tc
None
}
+/// Returns `true` if the lint is `#[allow]`ed or `#[expect]`ed at any of the `ids`, fulfilling all
+/// of the expectations in `ids`
+///
+/// This should only be used when the lint would otherwise be emitted, for a way to check if a lint
+/// is allowed early to skip work see [`is_lint_allowed`]
+///
+/// To emit at a lint at a different context than the one current see
+/// [`span_lint_hir`](diagnostics::span_lint_hir) or
+/// [`span_lint_hir_and_then`](diagnostics::span_lint_hir_and_then)
+pub fn fulfill_or_allowed(cx: &LateContext<'_>, lint: &'static Lint, ids: impl IntoIterator<Item = HirId>) -> bool {
+ let mut suppress_lint = false;
+
+ for id in ids {
+ let (level, _) = cx.tcx.lint_level_at_node(lint, id);
+ if let Some(expectation) = level.get_expectation_id() {
+ cx.fulfill_expectation(expectation);
+ }
+
+ match level {
+ Level::Allow | Level::Expect(_) => suppress_lint = true,
+ Level::Warn | Level::ForceWarn(_) | Level::Deny | Level::Forbid => {},
+ }
+ }
+
+ suppress_lint
+}
+
/// Returns `true` if the lint is allowed in the current context. This is useful for
/// skipping long running code when it's unnecessary
///
diff --git a/tests/ui/used_underscore_binding.rs b/tests/ui/used_underscore_binding.rs
index c672eff1c27..a8f404b1400 100644
--- a/tests/ui/used_underscore_binding.rs
+++ b/tests/ui/used_underscore_binding.rs
@@ -1,6 +1,5 @@
//@aux-build:proc_macro_derive.rs
-#![feature(rustc_private)]
-#![warn(clippy::all)]
+#![feature(rustc_private, lint_reasons)]
#![warn(clippy::used_underscore_binding)]
#![allow(clippy::disallowed_names, clippy::eq_op, clippy::uninlined_format_args)]
@@ -107,6 +106,31 @@ async fn await_desugaring() {
.await
}
+struct PhantomField<T> {
+ _marker: std::marker::PhantomData<T>,
+}
+
+impl<T> std::fmt::Debug for PhantomField<T> {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ f.debug_struct("PhantomField").field("_marker", &self._marker).finish()
+ }
+}
+
+struct AllowedField {
+ #[allow(clippy::used_underscore_binding)]
+ _allowed: usize,
+}
+
+struct ExpectedField {
+ #[expect(clippy::used_underscore_binding)]
+ _expected: usize,
+}
+
+fn lint_levels(allowed: AllowedField, expected: ExpectedField) {
+ let _ = allowed._allowed;
+ let _ = expected._expected;
+}
+
fn main() {
let foo = 0u32;
// tests of unused_underscore lint
diff --git a/tests/ui/used_underscore_binding.stderr b/tests/ui/used_underscore_binding.stderr
index 289519b172e..78d8279810c 100644
--- a/tests/ui/used_underscore_binding.stderr
+++ b/tests/ui/used_underscore_binding.stderr
@@ -1,41 +1,76 @@
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:24:5
+ --> $DIR/used_underscore_binding.rs:23:5
|
LL | _foo + 1
| ^^^^
|
+note: `_foo` is defined here
+ --> $DIR/used_underscore_binding.rs:22:22
+ |
+LL | fn prefix_underscore(_foo: u32) -> u32 {
+ | ^^^^
= note: `-D clippy::used-underscore-binding` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::used_underscore_binding)]`
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:29:20
+ --> $DIR/used_underscore_binding.rs:28:20
|
LL | println!("{}", _foo);
| ^^^^
+ |
+note: `_foo` is defined here
+ --> $DIR/used_underscore_binding.rs:27:24
+ |
+LL | fn in_macro_or_desugar(_foo: u32) {
+ | ^^^^
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:30:16
+ --> $DIR/used_underscore_binding.rs:29:16
|
LL | assert_eq!(_foo, _foo);
| ^^^^
+ |
+note: `_foo` is defined here
+ --> $DIR/used_underscore_binding.rs:27:24
+ |
+LL | fn in_macro_or_desugar(_foo: u32) {
+ | ^^^^
error: used binding `_foo` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:30:22
+ --> $DIR/used_underscore_binding.rs:29:22
|
LL | assert_eq!(_foo, _foo);
| ^^^^
+ |
+note: `_foo` is defined here
+ --> $DIR/used_underscore_binding.rs:27:24
+ |
+LL | fn in_macro_or_desugar(_foo: u32) {
+ | ^^^^
error: used binding `_underscore_field` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:43:5
+ --> $DIR/used_underscore_binding.rs:42:5
|
LL | s._underscore_field += 1;
| ^^^^^^^^^^^^^^^^^^^
+ |
+note: `_underscore_field` is defined here
+ --> $DIR/used_underscore_binding.rs:36:5
+ |
+LL | _underscore_field: u32,
+ | ^^^^^^^^^^^^^^^^^^^^^^
error: used binding `_i` which is prefixed with an underscore. A leading underscore signals that a binding will not be used
- --> $DIR/used_underscore_binding.rs:104:16
+ --> $DIR/used_underscore_binding.rs:103:16
|
LL | uses_i(_i);
| ^^
+ |
+note: `_i` is defined here
+ --> $DIR/used_underscore_binding.rs:102:13
+ |
+LL | let _i = 5;
+ | ^^
error: aborting due to 6 previous errors