Skip to content

Commit ab19d0a

Browse files
committed
Detect const in pattern with typo
When writing a constant name incorrectly in a pattern, the pattern will be identified as a new binding. We look for consts in the current crate, consts that where imported in the current crate and for local `let` bindings in case someone got them confused with `const`s. ``` error: unreachable pattern --> $DIR/const-with-typo-in-pattern-binding.rs:30:9 | LL | GOOOD => {} | ----- matches any value LL | LL | _ => {} | ^ no value can reach this | help: you might have meant to pattern match against the value of similarly named constant `GOOD` instead of introducing a new catch-all binding | LL | GOOD => {} | ~~~~ ``` Fix #132582.
1 parent 7c7bb7d commit ab19d0a

File tree

5 files changed

+315
-1
lines changed

5 files changed

+315
-1
lines changed

compiler/rustc_mir_build/messages.ftl

+7
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,13 @@ mir_build_unreachable_pattern = unreachable pattern
338338
.unreachable_covered_by_catchall = matches any value
339339
.unreachable_covered_by_one = matches all the relevant values
340340
.unreachable_covered_by_many = multiple earlier patterns match some of the same values
341+
.unreachable_pattern_const_reexport_accessible = there is a constant of the same name imported in another scope, which could have been used to pattern match against its value instead of introducing a new catch-all binding, but it needs to be imported in the pattern's scope
342+
.unreachable_pattern_wanted_const = you might have meant to pattern match against the value of {$is_typo ->
343+
[true] similarly named constant
344+
*[false] constant
345+
} `{$const_name}` instead of introducing a new catch-all binding
346+
.unreachable_pattern_const_inaccessible = there is a constant of the same name, which could have been used to pattern match against its value instead of introducing a new catch-all binding, but it is not accessible from this scope
347+
.unreachable_pattern_let_binding = there is a binding of the same name; if you meant to pattern match against the value of that binding, that is a feature of constants that is not available for `let` bindings
341348
.suggestion = remove the match arm
342349
343350
mir_build_unsafe_fn_safe_body = an unsafe function restricts its caller, but its body is safe by default

compiler/rustc_mir_build/src/errors.rs

+22
Original file line numberDiff line numberDiff line change
@@ -593,6 +593,14 @@ pub(crate) struct UnreachablePattern<'tcx> {
593593
pub(crate) uninhabited_note: Option<()>,
594594
#[label(mir_build_unreachable_covered_by_catchall)]
595595
pub(crate) covered_by_catchall: Option<Span>,
596+
#[subdiagnostic]
597+
pub(crate) wanted_constant: Option<WantedConstant>,
598+
#[note(mir_build_unreachable_pattern_const_reexport_accessible)]
599+
pub(crate) accessible_constant: Option<Span>,
600+
#[note(mir_build_unreachable_pattern_const_inaccessible)]
601+
pub(crate) inaccessible_constant: Option<Span>,
602+
#[note(mir_build_unreachable_pattern_let_binding)]
603+
pub(crate) pattern_let_binding: Option<Span>,
596604
#[label(mir_build_unreachable_covered_by_one)]
597605
pub(crate) covered_by_one: Option<Span>,
598606
#[note(mir_build_unreachable_covered_by_many)]
@@ -602,6 +610,20 @@ pub(crate) struct UnreachablePattern<'tcx> {
602610
pub(crate) suggest_remove: Option<Span>,
603611
}
604612

613+
#[derive(Subdiagnostic)]
614+
#[suggestion(
615+
mir_build_unreachable_pattern_wanted_const,
616+
code = "{const_path}",
617+
applicability = "machine-applicable"
618+
)]
619+
pub(crate) struct WantedConstant {
620+
#[primary_span]
621+
pub(crate) span: Span,
622+
pub(crate) is_typo: bool,
623+
pub(crate) const_name: String,
624+
pub(crate) const_path: String,
625+
}
626+
605627
#[derive(Diagnostic)]
606628
#[diag(mir_build_const_pattern_depends_on_generic_parameter, code = E0158)]
607629
pub(crate) struct ConstPatternDependsOnGenericParameter {

compiler/rustc_mir_build/src/thir/pattern/check_match.rs

+163-1
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan, struct_span_code_e
77
use rustc_hir::def::*;
88
use rustc_hir::def_id::LocalDefId;
99
use rustc_hir::{self as hir, BindingMode, ByRef, HirId};
10+
use rustc_infer::infer::TyCtxtInferExt;
11+
use rustc_lint::Level;
1012
use rustc_middle::bug;
1113
use rustc_middle::middle::limits::get_limit_size;
1214
use rustc_middle::thir::visit::Visitor;
@@ -21,8 +23,10 @@ use rustc_pattern_analysis::rustc::{
2123
use rustc_session::lint::builtin::{
2224
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
2325
};
26+
use rustc_span::edit_distance::find_best_match_for_name;
2427
use rustc_span::hygiene::DesugaringKind;
2528
use rustc_span::{Span, sym};
29+
use rustc_trait_selection::infer::InferCtxtExt;
2630
use tracing::instrument;
2731

2832
use crate::errors::*;
@@ -944,6 +948,10 @@ fn report_unreachable_pattern<'p, 'tcx>(
944948
covered_by_one: None,
945949
covered_by_many: None,
946950
covered_by_many_n_more_count: 0,
951+
wanted_constant: None,
952+
accessible_constant: None,
953+
inaccessible_constant: None,
954+
pattern_let_binding: None,
947955
suggest_remove: None,
948956
};
949957
match explanation.covered_by.as_slice() {
@@ -966,7 +974,10 @@ fn report_unreachable_pattern<'p, 'tcx>(
966974
});
967975
}
968976
[covering_pat] if pat_is_catchall(covering_pat) => {
969-
lint.covered_by_catchall = Some(covering_pat.data().span);
977+
// A binding pattern that matches all, a single binding name.
978+
let pat = covering_pat.data();
979+
lint.covered_by_catchall = Some(pat.span);
980+
find_fallback_pattern_typo(cx, hir_id, pat, &mut lint);
970981
}
971982
[covering_pat] => {
972983
lint.covered_by_one = Some(covering_pat.data().span);
@@ -999,6 +1010,157 @@ fn report_unreachable_pattern<'p, 'tcx>(
9991010
cx.tcx.emit_node_span_lint(UNREACHABLE_PATTERNS, hir_id, pat_span, lint);
10001011
}
10011012

1013+
/// Detect typos that were meant to be a `const` but were interpreted as a new pattern binding.
1014+
fn find_fallback_pattern_typo<'tcx>(
1015+
cx: &PatCtxt<'_, 'tcx>,
1016+
hir_id: HirId,
1017+
pat: &Pat<'tcx>,
1018+
lint: &mut UnreachablePattern<'_>,
1019+
) {
1020+
if let (Level::Allow, _) = cx.tcx.lint_level_at_node(UNREACHABLE_PATTERNS, hir_id) {
1021+
// This is because we use `with_no_trimmed_paths` later, so if we never emit the lint we'd
1022+
// ICE. At the same time, we don't really need to do all of this if we won't emit anything.
1023+
return;
1024+
}
1025+
if let PatKind::Binding { name, subpattern: None, ty, .. } = pat.kind {
1026+
// See if the binding might have been a `const` that was mistyped or out of scope.
1027+
let mut accessible = vec![];
1028+
let mut accessible_path = vec![];
1029+
let mut inaccessible = vec![];
1030+
let mut imported = vec![];
1031+
let mut imported_spans = vec![];
1032+
let infcx = cx.tcx.infer_ctxt().build(ty::TypingMode::non_body_analysis());
1033+
let parent = cx.tcx.hir().get_parent_item(hir_id);
1034+
1035+
for item in cx.tcx.hir_crate_items(()).free_items() {
1036+
if let DefKind::Use = cx.tcx.def_kind(item.owner_id) {
1037+
// Look for consts being re-exported.
1038+
let item = cx.tcx.hir().expect_item(item.owner_id.def_id);
1039+
let use_name = item.ident.name;
1040+
let hir::ItemKind::Use(path, _) = item.kind else {
1041+
continue;
1042+
};
1043+
for res in &path.res {
1044+
if let Res::Def(DefKind::Const, id) = res
1045+
&& infcx.can_eq(cx.param_env, ty, cx.tcx.type_of(id).instantiate_identity())
1046+
{
1047+
if cx.tcx.visibility(id).is_accessible_from(parent, cx.tcx) {
1048+
// The original const is accessible, suggest using it directly.
1049+
let item_name = cx.tcx.item_name(*id);
1050+
accessible.push(item_name);
1051+
accessible_path.push(with_no_trimmed_paths!(cx.tcx.def_path_str(id)));
1052+
} else if cx
1053+
.tcx
1054+
.visibility(item.owner_id)
1055+
.is_accessible_from(parent, cx.tcx)
1056+
{
1057+
// The const is accessible only through the re-export, point at
1058+
// the `use`.
1059+
imported.push(use_name);
1060+
imported_spans.push(item.ident.span);
1061+
}
1062+
}
1063+
}
1064+
}
1065+
if let DefKind::Const = cx.tcx.def_kind(item.owner_id)
1066+
&& infcx.can_eq(
1067+
cx.param_env,
1068+
ty,
1069+
cx.tcx.type_of(item.owner_id).instantiate_identity(),
1070+
)
1071+
{
1072+
// Look for local consts.
1073+
let item_name = cx.tcx.item_name(item.owner_id.into());
1074+
let vis = cx.tcx.visibility(item.owner_id);
1075+
if vis.is_accessible_from(parent, cx.tcx) {
1076+
accessible.push(item_name);
1077+
let path = if item_name == name {
1078+
// We know that the const wasn't in scope because it has the exact
1079+
// same name, so we suggest the full path.
1080+
with_no_trimmed_paths!(cx.tcx.def_path_str(item.owner_id))
1081+
} else {
1082+
// The const is likely just typoed, and nothing else.
1083+
cx.tcx.def_path_str(item.owner_id)
1084+
};
1085+
accessible_path.push(path);
1086+
} else if name == item_name {
1087+
// The const exists somewhere in this crate, but it can't be imported
1088+
// from this pattern's scope. We'll just point at its definition.
1089+
inaccessible.push(cx.tcx.def_span(item.owner_id));
1090+
}
1091+
}
1092+
}
1093+
if let Some((i, &const_name)) =
1094+
accessible.iter().enumerate().find(|(_, &const_name)| const_name == name)
1095+
{
1096+
// The pattern name is an exact match, so the pattern needed to be imported.
1097+
lint.wanted_constant = Some(WantedConstant {
1098+
span: pat.span,
1099+
is_typo: false,
1100+
const_name: const_name.to_string(),
1101+
const_path: accessible_path[i].clone(),
1102+
});
1103+
} else if let Some(name) = find_best_match_for_name(&accessible, name, None) {
1104+
// The pattern name is likely a typo.
1105+
lint.wanted_constant = Some(WantedConstant {
1106+
span: pat.span,
1107+
is_typo: true,
1108+
const_name: name.to_string(),
1109+
const_path: name.to_string(),
1110+
});
1111+
} else if let Some(i) =
1112+
imported.iter().enumerate().find(|(_, &const_name)| const_name == name).map(|(i, _)| i)
1113+
{
1114+
// The const with the exact name wasn't re-exported from an import in this
1115+
// crate, we point at the import.
1116+
lint.accessible_constant = Some(imported_spans[i]);
1117+
} else if let Some(name) = find_best_match_for_name(&imported, name, None) {
1118+
// The typoed const wasn't re-exported by an import in this crate, we suggest
1119+
// the right name (which will likely require another follow up suggestion).
1120+
lint.wanted_constant = Some(WantedConstant {
1121+
span: pat.span,
1122+
is_typo: true,
1123+
const_path: name.to_string(),
1124+
const_name: name.to_string(),
1125+
});
1126+
} else if !inaccessible.is_empty() {
1127+
for span in inaccessible {
1128+
// The const with the exact name match isn't accessible, we just point at it.
1129+
lint.inaccessible_constant = Some(span);
1130+
}
1131+
} else {
1132+
// Look for local bindings for people that might have gotten confused with how
1133+
// `let` and `const` works.
1134+
for (_, node) in cx.tcx.hir().parent_iter(hir_id) {
1135+
match node {
1136+
hir::Node::Stmt(hir::Stmt { kind: hir::StmtKind::Let(let_stmt), .. }) => {
1137+
if let hir::PatKind::Binding(_, _, binding_name, _) = let_stmt.pat.kind {
1138+
if name == binding_name.name {
1139+
lint.pattern_let_binding = Some(binding_name.span);
1140+
}
1141+
}
1142+
}
1143+
hir::Node::Block(hir::Block { stmts, .. }) => {
1144+
for stmt in *stmts {
1145+
if let hir::StmtKind::Let(let_stmt) = stmt.kind {
1146+
if let hir::PatKind::Binding(_, _, binding_name, _) =
1147+
let_stmt.pat.kind
1148+
{
1149+
if name == binding_name.name {
1150+
lint.pattern_let_binding = Some(binding_name.span);
1151+
}
1152+
}
1153+
}
1154+
}
1155+
}
1156+
hir::Node::Item(_) => break,
1157+
_ => {}
1158+
}
1159+
}
1160+
}
1161+
}
1162+
}
1163+
10021164
/// Report unreachable arms, if any.
10031165
fn report_arm_reachability<'p, 'tcx>(
10041166
cx: &PatCtxt<'p, 'tcx>,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
#![deny(unreachable_patterns)] //~ NOTE the lint level is defined here
2+
#![allow(non_snake_case, non_upper_case_globals)]
3+
mod x {
4+
pub use std::env::consts::ARCH;
5+
const X: i32 = 0; //~ NOTE there is a constant of the same name
6+
}
7+
fn main() {
8+
let input: i32 = 42;
9+
10+
const god: i32 = 1;
11+
const GOOD: i32 = 1;
12+
const BAD: i32 = 2;
13+
14+
let name: i32 = 42; //~ NOTE there is a binding of the same name
15+
16+
match input {
17+
X => {} //~ NOTE matches any value
18+
_ => {} //~ ERROR unreachable pattern
19+
//~^ NOTE no value can reach this
20+
}
21+
match input {
22+
GOD => {} //~ HELP you might have meant to pattern match against the value of similarly named constant `god`
23+
//~^ NOTE matches any value
24+
_ => {} //~ ERROR unreachable pattern
25+
//~^ NOTE no value can reach this
26+
}
27+
match input {
28+
GOOOD => {} //~ HELP you might have meant to pattern match against the value of similarly named constant `GOOD`
29+
//~^ NOTE matches any value
30+
_ => {} //~ ERROR unreachable pattern
31+
//~^ NOTE no value can reach this
32+
}
33+
match input {
34+
name => {}
35+
//~^ NOTE matches any value
36+
_ => {} //~ ERROR unreachable pattern
37+
//~^ NOTE no value can reach this
38+
}
39+
match "" {
40+
ARCH => {} //~ HELP you might have meant to pattern match against the value of constant `ARCH`
41+
//~^ NOTE matches any value
42+
_ => {} //~ ERROR unreachable pattern
43+
//~^ NOTE no value can reach this
44+
}
45+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
error: unreachable pattern
2+
--> $DIR/const-with-typo-in-pattern-binding.rs:18:9
3+
|
4+
LL | X => {}
5+
| - matches any value
6+
LL | _ => {}
7+
| ^ no value can reach this
8+
|
9+
note: there is a constant of the same name, which could have been used to pattern match against its value instead of introducing a new catch-all binding, but it is not accessible from this scope
10+
--> $DIR/const-with-typo-in-pattern-binding.rs:5:5
11+
|
12+
LL | const X: i32 = 0;
13+
| ^^^^^^^^^^^^
14+
note: the lint level is defined here
15+
--> $DIR/const-with-typo-in-pattern-binding.rs:1:9
16+
|
17+
LL | #![deny(unreachable_patterns)]
18+
| ^^^^^^^^^^^^^^^^^^^^
19+
20+
error: unreachable pattern
21+
--> $DIR/const-with-typo-in-pattern-binding.rs:24:9
22+
|
23+
LL | GOD => {}
24+
| --- matches any value
25+
LL |
26+
LL | _ => {}
27+
| ^ no value can reach this
28+
|
29+
help: you might have meant to pattern match against the value of similarly named constant `god` instead of introducing a new catch-all binding
30+
|
31+
LL | god => {}
32+
| ~~~
33+
34+
error: unreachable pattern
35+
--> $DIR/const-with-typo-in-pattern-binding.rs:30:9
36+
|
37+
LL | GOOOD => {}
38+
| ----- matches any value
39+
LL |
40+
LL | _ => {}
41+
| ^ no value can reach this
42+
|
43+
help: you might have meant to pattern match against the value of similarly named constant `GOOD` instead of introducing a new catch-all binding
44+
|
45+
LL | GOOD => {}
46+
| ~~~~
47+
48+
error: unreachable pattern
49+
--> $DIR/const-with-typo-in-pattern-binding.rs:36:9
50+
|
51+
LL | name => {}
52+
| ---- matches any value
53+
LL |
54+
LL | _ => {}
55+
| ^ no value can reach this
56+
|
57+
note: there is a binding of the same name; if you meant to pattern match against the value of that binding, that is a feature of constants that is not available for `let` bindings
58+
--> $DIR/const-with-typo-in-pattern-binding.rs:14:9
59+
|
60+
LL | let name: i32 = 42;
61+
| ^^^^
62+
63+
error: unreachable pattern
64+
--> $DIR/const-with-typo-in-pattern-binding.rs:42:9
65+
|
66+
LL | ARCH => {}
67+
| ---- matches any value
68+
LL |
69+
LL | _ => {}
70+
| ^ no value can reach this
71+
|
72+
help: you might have meant to pattern match against the value of constant `ARCH` instead of introducing a new catch-all binding
73+
|
74+
LL | std::env::consts::ARCH => {}
75+
| ~~~~~~~~~~~~~~~~~~~~~~
76+
77+
error: aborting due to 5 previous errors
78+

0 commit comments

Comments
 (0)