From 4fb57e0796cc61bec9d6a8a0392bbfe5855d693e Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Fri, 24 Nov 2017 11:21:43 -0800 Subject: [PATCH] =?UTF-8?q?one-time=20diagnostic=20and=20suggestion=20for?= =?UTF-8?q?=20re=C3=ABxporting=20private=20variant=20error?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/librustc_resolve/resolve_imports.rs | 60 +++++++++++++++++-- ...sue-46209-private-enum-variant-reexport.rs | 51 ++++++++++++++++ .../compile-fail/private-variant-reexport.rs | 8 +-- 3 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 src/test/compile-fail/issue-46209-private-enum-variant-reexport.rs diff --git a/src/librustc_resolve/resolve_imports.rs b/src/librustc_resolve/resolve_imports.rs index d72253e5a8a48..d4a08d643ab67 100644 --- a/src/librustc_resolve/resolve_imports.rs +++ b/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, .. } 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 index 0000000000000..5b23e5e815053 --- /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 or the MIT license +// , 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 c77a7532e34a2..1280aba3076ab 100644 --- 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 }