When encountering a binding that could be a const or unit variant, suggest the right path

Author: estebank

compiler/rustc_resolve/src/diagnostics.rs

@@ -134,6 +134,7 @@ impl<'a> Resolver<'a> {
                     &candidates,
                     instead,
                     found_use,
+                    false,
                 );
             } else if let Some((span, msg, sugg, appl)) = suggestion {
                 err.span_suggestion(span, msg, sugg, appl);
@@ -493,14 +494,14 @@ impl<'a> Resolver<'a> {
     ///
     /// This takes the error provided, combines it with the span and any additional spans inside the
     /// error and emits it.
-    crate fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) {
+    crate fn report_error(&mut self, span: Span, resolution_error: ResolutionError<'a>) {
         self.into_struct_error(span, resolution_error).emit();
     }
 
     crate fn into_struct_error(
-        &self,
+        &mut self,
         span: Span,
-        resolution_error: ResolutionError<'_>,
+        resolution_error: ResolutionError<'a>,
     ) -> DiagnosticBuilder<'_, ErrorGuaranteed> {
         match resolution_error {
             ResolutionError::GenericParamsFromOuterFunction(outer_res, has_generic_params) => {
@@ -650,7 +651,7 @@ impl<'a> Resolver<'a> {
                 }
                 err
             }
-            ResolutionError::VariableNotBoundInPattern(binding_error) => {
+            ResolutionError::VariableNotBoundInPattern(binding_error, parent_scope) => {
                 let BindingError { name, target, origin, could_be_path } = binding_error;
 
                 let target_sp = target.iter().copied().collect::<Vec<_>>();
@@ -670,13 +671,41 @@ impl<'a> Resolver<'a> {
                 for sp in origin_sp {
                     err.span_label(sp, "variable not in all patterns");
                 }
-                if *could_be_path {
-                    let help_msg = format!(
-                        "if you meant to match on a variant or a `const` item, consider \
-                         making the path in the pattern qualified: `?::{}`",
-                        name,
+                if could_be_path {
+                    let import_suggestions = self.lookup_import_candidates(
+                        Ident::with_dummy_span(name),
+                        Namespace::ValueNS,
+                        &parent_scope,
+                        &|res: Res| match res {
+                            Res::Def(
+                                DefKind::Ctor(CtorOf::Variant, CtorKind::Const)
+                                | DefKind::Ctor(CtorOf::Struct, CtorKind::Const)
+                                | DefKind::Const
+                                | DefKind::AssocConst,
+                                _,
+                            ) => true,
+                            _ => false,
+                        },
+                    );
+
+                    if import_suggestions.is_empty() {
+                        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,
+                        );
+                        err.span_help(span, &help_msg);
+                    }
+                    show_candidates(
+                        &self.definitions,
+                        self.session,
+                        &mut err,
+                        Some(span),
+                        &import_suggestions,
+                        false,
+                        true,
+                        true,
                     );
-                    err.span_help(span, &help_msg);
                 }
                 err
             }
@@ -1022,7 +1051,7 @@ impl<'a> Resolver<'a> {
     }
 
     crate fn report_vis_error(
-        &self,
+        &mut self,
         vis_resolution_error: VisResolutionError<'_>,
     ) -> ErrorGuaranteed {
         match vis_resolution_error {
@@ -1455,6 +1484,7 @@ impl<'a> Resolver<'a> {
             &import_suggestions,
             false,
             true,
+            false,
         );
 
         if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
@@ -2402,6 +2432,7 @@ fn show_candidates(
     candidates: &[ImportSuggestion],
     instead: bool,
     found_use: bool,
+    is_pattern: bool,
 ) {
     if candidates.is_empty() {
         return;
@@ -2428,20 +2459,34 @@ fn show_candidates(
     }
 
     if !accessible_path_strings.is_empty() {
-        let (determiner, kind) = if accessible_path_strings.len() == 1 {
-            ("this", accessible_path_strings[0].1)
+        let (determiner, kind, name) = if accessible_path_strings.len() == 1 {
+            ("this", accessible_path_strings[0].1, format!(" `{}`", accessible_path_strings[0].0))
         } else {
-            ("one of these", "items")
+            ("one of these", "items", String::new())
         };
 
         let instead = if instead { " instead" } else { "" };
-        let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);
+        let mut msg = if is_pattern {
+            format!(
+                "if you meant to match on {}{}{}, use the full path in the pattern",
+                kind, instead, name
+            )
+        } else {
+            format!("consider importing {} {}{}", determiner, kind, instead)
+        };
 
         for note in accessible_path_strings.iter().flat_map(|cand| cand.3.as_ref()) {
             err.note(note);
         }
 
-        if let Some(span) = use_placement_span {
+        if let (true, Some(span)) = (is_pattern, use_placement_span) {
+            err.span_suggestions(
+                span,
+                &msg,
+                accessible_path_strings.into_iter().map(|a| a.0),
+                Applicability::MaybeIncorrect,
+            );
+        } else if let Some(span) = use_placement_span {
             for candidate in &mut accessible_path_strings {
                 // produce an additional newline to separate the new use statement
                 // from the directly following item.
@@ -2453,7 +2498,7 @@ fn show_candidates(
                 span,
                 &msg,
                 accessible_path_strings.into_iter().map(|a| a.0),
-                Applicability::Unspecified,
+                Applicability::MaybeIncorrect,
             );
         } else {
             msg.push(':');
@@ -2468,9 +2513,16 @@ fn show_candidates(
     } else {
         assert!(!inaccessible_path_strings.is_empty());
 
+        let prefix = if is_pattern { "you might have meant to match on " } else { "" };
         if inaccessible_path_strings.len() == 1 {
             let (name, descr, def_id, note) = &inaccessible_path_strings[0];
-            let msg = format!("{} `{}` exists but is inaccessible", descr, name);
+            let msg = format!(
+                "{}{} `{}`{} exists but is inaccessible",
+                prefix,
+                descr,
+                name,
+                if is_pattern { ", which" } else { "" }
+            );
 
             if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
                 let span = definitions.def_span(local_def_id);
@@ -2496,7 +2548,7 @@ fn show_candidates(
                 "item".to_string()
             };
 
-            let mut msg = format!("these {}s exist but are inaccessible", descr);
+            let mut msg = format!("{}these {}s exist but are inaccessible", prefix, descr);
             let mut has_colon = false;
 
             let mut spans = Vec::new();

compiler/rustc_resolve/src/late.rs

@@ -1422,7 +1422,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
     /// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved
     /// label and reports an error if the label is not found or is unreachable.
-    fn resolve_label(&self, mut label: Ident) -> Option<NodeId> {
+    fn resolve_label(&mut self, mut label: Ident) -> Option<NodeId> {
         let mut suggestion = None;
 
         // Preserve the original span so that errors contain "in this macro invocation"
@@ -1442,14 +1442,15 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
 
             let ident = label.normalize_to_macro_rules();
             if let Some((ident, id)) = rib.bindings.get_key_value(&ident) {
+                let definition_span = ident.span;
                 return if self.is_label_valid_from_rib(i) {
                     Some(*id)
                 } else {
                     self.report_error(
                         original_span,
                         ResolutionError::UnreachableLabel {
                             name: label.name,
-                            definition_span: ident.span,
+                            definition_span,
                             suggestion,
                         },
                     );
@@ -2135,7 +2136,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         span: Span,
         err: F,
     ) where
-        F: FnOnce(Ident, &str, Option<Symbol>) -> ResolutionError<'_>,
+        F: FnOnce(Ident, String, Option<Symbol>) -> ResolutionError<'a>,
     {
         // If there is a TraitRef in scope for an impl, then the method must be in the trait.
         let Some((module, _)) = &self.current_trait_ref else { return; };
@@ -2159,7 +2160,8 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             // We could not find the method: report an error.
             let candidate = self.find_similarly_named_assoc_item(ident.name, kind);
             let path = &self.current_trait_ref.as_ref().unwrap().1.path;
-            self.report_error(span, err(ident, &path_names_to_string(path), candidate));
+            let path_names = path_names_to_string(path);
+            self.report_error(span, err(ident, path_names, candidate));
             return;
         };
 
@@ -2183,13 +2185,14 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
             AssocItemKind::TyAlias(..) => (rustc_errors::error_code!(E0325), "type"),
             AssocItemKind::MacCall(..) => span_bug!(span, "unexpanded macro"),
         };
+        let trait_path = path_names_to_string(path);
         self.report_error(
             span,
             ResolutionError::TraitImplMismatch {
                 name: ident.name,
                 kind,
                 code,
-                trait_path: path_names_to_string(path),
+                trait_path,
                 trait_item_span: binding.span,
             },
         );
@@ -2304,16 +2307,16 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
         }
 
         // 3) Report all missing variables we found.
-        let mut missing_vars = missing_vars.iter_mut().collect::<Vec<_>>();
-        missing_vars.sort_by_key(|(sym, _err)| sym.as_str());
+        let mut missing_vars = missing_vars.into_iter().collect::<Vec<_>>();
+        missing_vars.sort_by_key(|&(sym, ref _err)| sym);
 
-        for (name, mut v) in missing_vars {
-            if inconsistent_vars.contains_key(name) {
+        for (name, mut v) in missing_vars.into_iter() {
+            if inconsistent_vars.contains_key(&name) {
                 v.could_be_path = false;
             }
             self.report_error(
                 *v.origin.iter().next().unwrap(),
-                ResolutionError::VariableNotBoundInPattern(v),
+                ResolutionError::VariableNotBoundInPattern(v, self.parent_scope),
             );
         }
 
@@ -2815,7 +2818,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
     /// A wrapper around [`Resolver::report_error`].
     ///
     /// This doesn't emit errors for function bodies if this is rustdoc.
-    fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) {
+    fn report_error(&mut self, span: Span, resolution_error: ResolutionError<'a>) {
         if self.should_report_errs() {
             self.r.report_error(span, resolution_error);
         }

compiler/rustc_resolve/src/lib.rs

@@ -201,13 +201,13 @@ enum ResolutionError<'a> {
     /// parameter list.
     NameAlreadyUsedInParameterList(Symbol, Span),
     /// Error E0407: method is not a member of trait.
-    MethodNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
+    MethodNotMemberOfTrait(Ident, String, Option<Symbol>),
     /// Error E0437: type is not a member of trait.
-    TypeNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
+    TypeNotMemberOfTrait(Ident, String, Option<Symbol>),
     /// Error E0438: const is not a member of trait.
-    ConstNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
+    ConstNotMemberOfTrait(Ident, String, Option<Symbol>),
     /// Error E0408: variable `{}` is not bound in all patterns.
-    VariableNotBoundInPattern(&'a BindingError),
+    VariableNotBoundInPattern(BindingError, ParentScope<'a>),
     /// Error E0409: variable `{}` is bound in inconsistent ways within the same match arm.
     VariableBoundWithDifferentMode(Symbol, Span),
     /// Error E0415: identifier is bound more than once in this parameter list.

compiler/rustc_typeck/src/check/pat.rs

@@ -643,6 +643,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             });
             let pre = if in_match { "in the same arm, " } else { "" };
             err.note(&format!("{}a binding must have the same type in all alternatives", pre));
+            // FIXME: check if `var_ty` and `ty` can be made the same type by adding or removing
+            // `ref` or `&` to the pattern.
             err.emit();
         }
     }

src/test/ui/or-patterns/missing-bindings.stderr

@@ -103,6 +103,22 @@ LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
    |            |
    |            pattern doesn't bind `c`
 
+error[E0408]: variable `d` is not bound in all patterns
+  --> $DIR/missing-bindings.rs:45:33
+   |
+LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
+   |                            -    ^^^^ pattern doesn't bind `d`
+   |                            |
+   |                            variable not in all patterns
+
+error[E0408]: variable `e` is not bound in all patterns
+  --> $DIR/missing-bindings.rs:45:10
+   |
+LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
+   |          ^^^^^^^^^^^^^^^^^^^^     - variable not in all patterns
+   |          |
+   |          pattern doesn't bind `e`
+
 error[E0408]: variable `a` is not bound in all patterns
   --> $DIR/missing-bindings.rs:45:33
    |
@@ -127,22 +143,6 @@ LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
    |                        |
    |                        variable not in all patterns
 
-error[E0408]: variable `d` is not bound in all patterns
-  --> $DIR/missing-bindings.rs:45:33
-   |
-LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
-   |                            -    ^^^^ pattern doesn't bind `d`
-   |                            |
-   |                            variable not in all patterns
-
-error[E0408]: variable `e` is not bound in all patterns
-  --> $DIR/missing-bindings.rs:45:10
-   |
-LL |     let (A(A(a, b) | B(c), d) | B(e)) = Y;
-   |          ^^^^^^^^^^^^^^^^^^^^     - variable not in all patterns
-   |          |
-   |          pattern doesn't bind `e`
-
 error[E0408]: variable `a` is not bound in all patterns
   --> $DIR/missing-bindings.rs:61:29
    |

src/test/ui/resolve/resolve-inconsistent-names.rs

@@ -2,9 +2,9 @@
 
 enum E { A, B, c }
 
-mod m {
+pub mod m {
     const CONST1: usize = 10;
-    const Const2: usize = 20;
+    pub const Const2: usize = 20;
 }
 
 fn main() {
@@ -22,15 +22,14 @@ fn main() {
         //~| ERROR variable `B` is bound inconsistently
         //~| ERROR mismatched types
         //~| ERROR variable `c` is not bound in all patterns
-        //~| HELP consider making the path in the pattern qualified: `?::A`
+        //~| HELP if you meant to match on unit variant `E::A`, use the full path in the pattern
     }
 
     let z = (10, 20);
     match z {
         (CONST1, _) | (_, Const2) => ()
         //~^ ERROR variable `CONST1` is not bound in all patterns
-        //~| HELP consider making the path in the pattern qualified: `?::CONST1`
         //~| ERROR variable `Const2` is not bound in all patterns
-        //~| HELP consider making the path in the pattern qualified: `?::Const2`
+        //~| HELP if you meant to match on constant `m::Const2`, use the full path in the pattern
     }
 }