Implement mut ref/mut ref mut

This commit is contained in:
Jules Bertholet 2024-03-23 21:04:45 -04:00
parent 4b10cb20bc
commit 11b28d44bd
14 changed files with 34 additions and 29 deletions

View File

@ -24,13 +24,13 @@ pub fn check_fn(cx: &LateContext<'_>, kind: FnKind<'_>, decl: &FnDecl<'_>, body:
let name = ident.name.as_str(); let name = ident.name.as_str();
let name = match decl.implicit_self { let name = match decl.implicit_self {
ImplicitSelfKind::MutRef => { ImplicitSelfKind::RefMut => {
let Some(name) = name.strip_suffix("_mut") else { let Some(name) = name.strip_suffix("_mut") else {
return; return;
}; };
name name
}, },
ImplicitSelfKind::Imm | ImplicitSelfKind::Mut | ImplicitSelfKind::ImmRef => name, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut | ImplicitSelfKind::RefImm => name,
ImplicitSelfKind::None => return, ImplicitSelfKind::None => return,
}; };

View File

@ -97,7 +97,7 @@ fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap<hir
value_hir_id, value_hir_id,
ident, ident,
sub_pat, sub_pat,
) = pat.kind ) = pat.kind && by_ref != hir::ByRef::Yes(hir::Mutability::Mut)
{ {
// This block catches bindings with sub patterns. It would be hard to build a correct suggestion // This block catches bindings with sub patterns. It would be hard to build a correct suggestion
// for them and it's likely that the user knows what they are doing in such a case. // for them and it's likely that the user knows what they are doing in such a case.
@ -115,7 +115,7 @@ fn find_slice_values(cx: &LateContext<'_>, pat: &hir::Pat<'_>) -> FxIndexMap<hir
if let ty::Slice(inner_ty) | ty::Array(inner_ty, _) = bound_ty.peel_refs().kind() { if let ty::Slice(inner_ty) | ty::Array(inner_ty, _) = bound_ty.peel_refs().kind() {
// The values need to use the `ref` keyword if they can't be copied. // The values need to use the `ref` keyword if they can't be copied.
// This will need to be adjusted if the lint want to support mutable access in the future // This will need to be adjusted if the lint want to support mutable access in the future
let src_is_ref = bound_ty.is_ref() && by_ref != hir::ByRef::Yes; let src_is_ref = bound_ty.is_ref() && by_ref == hir::ByRef::No;
let needs_ref = !(src_is_ref || is_copy(cx, *inner_ty)); let needs_ref = !(src_is_ref || is_copy(cx, *inner_ty));
let slice_info = slices let slice_info = slices

View File

@ -216,8 +216,8 @@ fn {expected_method_name}({ref_self}self) -> {iter_ty} {{
fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) { fn check_impl_item(&mut self, cx: &LateContext<'_>, item: &rustc_hir::ImplItem<'_>) {
let item_did = item.owner_id.to_def_id(); let item_did = item.owner_id.to_def_id();
let (borrow_prefix, expected_implicit_self) = match item.ident.name { let (borrow_prefix, expected_implicit_self) = match item.ident.name {
sym::iter => ("&", ImplicitSelfKind::ImmRef), sym::iter => ("&", ImplicitSelfKind::RefImm),
sym::iter_mut => ("&mut ", ImplicitSelfKind::MutRef), sym::iter_mut => ("&mut ", ImplicitSelfKind::RefMut),
_ => return, _ => return,
}; };

View File

@ -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 { fn expected_sig(self, self_kind: ImplicitSelfKind) -> String {
let self_ref = match self_kind { let self_ref = match self_kind {
ImplicitSelfKind::ImmRef => "&", ImplicitSelfKind::RefImm => "&",
ImplicitSelfKind::MutRef => "&mut ", ImplicitSelfKind::RefMut => "&mut ",
_ => "", _ => "",
}; };
match self { match self {
@ -411,8 +411,8 @@ fn check_is_empty_sig<'tcx>(
[arg, res] if len_output.matches_is_empty_output(cx, *res) => { [arg, res] if len_output.matches_is_empty_output(cx, *res) => {
matches!( matches!(
(arg.kind(), self_kind), (arg.kind(), self_kind),
(ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::ImmRef) (ty::Ref(_, _, Mutability::Not), ImplicitSelfKind::RefImm)
| (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::MutRef) | (ty::Ref(_, _, Mutability::Mut), ImplicitSelfKind::RefMut)
) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut)) ) || (!arg.is_ref() && matches!(self_kind, ImplicitSelfKind::Imm | ImplicitSelfKind::Mut))
}, },
_ => false, _ => false,

View File

@ -2,7 +2,7 @@
use clippy_utils::source::snippet_with_applicability; use clippy_utils::source::snippet_with_applicability;
use clippy_utils::{path_to_local_id, peel_blocks, strip_pat_refs}; use clippy_utils::{path_to_local_id, peel_blocks, strip_pat_refs};
use rustc_errors::Applicability; 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 rustc_lint::LateContext;
use super::INFALLIBLE_DESTRUCTURING_MATCH; use super::INFALLIBLE_DESTRUCTURING_MATCH;
@ -30,7 +30,7 @@ pub(crate) fn check(cx: &LateContext<'_>, local: &LetStmt<'_>) -> bool {
format!( format!(
"let {}({}{}) = {};", "let {}({}{}) = {};",
snippet_with_applicability(cx, variant_name.span, "..", &mut applicability), 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, local.pat.span, "..", &mut applicability),
snippet_with_applicability(cx, target.span, "..", &mut applicability), snippet_with_applicability(cx, target.span, "..", &mut applicability),
), ),

View File

@ -67,7 +67,7 @@ fn is_none_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> bool {
fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> { fn is_ref_some_arm(cx: &LateContext<'_>, arm: &Arm<'_>) -> Option<Mutability> {
if let PatKind::TupleStruct(ref qpath, [first_pat, ..], _) = arm.pat.kind 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) && 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 && let ExprKind::Call(e, [arg]) = peel_blocks(arm.body).kind
&& is_res_lang_ctor(cx, path_res(cx, e), LangItem::OptionSome) && is_res_lang_ctor(cx, path_res(cx, e), LangItem::OptionSome)
&& let ExprKind::Path(QPath::Resolved(_, path2)) = arg.kind && let ExprKind::Path(QPath::Resolved(_, path2)) = arg.kind

View File

@ -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` // Example: `Custom::TypeA => Custom::TypeB`, or `None => None`
(PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => { (PatKind::Path(QPath::Resolved(_, p_path)), ExprKind::Path(QPath::Resolved(_, e_path))) => {

View File

@ -182,7 +182,7 @@ fn get_pat_binding<'tcx>(
if let PatKind::Binding(bind_annot, hir_id, ident, _) = pat.kind if let PatKind::Binding(bind_annot, hir_id, ident, _) = pat.kind
&& hir_id == local && 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); let _ = byref_ident.insert(ident);
} }
// the second call of `replace()` returns a `Some(span)`, meaning a multi-binding pattern // the second call of `replace()` returns a `Some(span)`, meaning a multi-binding pattern

View File

@ -69,7 +69,7 @@ pub(super) fn check(
_ => false, _ => false,
}, },
// local binding capturing a reference // 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; return;
}, },
_ => false, _ => false,

View File

@ -60,8 +60,6 @@ pub(super) fn check<'tcx>(
applicability, applicability,
); );
} else { } 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( span_lint_and_sugg(
cx, cx,
ITER_KV_MAP, ITER_KV_MAP,
@ -69,7 +67,8 @@ pub(super) fn check<'tcx>(
&format!("iterating on a map's {replacement_kind}s"), &format!("iterating on a map's {replacement_kind}s"),
"try", "try",
format!( 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) snippet_with_applicability(cx, body_expr.span, "/* body */", &mut applicability)
), ),
applicability, applicability,

View File

@ -129,7 +129,7 @@ fn check_fn(
if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) { if !is_lint_allowed(cx, REF_PATTERNS, arg.pat.hir_id) {
return; return;
} }
if let PatKind::Binding(BindingAnnotation(ByRef::Yes, _), ..) = arg.pat.kind { if let PatKind::Binding(BindingAnnotation(ByRef::Yes(_), _), ..) = arg.pat.kind {
span_lint( span_lint(
cx, cx,
TOPLEVEL_REF_ARG, TOPLEVEL_REF_ARG,
@ -144,7 +144,7 @@ fn check_fn(
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) { fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
if !in_external_macro(cx.tcx.sess, stmt.span) if !in_external_macro(cx.tcx.sess, stmt.span)
&& let StmtKind::Let(local) = stmt.kind && 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 && let Some(init) = local.init
// Do not emit if clippy::ref_patterns is not allowed to avoid having two lints for the same issue. // 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) && is_lint_allowed(cx, REF_PATTERNS, local.pat.hir_id)

View File

@ -14,7 +14,7 @@
use rustc_hir::def::Res; use rustc_hir::def::Res;
use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::LangItem::{self, OptionNone, OptionSome, ResultErr, ResultOk};
use rustc_hir::{ 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_lint::{LateContext, LateLintPass};
use rustc_middle::ty::Ty; 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 mut applicability = Applicability::MachineApplicable;
let receiver_str = snippet_with_applicability(cx, let_expr.span, "..", &mut applicability); 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 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!( let sugg = format!(
"{receiver_str}{}?{}", "{receiver_str}{method_call_str}?{}",
if by_ref == ByRef::Yes { ".as_ref()" } else { "" },
if requires_semi { ";" } else { "" } if requires_semi { ";" } else { "" }
); );
span_lint_and_sugg( span_lint_and_sugg(

View File

@ -649,6 +649,8 @@ macro_rules! kind {
BindingAnnotation::REF => "REF", BindingAnnotation::REF => "REF",
BindingAnnotation::MUT => "MUT", BindingAnnotation::MUT => "MUT",
BindingAnnotation::REF_MUT => "REF_MUT", BindingAnnotation::REF_MUT => "REF_MUT",
BindingAnnotation::MUT_REF => "MUT_REF",
BindingAnnotation::MUT_REF_MUT => "MUT_REF_MUT",
}; };
kind!("Binding(BindingAnnotation::{ann}, _, {name}, {sub})"); kind!("Binding(BindingAnnotation::{ann}, _, {name}, {sub})");
self.ident(name); self.ident(name);

View File

@ -97,7 +97,7 @@
use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor};
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk}; use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
use rustc_hir::{ 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, ExprField, ExprKind, FnDecl, FnRetTy, GenericArgs, HirId, Impl, ImplItem, ImplItemKind, ImplItemRef, Item,
ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, PathSegment, PrimTy, ItemKind, LangItem, LetStmt, MatchSource, Mutability, Node, OwnerId, Param, Pat, PatKind, Path, PathSegment, PrimTy,
QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitItemRef, TraitRef, TyKind, UnOp,
@ -107,7 +107,6 @@
use rustc_middle::hir::place::PlaceBase; use rustc_middle::hir::place::PlaceBase;
use rustc_middle::mir::Const; use rustc_middle::mir::Const;
use rustc_middle::ty::adjustment::{Adjust, Adjustment, AutoBorrow}; 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::fast_reject::SimplifiedType;
use rustc_middle::ty::layout::IntegerExt; use rustc_middle::ty::layout::IntegerExt;
use rustc_middle::ty::{ use rustc_middle::ty::{
@ -1006,11 +1005,12 @@ fn pat_capture_kind(cx: &LateContext<'_>, pat: &Pat<'_>) -> CaptureKind {
.typeck_results() .typeck_results()
.extract_binding_mode(cx.sess(), id, span) .extract_binding_mode(cx.sess(), id, span)
.unwrap() .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; capture = CaptureKind::Value;
}, },
BindingMode::BindByReference(Mutability::Mut) if capture != CaptureKind::Value => { ByRef::Yes(Mutability::Mut) if capture != CaptureKind::Value => {
capture = CaptureKind::Ref(Mutability::Mut); capture = CaptureKind::Ref(Mutability::Mut);
}, },
_ => (), _ => (),
@ -2035,7 +2035,7 @@ fn check_pat(cx: &LateContext<'_>, pat: &Pat<'_>, expr: &Expr<'_>) -> bool {
.typeck_results() .typeck_results()
.pat_binding_modes() .pat_binding_modes()
.get(pat.hir_id) .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, // 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 // the inner patterns become references. Don't consider this the identity function