diff --git a/src/librustc/lint/mod.rs b/src/librustc/lint/mod.rs index 906cae53710ff..f0761ce617865 100644 --- a/src/librustc/lint/mod.rs +++ b/src/librustc/lint/mod.rs @@ -37,7 +37,7 @@ use errors::{DiagnosticBuilder, DiagnosticId}; use hir::def_id::{CrateNum, LOCAL_CRATE}; use hir::intravisit::{self, FnKind}; use hir; -use session::Session; +use session::{Session, DiagnosticMessageId}; use std::hash; use syntax::ast; use syntax::codemap::MultiSpan; @@ -423,7 +423,7 @@ pub fn struct_lint_level<'a>(sess: &'a Session, LintSource::Default => { sess.diag_note_once( &mut err, - lint, + DiagnosticMessageId::from(lint), &format!("#[{}({})] on by default", level.as_str(), name)); } LintSource::CommandLine(lint_flag_val) => { @@ -437,24 +437,25 @@ pub fn struct_lint_level<'a>(sess: &'a Session, if lint_flag_val.as_str() == name { sess.diag_note_once( &mut err, - lint, + DiagnosticMessageId::from(lint), &format!("requested on the command line with `{} {}`", flag, hyphen_case_lint_name)); } else { let hyphen_case_flag_val = lint_flag_val.as_str().replace("_", "-"); sess.diag_note_once( &mut err, - lint, + DiagnosticMessageId::from(lint), &format!("`{} {}` implied by `{} {}`", flag, hyphen_case_lint_name, flag, hyphen_case_flag_val)); } } LintSource::Node(lint_attr_name, src) => { - sess.diag_span_note_once(&mut err, lint, src, "lint level defined here"); + sess.diag_span_note_once(&mut err, DiagnosticMessageId::from(lint), + src, "lint level defined here"); if lint_attr_name.as_str() != name { let level_str = level.as_str(); - sess.diag_note_once(&mut err, lint, + sess.diag_note_once(&mut err, DiagnosticMessageId::from(lint), &format!("#[{}({})] implied by #[{}({})]", level_str, name, level_str, lint_attr_name)); } diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs index df5805bacd41a..36c1966bdc834 100644 --- a/src/librustc/session/mod.rs +++ b/src/librustc/session/mod.rs @@ -161,6 +161,7 @@ pub struct PerfStats { enum DiagnosticBuilderMethod { Note, SpanNote, + SpanSuggestion(String), // suggestion // add more variants as needed to support one-time diagnostics } @@ -173,6 +174,12 @@ pub enum DiagnosticMessageId { StabilityId(u32) // issue number } +impl From<&'static lint::Lint> for DiagnosticMessageId { + fn from(lint: &'static lint::Lint) -> Self { + DiagnosticMessageId::LintId(lint::LintId::of(lint)) + } +} + impl Session { pub fn local_crate_disambiguator(&self) -> CrateDisambiguator { match *self.crate_disambiguator.borrow() { @@ -358,10 +365,11 @@ impl Session { fn diag_once<'a, 'b>(&'a self, diag_builder: &'b mut DiagnosticBuilder<'a>, method: DiagnosticBuilderMethod, - lint: &'static lint::Lint, message: &str, span: Option) { + msg_id: DiagnosticMessageId, + message: &str, + span_maybe: Option) { - let lint_id = DiagnosticMessageId::LintId(lint::LintId::of(lint)); - let id_span_message = (lint_id, span, message.to_owned()); + let id_span_message = (msg_id, span_maybe, message.to_owned()); let fresh = self.one_time_diagnostics.borrow_mut().insert(id_span_message); if fresh { match method { @@ -369,7 +377,12 @@ impl Session { diag_builder.note(message); }, DiagnosticBuilderMethod::SpanNote => { - diag_builder.span_note(span.expect("span_note expects a span"), message); + let span = span_maybe.expect("span_note needs a span"); + diag_builder.span_note(span, message); + }, + DiagnosticBuilderMethod::SpanSuggestion(suggestion) => { + let span = span_maybe.expect("span_suggestion needs a span"); + diag_builder.span_suggestion(span, message, suggestion); } } } @@ -377,14 +390,25 @@ impl Session { pub fn diag_span_note_once<'a, 'b>(&'a self, diag_builder: &'b mut DiagnosticBuilder<'a>, - lint: &'static lint::Lint, span: Span, message: &str) { - self.diag_once(diag_builder, DiagnosticBuilderMethod::SpanNote, lint, message, Some(span)); + msg_id: DiagnosticMessageId, span: Span, message: &str) { + self.diag_once(diag_builder, DiagnosticBuilderMethod::SpanNote, + msg_id, message, Some(span)); } pub fn diag_note_once<'a, 'b>(&'a self, diag_builder: &'b mut DiagnosticBuilder<'a>, - lint: &'static lint::Lint, message: &str) { - self.diag_once(diag_builder, DiagnosticBuilderMethod::Note, lint, message, None); + msg_id: DiagnosticMessageId, message: &str) { + self.diag_once(diag_builder, DiagnosticBuilderMethod::Note, msg_id, message, None); + } + + pub fn diag_span_suggestion_once<'a, 'b>(&'a self, + diag_builder: &'b mut DiagnosticBuilder<'a>, + msg_id: DiagnosticMessageId, + span: Span, + message: &str, + suggestion: String) { + self.diag_once(diag_builder, DiagnosticBuilderMethod::SpanSuggestion(suggestion), + msg_id, message, Some(span)); } pub fn codemap<'a>(&'a self) -> &'a codemap::CodeMap { 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 }