Skip to content

Commit b6a1b9e

Browse files
committed
Fix unused_variables typo suggestions for const patterns
1 parent 5b686bc commit b6a1b9e

4 files changed

Lines changed: 288 additions & 4 deletions

File tree

compiler/rustc_mir_transform/src/liveness.rs

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -168,13 +168,15 @@ fn is_capture(place: PlaceRef<'_>) -> bool {
168168
}
169169
}
170170

171-
/// Give a diagnostic when an unused variable may be a typo of a unit variant or a struct.
171+
/// Give a diagnostic when an unused variable may be a typo of a unit variant,
172+
/// unit struct, or const.
172173
fn maybe_suggest_unit_pattern_typo<'tcx>(
173174
tcx: TyCtxt<'tcx>,
174175
body_def_id: DefId,
175176
name: Symbol,
176177
span: Span,
177178
ty: Ty<'tcx>,
179+
allow_consts: bool,
178180
) -> Option<errors::PatternTypo> {
179181
if let ty::Adt(adt_def, _) = ty.peel_refs().kind() {
180182
let variant_names: Vec<_> = adt_def
@@ -191,13 +193,17 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
191193
{
192194
return Some(errors::PatternTypo {
193195
span,
194-
code: with_no_trimmed_paths!(tcx.def_path_str(variant.def_id)),
196+
code: pattern_item_path(tcx, body_def_id, variant.def_id),
195197
kind: tcx.def_descr(variant.def_id),
196198
item_name: variant.name,
197199
});
198200
}
199201
}
200202

203+
if !allow_consts {
204+
return None;
205+
}
206+
201207
// Look for consts of the same type with similar names as well,
202208
// not just unit structs and variants.
203209
let constants = tcx
@@ -215,7 +221,7 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
215221
{
216222
return Some(errors::PatternTypo {
217223
span,
218-
code: with_no_trimmed_paths!(tcx.def_path_str(def_id)),
224+
code: pattern_item_path(tcx, body_def_id, def_id.to_def_id()),
219225
kind: "constant",
220226
item_name,
221227
});
@@ -224,6 +230,64 @@ fn maybe_suggest_unit_pattern_typo<'tcx>(
224230
None
225231
}
226232

233+
fn pattern_item_path(tcx: TyCtxt<'_>, body_def_id: DefId, item_def_id: DefId) -> String {
234+
if is_nested_in(tcx, item_def_id, body_def_id) {
235+
return relative_item_path(tcx, body_def_id, item_def_id);
236+
}
237+
238+
let body_mod = nearest_parent_module(tcx, body_def_id);
239+
if body_mod == nearest_parent_module(tcx, item_def_id) {
240+
relative_item_path(tcx, body_mod, item_def_id)
241+
} else if item_def_id.is_local() && !body_mod.is_top_level_module() {
242+
format!("{}::{}", kw::Crate, with_no_trimmed_paths!(tcx.def_path_str(item_def_id)))
243+
} else {
244+
with_no_trimmed_paths!(tcx.def_path_str(item_def_id))
245+
}
246+
}
247+
248+
fn is_nested_in(tcx: TyCtxt<'_>, mut def_id: DefId, parent_def_id: DefId) -> bool {
249+
while let Some(parent) = tcx.opt_parent(def_id) {
250+
if parent == parent_def_id {
251+
return true;
252+
}
253+
def_id = parent;
254+
}
255+
256+
false
257+
}
258+
259+
fn relative_item_path(tcx: TyCtxt<'_>, parent_def_id: DefId, item_def_id: DefId) -> String {
260+
let mut def_id = item_def_id;
261+
let mut path = Vec::new();
262+
263+
loop {
264+
if def_id == parent_def_id {
265+
break;
266+
}
267+
if let Some(name) = tcx.opt_item_name(def_id) {
268+
path.push(name.to_string());
269+
}
270+
let Some(parent) = tcx.opt_parent(def_id) else {
271+
return with_no_trimmed_paths!(tcx.def_path_str(item_def_id));
272+
};
273+
def_id = parent;
274+
}
275+
276+
path.reverse();
277+
path.join("::")
278+
}
279+
280+
fn nearest_parent_module(tcx: TyCtxt<'_>, mut def_id: DefId) -> DefId {
281+
while let Some(parent) = tcx.opt_parent(def_id) {
282+
def_id = parent;
283+
if matches!(tcx.def_kind(def_id), DefKind::Mod) {
284+
return def_id;
285+
}
286+
}
287+
288+
def_id
289+
}
290+
227291
/// Return whether we should consider the current place as a drop guard and skip reporting.
228292
fn maybe_drop_guard<'tcx>(
229293
tcx: TyCtxt<'tcx>,
@@ -977,15 +1041,19 @@ impl<'a, 'tcx> AssignmentResult<'a, 'tcx> {
9771041
&& introductions.iter().any(|intro| intro.span.eq_ctxt(def_span));
9781042

9791043
let maybe_suggest_typo = || {
980-
if let LocalKind::Arg = self.body.local_kind(local) {
1044+
if matches!(self.body.local_kind(local), LocalKind::Arg) {
9811045
None
9821046
} else {
1047+
// In a bare `let x = ...`, replacing `x` with a const path would make the
1048+
// pattern refutable. Keep the existing unit ADT typo behavior unchanged.
1049+
let allow_consts = !matches!(binding.opt_match_place, Some((None, _)));
9831050
maybe_suggest_unit_pattern_typo(
9841051
tcx,
9851052
self.body.source.def_id(),
9861053
name,
9871054
def_span,
9881055
decl.ty,
1056+
allow_consts,
9891057
)
9901058
}
9911059
};
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
error: unused variable: `x`
2+
--> $DIR/unused-variables-const-pattern-typo.rs:17:18
3+
|
4+
LL | let Some(x) = x else { return };
5+
| ^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused-variables-const-pattern-typo.rs:9:9
9+
|
10+
LL | #![deny(unused_variables)]
11+
| ^^^^^^^^^^^^^^^^
12+
help: you might have meant to pattern match on the similarly named constant `X`
13+
|
14+
LL - let Some(x) = x else { return };
15+
LL + let Some(crate::enclosed::X) = x else { return };
16+
|
17+
help: if this is intentional, prefix it with an underscore
18+
|
19+
LL | let Some(_x) = x else { return };
20+
| +
21+
22+
error: unused variable: `x`
23+
--> $DIR/unused-variables-const-pattern-typo.rs:25:13
24+
|
25+
LL | let x = system::Y;
26+
| ^ help: if this is intentional, prefix it with an underscore: `_x`
27+
28+
error: unused variable: `good`
29+
--> $DIR/unused-variables-const-pattern-typo.rs:34:18
30+
|
31+
LL | let Some(good) = x else { return };
32+
| ^^^^
33+
|
34+
help: you might have meant to pattern match on the similarly named constant `GOOD`
35+
|
36+
LL - let Some(good) = x else { return };
37+
LL + let Some(GOOD) = x else { return };
38+
|
39+
help: if this is intentional, prefix it with an underscore
40+
|
41+
LL | let Some(_good) = x else { return };
42+
| +
43+
44+
error: unused variable: `local`
45+
--> $DIR/unused-variables-const-pattern-typo.rs:41:14
46+
|
47+
LL | let Some(local) = x else { return };
48+
| ^^^^^
49+
|
50+
help: you might have meant to pattern match on the similarly named constant `LOCAL`
51+
|
52+
LL - let Some(local) = x else { return };
53+
LL + let Some(LOCAL) = x else { return };
54+
|
55+
help: if this is intentional, prefix it with an underscore
56+
|
57+
LL | let Some(_local) = x else { return };
58+
| +
59+
60+
error: unused variable: `ready`
61+
--> $DIR/unused-variables-const-pattern-typo.rs:53:18
62+
|
63+
LL | let Some(ready) = x else { return };
64+
| ^^^^^
65+
|
66+
help: you might have meant to pattern match on the similarly named variant `Ready`
67+
|
68+
LL - let Some(ready) = x else { return };
69+
LL + let Some(crate::variants::State::Ready) = x else { return };
70+
|
71+
help: if this is intentional, prefix it with an underscore
72+
|
73+
LL | let Some(_ready) = x else { return };
74+
| +
75+
76+
error: aborting due to 5 previous errors
77+
Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
error: unused variable: `x`
2+
--> $DIR/unused-variables-const-pattern-typo.rs:17:18
3+
|
4+
LL | let Some(x) = x else { return };
5+
| ^
6+
|
7+
note: the lint level is defined here
8+
--> $DIR/unused-variables-const-pattern-typo.rs:9:9
9+
|
10+
LL | #![deny(unused_variables)]
11+
| ^^^^^^^^^^^^^^^^
12+
help: you might have meant to pattern match on the similarly named constant `X`
13+
|
14+
LL - let Some(x) = x else { return };
15+
LL + let Some(crate::enclosed::X) = x else { return };
16+
|
17+
help: if this is intentional, prefix it with an underscore
18+
|
19+
LL | let Some(_x) = x else { return };
20+
| +
21+
22+
error: unused variable: `x`
23+
--> $DIR/unused-variables-const-pattern-typo.rs:25:13
24+
|
25+
LL | let x = system::Y;
26+
| ^ help: if this is intentional, prefix it with an underscore: `_x`
27+
28+
error: unused variable: `good`
29+
--> $DIR/unused-variables-const-pattern-typo.rs:34:18
30+
|
31+
LL | let Some(good) = x else { return };
32+
| ^^^^
33+
|
34+
help: you might have meant to pattern match on the similarly named constant `GOOD`
35+
|
36+
LL - let Some(good) = x else { return };
37+
LL + let Some(GOOD) = x else { return };
38+
|
39+
help: if this is intentional, prefix it with an underscore
40+
|
41+
LL | let Some(_good) = x else { return };
42+
| +
43+
44+
error: unused variable: `local`
45+
--> $DIR/unused-variables-const-pattern-typo.rs:41:14
46+
|
47+
LL | let Some(local) = x else { return };
48+
| ^^^^^
49+
|
50+
help: you might have meant to pattern match on the similarly named constant `LOCAL`
51+
|
52+
LL - let Some(local) = x else { return };
53+
LL + let Some(LOCAL) = x else { return };
54+
|
55+
help: if this is intentional, prefix it with an underscore
56+
|
57+
LL | let Some(_local) = x else { return };
58+
| +
59+
60+
error: unused variable: `ready`
61+
--> $DIR/unused-variables-const-pattern-typo.rs:53:18
62+
|
63+
LL | let Some(ready) = x else { return };
64+
| ^^^^^
65+
|
66+
help: you might have meant to pattern match on the similarly named variant `Ready`
67+
|
68+
LL - let Some(ready) = x else { return };
69+
LL + let Some(crate::variants::State::Ready) = x else { return };
70+
|
71+
help: if this is intentional, prefix it with an underscore
72+
|
73+
LL | let Some(_ready) = x else { return };
74+
| +
75+
76+
error: aborting due to 5 previous errors
77+
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
//@ revisions: e2015 e2021
2+
//@[e2015] edition:2015
3+
//@[e2021] edition:2021
4+
5+
// Regression test for https://github.com/rust-lang/rust/issues/147595.
6+
// The `unused_variables` typo suggestion must not print a pattern path that
7+
// cannot be resolved from the binding site.
8+
9+
#![deny(unused_variables)]
10+
11+
mod enclosed {
12+
pub const X: i32 = 1;
13+
}
14+
15+
mod separated {
16+
pub fn let_else_path_qualification(x: Option<i32>) {
17+
let Some(x) = x else { return };
18+
//~^ ERROR unused variable: `x`
19+
}
20+
}
21+
22+
mod utils {
23+
pub fn simple_binding_with_imported_const() {
24+
use crate::system;
25+
let x = system::Y;
26+
//~^ ERROR unused variable: `x`
27+
}
28+
}
29+
30+
mod same_module {
31+
const GOOD: i32 = 0;
32+
33+
pub fn same_module_const_suggestion(x: Option<i32>) {
34+
let Some(good) = x else { return };
35+
//~^ ERROR unused variable: `good`
36+
}
37+
}
38+
39+
fn function_local_const_suggestion(x: Option<i32>) {
40+
const LOCAL: i32 = 0;
41+
let Some(local) = x else { return };
42+
//~^ ERROR unused variable: `local`
43+
}
44+
45+
mod variants {
46+
pub enum State {
47+
Ready,
48+
}
49+
}
50+
51+
mod separated_variant {
52+
pub fn cross_module_variant_path(x: Option<crate::variants::State>) {
53+
let Some(ready) = x else { return };
54+
//~^ ERROR unused variable: `ready`
55+
}
56+
}
57+
58+
mod system {
59+
pub const Y: u32 = 0;
60+
}
61+
62+
fn main() {}

0 commit comments

Comments
 (0)