Merge commit '37f4fbb929' into clippy-subtree-update

This commit is contained in:
Philipp Krones 2024-07-25 18:29:17 +02:00
parent 6d674685ae
commit 4e6851e50b
291 changed files with 4890 additions and 5205 deletions

View file

@ -10,59 +10,80 @@ use rustc_hir::{LangItem, QPath};
use rustc_lint::LateContext;
use rustc_span::Span;
pub(crate) struct OptionAndThenSome;
impl BindInsteadOfMap for OptionAndThenSome {
const VARIANT_LANG_ITEM: LangItem = LangItem::OptionSome;
const BAD_METHOD_NAME: &'static str = "and_then";
const GOOD_METHOD_NAME: &'static str = "map";
pub(super) fn check_and_then_some(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
arg: &hir::Expr<'_>,
) -> bool {
BindInsteadOfMap {
variant_lang_item: LangItem::OptionSome,
bad_method_name: "and_then",
good_method_name: "map",
}
.check(cx, expr, recv, arg)
}
pub(crate) struct ResultAndThenOk;
impl BindInsteadOfMap for ResultAndThenOk {
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultOk;
const BAD_METHOD_NAME: &'static str = "and_then";
const GOOD_METHOD_NAME: &'static str = "map";
pub(super) fn check_and_then_ok(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
arg: &hir::Expr<'_>,
) -> bool {
BindInsteadOfMap {
variant_lang_item: LangItem::ResultOk,
bad_method_name: "and_then",
good_method_name: "map",
}
.check(cx, expr, recv, arg)
}
pub(crate) struct ResultOrElseErrInfo;
impl BindInsteadOfMap for ResultOrElseErrInfo {
const VARIANT_LANG_ITEM: LangItem = LangItem::ResultErr;
const BAD_METHOD_NAME: &'static str = "or_else";
const GOOD_METHOD_NAME: &'static str = "map_err";
pub(super) fn check_or_else_err(
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
arg: &hir::Expr<'_>,
) -> bool {
BindInsteadOfMap {
variant_lang_item: LangItem::ResultErr,
bad_method_name: "or_else",
good_method_name: "map_err",
}
.check(cx, expr, recv, arg)
}
pub(crate) trait BindInsteadOfMap {
const VARIANT_LANG_ITEM: LangItem;
const BAD_METHOD_NAME: &'static str;
const GOOD_METHOD_NAME: &'static str;
struct BindInsteadOfMap {
variant_lang_item: LangItem,
bad_method_name: &'static str,
good_method_name: &'static str,
}
fn no_op_msg(cx: &LateContext<'_>) -> Option<String> {
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
impl BindInsteadOfMap {
fn no_op_msg(&self, cx: &LateContext<'_>) -> Option<String> {
let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
let item_id = cx.tcx.parent(variant_id);
Some(format!(
"using `{}.{}({})`, which is a no-op",
cx.tcx.item_name(item_id),
Self::BAD_METHOD_NAME,
self.bad_method_name,
cx.tcx.item_name(variant_id),
))
}
fn lint_msg(cx: &LateContext<'_>) -> Option<String> {
let variant_id = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)?;
fn lint_msg(&self, cx: &LateContext<'_>) -> Option<String> {
let variant_id = cx.tcx.lang_items().get(self.variant_lang_item)?;
let item_id = cx.tcx.parent(variant_id);
Some(format!(
"using `{}.{}(|x| {}(y))`, which is more succinctly expressed as `{}(|x| y)`",
cx.tcx.item_name(item_id),
Self::BAD_METHOD_NAME,
self.bad_method_name,
cx.tcx.item_name(variant_id),
Self::GOOD_METHOD_NAME
self.good_method_name,
))
}
fn lint_closure_autofixable(
&self,
cx: &LateContext<'_>,
expr: &hir::Expr<'_>,
recv: &hir::Expr<'_>,
@ -71,9 +92,9 @@ pub(crate) trait BindInsteadOfMap {
) -> bool {
if let hir::ExprKind::Call(some_expr, [inner_expr]) = closure_expr.kind
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = some_expr.kind
&& Self::is_variant(cx, path.res)
&& self.is_variant(cx, path.res)
&& !contains_return(inner_expr)
&& let Some(msg) = Self::lint_msg(cx)
&& let Some(msg) = self.lint_msg(cx)
{
let mut app = Applicability::MachineApplicable;
let some_inner_snip = snippet_with_context(cx, inner_expr.span, closure_expr.span.ctxt(), "_", &mut app).0;
@ -82,7 +103,7 @@ pub(crate) trait BindInsteadOfMap {
let option_snip = snippet(cx, recv.span, "..");
let note = format!(
"{option_snip}.{}({closure_args_snip} {some_inner_snip})",
Self::GOOD_METHOD_NAME
self.good_method_name
);
span_lint_and_sugg(cx, BIND_INSTEAD_OF_MAP, expr.span, msg, "try", note, app);
true
@ -91,13 +112,13 @@ pub(crate) trait BindInsteadOfMap {
}
}
fn lint_closure(cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
fn lint_closure(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, closure_expr: &hir::Expr<'_>) -> bool {
let mut suggs = Vec::new();
let can_sugg: bool = find_all_ret_expressions(cx, closure_expr, |ret_expr| {
if !ret_expr.span.from_expansion()
&& let hir::ExprKind::Call(func_path, [arg]) = ret_expr.kind
&& let hir::ExprKind::Path(QPath::Resolved(_, path)) = func_path.kind
&& Self::is_variant(cx, path.res)
&& self.is_variant(cx, path.res)
&& !contains_return(arg)
{
suggs.push((ret_expr.span, arg.span.source_callsite()));
@ -108,7 +129,7 @@ pub(crate) trait BindInsteadOfMap {
});
let (span, msg) = if can_sugg
&& let hir::ExprKind::MethodCall(segment, ..) = expr.kind
&& let Some(msg) = Self::lint_msg(cx)
&& let Some(msg) = self.lint_msg(cx)
{
(segment.ident.span, msg)
} else {
@ -119,7 +140,7 @@ pub(crate) trait BindInsteadOfMap {
diag,
"try",
Applicability::MachineApplicable,
std::iter::once((span, Self::GOOD_METHOD_NAME.into())).chain(
std::iter::once((span, self.good_method_name.into())).chain(
suggs
.into_iter()
.map(|(span1, span2)| (span1, snippet(cx, span2, "_").into())),
@ -130,9 +151,9 @@ pub(crate) trait BindInsteadOfMap {
}
/// Lint use of `_.and_then(|x| Some(y))` for `Option`s
fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
fn check(&self, cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr<'_>, arg: &hir::Expr<'_>) -> bool {
if let Some(adt) = cx.typeck_results().expr_ty(recv).ty_adt_def()
&& let Some(vid) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM)
&& let Some(vid) = cx.tcx.lang_items().get(self.variant_lang_item)
&& adt.did() == cx.tcx.parent(vid)
{
} else {
@ -144,15 +165,15 @@ pub(crate) trait BindInsteadOfMap {
let closure_body = cx.tcx.hir().body(body);
let closure_expr = peel_blocks(closure_body.value);
if Self::lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
if self.lint_closure_autofixable(cx, expr, recv, closure_expr, fn_decl_span) {
true
} else {
Self::lint_closure(cx, expr, closure_expr)
self.lint_closure(cx, expr, closure_expr)
}
},
// `_.and_then(Some)` case, which is no-op.
hir::ExprKind::Path(QPath::Resolved(_, path)) if Self::is_variant(cx, path.res) => {
if let Some(msg) = Self::no_op_msg(cx) {
hir::ExprKind::Path(QPath::Resolved(_, path)) if self.is_variant(cx, path.res) => {
if let Some(msg) = self.no_op_msg(cx) {
span_lint_and_sugg(
cx,
BIND_INSTEAD_OF_MAP,
@ -169,9 +190,9 @@ pub(crate) trait BindInsteadOfMap {
}
}
fn is_variant(cx: &LateContext<'_>, res: Res) -> bool {
fn is_variant(&self, cx: &LateContext<'_>, res: Res) -> bool {
if let Res::Def(DefKind::Ctor(CtorOf::Variant, CtorKind::Fn), id) = res {
if let Some(variant_id) = cx.tcx.lang_items().get(Self::VARIANT_LANG_ITEM) {
if let Some(variant_id) = cx.tcx.lang_items().get(self.variant_lang_item) {
return cx.tcx.parent(id) == variant_id;
}
}

View file

@ -131,8 +131,8 @@ mod waker_clone_wake;
mod wrong_self_convention;
mod zst_offset;
use bind_instead_of_map::BindInsteadOfMap;
use clippy_config::msrvs::{self, Msrv};
use clippy_config::Conf;
use clippy_utils::consts::{constant, Constant};
use clippy_utils::diagnostics::{span_lint, span_lint_and_help};
use clippy_utils::macros::FormatArgsStorage;
@ -4131,27 +4131,20 @@ pub struct Methods {
msrv: Msrv,
allow_expect_in_tests: bool,
allow_unwrap_in_tests: bool,
allowed_dotfiles: FxHashSet<String>,
allowed_dotfiles: FxHashSet<&'static str>,
format_args: FormatArgsStorage,
}
impl Methods {
#[must_use]
pub fn new(
avoid_breaking_exported_api: bool,
msrv: Msrv,
allow_expect_in_tests: bool,
allow_unwrap_in_tests: bool,
mut allowed_dotfiles: FxHashSet<String>,
format_args: FormatArgsStorage,
) -> Self {
allowed_dotfiles.extend(DEFAULT_ALLOWED_DOTFILES.iter().map(ToString::to_string));
pub fn new(conf: &'static Conf, format_args: FormatArgsStorage) -> Self {
let mut allowed_dotfiles: FxHashSet<_> = conf.allowed_dotfiles.iter().map(|s| &**s).collect();
allowed_dotfiles.extend(DEFAULT_ALLOWED_DOTFILES);
Self {
avoid_breaking_exported_api,
msrv,
allow_expect_in_tests,
allow_unwrap_in_tests,
avoid_breaking_exported_api: conf.avoid_breaking_exported_api,
msrv: conf.msrv.clone(),
allow_expect_in_tests: conf.allow_expect_in_tests,
allow_unwrap_in_tests: conf.allow_unwrap_in_tests,
allowed_dotfiles,
format_args,
}
@ -4512,8 +4505,8 @@ impl Methods {
}
},
("and_then", [arg]) => {
let biom_option_linted = bind_instead_of_map::OptionAndThenSome::check(cx, expr, recv, arg);
let biom_result_linted = bind_instead_of_map::ResultAndThenOk::check(cx, expr, recv, arg);
let biom_option_linted = bind_instead_of_map::check_and_then_some(cx, expr, recv, arg);
let biom_result_linted = bind_instead_of_map::check_and_then_ok(cx, expr, recv, arg);
if !biom_option_linted && !biom_result_linted {
unnecessary_lazy_eval::check(cx, expr, recv, arg, "and");
}
@ -4853,7 +4846,7 @@ impl Methods {
open_options::check(cx, expr, recv);
},
("or_else", [arg]) => {
if !bind_instead_of_map::ResultOrElseErrInfo::check(cx, expr, recv, arg) {
if !bind_instead_of_map::check_or_else_err(cx, expr, recv, arg) {
unnecessary_lazy_eval::check(cx, expr, recv, arg, "or");
}
},

View file

@ -10,6 +10,7 @@ use rustc_hir::{ExprKind, HirId, Node, PatKind, Path, QPath};
use rustc_lint::LateContext;
use rustc_middle::hir::nested_filter;
use rustc_span::{sym, Span};
use std::ops::ControlFlow;
use super::MAP_UNWRAP_OR;
@ -54,15 +55,14 @@ pub(super) fn check<'tcx>(
let mut reference_visitor = ReferenceVisitor {
cx,
identifiers: unwrap_visitor.identifiers,
found_reference: false,
unwrap_or_span: unwrap_arg.span,
};
let map = cx.tcx.hir();
let body = map.body_owned_by(map.enclosing_body_owner(expr.hir_id));
reference_visitor.visit_body(body);
if reference_visitor.found_reference {
// Visit the body, and return if we've found a reference
if reference_visitor.visit_body(body).is_break() {
return;
}
}
@ -151,29 +151,27 @@ impl<'a, 'tcx> Visitor<'tcx> for UnwrapVisitor<'a, 'tcx> {
struct ReferenceVisitor<'a, 'tcx> {
cx: &'a LateContext<'tcx>,
identifiers: FxHashSet<HirId>,
found_reference: bool,
unwrap_or_span: Span,
}
impl<'a, 'tcx> Visitor<'tcx> for ReferenceVisitor<'a, 'tcx> {
type NestedFilter = nested_filter::All;
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) {
type Result = ControlFlow<()>;
fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> {
// If we haven't found a reference yet, check if this references
// one of the locals that was moved in the `unwrap_or` argument.
// We are only interested in exprs that appear before the `unwrap_or` call.
if !self.found_reference {
if expr.span < self.unwrap_or_span
&& let ExprKind::Path(ref path) = expr.kind
&& let QPath::Resolved(_, path) = path
&& let Res::Local(local_id) = path.res
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
&& let PatKind::Binding(_, local_id, ..) = pat.kind
&& self.identifiers.contains(&local_id)
{
self.found_reference = true;
}
rustc_hir::intravisit::walk_expr(self, expr);
if expr.span < self.unwrap_or_span
&& let ExprKind::Path(ref path) = expr.kind
&& let QPath::Resolved(_, path) = path
&& let Res::Local(local_id) = path.res
&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
&& let PatKind::Binding(_, local_id, ..) = pat.kind
&& self.identifiers.contains(&local_id)
{
return ControlFlow::Break(());
}
rustc_hir::intravisit::walk_expr(self, expr)
}
fn nested_visit_map(&mut self) -> Self::Map {

View file

@ -1,8 +1,13 @@
use std::ops::ControlFlow;
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::eager_or_lazy::switch_to_lazy_eval;
use clippy_utils::source::snippet_with_context;
use clippy_utils::ty::{expr_type_is_certain, implements_trait, is_type_diagnostic_item};
use clippy_utils::{contains_return, is_default_equivalent, is_default_equivalent_call, last_path_segment};
use clippy_utils::visitors::for_each_expr;
use clippy_utils::{
contains_return, is_default_equivalent, is_default_equivalent_call, last_path_segment, peel_blocks,
};
use rustc_errors::Applicability;
use rustc_lint::LateContext;
use rustc_middle::ty;
@ -13,7 +18,7 @@ use {rustc_ast as ast, rustc_hir as hir};
use super::{OR_FUN_CALL, UNWRAP_OR_DEFAULT};
/// Checks for the `OR_FUN_CALL` lint.
#[allow(clippy::too_many_lines)]
#[expect(clippy::too_many_lines)]
pub(super) fn check<'tcx>(
cx: &LateContext<'tcx>,
expr: &hir::Expr<'_>,
@ -26,7 +31,6 @@ pub(super) fn check<'tcx>(
/// `or_insert(T::new())` or `or_insert(T::default())`.
/// Similarly checks for `unwrap_or_else(T::new)`, `unwrap_or_else(T::default)`,
/// `or_insert_with(T::new)` or `or_insert_with(T::default)`.
#[allow(clippy::too_many_arguments)]
fn check_unwrap_or_default(
cx: &LateContext<'_>,
name: &str,
@ -70,18 +74,31 @@ pub(super) fn check<'tcx>(
};
let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver).peel_refs();
let has_suggested_method = receiver_ty.ty_adt_def().is_some_and(|adt_def| {
let Some(suggested_method_def_id) = receiver_ty.ty_adt_def().and_then(|adt_def| {
cx.tcx
.inherent_impls(adt_def.did())
.into_iter()
.flatten()
.flat_map(|impl_id| cx.tcx.associated_items(impl_id).filter_by_name_unhygienic(sugg))
.any(|assoc| {
assoc.fn_has_self_parameter
.find_map(|assoc| {
if assoc.fn_has_self_parameter
&& cx.tcx.fn_sig(assoc.def_id).skip_binder().inputs().skip_binder().len() == 1
{
Some(assoc.def_id)
} else {
None
}
})
});
if !has_suggested_method {
}) else {
return false;
};
let in_sugg_method_implementation = {
matches!(
suggested_method_def_id.as_local(),
Some(local_def_id) if local_def_id == cx.tcx.hir().get_parent_item(receiver.hir_id).def_id
)
};
if in_sugg_method_implementation {
return false;
}
@ -110,8 +127,8 @@ pub(super) fn check<'tcx>(
}
/// Checks for `*or(foo())`.
#[allow(clippy::too_many_arguments)]
fn check_general_case<'tcx>(
#[expect(clippy::too_many_arguments)]
fn check_or_fn_call<'tcx>(
cx: &LateContext<'tcx>,
name: &str,
method_span: Span,
@ -122,7 +139,7 @@ pub(super) fn check<'tcx>(
span: Span,
// None if lambda is required
fun_span: Option<Span>,
) {
) -> bool {
// (path, fn_has_argument, methods, suffix)
const KNOW_TYPES: [(Symbol, bool, &[&str], &str); 4] = [
(sym::BTreeEntry, false, &["or_insert"], "with"),
@ -172,54 +189,68 @@ pub(super) fn check<'tcx>(
format!("{name}_{suffix}({sugg})"),
app,
);
true
} else {
false
}
}
let extract_inner_arg = |arg: &'tcx hir::Expr<'_>| {
if let hir::ExprKind::Block(
hir::Block {
stmts: [],
expr: Some(expr),
..
},
_,
) = arg.kind
{
expr
} else {
arg
}
};
if let [arg] = args {
let inner_arg = extract_inner_arg(arg);
match inner_arg.kind {
hir::ExprKind::Call(fun, or_args) => {
let or_has_args = !or_args.is_empty();
if or_has_args
|| !check_unwrap_or_default(cx, name, receiver, fun, Some(inner_arg), expr.span, method_span)
{
let fun_span = if or_has_args { None } else { Some(fun.span) };
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, fun_span);
}
},
hir::ExprKind::Path(..) | hir::ExprKind::Closure(..) => {
check_unwrap_or_default(cx, name, receiver, inner_arg, None, expr.span, method_span);
},
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_general_case(cx, name, method_span, receiver, arg, None, expr.span, None);
},
_ => (),
}
let inner_arg = peel_blocks(arg);
for_each_expr(cx, inner_arg, |ex| {
// `or_fun_call` lint needs to take nested expr into account,
// but `unwrap_or_default` lint doesn't, we don't want something like:
// `opt.unwrap_or(Foo { inner: String::default(), other: 1 })` to get replaced by
// `opt.unwrap_or_default()`.
let is_nested_expr = ex.hir_id != inner_arg.hir_id;
let is_triggered = match ex.kind {
hir::ExprKind::Call(fun, fun_args) => {
let inner_fun_has_args = !fun_args.is_empty();
let fun_span = if inner_fun_has_args || is_nested_expr {
None
} else {
Some(fun.span)
};
(!inner_fun_has_args
&& !is_nested_expr
&& check_unwrap_or_default(cx, name, receiver, fun, Some(ex), expr.span, method_span))
|| check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, fun_span)
},
hir::ExprKind::Path(..) | hir::ExprKind::Closure(..) if !is_nested_expr => {
check_unwrap_or_default(cx, name, receiver, ex, None, expr.span, method_span)
},
hir::ExprKind::Index(..) | hir::ExprKind::MethodCall(..) => {
check_or_fn_call(cx, name, method_span, receiver, arg, None, expr.span, None)
},
_ => false,
};
if is_triggered {
ControlFlow::Break(())
} else {
ControlFlow::Continue(())
}
});
}
// `map_or` takes two arguments
if let [arg, lambda] = args {
let inner_arg = extract_inner_arg(arg);
if let hir::ExprKind::Call(fun, or_args) = inner_arg.kind {
let fun_span = if or_args.is_empty() { Some(fun.span) } else { None };
check_general_case(cx, name, method_span, receiver, arg, Some(lambda), expr.span, fun_span);
}
let inner_arg = peel_blocks(arg);
for_each_expr(cx, inner_arg, |ex| {
let is_top_most_expr = ex.hir_id == inner_arg.hir_id;
if let hir::ExprKind::Call(fun, fun_args) = ex.kind {
let fun_span = if fun_args.is_empty() && is_top_most_expr {
Some(fun.span)
} else {
None
};
if check_or_fn_call(cx, name, method_span, receiver, arg, Some(lambda), expr.span, fun_span) {
return ControlFlow::Break(());
}
}
ControlFlow::Continue(())
});
}
}

View file

@ -21,7 +21,7 @@ pub(super) fn check(
path: &Expr<'_>,
expr: &Expr<'_>,
msrv: &Msrv,
allowed_dotfiles: &FxHashSet<String>,
allowed_dotfiles: &FxHashSet<&'static str>,
) {
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(recv).peel_refs(), sym::Path)
&& !path.span.from_expansion()