From 0e2d96e88c660c574723dfc6cf8aab084917cb8c Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Sat, 2 Feb 2019 01:08:15 +0000 Subject: [PATCH 1/5] Factored out error reporting from `smart_resolve_path_fragment` fn. --- src/librustc_resolve/lib.rs | 679 ++++++++++++++++++------------------ 1 file changed, 347 insertions(+), 332 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 270f242419714..30772cd0d45a2 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3124,391 +3124,405 @@ impl<'a> Resolver<'a> { ) } - fn smart_resolve_path_fragment(&mut self, - id: NodeId, - qself: Option<&QSelf>, - path: &[Segment], - span: Span, - source: PathSource<'_>, - crate_lint: CrateLint) - -> PathResolution { + /// Handles error reporting for `smart_resolve_path_fragment` function. + /// Creates base error and amends it with one short label and possibly some longer helps/notes. + fn smart_resolve_report_errors( + &mut self, + path: &[Segment], + span: Span, + source: PathSource<'_>, + def: Option, + ) -> (DiagnosticBuilder<'a>, Vec) { let ident_span = path.last().map_or(span, |ident| ident.ident.span); let ns = source.namespace(); let is_expected = &|def| source.is_expected(def); let is_enum_variant = &|def| if let Def::Variant(..) = def { true } else { false }; - - // Base error is amended with one short label and possibly some longer helps/notes. - let report_errors = |this: &mut Self, def: Option| { - // Make the base error. - let expected = source.descr_expected(); - let path_str = Segment::names_to_string(path); - let item_str = path.last().unwrap().ident; - let code = source.error_code(def.is_some()); - let (base_msg, fallback_label, base_span) = if let Some(def) = def { - (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str), - format!("not a {}", expected), - span) + + // Make the base error. + let expected = source.descr_expected(); + let path_str = Segment::names_to_string(path); + let item_str = path.last().unwrap().ident; + let code = source.error_code(def.is_some()); + let (base_msg, fallback_label, base_span) = if let Some(def) = def { + (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str), + format!("not a {}", expected), + span) + } else { + let item_span = path.last().unwrap().ident.span; + let (mod_prefix, mod_str) = if path.len() == 1 { + (String::new(), "this scope".to_string()) + } else if path.len() == 2 && path[0].ident.name == keywords::PathRoot.name() { + (String::new(), "the crate root".to_string()) } else { - let item_span = path.last().unwrap().ident.span; - let (mod_prefix, mod_str) = if path.len() == 1 { - (String::new(), "this scope".to_string()) - } else if path.len() == 2 && path[0].ident.name == keywords::PathRoot.name() { - (String::new(), "the crate root".to_string()) - } else { - let mod_path = &path[..path.len() - 1]; - let mod_prefix = match this.resolve_path_without_parent_scope( - mod_path, Some(TypeNS), false, span, CrateLint::No - ) { - PathResult::Module(ModuleOrUniformRoot::Module(module)) => - module.def(), - _ => None, - }.map_or(String::new(), |def| format!("{} ", def.kind_name())); - (mod_prefix, format!("`{}`", Segment::names_to_string(mod_path))) - }; - (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str), - format!("not found in {}", mod_str), - item_span) + let mod_path = &path[..path.len() - 1]; + let mod_prefix = match self.resolve_path_without_parent_scope( + mod_path, Some(TypeNS), false, span, CrateLint::No + ) { + PathResult::Module(ModuleOrUniformRoot::Module(module)) => + module.def(), + _ => None, + }.map_or(String::new(), |def| format!("{} ", def.kind_name())); + (mod_prefix, format!("`{}`", Segment::names_to_string(mod_path))) }; + (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str), + format!("not found in {}", mod_str), + item_span) + }; - let code = DiagnosticId::Error(code.into()); - let mut err = this.session.struct_span_err_with_code(base_span, &base_msg, code); + let code = DiagnosticId::Error(code.into()); + let mut err = self.session.struct_span_err_with_code(base_span, &base_msg, code); - // Emit help message for fake-self from other languages like `this`(javascript) - if ["this", "my"].contains(&&*item_str.as_str()) - && this.self_value_is_available(path[0].ident.span, span) { - err.span_suggestion( + // Emit help message for fake-self from other languages (e.g., `this` in Javascript). + if ["this", "my"].contains(&&*item_str.as_str()) + && self.self_value_is_available(path[0].ident.span, span) { + err.span_suggestion( + span, + "did you mean", + "self".to_string(), + Applicability::MaybeIncorrect, + ); + } + + // Emit special messages for unresolved `Self` and `self`. + if is_self_type(path, ns) { + __diagnostic_used!(E0411); + err.code(DiagnosticId::Error("E0411".into())); + err.span_label(span, format!("`Self` is only available in impls, traits, \ + and type definitions")); + return (err, Vec::new()); + } + if is_self_value(path, ns) { + debug!("smart_resolve_path_fragment: E0424, source={:?}", source); + + __diagnostic_used!(E0424); + err.code(DiagnosticId::Error("E0424".into())); + err.span_label(span, match source { + PathSource::Pat => { + format!("`self` value is a keyword \ + and may not be bound to \ + variables or shadowed") + } + _ => { + format!("`self` value is a keyword \ + only available in methods \ + with `self` parameter") + } + }); + return (err, Vec::new()); + } + + // Try to lookup name in more relaxed fashion for better error reporting. + let ident = path.last().unwrap().ident; + let candidates = self.lookup_import_candidates(ident, ns, is_expected); + if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { + let enum_candidates = + self.lookup_import_candidates(ident, ns, is_enum_variant); + let mut enum_candidates = enum_candidates.iter() + .map(|suggestion| { + import_candidate_to_enum_paths(&suggestion) + }).collect::>(); + enum_candidates.sort(); + + if !enum_candidates.is_empty() { + // Contextualize for E0412 "cannot find type", but don't belabor the point + // (that it's a variant) for E0573 "expected type, found variant". + let preamble = if def.is_none() { + let others = match enum_candidates.len() { + 1 => String::new(), + 2 => " and 1 other".to_owned(), + n => format!(" and {} others", n) + }; + format!("there is an enum variant `{}`{}; ", + enum_candidates[0].0, others) + } else { + String::new() + }; + let msg = format!("{}try using the variant's enum", preamble); + + err.span_suggestions( span, - "did you mean", - "self".to_string(), - Applicability::MaybeIncorrect, + &msg, + enum_candidates.into_iter() + .map(|(_variant_path, enum_ty_path)| enum_ty_path) + // Variants re-exported in prelude doesn't mean `prelude::v1` is the + // type name! + // FIXME: is there a more principled way to do this that + // would work for other re-exports? + .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") + // Also write `Option` rather than `std::prelude::v1::Option`. + .map(|enum_ty_path| { + // FIXME #56861: DRYer prelude filtering. + enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() + }), + Applicability::MachineApplicable, ); } - - // Emit special messages for unresolved `Self` and `self`. - if is_self_type(path, ns) { - __diagnostic_used!(E0411); - err.code(DiagnosticId::Error("E0411".into())); - err.span_label(span, format!("`Self` is only available in impls, traits, \ - and type definitions")); - return (err, Vec::new()); - } - if is_self_value(path, ns) { - debug!("smart_resolve_path_fragment E0424 source:{:?}", source); - - __diagnostic_used!(E0424); - err.code(DiagnosticId::Error("E0424".into())); - err.span_label(span, match source { - PathSource::Pat => { - format!("`self` value is a keyword \ - and may not be bound to \ - variables or shadowed") + } + if path.len() == 1 && self.self_type_is_available(span) { + if let Some(candidate) = self.lookup_assoc_candidate(ident, ns, is_expected) { + let self_is_available = self.self_value_is_available(path[0].ident.span, span); + match candidate { + AssocSuggestion::Field => { + err.span_suggestion( + span, + "try", + format!("self.{}", path_str), + Applicability::MachineApplicable, + ); + if !self_is_available { + err.span_label(span, format!("`self` value is a keyword \ + only available in \ + methods with `self` parameter")); + } } - _ => { - format!("`self` value is a keyword \ - only available in methods \ - with `self` parameter") + AssocSuggestion::MethodWithSelf if self_is_available => { + err.span_suggestion( + span, + "try", + format!("self.{}", path_str), + Applicability::MachineApplicable, + ); } - }); - return (err, Vec::new()); + AssocSuggestion::MethodWithSelf | AssocSuggestion::AssocItem => { + err.span_suggestion( + span, + "try", + format!("Self::{}", path_str), + Applicability::MachineApplicable, + ); + } + } + return (err, candidates); } + } - // Try to lookup the name in more relaxed fashion for better error reporting. - let ident = path.last().unwrap().ident; - let candidates = this.lookup_import_candidates(ident, ns, is_expected); - if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { - let enum_candidates = - this.lookup_import_candidates(ident, ns, is_enum_variant); - let mut enum_candidates = enum_candidates.iter() - .map(|suggestion| { - import_candidate_to_enum_paths(&suggestion) - }).collect::>(); - enum_candidates.sort(); - - if !enum_candidates.is_empty() { - // contextualize for E0412 "cannot find type", but don't belabor the point - // (that it's a variant) for E0573 "expected type, found variant" - let preamble = if def.is_none() { - let others = match enum_candidates.len() { - 1 => String::new(), - 2 => " and 1 other".to_owned(), - n => format!(" and {} others", n) - }; - format!("there is an enum variant `{}`{}; ", - enum_candidates[0].0, others) - } else { - String::new() - }; - let msg = format!("{}try using the variant's enum", preamble); + let mut levenshtein_worked = false; - err.span_suggestions( + // Try Levenshtein algorithm. + let suggestion = self.lookup_typo_candidate(path, ns, is_expected, span); + if let Some(suggestion) = suggestion { + let msg = format!( + "{} {} with a similar name exists", + suggestion.article, suggestion.kind + ); + err.span_suggestion( + ident_span, + &msg, + suggestion.candidate.to_string(), + Applicability::MaybeIncorrect, + ); + + levenshtein_worked = true; + } + + // Try context-dependent help if relaxed lookup didn't work. + if let Some(def) = def { + match (def, source) { + (Def::Macro(..), _) => { + err.span_suggestion( span, - &msg, - enum_candidates.into_iter() - .map(|(_variant_path, enum_ty_path)| enum_ty_path) - // variants reëxported in prelude doesn't mean `prelude::v1` is the - // type name! FIXME: is there a more principled way to do this that - // would work for other reëxports? - .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") - // also say `Option` rather than `std::prelude::v1::Option` - .map(|enum_ty_path| { - // FIXME #56861: DRYer prelude filtering - enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() - }), - Applicability::MachineApplicable, + "use `!` to invoke the macro", + format!("{}!", path_str), + Applicability::MaybeIncorrect, ); + return (err, candidates); } - } - if path.len() == 1 && this.self_type_is_available(span) { - if let Some(candidate) = this.lookup_assoc_candidate(ident, ns, is_expected) { - let self_is_available = this.self_value_is_available(path[0].ident.span, span); - match candidate { - AssocSuggestion::Field => { - err.span_suggestion( - span, - "try", - format!("self.{}", path_str), - Applicability::MachineApplicable, - ); - if !self_is_available { - err.span_label(span, format!("`self` value is a keyword \ - only available in \ - methods with `self` parameter")); - } - } - AssocSuggestion::MethodWithSelf if self_is_available => { - err.span_suggestion( - span, - "try", - format!("self.{}", path_str), - Applicability::MachineApplicable, - ); - } - AssocSuggestion::MethodWithSelf | AssocSuggestion::AssocItem => { - err.span_suggestion( - span, - "try", - format!("Self::{}", path_str), - Applicability::MachineApplicable, - ); - } + (Def::TyAlias(..), PathSource::Trait(_)) => { + err.span_label(span, "type aliases cannot be used as traits"); + if nightly_options::is_nightly_build() { + err.note("did you mean to use a trait alias?"); } return (err, candidates); } - } - - let mut levenshtein_worked = false; - - // Try Levenshtein algorithm. - let suggestion = this.lookup_typo_candidate(path, ns, is_expected, span); - if let Some(suggestion) = suggestion { - let msg = format!( - "{} {} with a similar name exists", - suggestion.article, suggestion.kind - ); - err.span_suggestion( - ident_span, - &msg, - suggestion.candidate.to_string(), - Applicability::MaybeIncorrect, - ); - - levenshtein_worked = true; - } - - // Try context dependent help if relaxed lookup didn't work. - if let Some(def) = def { - match (def, source) { - (Def::Macro(..), _) => { + (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { + ExprKind::Field(_, ident) => { err.span_suggestion( - span, - "use `!` to invoke the macro", - format!("{}!", path_str), + parent.span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, ident), Applicability::MaybeIncorrect, ); return (err, candidates); } - (Def::TyAlias(..), PathSource::Trait(_)) => { - err.span_label(span, "type aliases cannot be used as traits"); - if nightly_options::is_nightly_build() { - err.note("did you mean to use a trait alias?"); - } + ExprKind::MethodCall(ref segment, ..) => { + let span = parent.span.with_hi(segment.ident.span.hi()); + err.span_suggestion( + span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, segment.ident), + Applicability::MaybeIncorrect, + ); return (err, candidates); } - (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { - ExprKind::Field(_, ident) => { - err.span_suggestion( - parent.span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, ident), - Applicability::MaybeIncorrect, - ); - return (err, candidates); - } - ExprKind::MethodCall(ref segment, ..) => { - let span = parent.span.with_hi(segment.ident.span.hi()); - err.span_suggestion( - span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, segment.ident), - Applicability::MaybeIncorrect, - ); - return (err, candidates); - } - _ => {} - }, - (Def::Enum(..), PathSource::TupleStruct) - | (Def::Enum(..), PathSource::Expr(..)) => { - if let Some(variants) = this.collect_enum_variants(def) { - err.note(&format!("did you mean to use one \ - of the following variants?\n{}", - variants.iter() - .map(|suggestion| path_names_to_string(suggestion)) - .map(|suggestion| format!("- `{}`", suggestion)) - .collect::>() - .join("\n"))); + _ => {} + }, + (Def::Enum(..), PathSource::TupleStruct) + | (Def::Enum(..), PathSource::Expr(..)) => { + if let Some(variants) = self.collect_enum_variants(def) { + err.note(&format!("did you mean to use one \ + of the following variants?\n{}", + variants.iter() + .map(|suggestion| path_names_to_string(suggestion)) + .map(|suggestion| format!("- `{}`", suggestion)) + .collect::>() + .join("\n"))); - } else { - err.note("did you mean to use one of the enum's variants?"); + } else { + err.note("did you mean to use one of the enum's variants?"); + } + return (err, candidates); + }, + (Def::Struct(def_id), _) if ns == ValueNS => { + if let Some((ctor_def, ctor_vis)) + = self.struct_constructors.get(&def_id).cloned() { + let accessible_ctor = self.is_accessible(ctor_vis); + if is_expected(ctor_def) && !accessible_ctor { + err.span_label(span, format!("constructor is not visible \ + here due to private fields")); } - return (err, candidates); - }, - (Def::Struct(def_id), _) if ns == ValueNS => { - if let Some((ctor_def, ctor_vis)) - = this.struct_constructors.get(&def_id).cloned() { - let accessible_ctor = this.is_accessible(ctor_vis); - if is_expected(ctor_def) && !accessible_ctor { - err.span_label(span, format!("constructor is not visible \ - here due to private fields")); - } - } else { - // HACK(estebank): find a better way to figure out that this was a - // parser issue where a struct literal is being used on an expression - // where a brace being opened means a block is being started. Look - // ahead for the next text to see if `span` is followed by a `{`. - let sm = this.session.source_map(); - let mut sp = span; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet.chars().any(|c| { !c.is_whitespace() }) { - break; - } + } else { + // HACK(estebank): find a better way to figure out that this was a + // parser issue where a struct literal is being used on an expression + // where a brace being opened means a block is being started. Look + // ahead for the next text to see if `span` is followed by a `{`. + let sm = self.session.source_map(); + let mut sp = span; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet.chars().any(|c| { !c.is_whitespace() }) { + break; } - _ => break, } + _ => break, } - let followed_by_brace = match sm.span_to_snippet(sp) { - Ok(ref snippet) if snippet == "{" => true, - _ => false, - }; - // In case this could be a struct literal that needs to be surrounded - // by parenthesis, find the appropriate span. - let mut i = 0; - let mut closing_brace = None; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet == "}" { - let sp = span.to(sp); - if let Ok(snippet) = sm.span_to_snippet(sp) { - closing_brace = Some((sp, snippet)); - } - break; + } + let followed_by_brace = match sm.span_to_snippet(sp) { + Ok(ref snippet) if snippet == "{" => true, + _ => false, + }; + // In case this could be a struct literal that needs to be surrounded + // by parenthesis, find the appropriate span. + let mut i = 0; + let mut closing_brace = None; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet == "}" { + let sp = span.to(sp); + if let Ok(snippet) = sm.span_to_snippet(sp) { + closing_brace = Some((sp, snippet)); } + break; } - _ => break, - } - i += 1; - if i > 100 { // The bigger the span the more likely we're - break; // incorrect. Bound it to 100 chars long. } + _ => break, } - match source { - PathSource::Expr(Some(parent)) => { - match parent.node { - ExprKind::MethodCall(ref path_assignment, _) => { - err.span_suggestion( - sm.start_point(parent.span) - .to(path_assignment.ident.span), - "use `::` to access an associated function", - format!("{}::{}", - path_str, - path_assignment.ident), - Applicability::MaybeIncorrect - ); - return (err, candidates); - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", - path_str), - ); - return (err, candidates); - }, - } - }, - PathSource::Expr(None) if followed_by_brace == true => { - if let Some((sp, snippet)) = closing_brace { + i += 1; + // The bigger the span, the more likely we're + // incorrect. Bound it to 100 chars long. + if i > 100 { + break; + } + } + match source { + PathSource::Expr(Some(parent)) => { + match parent.node { + ExprKind::MethodCall(ref path_assignment, _) => { err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), - Applicability::MaybeIncorrect, + sm.start_point(parent.span) + .to(path_assignment.ident.span), + "use `::` to access an associated function", + format!("{}::{}", + path_str, + path_assignment.ident), + Applicability::MaybeIncorrect ); - } else { + return (err, candidates); + }, + _ => { err.span_label( span, - format!("did you mean `({} {{ /* fields */ }})`?", + format!("did you mean `{} {{ /* fields */ }}`?", path_str), ); - } - return (err, candidates); - }, - _ => { + return (err, candidates); + }, + } + }, + PathSource::Expr(None) if followed_by_brace == true => { + if let Some((sp, snippet)) = closing_brace { + err.span_suggestion( + sp, + "surround the struct literal with parenthesis", + format!("({})", snippet), + Applicability::MaybeIncorrect, + ); + } else { err.span_label( span, - format!("did you mean `{} {{ /* fields */ }}`?", + format!("did you mean `({} {{ /* fields */ }})`?", path_str), ); - return (err, candidates); - }, - } + } + return (err, candidates); + }, + _ => { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", + path_str), + ); + return (err, candidates); + }, } - return (err, candidates); } - (Def::Union(..), _) | - (Def::Variant(..), _) | - (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { - err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", - path_str)); - return (err, candidates); - } - (Def::SelfTy(..), _) if ns == ValueNS => { - err.span_label(span, fallback_label); - err.note("can't use `Self` as a constructor, you must use the \ - implemented struct"); - return (err, candidates); - } - (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { - err.note("can't use a type alias as a constructor"); - return (err, candidates); - } - _ => {} + return (err, candidates); } + (Def::Union(..), _) | + (Def::Variant(..), _) | + (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { + err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", + path_str)); + return (err, candidates); + } + (Def::SelfTy(..), _) if ns == ValueNS => { + err.span_label(span, fallback_label); + err.note("can't use `Self` as a constructor, you must use the \ + implemented struct"); + return (err, candidates); + } + (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { + err.note("can't use a type alias as a constructor"); + return (err, candidates); + } + _ => {} } + } + + // Fallback label. + if !levenshtein_worked { + err.span_label(base_span, fallback_label); + self.type_ascription_suggestion(&mut err, base_span); + } + (err, candidates) + } + + fn smart_resolve_path_fragment(&mut self, + id: NodeId, + qself: Option<&QSelf>, + path: &[Segment], + span: Span, + source: PathSource, + crate_lint: CrateLint) + -> PathResolution { + let ns = source.namespace(); + let is_expected = &|def| source.is_expected(def); - // Fallback label. - if !levenshtein_worked { - err.span_label(base_span, fallback_label); - this.type_ascription_suggestion(&mut err, base_span); - } - (err, candidates) - }; let report_errors = |this: &mut Self, def: Option| { - let (err, candidates) = report_errors(this, def); + let (err, candidates) = this.smart_resolve_report_errors(path, span, source, def); let def_id = this.current_module.normal_ancestor_id; let node_id = this.definitions.as_local_node_id(def_id).unwrap(); let better = def.is_some(); @@ -3579,7 +3593,8 @@ impl<'a> Resolver<'a> { debug!("self.current_type_ascription {:?}", self.current_type_ascription); if let Some(sp) = self.current_type_ascription.last() { let mut sp = *sp; - loop { // try to find the `:`, bail on first non-':'/non-whitespace + loop { + // Try to find the `:`; bail on first non-':' / non-whitespace. sp = cm.next_point(sp); if let Ok(snippet) = cm.span_to_snippet(sp.to(cm.next_point(sp))) { debug!("snippet {:?}", snippet); From 14b674ab9d1f0b71763a168bd764eb7d53154008 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Sat, 2 Feb 2019 02:18:32 +0000 Subject: [PATCH 2/5] Factored out context-dependent help for error reporting. --- src/librustc_resolve/lib.rs | 378 +++++++++++++++++++----------------- 1 file changed, 195 insertions(+), 183 deletions(-) diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 30772cd0d45a2..1a5fce14fa8fd 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3124,6 +3124,193 @@ impl<'a> Resolver<'a> { ) } + /// Provides context-dependent help for errors reported by the `smart_resolve_path_fragment` + /// function. + /// Returns `true` if able to provide context-dependent help. + fn smart_resolve_context_dep_help( + &mut self, + err: &mut DiagnosticBuilder<'a>, + span: Span, + source: PathSource, + def: Def, + path_str: &str, + fallback_label: &str, + ) -> bool { + let ns = source.namespace(); + let is_expected = &|def| source.is_expected(def); + + match (def, source) { + (Def::Macro(..), _) => { + err.span_suggestion( + span, + "use `!` to invoke the macro", + format!("{}!", path_str), + Applicability::MaybeIncorrect, + ); + } + (Def::TyAlias(..), PathSource::Trait(_)) => { + err.span_label(span, "type aliases cannot be used as traits"); + if nightly_options::is_nightly_build() { + err.note("did you mean to use a trait alias?"); + } + } + (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { + ExprKind::Field(_, ident) => { + err.span_suggestion( + parent.span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, ident), + Applicability::MaybeIncorrect, + ); + } + ExprKind::MethodCall(ref segment, ..) => { + let span = parent.span.with_hi(segment.ident.span.hi()); + err.span_suggestion( + span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, segment.ident), + Applicability::MaybeIncorrect, + ); + } + _ => return false, + }, + (Def::Enum(..), PathSource::TupleStruct) + | (Def::Enum(..), PathSource::Expr(..)) => { + if let Some(variants) = self.collect_enum_variants(def) { + err.note(&format!("did you mean to use one \ + of the following variants?\n{}", + variants.iter() + .map(|suggestion| path_names_to_string(suggestion)) + .map(|suggestion| format!("- `{}`", suggestion)) + .collect::>() + .join("\n"))); + } else { + err.note("did you mean to use one of the enum's variants?"); + } + }, + (Def::Struct(def_id), _) if ns == ValueNS => { + if let Some((ctor_def, ctor_vis)) + = self.struct_constructors.get(&def_id).cloned() { + let accessible_ctor = self.is_accessible(ctor_vis); + if is_expected(ctor_def) && !accessible_ctor { + err.span_label(span, format!("constructor is not visible \ + here due to private fields")); + } + } else { + // HACK(estebank): find a better way to figure out that this was a + // parser issue where a struct literal is being used on an expression + // where a brace being opened means a block is being started. Look + // ahead for the next text to see if `span` is followed by a `{`. + let sm = self.session.source_map(); + let mut sp = span; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet.chars().any(|c| { !c.is_whitespace() }) { + break; + } + } + _ => break, + } + } + let followed_by_brace = match sm.span_to_snippet(sp) { + Ok(ref snippet) if snippet == "{" => true, + _ => false, + }; + // In case this could be a struct literal that needs to be surrounded + // by parenthesis, find the appropriate span. + let mut i = 0; + let mut closing_brace = None; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet == "}" { + let sp = span.to(sp); + if let Ok(snippet) = sm.span_to_snippet(sp) { + closing_brace = Some((sp, snippet)); + } + break; + } + } + _ => break, + } + i += 1; + // The bigger the span, the more likely we're incorrect -- + // bound it to 100 chars long. + if i > 100 { + break; + } + } + match source { + PathSource::Expr(Some(parent)) => { + match parent.node { + ExprKind::MethodCall(ref path_assignment, _) => { + err.span_suggestion( + sm.start_point(parent.span) + .to(path_assignment.ident.span), + "use `::` to access an associated function", + format!("{}::{}", + path_str, + path_assignment.ident), + Applicability::MaybeIncorrect + ); + }, + _ => { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", + path_str), + ); + }, + } + }, + PathSource::Expr(None) if followed_by_brace == true => { + if let Some((sp, snippet)) = closing_brace { + err.span_suggestion( + sp, + "surround the struct literal with parenthesis", + format!("({})", snippet), + Applicability::MaybeIncorrect, + ); + } else { + err.span_label( + span, + format!("did you mean `({} {{ /* fields */ }})`?", + path_str), + ); + } + }, + _ => { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", + path_str), + ); + }, + } + } + } + (Def::Union(..), _) | + (Def::Variant(..), _) | + (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { + err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", + path_str)); + } + (Def::SelfTy(..), _) if ns == ValueNS => { + err.span_label(span, fallback_label); + err.note("can't use `Self` as a constructor, you must use the \ + implemented struct"); + } + (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { + err.note("can't use a type alias as a constructor"); + } + _ => return false, + } + true + } + /// Handles error reporting for `smart_resolve_path_fragment` function. /// Creates base error and amends it with one short label and possibly some longer helps/notes. fn smart_resolve_report_errors( @@ -3137,7 +3324,7 @@ impl<'a> Resolver<'a> { let ns = source.namespace(); let is_expected = &|def| source.is_expected(def); let is_enum_variant = &|def| if let Def::Variant(..) = def { true } else { false }; - + // Make the base error. let expected = source.descr_expected(); let path_str = Segment::names_to_string(path); @@ -3317,188 +3504,13 @@ impl<'a> Resolver<'a> { // Try context-dependent help if relaxed lookup didn't work. if let Some(def) = def { - match (def, source) { - (Def::Macro(..), _) => { - err.span_suggestion( - span, - "use `!` to invoke the macro", - format!("{}!", path_str), - Applicability::MaybeIncorrect, - ); - return (err, candidates); - } - (Def::TyAlias(..), PathSource::Trait(_)) => { - err.span_label(span, "type aliases cannot be used as traits"); - if nightly_options::is_nightly_build() { - err.note("did you mean to use a trait alias?"); - } - return (err, candidates); - } - (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { - ExprKind::Field(_, ident) => { - err.span_suggestion( - parent.span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, ident), - Applicability::MaybeIncorrect, - ); - return (err, candidates); - } - ExprKind::MethodCall(ref segment, ..) => { - let span = parent.span.with_hi(segment.ident.span.hi()); - err.span_suggestion( - span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, segment.ident), - Applicability::MaybeIncorrect, - ); - return (err, candidates); - } - _ => {} - }, - (Def::Enum(..), PathSource::TupleStruct) - | (Def::Enum(..), PathSource::Expr(..)) => { - if let Some(variants) = self.collect_enum_variants(def) { - err.note(&format!("did you mean to use one \ - of the following variants?\n{}", - variants.iter() - .map(|suggestion| path_names_to_string(suggestion)) - .map(|suggestion| format!("- `{}`", suggestion)) - .collect::>() - .join("\n"))); - - } else { - err.note("did you mean to use one of the enum's variants?"); - } - return (err, candidates); - }, - (Def::Struct(def_id), _) if ns == ValueNS => { - if let Some((ctor_def, ctor_vis)) - = self.struct_constructors.get(&def_id).cloned() { - let accessible_ctor = self.is_accessible(ctor_vis); - if is_expected(ctor_def) && !accessible_ctor { - err.span_label(span, format!("constructor is not visible \ - here due to private fields")); - } - } else { - // HACK(estebank): find a better way to figure out that this was a - // parser issue where a struct literal is being used on an expression - // where a brace being opened means a block is being started. Look - // ahead for the next text to see if `span` is followed by a `{`. - let sm = self.session.source_map(); - let mut sp = span; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet.chars().any(|c| { !c.is_whitespace() }) { - break; - } - } - _ => break, - } - } - let followed_by_brace = match sm.span_to_snippet(sp) { - Ok(ref snippet) if snippet == "{" => true, - _ => false, - }; - // In case this could be a struct literal that needs to be surrounded - // by parenthesis, find the appropriate span. - let mut i = 0; - let mut closing_brace = None; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet == "}" { - let sp = span.to(sp); - if let Ok(snippet) = sm.span_to_snippet(sp) { - closing_brace = Some((sp, snippet)); - } - break; - } - } - _ => break, - } - i += 1; - // The bigger the span, the more likely we're - // incorrect. Bound it to 100 chars long. - if i > 100 { - break; - } - } - match source { - PathSource::Expr(Some(parent)) => { - match parent.node { - ExprKind::MethodCall(ref path_assignment, _) => { - err.span_suggestion( - sm.start_point(parent.span) - .to(path_assignment.ident.span), - "use `::` to access an associated function", - format!("{}::{}", - path_str, - path_assignment.ident), - Applicability::MaybeIncorrect - ); - return (err, candidates); - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", - path_str), - ); - return (err, candidates); - }, - } - }, - PathSource::Expr(None) if followed_by_brace == true => { - if let Some((sp, snippet)) = closing_brace { - err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), - Applicability::MaybeIncorrect, - ); - } else { - err.span_label( - span, - format!("did you mean `({} {{ /* fields */ }})`?", - path_str), - ); - } - return (err, candidates); - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", - path_str), - ); - return (err, candidates); - }, - } - } - return (err, candidates); - } - (Def::Union(..), _) | - (Def::Variant(..), _) | - (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { - err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", - path_str)); - return (err, candidates); - } - (Def::SelfTy(..), _) if ns == ValueNS => { - err.span_label(span, fallback_label); - err.note("can't use `Self` as a constructor, you must use the \ - implemented struct"); - return (err, candidates); - } - (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { - err.note("can't use a type alias as a constructor"); - return (err, candidates); - } - _ => {} + if self.smart_resolve_context_dep_help(&mut err, + span, + source, + def, + &path_str, + &fallback_label) { + return (err, candidates); } } From 497a772d04731d9e93abb71ae99696684e514bb3 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Mon, 4 Feb 2019 11:25:29 +0100 Subject: [PATCH 3/5] Addressed review points. --- src/librustc_resolve/error_reporting.rs | 417 +++++++++++++++++++++++- src/librustc_resolve/lib.rs | 399 ----------------------- 2 files changed, 412 insertions(+), 404 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index b131a6b62f9bf..88892ae2cd3d8 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -1,13 +1,420 @@ -use crate::{CrateLint, PathResult, Segment}; -use crate::macros::ParentScope; -use crate::resolve_imports::ImportResolver; +use std::cmp::Reverse; +use log::debug; +use rustc::hir::def::*; +use rustc::hir::def::Namespace::*; +use rustc::hir::def_id::{CRATE_DEF_INDEX, DefId}; +use rustc::session::config::nightly_options; +use syntax::ast::{ExprKind}; use syntax::symbol::keywords; use syntax_pos::Span; -use log::debug; +use crate::errors::{Applicability, DiagnosticBuilder, DiagnosticId}; +use crate::macros::ParentScope; +use crate::resolve_imports::ImportResolver; +use crate::{import_candidate_to_enum_paths, is_self_type, is_self_value, path_names_to_string}; +use crate::{AssocSuggestion, CrateLint, ImportSuggestion, ModuleOrUniformRoot, PathResult, + PathSource, Resolver, Segment}; -use std::cmp::Reverse; +impl<'a> Resolver<'a> { + /// Handles error reporting for `smart_resolve_path_fragment` function. + /// Creates base error and amends it with one short label and possibly some longer helps/notes. + pub(crate) fn smart_resolve_report_errors( + &mut self, + path: &[Segment], + span: Span, + source: PathSource, + def: Option, + ) -> (DiagnosticBuilder<'a>, Vec) { + let ident_span = path.last().map_or(span, |ident| ident.ident.span); + let ns = source.namespace(); + let is_expected = &|def| source.is_expected(def); + let is_enum_variant = &|def| if let Def::Variant(..) = def { true } else { false }; + + // Make the base error. + let expected = source.descr_expected(); + let path_str = Segment::names_to_string(path); + let item_str = path.last().unwrap().ident; + let code = source.error_code(def.is_some()); + let (base_msg, fallback_label, base_span) = if let Some(def) = def { + (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str), + format!("not a {}", expected), + span) + } else { + let item_span = path.last().unwrap().ident.span; + let (mod_prefix, mod_str) = if path.len() == 1 { + (String::new(), "this scope".to_string()) + } else if path.len() == 2 && path[0].ident.name == keywords::PathRoot.name() { + (String::new(), "the crate root".to_string()) + } else { + let mod_path = &path[..path.len() - 1]; + let mod_prefix = match self.resolve_path_without_parent_scope( + mod_path, Some(TypeNS), false, span, CrateLint::No + ) { + PathResult::Module(ModuleOrUniformRoot::Module(module)) => + module.def(), + _ => None, + }.map_or(String::new(), |def| format!("{} ", def.kind_name())); + (mod_prefix, format!("`{}`", Segment::names_to_string(mod_path))) + }; + (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str), + format!("not found in {}", mod_str), + item_span) + }; + + let code = DiagnosticId::Error(code.into()); + let mut err = self.session.struct_span_err_with_code(base_span, &base_msg, code); + + // Emit help message for fake-self from other languages (e.g., `this` in Javascript). + if ["this", "my"].contains(&&*item_str.as_str()) + && self.self_value_is_available(path[0].ident.span, span) { + err.span_suggestion( + span, + "did you mean", + "self".to_string(), + Applicability::MaybeIncorrect, + ); + } + + // Emit special messages for unresolved `Self` and `self`. + if is_self_type(path, ns) { + __diagnostic_used!(E0411); + err.code(DiagnosticId::Error("E0411".into())); + err.span_label(span, format!("`Self` is only available in impls, traits, \ + and type definitions")); + return (err, Vec::new()); + } + if is_self_value(path, ns) { + debug!("smart_resolve_path_fragment: E0424, source={:?}", source); + + __diagnostic_used!(E0424); + err.code(DiagnosticId::Error("E0424".into())); + err.span_label(span, match source { + PathSource::Pat => { + format!("`self` value is a keyword \ + and may not be bound to \ + variables or shadowed") + } + _ => { + format!("`self` value is a keyword \ + only available in methods \ + with `self` parameter") + } + }); + return (err, Vec::new()); + } + + // Try to lookup name in more relaxed fashion for better error reporting. + let ident = path.last().unwrap().ident; + let candidates = self.lookup_import_candidates(ident, ns, is_expected); + if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { + let enum_candidates = + self.lookup_import_candidates(ident, ns, is_enum_variant); + let mut enum_candidates = enum_candidates.iter() + .map(|suggestion| { + import_candidate_to_enum_paths(&suggestion) + }).collect::>(); + enum_candidates.sort(); + + if !enum_candidates.is_empty() { + // Contextualize for E0412 "cannot find type", but don't belabor the point + // (that it's a variant) for E0573 "expected type, found variant". + let preamble = if def.is_none() { + let others = match enum_candidates.len() { + 1 => String::new(), + 2 => " and 1 other".to_owned(), + n => format!(" and {} others", n) + }; + format!("there is an enum variant `{}`{}; ", + enum_candidates[0].0, others) + } else { + String::new() + }; + let msg = format!("{}try using the variant's enum", preamble); + + err.span_suggestions( + span, + &msg, + enum_candidates.into_iter() + .map(|(_variant_path, enum_ty_path)| enum_ty_path) + // Variants re-exported in prelude doesn't mean `prelude::v1` is the + // type name! + // FIXME: is there a more principled way to do this that + // would work for other re-exports? + .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") + // Also write `Option` rather than `std::prelude::v1::Option`. + .map(|enum_ty_path| { + // FIXME #56861: DRY-er prelude filtering. + enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() + }), + Applicability::MachineApplicable, + ); + } + } + if path.len() == 1 && self.self_type_is_available(span) { + if let Some(candidate) = self.lookup_assoc_candidate(ident, ns, is_expected) { + let self_is_available = self.self_value_is_available(path[0].ident.span, span); + match candidate { + AssocSuggestion::Field => { + err.span_suggestion( + span, + "try", + format!("self.{}", path_str), + Applicability::MachineApplicable, + ); + if !self_is_available { + err.span_label(span, format!("`self` value is a keyword \ + only available in \ + methods with `self` parameter")); + } + } + AssocSuggestion::MethodWithSelf if self_is_available => { + err.span_suggestion( + span, + "try", + format!("self.{}", path_str), + Applicability::MachineApplicable, + ); + } + AssocSuggestion::MethodWithSelf | AssocSuggestion::AssocItem => { + err.span_suggestion( + span, + "try", + format!("Self::{}", path_str), + Applicability::MachineApplicable, + ); + } + } + return (err, candidates); + } + } + + let mut levenshtein_worked = false; + + // Try Levenshtein algorithm. + let suggestion = self.lookup_typo_candidate(path, ns, is_expected, span); + if let Some(suggestion) = suggestion { + let msg = format!( + "{} {} with a similar name exists", + suggestion.article, suggestion.kind + ); + err.span_suggestion( + ident_span, + &msg, + suggestion.candidate.to_string(), + Applicability::MaybeIncorrect, + ); + + levenshtein_worked = true; + } + + // Try context-dependent help if relaxed lookup didn't work. + if let Some(def) = def { + if self.smart_resolve_context_dependent_help(&mut err, + span, + source, + def, + &path_str, + &fallback_label) { + return (err, candidates); + } + } + + // Fallback label. + if !levenshtein_worked { + err.span_label(base_span, fallback_label); + self.type_ascription_suggestion(&mut err, base_span); + } + (err, candidates) + } + + /// Provides context-dependent help for errors reported by the `smart_resolve_path_fragment` + /// function. + /// Returns `true` if able to provide context-dependent help. + fn smart_resolve_context_dependent_help( + &mut self, + err: &mut DiagnosticBuilder<'a>, + span: Span, + source: PathSource, + def: Def, + path_str: &str, + fallback_label: &str, + ) -> bool { + let ns = source.namespace(); + let is_expected = &|def| source.is_expected(def); + + match (def, source) { + (Def::Macro(..), _) => { + err.span_suggestion( + span, + "use `!` to invoke the macro", + format!("{}!", path_str), + Applicability::MaybeIncorrect, + ); + } + (Def::TyAlias(..), PathSource::Trait(_)) => { + err.span_label(span, "type aliases cannot be used as traits"); + if nightly_options::is_nightly_build() { + err.note("did you mean to use a trait alias?"); + } + } + (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { + ExprKind::Field(_, ident) => { + err.span_suggestion( + parent.span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, ident), + Applicability::MaybeIncorrect, + ); + } + ExprKind::MethodCall(ref segment, ..) => { + let span = parent.span.with_hi(segment.ident.span.hi()); + err.span_suggestion( + span, + "use the path separator to refer to an item", + format!("{}::{}", path_str, segment.ident), + Applicability::MaybeIncorrect, + ); + } + _ => return false, + }, + (Def::Enum(..), PathSource::TupleStruct) + | (Def::Enum(..), PathSource::Expr(..)) => { + if let Some(variants) = self.collect_enum_variants(def) { + err.note(&format!("did you mean to use one \ + of the following variants?\n{}", + variants.iter() + .map(|suggestion| path_names_to_string(suggestion)) + .map(|suggestion| format!("- `{}`", suggestion)) + .collect::>() + .join("\n"))); + } else { + err.note("did you mean to use one of the enum's variants?"); + } + }, + (Def::Struct(def_id), _) if ns == ValueNS => { + if let Some((ctor_def, ctor_vis)) + = self.struct_constructors.get(&def_id).cloned() { + let accessible_ctor = self.is_accessible(ctor_vis); + if is_expected(ctor_def) && !accessible_ctor { + err.span_label(span, format!("constructor is not visible \ + here due to private fields")); + } + } else { + // HACK(estebank): find a better way to figure out that this was a + // parser issue where a struct literal is being used on an expression + // where a brace being opened means a block is being started. Look + // ahead for the next text to see if `span` is followed by a `{`. + let sm = self.session.source_map(); + let mut sp = span; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet.chars().any(|c| { !c.is_whitespace() }) { + break; + } + } + _ => break, + } + } + let followed_by_brace = match sm.span_to_snippet(sp) { + Ok(ref snippet) if snippet == "{" => true, + _ => false, + }; + // In case this could be a struct literal that needs to be surrounded + // by parenthesis, find the appropriate span. + let mut i = 0; + let mut closing_brace = None; + loop { + sp = sm.next_point(sp); + match sm.span_to_snippet(sp) { + Ok(ref snippet) => { + if snippet == "}" { + let sp = span.to(sp); + if let Ok(snippet) = sm.span_to_snippet(sp) { + closing_brace = Some((sp, snippet)); + } + break; + } + } + _ => break, + } + i += 1; + // The bigger the span, the more likely we're incorrect -- + // bound it to 100 chars long. + if i > 100 { + break; + } + } + match source { + PathSource::Expr(Some(parent)) => { + match parent.node { + ExprKind::MethodCall(ref path_assignment, _) => { + err.span_suggestion( + sm.start_point(parent.span) + .to(path_assignment.ident.span), + "use `::` to access an associated function", + format!("{}::{}", + path_str, + path_assignment.ident), + Applicability::MaybeIncorrect + ); + }, + _ => { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", + path_str), + ); + }, + } + }, + PathSource::Expr(None) if followed_by_brace == true => { + if let Some((sp, snippet)) = closing_brace { + err.span_suggestion( + sp, + "surround the struct literal with parenthesis", + format!("({})", snippet), + Applicability::MaybeIncorrect, + ); + } else { + err.span_label( + span, + format!("did you mean `({} {{ /* fields */ }})`?", + path_str), + ); + } + }, + _ => { + err.span_label( + span, + format!("did you mean `{} {{ /* fields */ }}`?", + path_str), + ); + }, + } + } + } + (Def::Union(..), _) | + (Def::Variant(..), _) | + (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { + err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", + path_str)); + } + (Def::SelfTy(..), _) if ns == ValueNS => { + err.span_label(span, fallback_label); + err.note("can't use `Self` as a constructor, you must use the \ + implemented struct"); + } + (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { + err.note("can't use a type alias as a constructor"); + } + _ => return false, + } + true + } +} impl<'a, 'b:'a> ImportResolver<'a, 'b> { /// Add suggestions for a path that cannot be resolved. diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 1a5fce14fa8fd..590dcb8947db4 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -25,7 +25,6 @@ use rustc::hir::def::*; use rustc::hir::def::Namespace::*; use rustc::hir::def_id::{CRATE_DEF_INDEX, LOCAL_CRATE, DefId}; use rustc::hir::{Freevar, FreevarMap, TraitCandidate, TraitMap, GlobMap}; -use rustc::session::config::nightly_options; use rustc::ty; use rustc::util::nodemap::{NodeMap, NodeSet, FxHashMap, FxHashSet, DefIdMap}; use rustc::{bug, span_bug}; @@ -3124,404 +3123,6 @@ impl<'a> Resolver<'a> { ) } - /// Provides context-dependent help for errors reported by the `smart_resolve_path_fragment` - /// function. - /// Returns `true` if able to provide context-dependent help. - fn smart_resolve_context_dep_help( - &mut self, - err: &mut DiagnosticBuilder<'a>, - span: Span, - source: PathSource, - def: Def, - path_str: &str, - fallback_label: &str, - ) -> bool { - let ns = source.namespace(); - let is_expected = &|def| source.is_expected(def); - - match (def, source) { - (Def::Macro(..), _) => { - err.span_suggestion( - span, - "use `!` to invoke the macro", - format!("{}!", path_str), - Applicability::MaybeIncorrect, - ); - } - (Def::TyAlias(..), PathSource::Trait(_)) => { - err.span_label(span, "type aliases cannot be used as traits"); - if nightly_options::is_nightly_build() { - err.note("did you mean to use a trait alias?"); - } - } - (Def::Mod(..), PathSource::Expr(Some(parent))) => match parent.node { - ExprKind::Field(_, ident) => { - err.span_suggestion( - parent.span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, ident), - Applicability::MaybeIncorrect, - ); - } - ExprKind::MethodCall(ref segment, ..) => { - let span = parent.span.with_hi(segment.ident.span.hi()); - err.span_suggestion( - span, - "use the path separator to refer to an item", - format!("{}::{}", path_str, segment.ident), - Applicability::MaybeIncorrect, - ); - } - _ => return false, - }, - (Def::Enum(..), PathSource::TupleStruct) - | (Def::Enum(..), PathSource::Expr(..)) => { - if let Some(variants) = self.collect_enum_variants(def) { - err.note(&format!("did you mean to use one \ - of the following variants?\n{}", - variants.iter() - .map(|suggestion| path_names_to_string(suggestion)) - .map(|suggestion| format!("- `{}`", suggestion)) - .collect::>() - .join("\n"))); - } else { - err.note("did you mean to use one of the enum's variants?"); - } - }, - (Def::Struct(def_id), _) if ns == ValueNS => { - if let Some((ctor_def, ctor_vis)) - = self.struct_constructors.get(&def_id).cloned() { - let accessible_ctor = self.is_accessible(ctor_vis); - if is_expected(ctor_def) && !accessible_ctor { - err.span_label(span, format!("constructor is not visible \ - here due to private fields")); - } - } else { - // HACK(estebank): find a better way to figure out that this was a - // parser issue where a struct literal is being used on an expression - // where a brace being opened means a block is being started. Look - // ahead for the next text to see if `span` is followed by a `{`. - let sm = self.session.source_map(); - let mut sp = span; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet.chars().any(|c| { !c.is_whitespace() }) { - break; - } - } - _ => break, - } - } - let followed_by_brace = match sm.span_to_snippet(sp) { - Ok(ref snippet) if snippet == "{" => true, - _ => false, - }; - // In case this could be a struct literal that needs to be surrounded - // by parenthesis, find the appropriate span. - let mut i = 0; - let mut closing_brace = None; - loop { - sp = sm.next_point(sp); - match sm.span_to_snippet(sp) { - Ok(ref snippet) => { - if snippet == "}" { - let sp = span.to(sp); - if let Ok(snippet) = sm.span_to_snippet(sp) { - closing_brace = Some((sp, snippet)); - } - break; - } - } - _ => break, - } - i += 1; - // The bigger the span, the more likely we're incorrect -- - // bound it to 100 chars long. - if i > 100 { - break; - } - } - match source { - PathSource::Expr(Some(parent)) => { - match parent.node { - ExprKind::MethodCall(ref path_assignment, _) => { - err.span_suggestion( - sm.start_point(parent.span) - .to(path_assignment.ident.span), - "use `::` to access an associated function", - format!("{}::{}", - path_str, - path_assignment.ident), - Applicability::MaybeIncorrect - ); - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", - path_str), - ); - }, - } - }, - PathSource::Expr(None) if followed_by_brace == true => { - if let Some((sp, snippet)) = closing_brace { - err.span_suggestion( - sp, - "surround the struct literal with parenthesis", - format!("({})", snippet), - Applicability::MaybeIncorrect, - ); - } else { - err.span_label( - span, - format!("did you mean `({} {{ /* fields */ }})`?", - path_str), - ); - } - }, - _ => { - err.span_label( - span, - format!("did you mean `{} {{ /* fields */ }}`?", - path_str), - ); - }, - } - } - } - (Def::Union(..), _) | - (Def::Variant(..), _) | - (Def::VariantCtor(_, CtorKind::Fictive), _) if ns == ValueNS => { - err.span_label(span, format!("did you mean `{} {{ /* fields */ }}`?", - path_str)); - } - (Def::SelfTy(..), _) if ns == ValueNS => { - err.span_label(span, fallback_label); - err.note("can't use `Self` as a constructor, you must use the \ - implemented struct"); - } - (Def::TyAlias(_), _) | (Def::AssociatedTy(..), _) if ns == ValueNS => { - err.note("can't use a type alias as a constructor"); - } - _ => return false, - } - true - } - - /// Handles error reporting for `smart_resolve_path_fragment` function. - /// Creates base error and amends it with one short label and possibly some longer helps/notes. - fn smart_resolve_report_errors( - &mut self, - path: &[Segment], - span: Span, - source: PathSource<'_>, - def: Option, - ) -> (DiagnosticBuilder<'a>, Vec) { - let ident_span = path.last().map_or(span, |ident| ident.ident.span); - let ns = source.namespace(); - let is_expected = &|def| source.is_expected(def); - let is_enum_variant = &|def| if let Def::Variant(..) = def { true } else { false }; - - // Make the base error. - let expected = source.descr_expected(); - let path_str = Segment::names_to_string(path); - let item_str = path.last().unwrap().ident; - let code = source.error_code(def.is_some()); - let (base_msg, fallback_label, base_span) = if let Some(def) = def { - (format!("expected {}, found {} `{}`", expected, def.kind_name(), path_str), - format!("not a {}", expected), - span) - } else { - let item_span = path.last().unwrap().ident.span; - let (mod_prefix, mod_str) = if path.len() == 1 { - (String::new(), "this scope".to_string()) - } else if path.len() == 2 && path[0].ident.name == keywords::PathRoot.name() { - (String::new(), "the crate root".to_string()) - } else { - let mod_path = &path[..path.len() - 1]; - let mod_prefix = match self.resolve_path_without_parent_scope( - mod_path, Some(TypeNS), false, span, CrateLint::No - ) { - PathResult::Module(ModuleOrUniformRoot::Module(module)) => - module.def(), - _ => None, - }.map_or(String::new(), |def| format!("{} ", def.kind_name())); - (mod_prefix, format!("`{}`", Segment::names_to_string(mod_path))) - }; - (format!("cannot find {} `{}` in {}{}", expected, item_str, mod_prefix, mod_str), - format!("not found in {}", mod_str), - item_span) - }; - - let code = DiagnosticId::Error(code.into()); - let mut err = self.session.struct_span_err_with_code(base_span, &base_msg, code); - - // Emit help message for fake-self from other languages (e.g., `this` in Javascript). - if ["this", "my"].contains(&&*item_str.as_str()) - && self.self_value_is_available(path[0].ident.span, span) { - err.span_suggestion( - span, - "did you mean", - "self".to_string(), - Applicability::MaybeIncorrect, - ); - } - - // Emit special messages for unresolved `Self` and `self`. - if is_self_type(path, ns) { - __diagnostic_used!(E0411); - err.code(DiagnosticId::Error("E0411".into())); - err.span_label(span, format!("`Self` is only available in impls, traits, \ - and type definitions")); - return (err, Vec::new()); - } - if is_self_value(path, ns) { - debug!("smart_resolve_path_fragment: E0424, source={:?}", source); - - __diagnostic_used!(E0424); - err.code(DiagnosticId::Error("E0424".into())); - err.span_label(span, match source { - PathSource::Pat => { - format!("`self` value is a keyword \ - and may not be bound to \ - variables or shadowed") - } - _ => { - format!("`self` value is a keyword \ - only available in methods \ - with `self` parameter") - } - }); - return (err, Vec::new()); - } - - // Try to lookup name in more relaxed fashion for better error reporting. - let ident = path.last().unwrap().ident; - let candidates = self.lookup_import_candidates(ident, ns, is_expected); - if candidates.is_empty() && is_expected(Def::Enum(DefId::local(CRATE_DEF_INDEX))) { - let enum_candidates = - self.lookup_import_candidates(ident, ns, is_enum_variant); - let mut enum_candidates = enum_candidates.iter() - .map(|suggestion| { - import_candidate_to_enum_paths(&suggestion) - }).collect::>(); - enum_candidates.sort(); - - if !enum_candidates.is_empty() { - // Contextualize for E0412 "cannot find type", but don't belabor the point - // (that it's a variant) for E0573 "expected type, found variant". - let preamble = if def.is_none() { - let others = match enum_candidates.len() { - 1 => String::new(), - 2 => " and 1 other".to_owned(), - n => format!(" and {} others", n) - }; - format!("there is an enum variant `{}`{}; ", - enum_candidates[0].0, others) - } else { - String::new() - }; - let msg = format!("{}try using the variant's enum", preamble); - - err.span_suggestions( - span, - &msg, - enum_candidates.into_iter() - .map(|(_variant_path, enum_ty_path)| enum_ty_path) - // Variants re-exported in prelude doesn't mean `prelude::v1` is the - // type name! - // FIXME: is there a more principled way to do this that - // would work for other re-exports? - .filter(|enum_ty_path| enum_ty_path != "std::prelude::v1") - // Also write `Option` rather than `std::prelude::v1::Option`. - .map(|enum_ty_path| { - // FIXME #56861: DRYer prelude filtering. - enum_ty_path.trim_start_matches("std::prelude::v1::").to_owned() - }), - Applicability::MachineApplicable, - ); - } - } - if path.len() == 1 && self.self_type_is_available(span) { - if let Some(candidate) = self.lookup_assoc_candidate(ident, ns, is_expected) { - let self_is_available = self.self_value_is_available(path[0].ident.span, span); - match candidate { - AssocSuggestion::Field => { - err.span_suggestion( - span, - "try", - format!("self.{}", path_str), - Applicability::MachineApplicable, - ); - if !self_is_available { - err.span_label(span, format!("`self` value is a keyword \ - only available in \ - methods with `self` parameter")); - } - } - AssocSuggestion::MethodWithSelf if self_is_available => { - err.span_suggestion( - span, - "try", - format!("self.{}", path_str), - Applicability::MachineApplicable, - ); - } - AssocSuggestion::MethodWithSelf | AssocSuggestion::AssocItem => { - err.span_suggestion( - span, - "try", - format!("Self::{}", path_str), - Applicability::MachineApplicable, - ); - } - } - return (err, candidates); - } - } - - let mut levenshtein_worked = false; - - // Try Levenshtein algorithm. - let suggestion = self.lookup_typo_candidate(path, ns, is_expected, span); - if let Some(suggestion) = suggestion { - let msg = format!( - "{} {} with a similar name exists", - suggestion.article, suggestion.kind - ); - err.span_suggestion( - ident_span, - &msg, - suggestion.candidate.to_string(), - Applicability::MaybeIncorrect, - ); - - levenshtein_worked = true; - } - - // Try context-dependent help if relaxed lookup didn't work. - if let Some(def) = def { - if self.smart_resolve_context_dep_help(&mut err, - span, - source, - def, - &path_str, - &fallback_label) { - return (err, candidates); - } - } - - // Fallback label. - if !levenshtein_worked { - err.span_label(base_span, fallback_label); - self.type_ascription_suggestion(&mut err, base_span); - } - (err, candidates) - } - fn smart_resolve_path_fragment(&mut self, id: NodeId, qself: Option<&QSelf>, From 5c87bc85e2d7395d671311a5b8b97013b2e73d87 Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Mon, 4 Feb 2019 11:24:09 +0100 Subject: [PATCH 4/5] Minor cosmetic changes. --- src/librustc_resolve/error_reporting.rs | 12 ++++++------ src/librustc_resolve/lib.rs | 5 ++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 88892ae2cd3d8..1f7ae7c3034f3 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -417,7 +417,7 @@ impl<'a> Resolver<'a> { } impl<'a, 'b:'a> ImportResolver<'a, 'b> { - /// Add suggestions for a path that cannot be resolved. + /// Adds suggestions for a path that cannot be resolved. pub(crate) fn make_path_suggestion( &mut self, span: Span, @@ -431,7 +431,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { // On 2015 `{{root}}` is usually added implicitly. (Some(fst), Some(snd)) if fst.ident.name == keywords::PathRoot.name() && !snd.ident.is_path_segment_keyword() => {} - // `ident::...` on 2018 + // `ident::...` on 2018. (Some(fst), _) if fst.ident.span.rust_2018() && !fst.ident.is_path_segment_keyword() => { // Insert a placeholder that's later replaced by `self`/`super`/etc. @@ -470,7 +470,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - /// Suggest a missing `crate::` if that resolves to an correct module. + /// Suggests a missing `crate::` if that resolves to an correct module. /// /// ``` /// | @@ -501,7 +501,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - /// Suggest a missing `super::` if that resolves to an correct module. + /// Suggests a missing `super::` if that resolves to an correct module. /// /// ``` /// | @@ -525,7 +525,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } } - /// Suggest a missing external crate name if that resolves to an correct module. + /// Suggests a missing external crate name if that resolves to an correct module. /// /// ``` /// | @@ -546,7 +546,7 @@ impl<'a, 'b:'a> ImportResolver<'a, 'b> { } // Sort extern crate names in reverse order to get - // 1) some consistent ordering for emitted dignostics and + // 1) some consistent ordering for emitted dignostics, and // 2) `std` suggestions before `core` suggestions. let mut extern_crate_names = self.resolver.extern_prelude.iter().map(|(ident, _)| ident.name).collect::>(); diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 590dcb8947db4..69f8722a82b9b 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -1737,7 +1737,7 @@ impl<'a> Resolver<'a> { } } - /// resolve_hir_path, but takes a callback in case there was an error + /// Like `resolve_hir_path`, but takes a callback in case there was an error. fn resolve_hir_path_cb( &mut self, path: &ast::Path, @@ -1750,7 +1750,7 @@ impl<'a> Resolver<'a> { let span = path.span; let segments = &path.segments; let path = Segment::from_path(&path); - // FIXME (Manishearth): Intra doc links won't get warned of epoch changes + // FIXME(Manishearth): intra-doc links won't get warned of epoch changes. let def = match self.resolve_path_without_parent_scope(&path, Some(namespace), true, span, CrateLint::No) { PathResult::Module(ModuleOrUniformRoot::Module(module)) => @@ -5066,7 +5066,6 @@ fn import_candidate_to_enum_paths(suggestion: &ImportSuggestion) -> (String, Str (variant_path_string, enum_path_string) } - /// 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 From fd70e8e8db6f0a43f9f5bc669e03ab9bbdce7fbc Mon Sep 17 00:00:00 2001 From: Alexander Regueiro Date: Thu, 7 Feb 2019 22:25:15 +0100 Subject: [PATCH 5/5] WIP --- src/librustc_resolve/error_reporting.rs | 4 ++-- src/librustc_resolve/lib.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_resolve/error_reporting.rs b/src/librustc_resolve/error_reporting.rs index 1f7ae7c3034f3..8300e691bbea4 100644 --- a/src/librustc_resolve/error_reporting.rs +++ b/src/librustc_resolve/error_reporting.rs @@ -23,7 +23,7 @@ impl<'a> Resolver<'a> { &mut self, path: &[Segment], span: Span, - source: PathSource, + source: PathSource<'_>, def: Option, ) -> (DiagnosticBuilder<'a>, Vec) { let ident_span = path.last().map_or(span, |ident| ident.ident.span); @@ -235,7 +235,7 @@ impl<'a> Resolver<'a> { &mut self, err: &mut DiagnosticBuilder<'a>, span: Span, - source: PathSource, + source: PathSource<'_>, def: Def, path_str: &str, fallback_label: &str, diff --git a/src/librustc_resolve/lib.rs b/src/librustc_resolve/lib.rs index 69f8722a82b9b..611b956035ba7 100644 --- a/src/librustc_resolve/lib.rs +++ b/src/librustc_resolve/lib.rs @@ -3128,7 +3128,7 @@ impl<'a> Resolver<'a> { qself: Option<&QSelf>, path: &[Segment], span: Span, - source: PathSource, + source: PathSource<'_>, crate_lint: CrateLint) -> PathResolution { let ns = source.namespace();