about summary refs log tree commit diff
path: root/compiler/rustc_resolve
diff options
context:
space:
mode:
authorStuart Cook <Zalathar@users.noreply.github.com>2025-09-04 10:01:54 +1000
committerGitHub <noreply@github.com>2025-09-04 10:01:54 +1000
commit3a6ae1167f56516fc0de1d304fcb9163cea0cb40 (patch)
tree8146219299de27bbdc1b03225d2458645082f880 /compiler/rustc_resolve
parentd71a9b6bcf9aae816291c560432b728f7d7f3494 (diff)
parent86085b4f65fae0ebc456202584e7e04b58a871f3 (diff)
downloadrust-3a6ae1167f56516fc0de1d304fcb9163cea0cb40.tar.gz
rust-3a6ae1167f56516fc0de1d304fcb9163cea0cb40.zip
Rollup merge of #145827 - estebank:issue-51976, r=jackh726
On unused binding or binding not present in all patterns, suggest potential typo of unit struct/variant or const

When encountering an or-pattern with a binding not available in all patterns, look for consts and unit struct/variants that have similar names as the binding to detect typos.

```
error[E0408]: variable `Ban` is not bound in all patterns
  --> $DIR/binding-typo.rs:22:9
   |
LL |         (Foo, _) | (Ban, Foo) => {}
   |         ^^^^^^^^    --- variable not in all patterns
   |         |
   |         pattern doesn't bind `Ban`
   |
help: you might have meant to use the similarly named unit variant `Bar`
   |
LL -         (Foo, _) | (Ban, Foo) => {}
LL +         (Foo, _) | (Bar, Foo) => {}
   |
```

For items that are not in the immedate scope, suggest the full path for them:

```
error[E0408]: variable `Non` is not bound in all patterns
  --> $DIR/binding-typo-2.rs:51:16
   |
LL |         (Non | Some(_))=> {}
   |          ---   ^^^^^^^ pattern doesn't bind `Non`
   |          |
   |          variable not in all patterns
   |
help: you might have meant to use the similarly named unit variant `None`
   |
LL -         (Non | Some(_))=> {}
LL +         (core::option::Option::None | Some(_))=> {}
   |
```

When encountering a typo in a pattern that gets interpreted as an unused binding, look for unit struct/variant of the same type as the binding:

```
error: unused variable: `Non`
  --> $DIR/binding-typo-2.rs:36:9
   |
LL |         Non => {}
   |         ^^^
   |
help: if this is intentional, prefix it with an underscore
   |
LL |         _Non => {}
   |         +
help: you might have meant to pattern match on the similarly named variant `None`
   |
LL -         Non => {}
LL +         std::prelude::v1::None => {}
   |
```

 Suggest constant on unused binding in a pattern

```
error: unused variable: `Batery`
  --> $DIR/binding-typo-2.rs:110:9
   |
LL |         Batery => {}
   |         ^^^^^^
   |
help: if this is intentional, prefix it with an underscore
   |
LL |         _Batery => {}
   |         +
help: you might have meant to pattern match on the similarly named constant `Battery`
   |
LL |         Battery => {}
   |            +
```

Fix rust-lang/rust#51976.
Diffstat (limited to 'compiler/rustc_resolve')
-rw-r--r--compiler/rustc_resolve/messages.ftl2
-rw-r--r--compiler/rustc_resolve/src/diagnostics.rs172
-rw-r--r--compiler/rustc_resolve/src/errors.rs12
-rw-r--r--compiler/rustc_resolve/src/late.rs52
-rw-r--r--compiler/rustc_resolve/src/lib.rs4
5 files changed, 205 insertions, 37 deletions
diff --git a/compiler/rustc_resolve/messages.ftl b/compiler/rustc_resolve/messages.ftl
index 47280a93677..0e566e20a12 100644
--- a/compiler/rustc_resolve/messages.ftl
+++ b/compiler/rustc_resolve/messages.ftl
@@ -470,6 +470,8 @@ resolve_variable_bound_with_different_mode =
     .label = bound in different ways
     .first_binding_span = first binding
 
+resolve_variable_is_a_typo = you might have meant to use the similarly named previously used binding `{$typo}`
+
 resolve_variable_is_not_bound_in_all_patterns =
     variable `{$name}` is not bound in all patterns
 
diff --git a/compiler/rustc_resolve/src/diagnostics.rs b/compiler/rustc_resolve/src/diagnostics.rs
index 5dfc4292a38..abbacc70b3e 100644
--- a/compiler/rustc_resolve/src/diagnostics.rs
+++ b/compiler/rustc_resolve/src/diagnostics.rs
@@ -661,8 +661,12 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
             ResolutionError::VariableNotBoundInPattern(binding_error, parent_scope) => {
                 let BindingError { name, target, origin, could_be_path } = binding_error;
 
-                let target_sp = target.iter().copied().collect::<Vec<_>>();
-                let origin_sp = origin.iter().copied().collect::<Vec<_>>();
+                let mut target_sp = target.iter().map(|pat| pat.span).collect::<Vec<_>>();
+                target_sp.sort();
+                target_sp.dedup();
+                let mut origin_sp = origin.iter().map(|(span, _)| *span).collect::<Vec<_>>();
+                origin_sp.sort();
+                origin_sp.dedup();
 
                 let msp = MultiSpan::from_spans(target_sp.clone());
                 let mut err = self
@@ -671,8 +675,35 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                 for sp in target_sp {
                     err.subdiagnostic(errors::PatternDoesntBindName { span: sp, name });
                 }
-                for sp in origin_sp {
-                    err.subdiagnostic(errors::VariableNotInAllPatterns { span: sp });
+                for sp in &origin_sp {
+                    err.subdiagnostic(errors::VariableNotInAllPatterns { span: *sp });
+                }
+                let mut suggested_typo = false;
+                if !target.iter().all(|pat| matches!(pat.kind, ast::PatKind::Ident(..)))
+                    && !origin.iter().all(|(_, pat)| matches!(pat.kind, ast::PatKind::Ident(..)))
+                {
+                    // The check above is so that when we encounter `match foo { (a | b) => {} }`,
+                    // we don't suggest `(a | a) => {}`, which would never be what the user wants.
+                    let mut target_visitor = BindingVisitor::default();
+                    for pat in &target {
+                        target_visitor.visit_pat(pat);
+                    }
+                    target_visitor.identifiers.sort();
+                    target_visitor.identifiers.dedup();
+                    let mut origin_visitor = BindingVisitor::default();
+                    for (_, pat) in &origin {
+                        origin_visitor.visit_pat(pat);
+                    }
+                    origin_visitor.identifiers.sort();
+                    origin_visitor.identifiers.dedup();
+                    // Find if the binding could have been a typo
+                    if let Some(typo) =
+                        find_best_match_for_name(&target_visitor.identifiers, name.name, None)
+                        && !origin_visitor.identifiers.contains(&typo)
+                    {
+                        err.subdiagnostic(errors::PatternBindingTypo { spans: origin_sp, typo });
+                        suggested_typo = true;
+                    }
                 }
                 if could_be_path {
                     let import_suggestions = self.lookup_import_candidates(
@@ -693,10 +724,86 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
                         },
                     );
 
-                    if import_suggestions.is_empty() {
+                    if import_suggestions.is_empty() && !suggested_typo {
+                        let kinds = [
+                            DefKind::Ctor(CtorOf::Variant, CtorKind::Const),
+                            DefKind::Ctor(CtorOf::Struct, CtorKind::Const),
+                            DefKind::Const,
+                            DefKind::AssocConst,
+                        ];
+                        let mut local_names = vec![];
+                        self.add_module_candidates(
+                            parent_scope.module,
+                            &mut local_names,
+                            &|res| matches!(res, Res::Def(_, _)),
+                            None,
+                        );
+                        let local_names: FxHashSet<_> = local_names
+                            .into_iter()
+                            .filter_map(|s| match s.res {
+                                Res::Def(_, def_id) => Some(def_id),
+                                _ => None,
+                            })
+                            .collect();
+
+                        let mut local_suggestions = vec![];
+                        let mut suggestions = vec![];
+                        for kind in kinds {
+                            if let Some(suggestion) = self.early_lookup_typo_candidate(
+                                ScopeSet::All(Namespace::ValueNS),
+                                &parent_scope,
+                                name,
+                                &|res: Res| match res {
+                                    Res::Def(k, _) => k == kind,
+                                    _ => false,
+                                },
+                            ) && let Res::Def(kind, mut def_id) = suggestion.res
+                            {
+                                if let DefKind::Ctor(_, _) = kind {
+                                    def_id = self.tcx.parent(def_id);
+                                }
+                                let kind = kind.descr(def_id);
+                                if local_names.contains(&def_id) {
+                                    // The item is available in the current scope. Very likely to
+                                    // be a typo. Don't use the full path.
+                                    local_suggestions.push((
+                                        suggestion.candidate,
+                                        suggestion.candidate.to_string(),
+                                        kind,
+                                    ));
+                                } else {
+                                    suggestions.push((
+                                        suggestion.candidate,
+                                        self.def_path_str(def_id),
+                                        kind,
+                                    ));
+                                }
+                            }
+                        }
+                        let suggestions = if !local_suggestions.is_empty() {
+                            // There is at least one item available in the current scope that is a
+                            // likely typo. We only show those.
+                            local_suggestions
+                        } else {
+                            suggestions
+                        };
+                        for (name, sugg, kind) in suggestions {
+                            err.span_suggestion_verbose(
+                                span,
+                                format!(
+                                    "you might have meant to use the similarly named {kind} `{name}`",
+                                ),
+                                sugg,
+                                Applicability::MaybeIncorrect,
+                            );
+                            suggested_typo = true;
+                        }
+                    }
+                    if import_suggestions.is_empty() && !suggested_typo {
                         let help_msg = format!(
-                            "if you meant to match on a variant or a `const` item, consider \
-                             making the path in the pattern qualified: `path::to::ModOrType::{name}`",
+                            "if you meant to match on a unit struct, unit variant or a `const` \
+                             item, consider making the path in the pattern qualified: \
+                             `path::to::ModOrType::{name}`",
                         );
                         err.span_help(span, help_msg);
                     }
@@ -1016,6 +1123,39 @@ impl<'ra, 'tcx> Resolver<'ra, 'tcx> {
         .emit()
     }
 
+    fn def_path_str(&self, mut def_id: DefId) -> String {
+        // We can't use `def_path_str` in resolve.
+        let mut path = vec![def_id];
+        while let Some(parent) = self.tcx.opt_parent(def_id) {
+            def_id = parent;
+            path.push(def_id);
+            if def_id.is_top_level_module() {
+                break;
+            }
+        }
+        // We will only suggest importing directly if it is accessible through that path.
+        path.into_iter()
+            .rev()
+            .map(|def_id| {
+                self.tcx
+                    .opt_item_name(def_id)
+                    .map(|name| {
+                        match (
+                            def_id.is_top_level_module(),
+                            def_id.is_local(),
+                            self.tcx.sess.edition(),
+                        ) {
+                            (true, true, Edition::Edition2015) => String::new(),
+                            (true, true, _) => kw::Crate.to_string(),
+                            (true, false, _) | (false, _, _) => name.to_string(),
+                        }
+                    })
+                    .unwrap_or_else(|| "_".to_string())
+            })
+            .collect::<Vec<String>>()
+            .join("::")
+    }
+
     pub(crate) fn add_scope_set_candidates(
         &mut self,
         suggestions: &mut Vec<TypoSuggestion>,
@@ -3396,7 +3536,7 @@ impl UsePlacementFinder {
     }
 }
 
-impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
+impl<'tcx> Visitor<'tcx> for UsePlacementFinder {
     fn visit_crate(&mut self, c: &Crate) {
         if self.target_module == CRATE_NODE_ID {
             let inject = c.spans.inject_use_span;
@@ -3424,6 +3564,22 @@ impl<'tcx> visit::Visitor<'tcx> for UsePlacementFinder {
     }
 }
 
+#[derive(Default)]
+struct BindingVisitor {
+    identifiers: Vec<Symbol>,
+    spans: FxHashMap<Symbol, Vec<Span>>,
+}
+
+impl<'tcx> Visitor<'tcx> for BindingVisitor {
+    fn visit_pat(&mut self, pat: &ast::Pat) {
+        if let ast::PatKind::Ident(_, ident, _) = pat.kind {
+            self.identifiers.push(ident.name);
+            self.spans.entry(ident.name).or_default().push(ident.span);
+        }
+        visit::walk_pat(self, pat);
+    }
+}
+
 fn search_for_any_use_in_items(items: &[Box<ast::Item>]) -> Option<Span> {
     for item in items {
         if let ItemKind::Use(..) = item.kind
diff --git a/compiler/rustc_resolve/src/errors.rs b/compiler/rustc_resolve/src/errors.rs
index 63d6fa23a14..72be94e58db 100644
--- a/compiler/rustc_resolve/src/errors.rs
+++ b/compiler/rustc_resolve/src/errors.rs
@@ -986,6 +986,18 @@ pub(crate) struct VariableNotInAllPatterns {
     pub(crate) span: Span,
 }
 
+#[derive(Subdiagnostic)]
+#[multipart_suggestion(
+    resolve_variable_is_a_typo,
+    applicability = "maybe-incorrect",
+    style = "verbose"
+)]
+pub(crate) struct PatternBindingTypo {
+    #[suggestion_part(code = "{typo}")]
+    pub(crate) spans: Vec<Span>,
+    pub(crate) typo: Symbol,
+}
+
 #[derive(Diagnostic)]
 #[diag(resolve_name_defined_multiple_time)]
 #[note]
diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs
index 6a12fafcb1e..521fef2b9d4 100644
--- a/compiler/rustc_resolve/src/late.rs
+++ b/compiler/rustc_resolve/src/late.rs
@@ -8,7 +8,6 @@
 
 use std::assert_matches::debug_assert_matches;
 use std::borrow::Cow;
-use std::collections::BTreeSet;
 use std::collections::hash_map::Entry;
 use std::mem::{replace, swap, take};
 
@@ -3682,31 +3681,30 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
         // 2) Record any missing bindings or binding mode inconsistencies.
         for (map_outer, pat_outer) in not_never_pats.iter() {
             // Check against all arms except for the same pattern which is always self-consistent.
-            let inners = not_never_pats
-                .iter()
-                .filter(|(_, pat)| pat.id != pat_outer.id)
-                .flat_map(|(map, _)| map);
-
-            for (&name, binding_inner) in inners {
-                match map_outer.get(&name) {
-                    None => {
-                        // The inner binding is missing in the outer.
-                        let binding_error =
-                            missing_vars.entry(name).or_insert_with(|| BindingError {
-                                name,
-                                origin: BTreeSet::new(),
-                                target: BTreeSet::new(),
-                                could_be_path: name.as_str().starts_with(char::is_uppercase),
-                            });
-                        binding_error.origin.insert(binding_inner.span);
-                        binding_error.target.insert(pat_outer.span);
-                    }
-                    Some(binding_outer) => {
-                        if binding_outer.annotation != binding_inner.annotation {
-                            // The binding modes in the outer and inner bindings differ.
-                            inconsistent_vars
-                                .entry(name)
-                                .or_insert((binding_inner.span, binding_outer.span));
+            let inners = not_never_pats.iter().filter(|(_, pat)| pat.id != pat_outer.id);
+
+            for (map, pat) in inners {
+                for (&name, binding_inner) in map {
+                    match map_outer.get(&name) {
+                        None => {
+                            // The inner binding is missing in the outer.
+                            let binding_error =
+                                missing_vars.entry(name).or_insert_with(|| BindingError {
+                                    name,
+                                    origin: Default::default(),
+                                    target: Default::default(),
+                                    could_be_path: name.as_str().starts_with(char::is_uppercase),
+                                });
+                            binding_error.origin.push((binding_inner.span, (***pat).clone()));
+                            binding_error.target.push((***pat_outer).clone());
+                        }
+                        Some(binding_outer) => {
+                            if binding_outer.annotation != binding_inner.annotation {
+                                // The binding modes in the outer and inner bindings differ.
+                                inconsistent_vars
+                                    .entry(name)
+                                    .or_insert((binding_inner.span, binding_outer.span));
+                            }
                         }
                     }
                 }
@@ -3719,7 +3717,7 @@ impl<'a, 'ast, 'ra, 'tcx> LateResolutionVisitor<'a, 'ast, 'ra, 'tcx> {
                 v.could_be_path = false;
             }
             self.report_error(
-                *v.origin.iter().next().unwrap(),
+                v.origin.iter().next().unwrap().0,
                 ResolutionError::VariableNotBoundInPattern(v, self.parent_scope),
             );
         }
diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs
index 9674c0356c2..0dea6ae3327 100644
--- a/compiler/rustc_resolve/src/lib.rs
+++ b/compiler/rustc_resolve/src/lib.rs
@@ -230,8 +230,8 @@ enum Used {
 #[derive(Debug)]
 struct BindingError {
     name: Ident,
-    origin: BTreeSet<Span>,
-    target: BTreeSet<Span>,
+    origin: Vec<(Span, ast::Pat)>,
+    target: Vec<ast::Pat>,
     could_be_path: bool,
 }