Skip to content

Commit 4438b32

Browse files
estebankestebank
committed
review comments and make test run-rustfix 
1 parent ec98df4 commit 4438b32

4 files changed

+100
-84
lines changed

‎compiler/rustc_hir_typeck/src/demand.rs

+64-74
Original file line numberDiff line numberDiff line change
@@ -822,82 +822,72 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
822822
expr: &hir::Expr<'_>,
823823
error: Option<TypeError<'tcx>>,
824824
) -> bool {
825-
let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error else { return false };
826-
let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind() else { return false };
827-
if !self.can_eq(self.param_env, *inner, found) {
828-
// The difference between the expected and found values isn't one level of borrowing.
829-
return false;
830-
}
831-
// We have an `ident = expr;` assignment.
832-
let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) =
833-
self.tcx.parent_hir_node(expr.hir_id)
834-
else {
835-
return false;
836-
};
837-
if rhs.hir_id != expr.hir_id || expected.is_closure() {
838-
return false;
839-
}
840-
// We are assigning to some binding.
841-
let hir::ExprKind::Path(hir::QPath::Resolved(
842-
None,
843-
hir::Path { res: hir::def::Res::Local(hir_id), .. },
844-
)) = lhs.kind
845-
else {
846-
return false;
847-
};
848-
let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id) else { return false };
849-
// The pattern we have is an fn argument.
850-
let hir::Node::Param(hir::Param { ty_span, .. }) = self.tcx.parent_hir_node(pat.hir_id)
851-
else {
852-
return false;
853-
};
854-
let item = self.tcx.hir().get_parent_item(pat.hir_id);
855-
let item = self.tcx.hir_owner_node(item);
856-
let Some(fn_decl) = item.fn_decl() else { return false };
825+
if let Some(TypeError::Sorts(ExpectedFound { expected, found })) = error
826+
&& let ty::Ref(_, inner, hir::Mutability::Not) = expected.kind()
857827

858-
// We have a mutable binding in the argument.
859-
let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind else {
860-
return false;
861-
};
828+
// The difference between the expected and found values is one level of borrowing.
829+
&& self.can_eq(self.param_env, *inner, found)
862830

863-
// Look for the type corresponding to the argument pattern we have in the argument list.
864-
let Some(ty_sugg) = fn_decl
865-
.inputs
866-
.iter()
867-
.filter_map(|ty| {
868-
if ty.span == *ty_span
869-
&& let hir::TyKind::Ref(lt, mut_ty) = ty.kind
870-
{
871-
// `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty`
872-
Some((
873-
mut_ty.ty.span.shrink_to_lo(),
874-
format!(
875-
"{}mut ",
876-
if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " }
877-
),
878-
))
879-
} else {
880-
None
881-
}
882-
})
883-
.next()
884-
else {
885-
return false;
886-
};
887-
let sugg = vec![
888-
ty_sugg,
889-
(pat.span.until(ident.span), String::new()),
890-
(lhs.span.shrink_to_lo(), "*".to_string()),
891-
];
892-
// We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the
893-
// assignment from `ident = val;` to `*ident = val;`.
894-
err.multipart_suggestion_verbose(
895-
"you might have meant to mutate the pointed at value being passed in, instead of \
896-
changing the reference in the local binding",
897-
sugg,
898-
Applicability::MaybeIncorrect,
899-
);
900-
return true;
831+
// We have an `ident = expr;` assignment.
832+
&& let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::Assign(lhs, rhs, _), .. }) =
833+
self.tcx.parent_hir_node(expr.hir_id)
834+
&& rhs.hir_id == expr.hir_id
835+
836+
// We are assigning to some binding.
837+
&& let hir::ExprKind::Path(hir::QPath::Resolved(
838+
None,
839+
hir::Path { res: hir::def::Res::Local(hir_id), .. },
840+
)) = lhs.kind
841+
&& let hir::Node::Pat(pat) = self.tcx.hir_node(*hir_id)
842+
843+
// The pattern we have is an fn argument.
844+
&& let hir::Node::Param(hir::Param { ty_span, .. }) =
845+
self.tcx.parent_hir_node(pat.hir_id)
846+
&& let item = self.tcx.hir().get_parent_item(pat.hir_id)
847+
&& let item = self.tcx.hir_owner_node(item)
848+
&& let Some(fn_decl) = item.fn_decl()
849+
850+
// We have a mutable binding in the argument.
851+
&& let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind
852+
853+
// Look for the type corresponding to the argument pattern we have in the argument list.
854+
&& let Some(ty_sugg) = fn_decl
855+
.inputs
856+
.iter()
857+
.filter_map(|ty| {
858+
if ty.span == *ty_span
859+
&& let hir::TyKind::Ref(lt, x) = ty.kind
860+
{
861+
// `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty`
862+
Some((
863+
x.ty.span.shrink_to_lo(),
864+
format!(
865+
"{}mut ",
866+
if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " }
867+
),
868+
))
869+
} else {
870+
None
871+
}
872+
})
873+
.next()
874+
{
875+
let sugg = vec![
876+
ty_sugg,
877+
(pat.span.until(ident.span), String::new()),
878+
(lhs.span.shrink_to_lo(), "*".to_string()),
879+
];
880+
// We suggest changing the argument from `mut ident: &Ty` to `ident: &'_ mut Ty` and the
881+
// assignment from `ident = val;` to `*ident = val;`.
882+
err.multipart_suggestion_verbose(
883+
"you might have meant to mutate the pointed at value being passed in, instead of \
884+
changing the reference in the local binding",
885+
sugg,
886+
Applicability::MaybeIncorrect,
887+
);
888+
return true;
889+
}
890+
false
901891
}
902892

903893
fn annotate_alternative_method_deref(

‎tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.fixed

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//@ run-rustfix
2+
#![deny(unused_assignments, unused_variables)]
3+
struct Object;
4+
5+
fn change_object(object: &mut Object) { //~ HELP you might have meant to mutate
6+
let object2 = Object;
7+
*object = object2; //~ ERROR mismatched types
8+
}
9+
10+
fn change_object2(object: &mut Object) { //~ ERROR variable `object` is assigned to, but never used
11+
//~^ HELP you might have meant to mutate
12+
let object2 = Object;
13+
*object = object2;
14+
//~^ ERROR `object2` does not live long enough
15+
//~| ERROR value assigned to `object` is never read
16+
}
17+
18+
fn main() {
19+
let mut object = Object;
20+
change_object(&mut object);
21+
change_object2(&mut object);
22+
}

‎tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,22 @@
1+
//@ run-rustfix
12
#![deny(unused_assignments, unused_variables)]
23
struct Object;
34

4-
fn change_object(mut object: &Object) {
5+
fn change_object(mut object: &Object) { //~ HELP you might have meant to mutate
56
let object2 = Object;
67
object = object2; //~ ERROR mismatched types
78
}
89

910
fn change_object2(mut object: &Object) { //~ ERROR variable `object` is assigned to, but never used
11+
//~^ HELP you might have meant to mutate
1012
let object2 = Object;
1113
object = &object2;
1214
//~^ ERROR `object2` does not live long enough
1315
//~| ERROR value assigned to `object` is never read
1416
}
1517

1618
fn main() {
17-
let object = Object;
18-
change_object(&object);
19-
change_object2(&object);
19+
let mut object = Object;
20+
change_object(&mut object);
21+
change_object2(&mut object);
2022
}

‎tests/ui/fn/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.stderr

+8-6
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error[E0308]: mismatched types
2-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:6:14
2+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:7:14
33
|
44
LL | fn change_object(mut object: &Object) {
55
| ------- expected due to this parameter type
@@ -15,41 +15,43 @@ LL ~ *object = object2;
1515
|
1616

1717
error: value assigned to `object` is never read
18-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5
18+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13:5
1919
|
2020
LL | object = &object2;
2121
| ^^^^^^
2222
|
2323
note: the lint level is defined here
24-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9
24+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2:9
2525
|
2626
LL | #![deny(unused_assignments, unused_variables)]
2727
| ^^^^^^^^^^^^^^^^^^
2828
help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding
2929
|
3030
LL ~ fn change_object2(object: &mut Object) {
31+
LL |
3132
LL | let object2 = Object;
3233
LL ~ *object = object2;
3334
|
3435

3536
error: variable `object` is assigned to, but never used
36-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23
37+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:10:23
3738
|
3839
LL | fn change_object2(mut object: &Object) {
3940
| ^^^^^^
4041
|
4142
= note: consider using `_object` instead
4243
note: the lint level is defined here
43-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:29
44+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:2:29
4445
|
4546
LL | #![deny(unused_assignments, unused_variables)]
4647
| ^^^^^^^^^^^^^^^^
4748

4849
error[E0597]: `object2` does not live long enough
49-
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:14
50+
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:13:14
5051
|
5152
LL | fn change_object2(mut object: &Object) {
5253
| - let's call the lifetime of this reference `'1`
54+
LL |
5355
LL | let object2 = Object;
5456
| ------- binding `object2` declared here
5557
LL | object = &object2;

0 commit comments

Comments
 (0)