about summary refs log tree commit diff
diff options
context:
space:
mode:
authorEsteban Küber <esteban@kuber.com.ar>2024-11-05 18:57:15 +0000
committerEsteban Küber <esteban@kuber.com.ar>2024-11-20 17:55:18 +0000
commit2487765b88fd9122fcaf57dca94024d81183d272 (patch)
tree1c8fa720dd984affe6812b88b94759740e82529d
parentbfe809d93c67de03e3a84b80549927fd828ec5d0 (diff)
downloadrust-2487765b88fd9122fcaf57dca94024d81183d272.tar.gz
rust-2487765b88fd9122fcaf57dca94024d81183d272.zip
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.
-rw-r--r--compiler/rustc_mir_build/messages.ftl7
-rw-r--r--compiler/rustc_mir_build/src/errors.rs22
-rw-r--r--compiler/rustc_mir_build/src/thir/pattern/check_match.rs164
-rw-r--r--tests/ui/resolve/const-with-typo-in-pattern-binding.rs45
-rw-r--r--tests/ui/resolve/const-with-typo-in-pattern-binding.stderr78
5 files changed, 315 insertions, 1 deletions
diff --git a/compiler/rustc_mir_build/messages.ftl b/compiler/rustc_mir_build/messages.ftl
index 55149570dbc..5cb5990ae59 100644
--- a/compiler/rustc_mir_build/messages.ftl
+++ b/compiler/rustc_mir_build/messages.ftl
@@ -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
diff --git a/compiler/rustc_mir_build/src/errors.rs b/compiler/rustc_mir_build/src/errors.rs
index 62c6d85b73f..4fbbc1e14b2 100644
--- a/compiler/rustc_mir_build/src/errors.rs
+++ b/compiler/rustc_mir_build/src/errors.rs
@@ -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)]
@@ -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 {
diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
index 033501c66db..48cd6b53fee 100644
--- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
+++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs
@@ -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;
@@ -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::*;
@@ -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() {
@@ -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);
@@ -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>,
diff --git a/tests/ui/resolve/const-with-typo-in-pattern-binding.rs b/tests/ui/resolve/const-with-typo-in-pattern-binding.rs
new file mode 100644
index 00000000000..fe45cee91db
--- /dev/null
+++ b/tests/ui/resolve/const-with-typo-in-pattern-binding.rs
@@ -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
+    }
+}
diff --git a/tests/ui/resolve/const-with-typo-in-pattern-binding.stderr b/tests/ui/resolve/const-with-typo-in-pattern-binding.stderr
new file mode 100644
index 00000000000..a0cdac3fa25
--- /dev/null
+++ b/tests/ui/resolve/const-with-typo-in-pattern-binding.stderr
@@ -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
+