Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Apr 30, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 104 additions & 30 deletions compiler/rustc_resolve/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ impl<'a> Resolver<'a> {
let (span, found_use) = if let Some(def_id) = def_id.as_local() {
UsePlacementFinder::check(krate, self.def_id_to_node_id[def_id])
} else {
(None, false)
(None, FoundUse::No)
};
if !candidates.is_empty() {
show_candidates(
Expand All @@ -132,8 +132,9 @@ impl<'a> Resolver<'a> {
&mut err,
span,
&candidates,
instead,
if instead { Instead::Yes } else { Instead::No },
found_use,
IsPattern::No,
);
} else if let Some((span, msg, sugg, appl)) = suggestion {
err.span_suggestion(span, msg, sugg, appl);
Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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<_>>();
Expand All @@ -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,
Instead::No,
FoundUse::Yes,
IsPattern::Yes,
);
err.span_help(span, &help_msg);
}
err
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1453,8 +1482,9 @@ impl<'a> Resolver<'a> {
err,
None,
&import_suggestions,
false,
true,
Instead::No,
FoundUse::Yes,
IsPattern::No,
);

if macro_kind == MacroKind::Derive && (ident.name == sym::Send || ident.name == sym::Sync) {
Expand Down Expand Up @@ -2390,6 +2420,27 @@ fn find_span_immediately_after_crate_name(
(next_left_bracket == after_second_colon, from_second_colon)
}

/// A suggestion has already been emitted, change the wording slightly to clarify that both are
/// independent options.
enum Instead {
Yes,
No,
}

/// Whether an existing place with an `use` item was found.
enum FoundUse {
Yes,
No,
}

/// Whether a binding is part of a pattern or an expression. Used for diagnostics.
enum IsPattern {
/// The binding is part of a pattern
Yes,
/// The binding is part of an expression
No,
}

/// When an entity with a given name is not available in scope, we search for
/// entities with that name in all crates. This method allows outputting the
/// results of this search in a programmer-friendly way
Expand All @@ -2400,8 +2451,9 @@ fn show_candidates(
// This is `None` if all placement locations are inside expansions
use_placement_span: Option<Span>,
candidates: &[ImportSuggestion],
instead: bool,
found_use: bool,
instead: Instead,
found_use: FoundUse,
is_pattern: IsPattern,
) {
if candidates.is_empty() {
return;
Expand All @@ -2428,32 +2480,46 @@ 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 instead = if let Instead::Yes = instead { " instead" } else { "" };
let mut msg = if let IsPattern::Yes = 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 (IsPattern::Yes, 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.
let additional_newline = if found_use { "" } else { "\n" };
let additional_newline = if let FoundUse::Yes = found_use { "" } else { "\n" };
candidate.0 = format!("use {};\n{}", &candidate.0, additional_newline);
}

err.span_suggestions(
span,
&msg,
accessible_path_strings.into_iter().map(|a| a.0),
Applicability::Unspecified,
Applicability::MaybeIncorrect,
);
} else {
msg.push(':');
Expand All @@ -2468,9 +2534,17 @@ fn show_candidates(
} else {
assert!(!inaccessible_path_strings.is_empty());

let prefix =
if let IsPattern::Yes = 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 let IsPattern::Yes = 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);
Expand All @@ -2496,7 +2570,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();
Expand Down Expand Up @@ -2537,14 +2611,14 @@ struct UsePlacementFinder {
}

impl UsePlacementFinder {
fn check(krate: &Crate, target_module: NodeId) -> (Option<Span>, bool) {
fn check(krate: &Crate, target_module: NodeId) -> (Option<Span>, FoundUse) {
let mut finder =
UsePlacementFinder { target_module, first_legal_span: None, first_use_span: None };
finder.visit_crate(krate);
if let Some(use_span) = finder.first_use_span {
(Some(use_span), true)
(Some(use_span), FoundUse::Yes)
} else {
(finder.first_legal_span, false)
(finder.first_legal_span, FoundUse::No)
}
}
}
Expand Down
25 changes: 14 additions & 11 deletions compiler/rustc_resolve/src/late.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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,
},
);
Expand Down Expand Up @@ -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; };
Expand All @@ -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;
};

Expand All @@ -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,
},
);
Expand Down Expand Up @@ -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),
);
}

Expand Down Expand Up @@ -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);
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_typeck/src/check/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
Expand Down
Loading