one-time diagnostic and suggestion for reëxporting private variant error

[rust.git] / src / librustc_resolve / resolve_imports.rs

diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index b85bf18ea800c4fec42d0a7947aa566c4d49d3e6..d4a08d643ab6768dbd7a77df2594993777e9b305 100644 (file)
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -21,9 +21,10 @@
 use rustc::lint::builtin::PUB_USE_OF_PRIVATE_EXTERN_CRATE;
 use rustc::hir::def_id::DefId;
 use rustc::hir::def::*;
+use rustc::session::DiagnosticMessageId;
 use rustc::util::nodemap::{FxHashMap, FxHashSet};
 
-use syntax::ast::{Ident, SpannedIdent, NodeId};
+use syntax::ast::{Ident, Name, SpannedIdent, NodeId};
 use syntax::ext::base::Determinacy::{self, Determined, Undetermined};
 use syntax::ext::hygiene::Mark;
 use syntax::parse::token;
@@ -48,7 +49,7 @@ pub enum ImportDirectiveSubclass<'a> {
         max_vis: Cell<ty::Visibility>, // The visibility of the greatest reexport.
         // n.b. `max_vis` is only used in `finalize_import` to check for reexport errors.
     },
-    ExternCrate,
+    ExternCrate(Option<Name>),
     MacroUse,
 }
 
@@ -72,7 +73,7 @@ pub fn is_glob(&self) -> bool {
     }
 }
 
-#[derive(Clone, Default)]
+#[derive(Clone, Default, Debug)]
 /// Records information about the resolution of a name in a namespace of a module.
 pub struct NameResolution<'a> {
     /// The single imports that define the name in the namespace.
@@ -606,10 +607,16 @@ fn finalize_import(&mut self, directive: &'b ImportDirective<'b>) -> Option<(Spa
         let module_result = self.resolve_path(&module_path, None, true, span);
         let module = match module_result {
             PathResult::Module(module) => module,
-            PathResult::Failed(span, msg, _) => {
+            PathResult::Failed(span, msg, false) => {
+                resolve_error(self, span, ResolutionError::FailedToResolve(&msg));
+                return None;
+            }
+            PathResult::Failed(span, msg, true) => {
                 let (mut self_path, mut self_result) = (module_path.clone(), None);
                 if !self_path.is_empty() &&
-                    !token::Ident(self_path[0].node).is_path_segment_keyword()
+                    !token::Ident(self_path[0].node).is_path_segment_keyword() &&
+                    !(self_path.len() > 1 &&
+                      token::Ident(self_path[1].node).is_path_segment_keyword())
                 {
                     self_path[0].node.name = keywords::SelfValue.name();
                     self_result = Some(self.resolve_path(&self_path, None, false, span));
@@ -861,12 +868,59 @@ fn finalize_resolutions_in(&mut self, module: Module<'b>) {
             }
 
             match binding.kind {
-                NameBindingKind::Import { binding: orig_binding, .. } => {
+                NameBindingKind::Import { binding: orig_binding, directive, .. } => {
                     if ns == TypeNS && orig_binding.is_variant() &&
-                       !orig_binding.vis.is_at_least(binding.vis, &*self) {
-                        let msg = format!("variant `{}` is private, and cannot be reexported, \
-                                           consider declaring its enum as `pub`", ident);
-                        self.session.span_err(binding.span, &msg);
+                        !orig_binding.vis.is_at_least(binding.vis, &*self) {
+                            let msg = match directive.subclass {
+                                ImportDirectiveSubclass::SingleImport { .. } => {
+                                    format!("variant `{}` is private and cannot be reexported",
+                                            ident)
+                                },
+                                ImportDirectiveSubclass::GlobImport { .. } => {
+                                    let msg = "enum is private and its variants \
+                                               cannot be reexported".to_owned();
+                                    let error_id = (DiagnosticMessageId::ErrorId(0), // no code?!
+                                                    Some(binding.span),
+                                                    msg.clone());
+                                    let fresh = self.session.one_time_diagnostics
+                                        .borrow_mut().insert(error_id);
+                                    if !fresh {
+                                        continue;
+                                    }
+                                    msg
+                                },
+                                ref s @ _ => bug!("unexpected import subclass {:?}", s)
+                            };
+                            let mut err = self.session.struct_span_err(binding.span, &msg);
+
+                            let imported_module = directive.imported_module.get()
+                                .expect("module should exist");
+                            let resolutions = imported_module.parent.expect("parent should exist")
+                                .resolutions.borrow();
+                            let enum_path_segment_index = directive.module_path.len() - 1;
+                            let enum_ident = directive.module_path[enum_path_segment_index].node;
+
+                            let enum_resolution = resolutions.get(&(enum_ident, TypeNS))
+                                .expect("resolution should exist");
+                            let enum_span = enum_resolution.borrow()
+                                .binding.expect("binding should exist")
+                                .span;
+                            let enum_def_span = self.session.codemap().def_span(enum_span);
+                            let enum_def_snippet = self.session.codemap()
+                                .span_to_snippet(enum_def_span).expect("snippet should exist");
+                            // potentially need to strip extant `crate`/`pub(path)` for suggestion
+                            let after_vis_index = enum_def_snippet.find("enum")
+                                .expect("`enum` keyword should exist in snippet");
+                            let suggestion = format!("pub {}",
+                                                     &enum_def_snippet[after_vis_index..]);
+
+                            self.session
+                                .diag_span_suggestion_once(&mut err,
+                                                           DiagnosticMessageId::ErrorId(0),
+                                                           enum_def_span,
+                                                           "consider making the enum public",
+                                                           suggestion);
+                            err.emit();
                     }
                 }
                 NameBindingKind::Ambiguity { b1, b2, .. }
@@ -923,7 +977,7 @@ fn import_directive_subclass_to_string(subclass: &ImportDirectiveSubclass) -> St
     match *subclass {
         SingleImport { source, .. } => source.to_string(),
         GlobImport { .. } => "*".to_string(),
-        ExternCrate => "<extern crate>".to_string(),
+        ExternCrate(_) => "<extern crate>".to_string(),
         MacroUse => "#[macro_use]".to_string(),
     }
 }