Skip to content

Commit ec98df4

Browse files
committed
On unused assign lint, detect mut arg: &Ty meant to be arg: &mut Ty
``` error: value assigned to `object` is never read --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:11:5 | LL | object = &object2; | ^^^^^^ | note: the lint level is defined here --> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9 | LL | #![deny(unused_assignments, unused_variables)] | ^^^^^^^^^^^^^^^^^^ help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding | LL ~ fn change_object2(object: &mut Object) { LL | let object2 = Object; LL ~ *object = object2; | ``` This might be the first thing someone tries to write to mutate the value *behind* an argument, trying to avoid an E0308.
1 parent c2ae386 commit ec98df4

File tree

4 files changed

+104
-6
lines changed

4 files changed

+104
-6
lines changed

compiler/rustc_passes/messages.ftl

+3
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,9 @@ passes_unused_assign = value assigned to `{$name}` is never read
799799
passes_unused_assign_passed = value passed to `{$name}` is never read
800800
.help = maybe it is overwritten before being read?
801801
802+
passes_unused_assign_suggestion =
803+
you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding
804+
802805
passes_unused_capture_maybe_capture_ref = value captured by `{$name}` is never read
803806
.help = did you mean to capture by reference instead?
804807

compiler/rustc_passes/src/errors.rs

+18-1
Original file line numberDiff line numberDiff line change
@@ -1787,9 +1787,26 @@ pub(crate) struct IneffectiveUnstableImpl;
17871787

17881788
#[derive(LintDiagnostic)]
17891789
#[diag(passes_unused_assign)]
1790-
#[help]
17911790
pub(crate) struct UnusedAssign {
17921791
pub name: String,
1792+
#[subdiagnostic]
1793+
pub suggestion: Option<UnusedAssignSuggestion>,
1794+
#[help]
1795+
pub help: bool,
1796+
}
1797+
1798+
#[derive(Subdiagnostic)]
1799+
#[multipart_suggestion(passes_unused_assign_suggestion, applicability = "maybe-incorrect")]
1800+
pub(crate) struct UnusedAssignSuggestion {
1801+
pub pre: &'static str,
1802+
#[suggestion_part(code = "{pre}mut ")]
1803+
pub ty_span: Span,
1804+
#[suggestion_part(code = "")]
1805+
pub ty_ref_span: Span,
1806+
#[suggestion_part(code = "*")]
1807+
pub ident_span: Span,
1808+
#[suggestion_part(code = "")]
1809+
pub expr_ref_span: Span,
17931810
}
17941811

17951812
#[derive(LintDiagnostic)]

compiler/rustc_passes/src/liveness.rs

+77-4
Original file line numberDiff line numberDiff line change
@@ -1360,7 +1360,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> {
13601360
fn visit_local(&mut self, local: &'tcx hir::LetStmt<'tcx>) {
13611361
self.check_unused_vars_in_pat(local.pat, None, None, |spans, hir_id, ln, var| {
13621362
if local.init.is_some() {
1363-
self.warn_about_dead_assign(spans, hir_id, ln, var);
1363+
self.warn_about_dead_assign(spans, hir_id, ln, var, None);
13641364
}
13651365
});
13661366

@@ -1460,7 +1460,8 @@ impl<'tcx> Liveness<'_, 'tcx> {
14601460
// as being used.
14611461
let ln = self.live_node(expr.hir_id, expr.span);
14621462
let var = self.variable(var_hid, expr.span);
1463-
self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var);
1463+
let sugg = self.annotate_mut_binding_to_immutable_binding(var_hid, expr);
1464+
self.warn_about_dead_assign(vec![expr.span], expr.hir_id, ln, var, sugg);
14641465
}
14651466
}
14661467
_ => {
@@ -1585,6 +1586,70 @@ impl<'tcx> Liveness<'_, 'tcx> {
15851586
}
15861587
}
15871588

1589+
/// Detect the following case
1590+
///
1591+
/// ```text
1592+
/// fn change_object(mut a: &Ty) {
1593+
/// let a = Ty::new();
1594+
/// b = &a;
1595+
/// }
1596+
/// ```
1597+
///
1598+
/// where the user likely meant to modify the value behind there reference, use `a` as an out
1599+
/// parameter, instead of mutating the local binding. When encountering this we suggest:
1600+
///
1601+
/// ```text
1602+
/// fn change_object(a: &'_ mut Ty) {
1603+
/// let a = Ty::new();
1604+
/// *b = a;
1605+
/// }
1606+
/// ```
1607+
fn annotate_mut_binding_to_immutable_binding(
1608+
&self,
1609+
var_hid: HirId,
1610+
expr: &'tcx Expr<'tcx>,
1611+
) -> Option<errors::UnusedAssignSuggestion> {
1612+
if let hir::Node::Expr(parent) = self.ir.tcx.parent_hir_node(expr.hir_id)
1613+
&& let hir::ExprKind::Assign(_, rhs, _) = parent.kind
1614+
&& let hir::ExprKind::AddrOf(borrow_kind, _mut, inner) = rhs.kind
1615+
&& let hir::BorrowKind::Ref = borrow_kind
1616+
&& let hir::Node::Pat(pat) = self.ir.tcx.hir_node(var_hid)
1617+
&& let hir::Node::Param(hir::Param { ty_span, .. }) =
1618+
self.ir.tcx.parent_hir_node(pat.hir_id)
1619+
&& let item_id = self.ir.tcx.hir().get_parent_item(pat.hir_id)
1620+
&& let item = self.ir.tcx.hir_owner_node(item_id)
1621+
&& let Some(fn_decl) = item.fn_decl()
1622+
&& let hir::PatKind::Binding(hir::BindingMode::MUT, _hir_id, ident, _) = pat.kind
1623+
&& let Some((ty_span, pre)) = fn_decl
1624+
.inputs
1625+
.iter()
1626+
.filter_map(|ty| {
1627+
if ty.span == *ty_span
1628+
&& let hir::TyKind::Ref(lt, mut_ty) = ty.kind
1629+
{
1630+
// `&'name Ty` -> `&'name mut Ty` or `&Ty` -> `&mut Ty`
1631+
Some((
1632+
mut_ty.ty.span.shrink_to_lo(),
1633+
if lt.ident.span.lo() == lt.ident.span.hi() { "" } else { " " },
1634+
))
1635+
} else {
1636+
None
1637+
}
1638+
})
1639+
.next()
1640+
{
1641+
Some(errors::UnusedAssignSuggestion {
1642+
ty_span,
1643+
pre,
1644+
ty_ref_span: pat.span.until(ident.span),
1645+
ident_span: expr.span.shrink_to_lo(),
1646+
expr_ref_span: rhs.span.until(inner.span),
1647+
})
1648+
} else {
1649+
None
1650+
}
1651+
}
1652+
15881653
#[instrument(skip(self), level = "INFO")]
15891654
fn report_unused(
15901655
&self,
@@ -1738,15 +1803,23 @@ impl<'tcx> Liveness<'_, 'tcx> {
17381803
suggs
17391804
}
17401805

1741-
fn warn_about_dead_assign(&self, spans: Vec<Span>, hir_id: HirId, ln: LiveNode, var: Variable) {
1806+
fn warn_about_dead_assign(
1807+
&self,
1808+
spans: Vec<Span>,
1809+
hir_id: HirId,
1810+
ln: LiveNode,
1811+
var: Variable,
1812+
suggestion: Option<errors::UnusedAssignSuggestion>,
1813+
) {
17421814
if !self.live_on_exit(ln, var)
17431815
&& let Some(name) = self.should_warn(var)
17441816
{
1817+
let help = suggestion.is_none();
17451818
self.ir.tcx.emit_node_span_lint(
17461819
lint::builtin::UNUSED_ASSIGNMENTS,
17471820
hir_id,
17481821
spans,
1749-
errors::UnusedAssign { name },
1822+
errors::UnusedAssign { name, suggestion, help },
17501823
);
17511824
}
17521825
}

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

+6-1
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,17 @@ error: value assigned to `object` is never read
2020
LL | object = &object2;
2121
| ^^^^^^
2222
|
23-
= help: maybe it is overwritten before being read?
2423
note: the lint level is defined here
2524
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:1:9
2625
|
2726
LL | #![deny(unused_assignments, unused_variables)]
2827
| ^^^^^^^^^^^^^^^^^^
28+
help: you might have meant to mutate the pointed at value being passed in, instead of changing the reference in the local binding
29+
|
30+
LL ~ fn change_object2(object: &mut Object) {
31+
LL | let object2 = Object;
32+
LL ~ *object = object2;
33+
|
2934

3035
error: variable `object` is assigned to, but never used
3136
--> $DIR/mut-arg-of-borrowed-type-meant-to-be-arg-of-mut-borrow.rs:9:23

0 commit comments

Comments
 (0)