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

We issue just one message for an erroneous glob private variant reëxport
(using the Session's one-time-diagnostics capability), but individual
(non-glob) such erroneous reëxports still get their own messages. The
suggestion to make the enum public is also one-time.

The enum variant reëxport error didn't have an associated error code
(and remedying this here is deemed out of the scope of this commit), so
we resort to the expediency of using 0 as the `DiagnosticMessageId`
value.

Adding Debug to NameResolution was helpful in development.

This resolves #46209.

Author: zackmdavis

src/librustc_resolve/resolve_imports.rs

@@ -21,6 +21,7 @@ use rustc::ty;
 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, Name, SpannedIdent, NodeId};
@@ -72,7 +73,7 @@ impl<'a> ImportDirective<'a> {
     }
 }
 
-#[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.
@@ -867,12 +868,59 @@ impl<'a, 'b:'a> ImportResolver<'a, '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, .. }

src/test/compile-fail/issue-46209-private-enum-variant-reexport.rs

@@ -0,0 +1,51 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#![feature(crate_visibility_modifier)]
+
+mod rank {
+    pub use self::Professor::*;
+    //~^ ERROR enum is private and its variants cannot be reexported
+    pub use self::Lieutenant::{JuniorGrade, Full};
+    //~^ ERROR variant `JuniorGrade` is private and cannot be reexported
+    //~| ERROR variant `Full` is private and cannot be reexported
+    pub use self::PettyOfficer::*;
+    //~^ ERROR enum is private and its variants cannot be reexported
+    pub use self::Crewman::*;
+    //~^ ERROR enum is private and its variants cannot be reexported
+
+    enum Professor {
+        Adjunct,
+        Assistant,
+        Associate,
+        Full
+    }
+
+    enum Lieutenant {
+        JuniorGrade,
+        Full,
+    }
+
+    pub(in rank) enum PettyOfficer {
+        SecondClass,
+        FirstClass,
+        Chief,
+        MasterChief
+    }
+
+    crate enum Crewman {
+        Recruit,
+        Apprentice,
+        Full
+    }
+
+}
+
+fn main() {}

src/test/compile-fail/private-variant-reexport.rs

@@ -9,19 +9,19 @@
 // except according to those terms.
 
 mod m1 {
-    pub use ::E::V; //~ ERROR variant `V` is private, and cannot be reexported
+    pub use ::E::V; //~ ERROR variant `V` is private and cannot be reexported
 }
 
 mod m2 {
-    pub use ::E::{V}; //~ ERROR variant `V` is private, and cannot be reexported
+    pub use ::E::{V}; //~ ERROR variant `V` is private and cannot be reexported
 }
 
 mod m3 {
-    pub use ::E::V::{self}; //~ ERROR variant `V` is private, and cannot be reexported
+    pub use ::E::V::{self}; //~ ERROR variant `V` is private and cannot be reexported
 }
 
 mod m4 {
-    pub use ::E::*; //~ ERROR variant `V` is private, and cannot be reexported
+    pub use ::E::*; //~ ERROR enum is private and its variants cannot be reexported
 }
 
 enum E { V }