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.

diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs
index d72253e5a8a48fc432108f2675c837b278cdaa87..d4a08d643ab6768dbd7a77df2594993777e9b305 100644 (file)
--- a/src/librustc_resolve/resolve_imports.rs
+++ b/src/librustc_resolve/resolve_imports.rs
@@ -21,6 +21,7 @@
 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 @@ 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.
@@ -867,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, .. }

diff --git a/src/test/compile-fail/issue-46209-private-enum-variant-reexport.rs b/src/test/compile-fail/issue-46209-private-enum-variant-reexport.rs
new file mode 100644 (file)
index 0000000..5b23e5e
--- /dev/null
+++ b/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() {}

diff --git a/src/test/compile-fail/private-variant-reexport.rs b/src/test/compile-fail/private-variant-reexport.rs
index c77a7532e34a281ac418c6a2e1464cc551900b2b..1280aba3076abbb3654f79619cbeadff461c64a2 100644 (file)
--- a/src/test/compile-fail/private-variant-reexport.rs
+++ b/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 }