Skip to content

Commit

Permalink
Auto merge of #90988 - estebank:binding-supposed-to-be-const, r=david…
Browse files Browse the repository at this point in the history 
…twco

When encountering a binding that could be a const or unit variant, suggest the right path
  • Loading branch information
@bors
bors committed Apr 30, 2022
2 parents 05c0738 + bce5ab2 commit 0c8e520
Show file tree
Hide file tree
Showing 8 changed files with 164 additions and 88 deletions.
134 changes: 104 additions & 30 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
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
View file
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
View file
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
View file
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

0 comments on commit 0c8e520

Please sign in to comment.