Skip to content

Commit

Permalink
Detect const in pattern with typo
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
estebank committed Nov 20, 2024
1 parent bfe809d commit 2487765
Show file tree
Hide file tree
Showing 5 changed files with 315 additions and 1 deletion.
7 changes: 7 additions & 0 deletions compiler/rustc_mir_build/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,13 @@ mir_build_unreachable_pattern = unreachable pattern
.unreachable_covered_by_catchall = matches any value
.unreachable_covered_by_one = matches all the relevant values
.unreachable_covered_by_many = multiple earlier patterns match some of the same values
.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
.unreachable_pattern_wanted_const = you might have meant to pattern match against the value of {$is_typo ->
[true] similarly named constant
*[false] constant
} `{$const_name}` instead of introducing a new catch-all binding
.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
.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
.suggestion = remove the match arm
mir_build_unsafe_fn_safe_body = an unsafe function restricts its caller, but its body is safe by default
Expand Down
22 changes: 22 additions & 0 deletions compiler/rustc_mir_build/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,14 @@ pub(crate) struct UnreachablePattern<'tcx> {
pub(crate) uninhabited_note: Option<()>,
#[label(mir_build_unreachable_covered_by_catchall)]
pub(crate) covered_by_catchall: Option<Span>,
#[subdiagnostic]
pub(crate) wanted_constant: Option<WantedConstant>,
#[note(mir_build_unreachable_pattern_const_reexport_accessible)]
pub(crate) accessible_constant: Option<Span>,
#[note(mir_build_unreachable_pattern_const_inaccessible)]
pub(crate) inaccessible_constant: Option<Span>,
#[note(mir_build_unreachable_pattern_let_binding)]
pub(crate) pattern_let_binding: Option<Span>,
#[label(mir_build_unreachable_covered_by_one)]
pub(crate) covered_by_one: Option<Span>,
#[note(mir_build_unreachable_covered_by_many)]
Expand All @@ -602,6 +610,20 @@ pub(crate) struct UnreachablePattern<'tcx> {
pub(crate) suggest_remove: Option<Span>,
}

#[derive(Subdiagnostic)]
#[suggestion(
mir_build_unreachable_pattern_wanted_const,
code = "{const_path}",
applicability = "machine-applicable"
)]
pub(crate) struct WantedConstant {
#[primary_span]
pub(crate) span: Span,
pub(crate) is_typo: bool,
pub(crate) const_name: String,
pub(crate) const_path: String,
}

#[derive(Diagnostic)]
#[diag(mir_build_const_pattern_depends_on_generic_parameter, code = E0158)]
pub(crate) struct ConstPatternDependsOnGenericParameter {
Expand Down
164 changes: 163 additions & 1 deletion compiler/rustc_mir_build/src/thir/pattern/check_match.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ use rustc_errors::{Applicability, ErrorGuaranteed, MultiSpan, struct_span_code_e
use rustc_hir::def::*;
use rustc_hir::def_id::LocalDefId;
use rustc_hir::{self as hir, BindingMode, ByRef, HirId};
use rustc_infer::infer::TyCtxtInferExt;
use rustc_infer::traits::Reveal;
use rustc_lint::Level;
use rustc_middle::bug;
use rustc_middle::middle::limits::get_limit_size;
use rustc_middle::thir::visit::Visitor;
Expand All @@ -22,8 +24,10 @@ use rustc_pattern_analysis::rustc::{
use rustc_session::lint::builtin::{
BINDINGS_WITH_VARIANT_NAME, IRREFUTABLE_LET_PATTERNS, UNREACHABLE_PATTERNS,
};
use rustc_span::edit_distance::find_best_match_for_name;
use rustc_span::hygiene::DesugaringKind;
use rustc_span::{Span, sym};
use rustc_trait_selection::infer::InferCtxtExt;
use tracing::instrument;

use crate::errors::*;
Expand Down Expand Up @@ -954,6 +958,10 @@ fn report_unreachable_pattern<'p, 'tcx>(
covered_by_one: None,
covered_by_many: None,
covered_by_many_n_more_count: 0,
wanted_constant: None,
accessible_constant: None,
inaccessible_constant: None,
pattern_let_binding: None,
suggest_remove: None,
};
match explanation.covered_by.as_slice() {
Expand All @@ -976,7 +984,10 @@ fn report_unreachable_pattern<'p, 'tcx>(
});
}
[covering_pat] if pat_is_catchall(covering_pat) => {
lint.covered_by_catchall = Some(covering_pat.data().span);
// A binding pattern that matches all, a single binding name.
let pat = covering_pat.data();
lint.covered_by_catchall = Some(pat.span);
find_fallback_pattern_typo(cx, hir_id, pat, &mut lint);
}
[covering_pat] => {
lint.covered_by_one = Some(covering_pat.data().span);
Expand Down Expand Up @@ -1009,6 +1020,157 @@ fn report_unreachable_pattern<'p, 'tcx>(
cx.tcx.emit_node_span_lint(UNREACHABLE_PATTERNS, hir_id, pat_span, lint);
}

/// Detect typos that were meant to be a `const` but were interpreted as a new pattern binding.
fn find_fallback_pattern_typo<'tcx>(
cx: &PatCtxt<'_, 'tcx>,
hir_id: HirId,
pat: &Pat<'tcx>,
lint: &mut UnreachablePattern<'_>,
) {
if let (Level::Allow, _) = cx.tcx.lint_level_at_node(UNREACHABLE_PATTERNS, hir_id) {
// This is because we use `with_no_trimmed_paths` later, so if we never emit the lint we'd
// ICE. At the same time, we don't really need to do all of this if we won't emit anything.
return;
}
if let PatKind::Binding { name, subpattern: None, ty, .. } = pat.kind {
// See if the binding might have been a `const` that was mistyped or out of scope.
let mut accessible = vec![];
let mut accessible_path = vec![];
let mut inaccessible = vec![];
let mut imported = vec![];
let mut imported_spans = vec![];
let infcx = cx.tcx.infer_ctxt().build(ty::TypingMode::non_body_analysis());
let parent = cx.tcx.hir().get_parent_item(hir_id);

for item in cx.tcx.hir_crate_items(()).free_items() {
if let DefKind::Use = cx.tcx.def_kind(item.owner_id) {
// Look for consts being re-exported.
let item = cx.tcx.hir().expect_item(item.owner_id.def_id);
let use_name = item.ident.name;
let hir::ItemKind::Use(path, _) = item.kind else {
continue;
};
for res in &path.res {
if let Res::Def(DefKind::Const, id) = res
&& infcx.can_eq(cx.param_env, ty, cx.tcx.type_of(id).instantiate_identity())
{
if cx.tcx.visibility(id).is_accessible_from(parent, cx.tcx) {
// The original const is accessible, suggest using it directly.
let item_name = cx.tcx.item_name(*id);
accessible.push(item_name);
accessible_path.push(with_no_trimmed_paths!(cx.tcx.def_path_str(id)));
} else if cx
.tcx
.visibility(item.owner_id)
.is_accessible_from(parent, cx.tcx)
{
// The const is accessible only through the re-export, point at
// the `use`.
imported.push(use_name);
imported_spans.push(item.ident.span);
}
}
}
}
if let DefKind::Const = cx.tcx.def_kind(item.owner_id)
&& infcx.can_eq(
cx.param_env,
ty,
cx.tcx.type_of(item.owner_id).instantiate_identity(),
)
{
// Look for local consts.
let item_name = cx.tcx.item_name(item.owner_id.into());
let vis = cx.tcx.visibility(item.owner_id);
if vis.is_accessible_from(parent, cx.tcx) {
accessible.push(item_name);
let path = if item_name == name {
// We know that the const wasn't in scope because it has the exact
// same name, so we suggest the full path.
with_no_trimmed_paths!(cx.tcx.def_path_str(item.owner_id))
} else {
// The const is likely just typoed, and nothing else.
cx.tcx.def_path_str(item.owner_id)
};
accessible_path.push(path);
} else if name == item_name {
// The const exists somewhere in this crate, but it can't be imported
// from this pattern's scope. We'll just point at its definition.
inaccessible.push(cx.tcx.def_span(item.owner_id));
}
}
}
if let Some((i, &const_name)) =
accessible.iter().enumerate().find(|(_, &const_name)| const_name == name)
{
// The pattern name is an exact match, so the pattern needed to be imported.
lint.wanted_constant = Some(WantedConstant {
span: pat.span,
is_typo: false,
const_name: const_name.to_string(),
const_path: accessible_path[i].clone(),
});
} else if let Some(name) = find_best_match_for_name(&accessible, name, None) {
// The pattern name is likely a typo.
lint.wanted_constant = Some(WantedConstant {
span: pat.span,
is_typo: true,
const_name: name.to_string(),
const_path: name.to_string(),
});
} else if let Some(i) =
imported.iter().enumerate().find(|(_, &const_name)| const_name == name).map(|(i, _)| i)
{
// The const with the exact name wasn't re-exported from an import in this
// crate, we point at the import.
lint.accessible_constant = Some(imported_spans[i]);
} else if let Some(name) = find_best_match_for_name(&imported, name, None) {
// The typoed const wasn't re-exported by an import in this crate, we suggest
// the right name (which will likely require another follow up suggestion).
lint.wanted_constant = Some(WantedConstant {
span: pat.span,
is_typo: true,
const_path: name.to_string(),
const_name: name.to_string(),
});
} else if !inaccessible.is_empty() {
for span in inaccessible {
// The const with the exact name match isn't accessible, we just point at it.
lint.inaccessible_constant = Some(span);
}
} else {
// Look for local bindings for people that might have gotten confused with how
// `let` and `const` works.
for (_, node) in cx.tcx.hir().parent_iter(hir_id) {
match node {
hir::Node::Stmt(hir::Stmt { kind: hir::StmtKind::Let(let_stmt), .. }) => {
if let hir::PatKind::Binding(_, _, binding_name, _) = let_stmt.pat.kind {
if name == binding_name.name {
lint.pattern_let_binding = Some(binding_name.span);
}
}
}
hir::Node::Block(hir::Block { stmts, .. }) => {
for stmt in *stmts {
if let hir::StmtKind::Let(let_stmt) = stmt.kind {
if let hir::PatKind::Binding(_, _, binding_name, _) =
let_stmt.pat.kind
{
if name == binding_name.name {
lint.pattern_let_binding = Some(binding_name.span);
}
}
}
}
}
hir::Node::Item(_) => break,
_ => {}
}
}
}
}
}

/// Report unreachable arms, if any.
fn report_arm_reachability<'p, 'tcx>(
cx: &PatCtxt<'p, 'tcx>,
Expand Down
45 changes: 45 additions & 0 deletions tests/ui/resolve/const-with-typo-in-pattern-binding.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#![deny(unreachable_patterns)] //~ NOTE the lint level is defined here
#![allow(non_snake_case, non_upper_case_globals)]
mod x {
pub use std::env::consts::ARCH;
const X: i32 = 0; //~ NOTE there is a constant of the same name
}
fn main() {
let input: i32 = 42;

const god: i32 = 1;
const GOOD: i32 = 1;
const BAD: i32 = 2;

let name: i32 = 42; //~ NOTE there is a binding of the same name

match input {
X => {} //~ NOTE matches any value
_ => {} //~ ERROR unreachable pattern
//~^ NOTE no value can reach this
}
match input {
GOD => {} //~ HELP you might have meant to pattern match against the value of similarly named constant `god`
//~^ NOTE matches any value
_ => {} //~ ERROR unreachable pattern
//~^ NOTE no value can reach this
}
match input {
GOOOD => {} //~ HELP you might have meant to pattern match against the value of similarly named constant `GOOD`
//~^ NOTE matches any value
_ => {} //~ ERROR unreachable pattern
//~^ NOTE no value can reach this
}
match input {
name => {}
//~^ NOTE matches any value
_ => {} //~ ERROR unreachable pattern
//~^ NOTE no value can reach this
}
match "" {
ARCH => {} //~ HELP you might have meant to pattern match against the value of constant `ARCH`
//~^ NOTE matches any value
_ => {} //~ ERROR unreachable pattern
//~^ NOTE no value can reach this
}
}
78 changes: 78 additions & 0 deletions tests/ui/resolve/const-with-typo-in-pattern-binding.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
error: unreachable pattern
--> $DIR/const-with-typo-in-pattern-binding.rs:18:9
|
LL | X => {}
| - matches any value
LL | _ => {}
| ^ no value can reach this
|
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
--> $DIR/const-with-typo-in-pattern-binding.rs:5:5
|
LL | const X: i32 = 0;
| ^^^^^^^^^^^^
note: the lint level is defined here
--> $DIR/const-with-typo-in-pattern-binding.rs:1:9
|
LL | #![deny(unreachable_patterns)]
| ^^^^^^^^^^^^^^^^^^^^

error: unreachable pattern
--> $DIR/const-with-typo-in-pattern-binding.rs:24:9
|
LL | GOD => {}
| --- 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 `god` instead of introducing a new catch-all binding
|
LL | god => {}
| ~~~

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 => {}
| ~~~~

error: unreachable pattern
--> $DIR/const-with-typo-in-pattern-binding.rs:36:9
|
LL | name => {}
| ---- matches any value
LL |
LL | _ => {}
| ^ no value can reach this
|
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
--> $DIR/const-with-typo-in-pattern-binding.rs:14:9
|
LL | let name: i32 = 42;
| ^^^^

error: unreachable pattern
--> $DIR/const-with-typo-in-pattern-binding.rs:42:9
|
LL | ARCH => {}
| ---- matches any value
LL |
LL | _ => {}
| ^ no value can reach this
|
help: you might have meant to pattern match against the value of constant `ARCH` instead of introducing a new catch-all binding
|
LL | std::env::consts::ARCH => {}
| ~~~~~~~~~~~~~~~~~~~~~~

error: aborting due to 5 previous errors

0 comments on commit 2487765

Please sign in to comment.