Skip to content

Commit bcf6667

Browse files
authored
Use #[must_use] determination from the compiler (#16633)
*[View all comments](https://triagebot.infra.rust-lang.org/gh-comments/rust-lang/rust-clippy/pull/16633)* changelog: [`double_must_use`, `let_underscore_must_use`, `must_use_candidates`]: better determination of `#[must_use]` needs by directly using the compiler algorithm For example, `Box<T>` is considered `#[must_use]` when `T` is.
2 parents 9b14e55 + 8197719 commit bcf6667

13 files changed

Lines changed: 257 additions & 102 deletions

clippy_lints/src/drop_forget_ref.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::is_must_use_func_call;
33
use clippy_utils::res::MaybeDef as _;
4-
use clippy_utils::ty::{is_copy, is_must_use_ty};
4+
use clippy_utils::ty::{is_copy, opt_must_use_path};
55
use rustc_hir::{Arm, Expr, ExprKind, LangItem, Node};
66
use rustc_lint::{LateContext, LateLintPass};
77
use rustc_session::declare_lint_pass;
@@ -98,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
9898
sym::mem_drop
9999
if !(arg_ty.needs_drop(cx.tcx, cx.typing_env())
100100
|| is_must_use_func_call(cx, arg)
101-
|| is_must_use_ty(cx, arg_ty)
101+
|| opt_must_use_path(cx, arg_ty).is_some()
102102
|| drop_is_single_call_in_arm) =>
103103
{
104104
(DROP_NON_DROP, DROP_NON_DROP_SUMMARY.into(), Some(arg.span))

clippy_lints/src/functions/mod.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ declare_clippy_lint! {
2525
/// ### What it does
2626
/// Checks for a `#[must_use]` attribute without
2727
/// further information on functions and methods that return a type already
28-
/// marked as `#[must_use]`.
28+
/// considered as `#[must_use]`.
2929
///
3030
/// ### Why is this bad?
3131
/// The attribute isn't needed. Not using the result
@@ -39,6 +39,12 @@ declare_clippy_lint! {
3939
/// unimplemented!();
4040
/// }
4141
/// ```
42+
///
43+
/// ### Note
44+
/// The compiler may consider a type as being indirectly `#[must_use]`. For
45+
/// example, although `Box<_>` itself is not `#[must_use]`, `Box<T>` will be
46+
/// considered `#[must_use]` if `T` is.
47+
/// ```
4248
#[clippy::version = "1.40.0"]
4349
pub DOUBLE_MUST_USE,
4450
style,

clippy_lints/src/functions/must_use.rs

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,17 @@
1-
use clippy_utils::res::MaybeDef as _;
21
use hir::FnSig;
32
use rustc_errors::Applicability;
43
use rustc_hir::def::Res;
54
use rustc_hir::def_id::DefIdSet;
65
use rustc_hir::{self as hir, Attribute, QPath, find_attr};
6+
use rustc_lint::unused::must_use::MustUsePath;
77
use rustc_lint::{LateContext, LintContext as _};
88
use rustc_middle::ty::{self, Ty};
99
use rustc_span::{Span, sym};
1010

1111
use clippy_utils::attrs::is_proc_macro;
1212
use clippy_utils::diagnostics::span_lint_and_then;
1313
use clippy_utils::source::snippet_indent;
14-
use clippy_utils::ty::is_must_use_ty;
14+
use clippy_utils::ty::{describe_must_use_type, opt_must_use_path};
1515
use clippy_utils::visitors::for_each_expr_without_closures;
1616
use clippy_utils::{is_entrypoint_fn, return_ty, trait_ref_of_method};
1717
use rustc_span::Symbol;
@@ -161,11 +161,13 @@ fn check_needless_must_use(
161161
}
162162
},
163163
);
164-
} else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
164+
} else if reason.is_none()
165+
&& let Some(return_must_use_path) = opt_must_use_path(cx, return_ty(cx, item_id))
166+
{
165167
// Ignore async functions unless Future::Output type is a must_use type
166168
if sig.header.is_async()
167169
&& let Some(future_ty) = cx.tcx.get_impl_future_output_ty(return_ty(cx, item_id))
168-
&& !is_must_use_ty(cx, future_ty)
170+
&& opt_must_use_path(cx, future_ty).is_none()
169171
{
170172
return;
171173
}
@@ -174,8 +176,18 @@ fn check_needless_must_use(
174176
cx,
175177
DOUBLE_MUST_USE,
176178
fn_header_span,
177-
"this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`",
179+
"this function has a `#[must_use]` attribute with no message, but returns a type already considered as `#[must_use]`",
178180
|diag| {
181+
// Add info about the reason why the return type is `#[must_use]` if it is a compound type.
182+
if !matches!(return_must_use_path, MustUsePath::Def(..)) {
183+
diag.span_note(
184+
sig.decl.output.span(),
185+
format!(
186+
"the return type is {}",
187+
describe_must_use_type(cx, &return_must_use_path)
188+
),
189+
);
190+
}
179191
// When there are multiple attributes, it is not sufficient to simply make `must_use` empty, see
180192
// issue #12320.
181193
// FIXME(jdonszelmann): this used to give a machine-applicable fix. However, it was super fragile,
@@ -206,7 +218,7 @@ fn check_must_use_candidate<'tcx>(
206218
|| item_span.in_external_macro(cx.sess().source_map())
207219
|| returns_unit(decl)
208220
|| !cx.effective_visibilities.is_exported(item_id.def_id)
209-
|| is_must_use_ty(cx, return_ty(cx, item_id))
221+
|| opt_must_use_path(cx, return_ty(cx, item_id)).is_some()
210222
|| item_span.from_expansion()
211223
|| is_entrypoint_fn(cx, item_id.def_id.to_def_id())
212224
{
@@ -220,13 +232,6 @@ fn check_must_use_candidate<'tcx>(
220232
format!("#[must_use]\n{indent}"),
221233
Applicability::MachineApplicable,
222234
);
223-
if let Some(msg) = match return_ty(cx, item_id).opt_diag_name(cx) {
224-
Some(sym::ControlFlow) => Some("`ControlFlow<B, C>` as `C` when `B` is uninhabited"),
225-
Some(sym::Result) => Some("`Result<T, E>` as `T` when `E` is uninhabited"),
226-
_ => None,
227-
} {
228-
diag.note(format!("a future version of Rust will treat {msg} wrt `#[must_use]`"));
229-
}
230235
});
231236
}
232237

clippy_lints/src/let_underscore.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
2-
use clippy_utils::ty::{implements_trait, is_must_use_ty};
2+
use clippy_utils::ty::{describe_must_use_type, implements_trait, opt_must_use_path};
33
use clippy_utils::{is_from_proc_macro, is_must_use_func_call, paths};
44
use rustc_hir::{LetStmt, LocalSource, PatKind};
55
use rustc_lint::{LateContext, LateLintPass};
@@ -176,15 +176,19 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
176176
diag.help("consider awaiting the future or dropping explicitly with `std::mem::drop`");
177177
},
178178
);
179-
} else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
180-
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
179+
} else if let Some(must_use_path) = opt_must_use_path(cx, cx.typeck_results().expr_ty(init)) {
181180
span_lint_and_then(
182181
cx,
183182
LET_UNDERSCORE_MUST_USE,
184183
local.span,
185184
"non-binding `let` on an expression with `#[must_use]` type",
186185
|diag| {
187186
diag.help("consider explicitly using expression value");
187+
let ty_span = local.ty.map_or(init.span, |ty| ty.span);
188+
diag.span_note(
189+
ty_span,
190+
format!("type is {}", describe_must_use_type(cx, &must_use_path)),
191+
);
188192
},
189193
);
190194
} else if is_must_use_func_call(cx, init) {

clippy_lints/src/return_self_not_must_use.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_help;
2-
use clippy_utils::ty::is_must_use_ty;
2+
use clippy_utils::ty::opt_must_use_path;
33
use clippy_utils::{nth_arg, return_ty};
44
use rustc_hir::def_id::LocalDefId;
55
use rustc_hir::intravisit::FnKind;
@@ -86,8 +86,8 @@ fn check_method(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_def: LocalDefId, spa
8686
// For this check, we don't want to remove the reference on the returned type because if
8787
// there is one, we shouldn't emit a warning!
8888
&& self_arg.peel_refs() == ret_ty
89-
// If `Self` is already marked as `#[must_use]`, no need for the attribute here.
90-
&& !is_must_use_ty(cx, ret_ty)
89+
// If `Self` is already considered as `#[must_use]`, no need for the attribute here.
90+
&& opt_must_use_path(cx, ret_ty).is_none()
9191
{
9292
span_lint_and_help(
9393
cx,

clippy_utils/src/ty/mod.rs

Lines changed: 104 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,19 @@
33
#![allow(clippy::module_name_repetitions)]
44

55
use core::ops::ControlFlow;
6+
use itertools::Itertools as _;
67
use rustc_abi::{BackendRepr, FieldsShape, VariantIdx, Variants};
78
use rustc_ast::ast::Mutability;
89
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
10+
use rustc_errors::pluralize;
911
use rustc_hir as hir;
1012
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
1113
use rustc_hir::def_id::DefId;
12-
use rustc_hir::{Expr, FnDecl, LangItem, find_attr};
14+
use rustc_hir::{Expr, ExprKind, FnDecl, LangItem};
1315
use rustc_hir_analysis::lower_ty;
1416
use rustc_infer::infer::TyCtxtInferExt as _;
1517
use rustc_lint::LateContext;
18+
use rustc_lint::unused::must_use::{IsTyMustUse, MustUsePath, is_ty_must_use};
1619
use rustc_middle::mir::ConstValue;
1720
use rustc_middle::mir::interpret::Scalar;
1821
use rustc_middle::traits::EvaluationResult;
@@ -319,54 +322,112 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
319322
}
320323
}
321324

322-
// Returns whether the `ty` has `#[must_use]` attribute. If `ty` is a `Result`/`ControlFlow`
323-
// whose `Err`/`Break` payload is an uninhabited type, the `Ok`/`Continue` payload type
324-
// will be used instead. See <https://github.com/rust-lang/rust/pull/148214>.
325-
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
326-
match ty.kind() {
327-
ty::Adt(adt, args) => match cx.tcx.get_diagnostic_name(adt.did()) {
328-
Some(sym::Result) if args.type_at(1).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
329-
is_must_use_ty(cx, args.type_at(0))
330-
},
331-
Some(sym::ControlFlow) if args.type_at(0).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
332-
is_must_use_ty(cx, args.type_at(1))
333-
},
334-
_ => find_attr!(cx.tcx, adt.did(), MustUse { .. }),
325+
/// Returns whether the `ty` has `#[must_use]` attribute, or acts like it does according to the
326+
/// compiler determination. For example, if `ty` is a `Result`/`ControlFlow` whose `Err`/`Break`
327+
/// payload is an uninhabited type, the `Ok`/`Continue` payload type will be used instead.
328+
///
329+
/// The [`MustUsePath`] can be used to describe the type through [`describe_must_use_type`].
330+
pub fn opt_must_use_path<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<MustUsePath> {
331+
// `is_ty_must_use` requires an expression, whose `hir_id` will be used to determine whether
332+
// certain types are visibly uninhabited from the module containing the expression.
333+
// `cx.last_node_with_lint_attrs` is initialized to the crate/module `hir_id` when linting
334+
// a new crate/module. If it is overriden, it is with an `hir_id` pertaining to the same
335+
// create/module. We can use this in a dummy expression instead of asking all callers
336+
// to provide a local `hir_id` which would not add more information.
337+
let dummy_expr = Expr {
338+
hir_id: cx.last_node_with_lint_attrs,
339+
span: DUMMY_SP,
340+
kind: ExprKind::Ret(None),
341+
};
342+
match is_ty_must_use(cx, ty, &dummy_expr) {
343+
IsTyMustUse::Yes(path) => Some(path),
344+
_ => None,
345+
}
346+
}
347+
348+
/// Describe a [`MustUsePath`] returned by [`is_must_use_ty`].
349+
pub fn describe_must_use_type(cx: &LateContext<'_>, path: &MustUsePath) -> String {
350+
describe_must_use_type_inner(cx, path, "", "", 1)
351+
}
352+
353+
// This is a rip-off from the compiler's `rustc_lint/src/unused/must_use.rs`
354+
fn describe_must_use_type_inner(
355+
cx: &LateContext<'_>,
356+
path: &MustUsePath,
357+
descr_pre: &str,
358+
descr_post: &str,
359+
plural_len: usize,
360+
) -> String {
361+
let plural_suffix = pluralize!(plural_len);
362+
363+
match path {
364+
MustUsePath::Boxed(path) => {
365+
let descr_pre = &format!("{descr_pre}boxed ");
366+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
335367
},
336-
ty::Foreign(did) => find_attr!(cx.tcx, *did, MustUse { .. }),
337-
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => {
338-
// for the Array case we don't need to care for the len == 0 case
339-
// because we don't want to lint functions returning empty arrays
340-
is_must_use_ty(cx, *ty)
368+
MustUsePath::Pinned(path) => {
369+
let descr_pre = &format!("{descr_pre}pinned ");
370+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
341371
},
342-
ty::Tuple(args) => args.iter().any(|ty| is_must_use_ty(cx, ty)),
343-
ty::Alias(
344-
_,
345-
AliasTy {
346-
kind: ty::Opaque { def_id },
347-
..
348-
},
349-
) => {
350-
for (predicate, _) in cx.tcx.explicit_item_self_bounds(*def_id).skip_binder() {
351-
if let ty::ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
352-
&& find_attr!(cx.tcx, trait_predicate.trait_ref.def_id, MustUse { .. })
353-
{
354-
return true;
372+
MustUsePath::Opaque(path) => {
373+
let descr_pre = &format!("{descr_pre}implementer{plural_suffix} of ");
374+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
375+
},
376+
MustUsePath::TraitObject(path) => {
377+
let descr_post = &format!(" trait object{plural_suffix}{descr_post}");
378+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
379+
},
380+
MustUsePath::TupleElement(elems) => elems
381+
.iter()
382+
.map(|(index, path)| {
383+
let descr_post = &format!(" in tuple element {index}");
384+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
385+
})
386+
.join(", "),
387+
MustUsePath::Result(path) => {
388+
let descr_post = &format!(" in a `Result` with an uninhabited error{descr_post}");
389+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
390+
},
391+
MustUsePath::ControlFlow(path) => {
392+
let descr_post = &format!(" in a `ControlFlow` with an uninhabited break{descr_post}");
393+
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
394+
},
395+
MustUsePath::Array(path, len) => {
396+
let descr_pre = &format!("{descr_pre}array{plural_suffix} of ");
397+
describe_must_use_type_inner(
398+
cx,
399+
path,
400+
descr_pre,
401+
descr_post,
402+
plural_len.saturating_add(usize::try_from(*len).unwrap_or(usize::MAX)),
403+
)
404+
},
405+
MustUsePath::Closure(_) => {
406+
format!(
407+
"{descr_pre}{} closure{plural_suffix}{descr_post}",
408+
if plural_len == 1 {
409+
"one".to_string()
410+
} else {
411+
plural_len.to_string()
355412
}
356-
}
357-
false
413+
)
358414
},
359-
ty::Dynamic(binder, _) => {
360-
for predicate in *binder {
361-
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder()
362-
&& find_attr!(cx.tcx, trait_ref.def_id, MustUse { .. })
363-
{
364-
return true;
415+
MustUsePath::Coroutine(_) => {
416+
format!(
417+
"{descr_pre}{} coroutine{plural_suffix}{descr_post}",
418+
if plural_len == 1 {
419+
"one".to_string()
420+
} else {
421+
plural_len.to_string()
365422
}
366-
}
367-
false
423+
)
424+
},
425+
MustUsePath::Def(_, def_id, _) => {
426+
format!(
427+
"{descr_pre}`{}`{plural_suffix}{descr_post}",
428+
cx.tcx.def_path_str(*def_id)
429+
)
368430
},
369-
_ => false,
370431
}
371432
}
372433

0 commit comments

Comments
 (0)