Skip to content

Commit

Permalink
one-time diagnostic and suggestion for reëxporting private variant error
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
zackmdavis committed Dec 10, 2017
1 parent 883f5e5 commit 4fb57e0
Show file tree
Hide file tree
Showing 3 changed files with 109 additions and 10 deletions.
60 changes: 54 additions & 6 deletions src/librustc_resolve/resolve_imports.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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, .. }
Expand Down
51 changes: 51 additions & 0 deletions src/test/compile-fail/issue-46209-private-enum-variant-reexport.rs
Original file line number Diff line number Diff line change
@@ -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() {}
8 changes: 4 additions & 4 deletions src/test/compile-fail/private-variant-reexport.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down

0 comments on commit 4fb57e0

Please sign in to comment.