Skip to content

Commit 09f3ea1

Browse files
estebankestebank
committed
When encountering a binding that could be a const or unit variant, suggest the right path
1 parent 05c0738 commit 09f3ea1

File tree

8 files changed

+131
-77
lines changed

8 files changed

+131
-77
lines changed

‎compiler/rustc_resolve/src/diagnostics.rs

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ impl<'a> Resolver<'a> {
134134
&candidates,
135135
instead,
136136
found_use,
137+
false,
137138
);
138139
} else if let Some((span, msg, sugg, appl)) = suggestion {
139140
err.span_suggestion(span, msg, sugg, appl);
@@ -493,14 +494,14 @@ impl<'a> Resolver<'a> {
493494
///
494495
/// This takes the error provided, combines it with the span and any additional spans inside the
495496
/// error and emits it.
496-
crate fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) {
497+
crate fn report_error(&mut self, span: Span, resolution_error: ResolutionError<'a>) {
497498
self.into_struct_error(span, resolution_error).emit();
498499
}
499500

500501
crate fn into_struct_error(
501-
&self,
502+
&mut self,
502503
span: Span,
503-
resolution_error: ResolutionError<'_>,
504+
resolution_error: ResolutionError<'a>,
504505
) -> DiagnosticBuilder<'_, ErrorGuaranteed> {
505506
match resolution_error {
506507
ResolutionError::GenericParamsFromOuterFunction(outer_res, has_generic_params) => {
@@ -650,7 +651,7 @@ impl<'a> Resolver<'a> {
650651
}
651652
err
652653
}
653-
ResolutionError::VariableNotBoundInPattern(binding_error) => {
654+
ResolutionError::VariableNotBoundInPattern(binding_error, parent_scope) => {
654655
let BindingError { name, target, origin, could_be_path } = binding_error;
655656

656657
let target_sp = target.iter().copied().collect::<Vec<_>>();
@@ -670,13 +671,41 @@ impl<'a> Resolver<'a> {
670671
for sp in origin_sp {
671672
err.span_label(sp, "variable not in all patterns");
672673
}
673-
if *could_be_path {
674-
let help_msg = format!(
675-
"if you meant to match on a variant or a `const` item, consider \
676-
making the path in the pattern qualified: `?::{}`",
677-
name,
674+
if could_be_path {
675+
let import_suggestions = self.lookup_import_candidates(
676+
Ident::with_dummy_span(name),
677+
Namespace::ValueNS,
678+
&parent_scope,
679+
&|res: Res| match res {
680+
Res::Def(
681+
DefKind::Ctor(CtorOf::Variant, CtorKind::Const)
682+
| DefKind::Ctor(CtorOf::Struct, CtorKind::Const)
683+
| DefKind::Const
684+
| DefKind::AssocConst,
685+
_,
686+
) => true,
687+
_ => false,
688+
},
689+
);
690+
691+
if import_suggestions.is_empty() {
692+
let help_msg = format!(
693+
"if you meant to match on a variant or a `const` item, consider \
694+
making the path in the pattern qualified: `path::to::ModOrType::{}`",
695+
name,
696+
);
697+
err.span_help(span, &help_msg);
698+
}
699+
show_candidates(
700+
&self.definitions,
701+
self.session,
702+
&mut err,
703+
Some(span),
704+
&import_suggestions,
705+
false,
706+
true,
707+
true,
678708
);
679-
err.span_help(span, &help_msg);
680709
}
681710
err
682711
}
@@ -1022,7 +1051,7 @@ impl<'a> Resolver<'a> {
10221051
}
10231052

10241053
crate fn report_vis_error(
1025-
&self,
1054+
&mut self,
10261055
vis_resolution_error: VisResolutionError<'_>,
10271056
) -> ErrorGuaranteed {
10281057
match vis_resolution_error {
@@ -1455,6 +1484,7 @@ impl<'a> Resolver<'a> {
14551484
&import_suggestions,
14561485
false,
14571486
true,
1487+
false,
14581488
);
14591489

14601490
if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
@@ -2402,6 +2432,7 @@ fn show_candidates(
24022432
candidates: &[ImportSuggestion],
24032433
instead: bool,
24042434
found_use: bool,
2435+
is_pattern: bool,
24052436
) {
24062437
if candidates.is_empty() {
24072438
return;
@@ -2428,20 +2459,34 @@ fn show_candidates(
24282459
}
24292460

24302461
if !accessible_path_strings.is_empty() {
2431-
let (determiner, kind) = if accessible_path_strings.len() == 1 {
2432-
("this", accessible_path_strings[0].1)
2462+
let (determiner, kind, name) = if accessible_path_strings.len() == 1 {
2463+
("this", accessible_path_strings[0].1, format!(" `{}`", accessible_path_strings[0].0))
24332464
} else {
2434-
("one of these", "items")
2465+
("one of these", "items", String::new())
24352466
};
24362467

24372468
let instead = if instead { " instead" } else { "" };
2438-
let mut msg = format!("consider importing {} {}{}", determiner, kind, instead);
2469+
let mut msg = if is_pattern {
2470+
format!(
2471+
"if you meant to match on {}{}{}, use the full path in the pattern",
2472+
kind, instead, name
2473+
)
2474+
} else {
2475+
format!("consider importing {} {}{}", determiner, kind, instead)
2476+
};
24392477

24402478
for note in accessible_path_strings.iter().flat_map(|cand| cand.3.as_ref()) {
24412479
err.note(note);
24422480
}
24432481

2444-
if let Some(span) = use_placement_span {
2482+
if let (true, Some(span)) = (is_pattern, use_placement_span) {
2483+
err.span_suggestions(
2484+
span,
2485+
&msg,
2486+
accessible_path_strings.into_iter().map(|a| a.0),
2487+
Applicability::MaybeIncorrect,
2488+
);
2489+
} else if let Some(span) = use_placement_span {
24452490
for candidate in &mut accessible_path_strings {
24462491
// produce an additional newline to separate the new use statement
24472492
// from the directly following item.
@@ -2453,7 +2498,7 @@ fn show_candidates(
24532498
span,
24542499
&msg,
24552500
accessible_path_strings.into_iter().map(|a| a.0),
2456-
Applicability::Unspecified,
2501+
Applicability::MaybeIncorrect,
24572502
);
24582503
} else {
24592504
msg.push(':');
@@ -2468,9 +2513,16 @@ fn show_candidates(
24682513
} else {
24692514
assert!(!inaccessible_path_strings.is_empty());
24702515

2516+
let prefix = if is_pattern { "you might have meant to match on " } else { "" };
24712517
if inaccessible_path_strings.len() == 1 {
24722518
let (name, descr, def_id, note) = &inaccessible_path_strings[0];
2473-
let msg = format!("{} `{}` exists but is inaccessible", descr, name);
2519+
let msg = format!(
2520+
"{}{} `{}`{} exists but is inaccessible",
2521+
prefix,
2522+
descr,
2523+
name,
2524+
if is_pattern { ", which" } else { "" }
2525+
);
24742526

24752527
if let Some(local_def_id) = def_id.and_then(|did| did.as_local()) {
24762528
let span = definitions.def_span(local_def_id);
@@ -2496,7 +2548,7 @@ fn show_candidates(
24962548
"item".to_string()
24972549
};
24982550

2499-
let mut msg = format!("these {}s exist but are inaccessible", descr);
2551+
let mut msg = format!("{}these {}s exist but are inaccessible", prefix, descr);
25002552
let mut has_colon = false;
25012553

25022554
let mut spans = Vec::new();

‎compiler/rustc_resolve/src/late.rs

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1422,7 +1422,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
14221422

14231423
/// Searches the current set of local scopes for labels. Returns the `NodeId` of the resolved
14241424
/// label and reports an error if the label is not found or is unreachable.
1425-
fn resolve_label(&self, mut label: Ident) -> Option<NodeId> {
1425+
fn resolve_label(&mut self, mut label: Ident) -> Option<NodeId> {
14261426
let mut suggestion = None;
14271427

14281428
// 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> {
14421442

14431443
let ident = label.normalize_to_macro_rules();
14441444
if let Some((ident, id)) = rib.bindings.get_key_value(&ident) {
1445+
let definition_span = ident.span;
14451446
return if self.is_label_valid_from_rib(i) {
14461447
Some(*id)
14471448
} else {
14481449
self.report_error(
14491450
original_span,
14501451
ResolutionError::UnreachableLabel {
14511452
name: label.name,
1452-
definition_span: ident.span,
1453+
definition_span,
14531454
suggestion,
14541455
},
14551456
);
@@ -2135,7 +2136,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
21352136
span: Span,
21362137
err: F,
21372138
) where
2138-
F: FnOnce(Ident, &str, Option<Symbol>) -> ResolutionError<'_>,
2139+
F: FnOnce(Ident, String, Option<Symbol>) -> ResolutionError<'a>,
21392140
{
21402141
// If there is a TraitRef in scope for an impl, then the method must be in the trait.
21412142
let Some((module, _)) = &self.current_trait_ref else { return; };
@@ -2159,7 +2160,8 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
21592160
// We could not find the method: report an error.
21602161
let candidate = self.find_similarly_named_assoc_item(ident.name, kind);
21612162
let path = &self.current_trait_ref.as_ref().unwrap().1.path;
2162-
self.report_error(span, err(ident, &path_names_to_string(path), candidate));
2163+
let path_names = path_names_to_string(path);
2164+
self.report_error(span, err(ident, path_names, candidate));
21632165
return;
21642166
};
21652167

@@ -2183,13 +2185,14 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
21832185
AssocItemKind::TyAlias(..) => (rustc_errors::error_code!(E0325), "type"),
21842186
AssocItemKind::MacCall(..) => span_bug!(span, "unexpanded macro"),
21852187
};
2188+
let trait_path = path_names_to_string(path);
21862189
self.report_error(
21872190
span,
21882191
ResolutionError::TraitImplMismatch {
21892192
name: ident.name,
21902193
kind,
21912194
code,
2192-
trait_path: path_names_to_string(path),
2195+
trait_path,
21932196
trait_item_span: binding.span,
21942197
},
21952198
);
@@ -2304,16 +2307,16 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
23042307
}
23052308

23062309
// 3) Report all missing variables we found.
2307-
let mut missing_vars = missing_vars.iter_mut().collect::<Vec<_>>();
2308-
missing_vars.sort_by_key(|(sym, _err)| sym.as_str());
2310+
let mut missing_vars = missing_vars.into_iter().collect::<Vec<_>>();
2311+
missing_vars.sort_by_key(|&(sym, ref _err)| sym);
23092312

2310-
for (name, mut v) in missing_vars {
2311-
if inconsistent_vars.contains_key(name) {
2313+
for (name, mut v) in missing_vars.into_iter() {
2314+
if inconsistent_vars.contains_key(&name) {
23122315
v.could_be_path = false;
23132316
}
23142317
self.report_error(
23152318
*v.origin.iter().next().unwrap(),
2316-
ResolutionError::VariableNotBoundInPattern(v),
2319+
ResolutionError::VariableNotBoundInPattern(v, self.parent_scope),
23172320
);
23182321
}
23192322

@@ -2815,7 +2818,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> {
28152818
/// A wrapper around [`Resolver::report_error`].
28162819
///
28172820
/// This doesn't emit errors for function bodies if this is rustdoc.
2818-
fn report_error(&self, span: Span, resolution_error: ResolutionError<'_>) {
2821+
fn report_error(&mut self, span: Span, resolution_error: ResolutionError<'a>) {
28192822
if self.should_report_errs() {
28202823
self.r.report_error(span, resolution_error);
28212824
}

‎compiler/rustc_resolve/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -201,13 +201,13 @@ enum ResolutionError<'a> {
201201
/// parameter list.
202202
NameAlreadyUsedInParameterList(Symbol, Span),
203203
/// Error E0407: method is not a member of trait.
204-
MethodNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
204+
MethodNotMemberOfTrait(Ident, String, Option<Symbol>),
205205
/// Error E0437: type is not a member of trait.
206-
TypeNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
206+
TypeNotMemberOfTrait(Ident, String, Option<Symbol>),
207207
/// Error E0438: const is not a member of trait.
208-
ConstNotMemberOfTrait(Ident, &'a str, Option<Symbol>),
208+
ConstNotMemberOfTrait(Ident, String, Option<Symbol>),
209209
/// Error E0408: variable `{}` is not bound in all patterns.
210-
VariableNotBoundInPattern(&'a BindingError),
210+
VariableNotBoundInPattern(BindingError, ParentScope<'a>),
211211
/// Error E0409: variable `{}` is bound in inconsistent ways within the same match arm.
212212
VariableBoundWithDifferentMode(Symbol, Span),
213213
/// Error E0415: identifier is bound more than once in this parameter list.

‎compiler/rustc_typeck/src/check/pat.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -643,6 +643,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
643643
});
644644
let pre = if in_match { "in the same arm, " } else { "" };
645645
err.note(&format!("{}a binding must have the same type in all alternatives", pre));
646+
// FIXME: check if `var_ty` and `ty` can be made the same type by adding or removing
647+
// `ref` or `&` to the pattern.
646648
err.emit();
647649
}
648650
}

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

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,22 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
103103
| |
104104
| pattern doesn't bind `c`
105105

106+
error[E0408]: variable `d` is not bound in all patterns
107+
--> $DIR/missing-bindings.rs:45:33
108+
|
109+
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
110+
| - ^^^^ pattern doesn't bind `d`
111+
| |
112+
| variable not in all patterns
113+
114+
error[E0408]: variable `e` is not bound in all patterns
115+
--> $DIR/missing-bindings.rs:45:10
116+
|
117+
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
118+
| ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns
119+
| |
120+
| pattern doesn't bind `e`
121+
106122
error[E0408]: variable `a` is not bound in all patterns
107123
--> $DIR/missing-bindings.rs:45:33
108124
|
@@ -127,22 +143,6 @@ LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
127143
| |
128144
| variable not in all patterns
129145

130-
error[E0408]: variable `d` is not bound in all patterns
131-
--> $DIR/missing-bindings.rs:45:33
132-
|
133-
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
134-
| - ^^^^ pattern doesn't bind `d`
135-
| |
136-
| variable not in all patterns
137-
138-
error[E0408]: variable `e` is not bound in all patterns
139-
--> $DIR/missing-bindings.rs:45:10
140-
|
141-
LL | let (A(A(a, b) | B(c), d) | B(e)) = Y;
142-
| ^^^^^^^^^^^^^^^^^^^^ - variable not in all patterns
143-
| |
144-
| pattern doesn't bind `e`
145-
146146
error[E0408]: variable `a` is not bound in all patterns
147147
--> $DIR/missing-bindings.rs:61:29
148148
|

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@
22

33
enum E { A, B, c }
44

5-
mod m {
5+
pub mod m {
66
const CONST1: usize = 10;
7-
const Const2: usize = 20;
7+
pub const Const2: usize = 20;
88
}
99

1010
fn main() {
@@ -22,15 +22,14 @@ fn main() {
2222
//~| ERROR variable `B` is bound inconsistently
2323
//~| ERROR mismatched types
2424
//~| ERROR variable `c` is not bound in all patterns
25-
//~| HELP consider making the path in the pattern qualified: `?::A`
25+
//~| HELP if you meant to match on unit variant `E::A`, use the full path in the pattern
2626
}
2727

2828
let z = (10, 20);
2929
match z {
3030
(CONST1, _) | (_, Const2) => ()
3131
//~^ ERROR variable `CONST1` is not bound in all patterns
32-
//~| HELP consider making the path in the pattern qualified: `?::CONST1`
3332
//~| ERROR variable `Const2` is not bound in all patterns
34-
//~| HELP consider making the path in the pattern qualified: `?::Const2`
33+
//~| HELP if you meant to match on constant `m::Const2`, use the full path in the pattern
3534
}
3635
}

0 commit comments

Comments
 (0)