Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
sym::mem_drop
if !(arg_ty.needs_drop(cx.tcx, cx.typing_env())
|| is_must_use_func_call(cx, arg)
|| is_must_use_ty(cx, arg_ty)
|| is_must_use_ty(cx, arg_ty).is_some()
|| drop_is_single_call_in_arm) =>
{
(DROP_NON_DROP, DROP_NON_DROP_SUMMARY.into(), Some(arg.span))
Expand Down
8 changes: 7 additions & 1 deletion clippy_lints/src/functions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ declare_clippy_lint! {
/// ### What it does
/// Checks for a `#[must_use]` attribute without
/// further information on functions and methods that return a type already
/// marked as `#[must_use]`.
/// considered as `#[must_use]`.
///
/// ### Why is this bad?
/// The attribute isn't needed. Not using the result
Expand All @@ -39,6 +39,12 @@ declare_clippy_lint! {
/// unimplemented!();
/// }
/// ```
///
/// ### Note
/// The compiler may consider a type as being indirectly `#[must_use]`. For
/// example, although `Box<_>` itself is not `#[must_use]`, `Box<T>` will be
/// considered `#[must_use]` if `T` is.
/// ```
#[clippy::version = "1.40.0"]
pub DOUBLE_MUST_USE,
style,
Expand Down
31 changes: 18 additions & 13 deletions clippy_lints/src/functions/must_use.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
use clippy_utils::res::MaybeDef as _;
use hir::FnSig;
use rustc_errors::Applicability;
use rustc_hir::def::Res;
use rustc_hir::def_id::DefIdSet;
use rustc_hir::{self as hir, Attribute, QPath, find_attr};
use rustc_lint::unused::must_use::MustUsePath;
use rustc_lint::{LateContext, LintContext};
use rustc_middle::ty::{self, Ty};
use rustc_span::{Span, sym};

use clippy_utils::attrs::is_proc_macro;
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::source::snippet_indent;
use clippy_utils::ty::is_must_use_ty;
use clippy_utils::ty::{describe_must_use_type, is_must_use_ty};
use clippy_utils::visitors::for_each_expr_without_closures;
use clippy_utils::{is_entrypoint_fn, return_ty, trait_ref_of_method};
use rustc_span::Symbol;
Expand Down Expand Up @@ -161,11 +161,13 @@ fn check_needless_must_use(
}
},
);
} else if reason.is_none() && is_must_use_ty(cx, return_ty(cx, item_id)) {
} else if reason.is_none()
&& let Some(return_must_use_path) = is_must_use_ty(cx, return_ty(cx, item_id))
{
// Ignore async functions unless Future::Output type is a must_use type
if sig.header.is_async()
&& let Some(future_ty) = cx.tcx.get_impl_future_output_ty(return_ty(cx, item_id))
&& !is_must_use_ty(cx, future_ty)
&& is_must_use_ty(cx, future_ty).is_none()
{
return;
}
Expand All @@ -174,8 +176,18 @@ fn check_needless_must_use(
cx,
DOUBLE_MUST_USE,
fn_header_span,
"this function has a `#[must_use]` attribute with no message, but returns a type already marked as `#[must_use]`",
"this function has a `#[must_use]` attribute with no message, but returns a type already considered as `#[must_use]`",
|diag| {
// Add info about the reason why the return type is `#[must_use]` if it is a compound type.
if !matches!(return_must_use_path, MustUsePath::Def(..)) {
diag.span_note(
sig.decl.output.span(),
format!(
"the return type is {}",
describe_must_use_type(cx, &return_must_use_path)
),
);
}
// When there are multiple attributes, it is not sufficient to simply make `must_use` empty, see
// issue #12320.
// FIXME(jdonszelmann): this used to give a machine-applicable fix. However, it was super fragile,
Expand Down Expand Up @@ -206,7 +218,7 @@ fn check_must_use_candidate<'tcx>(
|| item_span.in_external_macro(cx.sess().source_map())
|| returns_unit(decl)
|| !cx.effective_visibilities.is_exported(item_id.def_id)
|| is_must_use_ty(cx, return_ty(cx, item_id))
|| is_must_use_ty(cx, return_ty(cx, item_id)).is_some()
|| item_span.from_expansion()
|| is_entrypoint_fn(cx, item_id.def_id.to_def_id())
{
Expand All @@ -220,13 +232,6 @@ fn check_must_use_candidate<'tcx>(
format!("#[must_use]\n{indent}"),
Applicability::MachineApplicable,
);
if let Some(msg) = match return_ty(cx, item_id).opt_diag_name(cx) {
Some(sym::ControlFlow) => Some("`ControlFlow<B, C>` as `C` when `B` is uninhabited"),
Some(sym::Result) => Some("`Result<T, E>` as `T` when `E` is uninhabited"),
_ => None,
} {
diag.note(format!("a future version of Rust will treat {msg} wrt `#[must_use]`"));
}
});
}

Expand Down
10 changes: 7 additions & 3 deletions clippy_lints/src/let_underscore.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::ty::{implements_trait, is_must_use_ty};
use clippy_utils::ty::{describe_must_use_type, implements_trait, is_must_use_ty};
use clippy_utils::{is_from_proc_macro, is_must_use_func_call, paths};
use rustc_hir::{LetStmt, LocalSource, PatKind};
use rustc_lint::{LateContext, LateLintPass};
Expand Down Expand Up @@ -176,15 +176,19 @@ impl<'tcx> LateLintPass<'tcx> for LetUnderscore {
diag.help("consider awaiting the future or dropping explicitly with `std::mem::drop`");
},
);
} else if is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
#[expect(clippy::collapsible_span_lint_calls, reason = "rust-clippy#7797")]
} else if let Some(must_use_path) = is_must_use_ty(cx, cx.typeck_results().expr_ty(init)) {
span_lint_and_then(
cx,
LET_UNDERSCORE_MUST_USE,
local.span,
"non-binding `let` on an expression with `#[must_use]` type",
|diag| {
diag.help("consider explicitly using expression value");
let ty_span = local.ty.map_or(init.span, |ty| ty.span);
diag.span_note(
ty_span,
format!("type is {}", describe_must_use_type(cx, &must_use_path)),
);
},
);
} else if is_must_use_func_call(cx, init) {
Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/return_self_not_must_use.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ fn check_method(cx: &LateContext<'_>, decl: &FnDecl<'_>, fn_def: LocalDefId, spa
// For this check, we don't want to remove the reference on the returned type because if
// there is one, we shouldn't emit a warning!
&& self_arg.peel_refs() == ret_ty
// If `Self` is already marked as `#[must_use]`, no need for the attribute here.
&& !is_must_use_ty(cx, ret_ty)
// If `Self` is already considered as `#[must_use]`, no need for the attribute here.
&& is_must_use_ty(cx, ret_ty).is_none()
{
span_lint_and_help(
cx,
Expand Down
147 changes: 104 additions & 43 deletions clippy_utils/src/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,19 @@
#![allow(clippy::module_name_repetitions)]

use core::ops::ControlFlow;
use itertools::Itertools as _;
use rustc_abi::{BackendRepr, FieldsShape, VariantIdx, Variants};
use rustc_ast::ast::Mutability;
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_errors::pluralize;
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, CtorOf, DefKind, Res};
use rustc_hir::def_id::DefId;
use rustc_hir::{Expr, FnDecl, LangItem, find_attr};
use rustc_hir::{Expr, ExprKind, FnDecl, LangItem};
use rustc_hir_analysis::lower_ty;
use rustc_infer::infer::TyCtxtInferExt;
use rustc_lint::LateContext;
use rustc_lint::unused::must_use::{IsTyMustUse, MustUsePath, is_ty_must_use};
use rustc_middle::mir::ConstValue;
use rustc_middle::mir::interpret::Scalar;
use rustc_middle::traits::EvaluationResult;
Expand Down Expand Up @@ -319,54 +322,112 @@ pub fn has_drop<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
}
}

// Returns whether the `ty` has `#[must_use]` attribute. If `ty` is a `Result`/`ControlFlow`
// whose `Err`/`Break` payload is an uninhabited type, the `Ok`/`Continue` payload type
// will be used instead. See <https://github.com/rust-lang/rust/pull/148214>.
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
match ty.kind() {
ty::Adt(adt, args) => match cx.tcx.get_diagnostic_name(adt.did()) {
Some(sym::Result) if args.type_at(1).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
is_must_use_ty(cx, args.type_at(0))
},
Some(sym::ControlFlow) if args.type_at(0).is_privately_uninhabited(cx.tcx, cx.typing_env()) => {
is_must_use_ty(cx, args.type_at(1))
},
_ => find_attr!(cx.tcx, adt.did(), MustUse { .. }),
/// Returns whether the `ty` has `#[must_use]` attribute, or acts like it does according to the
/// compiler determination. For example, if `ty` is a `Result`/`ControlFlow` whose `Err`/`Break`
/// payload is an uninhabited type, the `Ok`/`Continue` payload type will be used instead.
///
/// The [`MustUsePath`] can be used to describe the type through [`describe_must_use_type`].
pub fn is_must_use_ty<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> Option<MustUsePath> {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename the function to better match the intent. Perhaps opt_must_use_path?

// `is_ty_must_use` requires an expression, whose `hir_id` will be used to determine whether
// certain types are visibly uninhabited from the module containing the expression.
// `cx.last_node_with_lint_attrs` is initialized to the crate/module `hir_id` when linting
// a new crate/module. If it is overriden, it is with an `hir_id` pertaining to the same
// create/module. We can use this in a dummy expression instead of asking all callers
// to provide a local `hir_id` which would not add more information.
let dummy_expr = Expr {
hir_id: cx.last_node_with_lint_attrs,
span: DUMMY_SP,
kind: ExprKind::Ret(None),
};
match is_ty_must_use(cx, ty, &dummy_expr) {
IsTyMustUse::Yes(path) => Some(path),
_ => None,
}
}

/// Describe a [`MustUsePath`] returned by [`is_must_use_ty`].
pub fn describe_must_use_type(cx: &LateContext<'_>, path: &MustUsePath) -> String {
describe_must_use_type_inner(cx, path, "", "", 1)
}

// This is a rip-off from the compiler's `rustc_lint/src/unused/must_use.rs`
fn describe_must_use_type_inner(
cx: &LateContext<'_>,
path: &MustUsePath,
descr_pre: &str,
descr_post: &str,
plural_len: usize,
) -> String {
let plural_suffix = pluralize!(plural_len);

match path {
MustUsePath::Boxed(path) => {
let descr_pre = &format!("{descr_pre}boxed ");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
ty::Foreign(did) => find_attr!(cx.tcx, *did, MustUse { .. }),
ty::Slice(ty) | ty::Array(ty, _) | ty::RawPtr(ty, _) | ty::Ref(_, ty, _) => {
// for the Array case we don't need to care for the len == 0 case
// because we don't want to lint functions returning empty arrays
is_must_use_ty(cx, *ty)
MustUsePath::Pinned(path) => {
let descr_pre = &format!("{descr_pre}pinned ");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
ty::Tuple(args) => args.iter().any(|ty| is_must_use_ty(cx, ty)),
ty::Alias(
_,
AliasTy {
kind: ty::Opaque { def_id },
..
},
) => {
for (predicate, _) in cx.tcx.explicit_item_self_bounds(*def_id).skip_binder() {
if let ty::ClauseKind::Trait(trait_predicate) = predicate.kind().skip_binder()
&& find_attr!(cx.tcx, trait_predicate.trait_ref.def_id, MustUse { .. })
{
return true;
MustUsePath::Opaque(path) => {
let descr_pre = &format!("{descr_pre}implementer{plural_suffix} of ");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
MustUsePath::TraitObject(path) => {
let descr_post = &format!(" trait object{plural_suffix}{descr_post}");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
MustUsePath::TupleElement(elems) => elems
.iter()
.map(|(index, path)| {
let descr_post = &format!(" in tuple element {index}");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
})
.join(", "),
MustUsePath::Result(path) => {
let descr_post = &format!(" in a `Result` with an uninhabited error{descr_post}");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
MustUsePath::ControlFlow(path) => {
let descr_post = &format!(" in a `ControlFlow` with an uninhabited break{descr_post}");
describe_must_use_type_inner(cx, path, descr_pre, descr_post, plural_len)
},
MustUsePath::Array(path, len) => {
let descr_pre = &format!("{descr_pre}array{plural_suffix} of ");
describe_must_use_type_inner(
cx,
path,
descr_pre,
descr_post,
plural_len.saturating_add(usize::try_from(*len).unwrap_or(usize::MAX)),
)
},
MustUsePath::Closure(_) => {
format!(
"{descr_pre}{} closure{plural_suffix}{descr_post}",
if plural_len == 1 {
"one".to_string()
} else {
plural_len.to_string()
}
}
false
)
},
ty::Dynamic(binder, _) => {
for predicate in *binder {
if let ty::ExistentialPredicate::Trait(ref trait_ref) = predicate.skip_binder()
&& find_attr!(cx.tcx, trait_ref.def_id, MustUse { .. })
{
return true;
MustUsePath::Coroutine(_) => {
format!(
"{descr_pre}{} coroutine{plural_suffix}{descr_post}",
if plural_len == 1 {
"one".to_string()
} else {
plural_len.to_string()
}
}
false
)
},
MustUsePath::Def(_, def_id, _) => {
format!(
"{descr_pre}`{}`{plural_suffix}{descr_post}",
cx.tcx.def_path_str(*def_id)
)
},
_ => false,
}
}

Expand Down
Loading