From 11b28d44bd428cdb4c939e51b064426c4b73a293 Mon Sep 17 00:00:00 2001 From: Jules Bertholet Date: Sat, 23 Mar 2024 21:04:45 -0400 Subject: [PATCH] Implement `mut ref`/`mut ref mut` --- clippy_lints/src/functions/misnamed_getters.rs | 4 ++-- clippy_lints/src/index_refutable_slice.rs | 4 ++-- clippy_lints/src/iter_without_into_iter.rs | 4 ++-- clippy_lints/src/len_zero.rs | 8 ++++---- .../src/matches/infallible_destructuring_match.rs | 4 ++-- clippy_lints/src/matches/match_as_ref.rs | 2 +- clippy_lints/src/matches/needless_match.rs | 2 +- clippy_lints/src/matches/redundant_guards.rs | 2 +- clippy_lints/src/methods/clone_on_copy.rs | 2 +- clippy_lints/src/methods/iter_kv_map.rs | 5 ++--- clippy_lints/src/misc.rs | 4 ++-- clippy_lints/src/question_mark.rs | 10 +++++++--- clippy_lints/src/utils/author.rs | 2 ++ clippy_utils/src/lib.rs | 10 +++++----- 14 files changed, 34 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/functions/misnamed_getters.rs b/clippy_lints/src/functions/misnamed_getters.rs index bf96c0d62b0..8ac17e17688 100644 --- a/clippy_lints/src/functions/misnamed_getters.rs +++ b/clippy_lints/src/functions/misnamed_getters.rs @@ -24,13 +24,13 @@ pub fn check_fn(cx: &LateContext<'_>, kind: FnKind<'_>, decl: &FnDecl<'_>, body: let name = ident.name.as_str(); let name = match decl.implicit_self { - ImplicitSelfKind::MutRef => { + ImplicitSelfKind::RefMut => { let Some(name) = name.strip_suffix("_mut") else { return; }; name }, - ImplicitSelfKind::Imm | ImplicitSelfKind::Mut | ImplicitSelfKind::ImmRef => name, + ImplicitSelfKind::Imm | ImplicitSelfKind::Mut | ImplicitSelfKind::RefImm => name, ImplicitSelfKind::None => return, }; diff --git a/clippy_lints/src/index_refutable_slice.rs b/clippy_lints/src/index_refutable_slice.rs index 5b5eb355f86..4d1f89b1d9d 100644 --- a/clippy_lints/src/index_refutable_slice.rs +++ b/clippy_lints/src/index_refutable_slice.rs @@ -97,7 +97,7 @@ fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap, pat: &hir::Pat<'_>) -> FxIndexMap {iter_ty} {{ fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) { let item_did = item.owner_id.to_def_id(); let (borrow_prefix, expected_implicit_self) = match item.ident.name { - sym::iter => ("&", ImplicitSelfKind::ImmRef), - sym::iter_mut => ("&mut ", ImplicitSelfKind::MutRef), + sym::iter => ("&", ImplicitSelfKind::RefImm), + sym::iter_mut => ("&mut ", ImplicitSelfKind::RefMut), _ => return, }; diff --git a/clippy_lints/src/len_zero.rs b/clippy_lints/src/len_zero.rs index 27d85cde532..cae9ada5a33 100644 --- a/clippy_lints/src/len_zero.rs +++ b/clippy_lints/src/len_zero.rs @@ -384,8 +384,8 @@ fn matches_is_empty_output<'tcx>(self, cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> fn expected_sig(self, self_kind: ImplicitSelfKind) -> String { let self_ref = match self_kind { - ImplicitSelfKind::ImmRef => "&", - ImplicitSelfKind::MutRef => "&mut ", + ImplicitSelfKind::RefImm => "&", + ImplicitSelfKind::RefMut => "&mut ", _ => "", }; match self { @@ -411,8 +411,8 @@ fn check_is_empty_sig<'tcx>( [arg, res] if len_output.matches_is_empty_output(cx, *res) => { matches!( (arg.kind(), self_kind), - (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::ImmRef) - | (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::MutRef) + (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm) + | (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::RefMut) ) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut)) }, _ => false, diff --git a/clippy_lints/src/matches/infallible_destructuring_match.rs b/clippy_lints/src/matches/infallible_destructuring_match.rs index 0f242e0b9e1..93d7683d2af 100644 --- a/clippy_lints/src/matches/infallible_destructuring_match.rs +++ b/clippy_lints/src/matches/infallible_destructuring_match.rs @@ -2,7 +2,7 @@ use clippy_utils::source::snippet_with_applicability; use clippy_utils::{path_to_local_id, peel_blocks, strip_pat_refs}; use rustc_errors::Applicability; -use rustc_hir::{ByRef, ExprKind, LetStmt, MatchSource, PatKind, QPath}; +use rustc_hir::{ExprKind, LetStmt, MatchSource, PatKind, QPath}; use rustc_lint::LateContext; use super::INFALLIBLE_DESTRUCTURING_MATCH; @@ -30,7 +30,7 @@ pub(crate) fn check(cx: &LateContext<'_>, local: &LetStmt<'_>) -> bool { format!( "let {}({}{}) = {};", snippet_with_applicability(cx, variant_name.span, "..", &mut applicability), - if binding.0 == ByRef::Yes { "ref " } else { "" }, + binding.prefix_str(), snippet_with_applicability(cx, local.pat.span, "..", &mut applicability), snippet_with_applicability(cx, target.span, "..", &mut applicability), ), diff --git a/clippy_lints/src/matches/match_as_ref.rs b/clippy_lints/src/matches/match_as_ref.rs index 3f737da92c0..6b484ff2749 100644 --- a/clippy_lints/src/matches/match_as_ref.rs +++ b/clippy_lints/src/matches/match_as_ref.rs @@ -67,7 +67,7 @@ fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool { fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option { if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind && is_res_lang_ctor(cx, cx.qpath_res(qpath, arm.pat.hir_id), LangItem::OptionSome) - && let PatKind::Binding(BindingAnnotation(ByRef::Yes, mutabl), .., ident, _) = first_pat.kind + && let PatKind::Binding(BindingAnnotation(ByRef::Yes(mutabl), _), .., ident, _) = first_pat.kind && let ExprKind::Call(e, [arg]) = peel_blocks(arm.body).kind && is_res_lang_ctor(cx, path_res(cx, e), LangItem::OptionSome) && let ExprKind::Path(QPath::Resolved(_, path2)) = arg.kind diff --git a/clippy_lints/src/matches/needless_match.rs b/clippy_lints/src/matches/needless_match.rs index cee77f62b61..fe83e784c3c 100644 --- a/clippy_lints/src/matches/needless_match.rs +++ b/clippy_lints/src/matches/needless_match.rs @@ -178,7 +178,7 @@ fn pat_same_as_expr(pat: &Pat<'_>, expr: &Expr<'_>) -> bool { }, )), ) => { - return !matches!(annot, BindingAnnotation(ByRef::Yes, _)) && pat_ident.name == first_seg.ident.name; + return !matches!(annot, BindingAnnotation(ByRef::Yes(_), _)) && pat_ident.name == first_seg.ident.name; }, // Example: `Custom::TypeA => Custom::TypeB`, or `None => None` (PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => { diff --git a/clippy_lints/src/matches/redundant_guards.rs b/clippy_lints/src/matches/redundant_guards.rs index 6bae51b45b8..50cbccc3968 100644 --- a/clippy_lints/src/matches/redundant_guards.rs +++ b/clippy_lints/src/matches/redundant_guards.rs @@ -182,7 +182,7 @@ fn get_pat_binding<'tcx>( if let PatKind::Binding(bind_annot, hir_id, ident, _) = pat.kind && hir_id == local { - if matches!(bind_annot.0, rustc_ast::ByRef::Yes) { + if matches!(bind_annot.0, rustc_ast::ByRef::Yes(_)) { let _ = byref_ident.insert(ident); } // the second call of `replace()` returns a `Some(span)`, meaning a multi-binding pattern diff --git a/clippy_lints/src/methods/clone_on_copy.rs b/clippy_lints/src/methods/clone_on_copy.rs index d4a5de3d1de..3e099004c47 100644 --- a/clippy_lints/src/methods/clone_on_copy.rs +++ b/clippy_lints/src/methods/clone_on_copy.rs @@ -69,7 +69,7 @@ pub(super) fn check( _ => false, }, // local binding capturing a reference - Node::LetStmt(l) if matches!(l.pat.kind, PatKind::Binding(BindingAnnotation(ByRef::Yes, _), ..)) => { + Node::LetStmt(l) if matches!(l.pat.kind, PatKind::Binding(BindingAnnotation(ByRef::Yes(_), _), ..)) => { return; }, _ => false, diff --git a/clippy_lints/src/methods/iter_kv_map.rs b/clippy_lints/src/methods/iter_kv_map.rs index 6394f35f860..1431a5db2d9 100644 --- a/clippy_lints/src/methods/iter_kv_map.rs +++ b/clippy_lints/src/methods/iter_kv_map.rs @@ -60,8 +60,6 @@ pub(super) fn check<'tcx>( applicability, ); } else { - let ref_annotation = if annotation.0 == ByRef::Yes { "ref " } else { "" }; - let mut_annotation = if annotation.1 == Mutability::Mut { "mut " } else { "" }; span_lint_and_sugg( cx, ITER_KV_MAP, @@ -69,7 +67,8 @@ pub(super) fn check<'tcx>( &format!("iterating on a map's {replacement_kind}s"), "try", format!( - "{recv_snippet}.{into_prefix}{replacement_kind}s().map(|{ref_annotation}{mut_annotation}{bound_ident}| {})", + "{recv_snippet}.{into_prefix}{replacement_kind}s().map(|{}{bound_ident}| {})", + annotation.prefix_str(), snippet_with_applicability(cx, body_expr.span, "/* body */", &mut applicability) ), applicability, diff --git a/clippy_lints/src/misc.rs b/clippy_lints/src/misc.rs index ea6e662b4be..11fecb7d72e 100644 --- a/clippy_lints/src/misc.rs +++ b/clippy_lints/src/misc.rs @@ -129,7 +129,7 @@ fn check_fn( if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) { return; } - if let PatKind::Binding(BindingAnnotation(ByRef::Yes, _), ..) = arg.pat.kind { + if let PatKind::Binding(BindingAnnotation(ByRef::Yes(_), _), ..) = arg.pat.kind { span_lint( cx, TOPLEVEL_REF_ARG, @@ -144,7 +144,7 @@ fn check_fn( fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { if !in_external_macro(cx.tcx.sess, stmt.span) && let StmtKind::Let(local) = stmt.kind - && let PatKind::Binding(BindingAnnotation(ByRef::Yes, mutabl), .., name, None) = local.pat.kind + && let PatKind::Binding(BindingAnnotation(ByRef::Yes(mutabl), _), .., name, None) = local.pat.kind && let Some(init) = local.init // Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id) diff --git a/clippy_lints/src/question_mark.rs b/clippy_lints/src/question_mark.rs index f1db571e113..b57220e6cf0 100644 --- a/clippy_lints/src/question_mark.rs +++ b/clippy_lints/src/question_mark.rs @@ -14,7 +14,7 @@ use rustc_hir::def::Res; use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::{ - BindingAnnotation, Block, ByRef, Expr, ExprKind, LetStmt, Node, PatKind, PathSegment, QPath, Stmt, StmtKind, + BindingAnnotation, Block, ByRef, Expr, ExprKind, LetStmt, Mutability, Node, PatKind, PathSegment, QPath, Stmt, StmtKind, }; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::Ty; @@ -283,9 +283,13 @@ fn check_if_let_some_or_err_and_early_return<'tcx>(cx: &LateContext<'tcx>, expr: let mut applicability = Applicability::MachineApplicable; let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); let requires_semi = matches!(cx.tcx.parent_hir_node(expr.hir_id), Node::Stmt(_)); + let method_call_str = match by_ref { + ByRef::Yes(Mutability::Mut) => ".as_mut()", + ByRef::Yes(Mutability::Not) => ".as_ref()", + ByRef::No => "", + }; let sugg = format!( - "{receiver_str}{}?{}", - if by_ref == ByRef::Yes { ".as_ref()" } else { "" }, + "{receiver_str}{method_call_str}?{}", if requires_semi { ";" } else { "" } ); span_lint_and_sugg( diff --git a/clippy_lints/src/utils/author.rs b/clippy_lints/src/utils/author.rs index 5319915b2ea..e45beb4910b 100644 --- a/clippy_lints/src/utils/author.rs +++ b/clippy_lints/src/utils/author.rs @@ -649,6 +649,8 @@ macro_rules! kind { BindingAnnotation::REF => "REF", BindingAnnotation::MUT => "MUT", BindingAnnotation::REF_MUT => "REF_MUT", + BindingAnnotation::MUT_REF => "MUT_REF", + BindingAnnotation::MUT_REF_MUT => "MUT_REF_MUT", }; kind!("Binding(BindingAnnotation::{ann}, _, {name}, {sub})"); self.ident(name); diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs index 8251bdf78fc..95bab5801d1 100644 --- a/clippy_utils/src/lib.rs +++ b/clippy_utils/src/lib.rs @@ -97,7 +97,7 @@ use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::{ - self as hir, def, Arm, ArrayLen, BindingAnnotation, Block, BlockCheckMode, Body, Closure, Destination, Expr, + self as hir, def, Arm, ArrayLen, BindingAnnotation, Block, BlockCheckMode, Body, ByRef, Closure, Destination, Expr, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, @@ -107,7 +107,6 @@ use rustc_middle::hir::place::PlaceBase; use rustc_middle::mir::Const; use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; -use rustc_middle::ty::binding::BindingMode; use rustc_middle::ty::fast_reject::SimplifiedType; use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::{ @@ -1006,11 +1005,12 @@ fn pat_capture_kind(cx: &LateContext<'_>, pat: &Pat<'_>) -> CaptureKind { .typeck_results() .extract_binding_mode(cx.sess(), id, span) .unwrap() + .0 { - BindingMode::BindByValue(_) if !is_copy(cx, cx.typeck_results().node_type(id)) => { + ByRef::No if !is_copy(cx, cx.typeck_results().node_type(id)) => { capture = CaptureKind::Value; }, - BindingMode::BindByReference(Mutability::Mut) if capture != CaptureKind::Value => { + ByRef::Yes(Mutability::Mut) if capture != CaptureKind::Value => { capture = CaptureKind::Ref(Mutability::Mut); }, _ => (), @@ -2035,7 +2035,7 @@ fn check_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>) -> bool { .typeck_results() .pat_binding_modes() .get(pat.hir_id) - .is_some_and(|mode| matches!(mode, BindingMode::BindByReference(_))) + .is_some_and(|mode| matches!(mode.0, ByRef::Yes(_))) { // If a tuple `(x, y)` is of type `&(i32, i32)`, then due to match ergonomics, // the inner patterns become references. Don't consider this the identity function