Skip to content

Commit

Permalink
unreachable-pub lint for pub items not reachable from crate root
Browse files Browse the repository at this point in the history
This is with deepest thanks to Vadim Petrochenkov for thorough review, and
resolves #45521.
  • Loading branch information
zackmdavis committed Nov 3, 2017
1 parent e340996 commit 085ec6d
Show file tree
Hide file tree
Showing 6 changed files with 469 additions and 0 deletions.
57 changes: 57 additions & 0 deletions src/librustc_lint/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1301,3 +1301,60 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnionsWithDropFields {
}
}
}

/// Lint for items marked `pub` that aren't reachable from other crates
pub struct UnreachablePub;

declare_lint! {
UNREACHABLE_PUB,
Allow,
"`pub` items not reachable from crate root"
}

impl LintPass for UnreachablePub {
fn get_lints(&self) -> LintArray {
lint_array!(UNREACHABLE_PUB)
}
}

impl UnreachablePub {
fn perform_lint(&self, cx: &LateContext, what: &str, id: ast::NodeId,
vis: &hir::Visibility, span: Span, exportable: bool) {
if !cx.access_levels.is_reachable(id) && *vis == hir::Visibility::Public {
let def_span = cx.tcx.sess.codemap().def_span(span);
let mut err = cx.struct_span_lint(UNREACHABLE_PUB, def_span,
&format!("unreachable `pub` {}", what));
// visibility is token at start of declaration (can be macro
// variable rather than literal `pub`)
let pub_span = cx.tcx.sess.codemap().span_until_char(def_span, ' ');
let replacement = if cx.tcx.sess.features.borrow().crate_visibility_modifier {
"crate"
} else {
"pub(crate)"
}.to_owned();
err.span_suggestion(pub_span, "consider restricting its visibility", replacement);
if exportable {
err.help("or consider exporting it for use by other crates");
}
err.emit();
}
}
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UnreachablePub {
fn check_item(&mut self, cx: &LateContext, item: &hir::Item) {
self.perform_lint(cx, "item", item.id, &item.vis, item.span, true);
}

fn check_foreign_item(&mut self, cx: &LateContext, foreign_item: &hir::ForeignItem) {
self.perform_lint(cx, "item", foreign_item.id, &foreign_item.vis, foreign_item.span, true);
}

fn check_struct_field(&mut self, cx: &LateContext, field: &hir::StructField) {
self.perform_lint(cx, "field", field.id, &field.vis, field.span, false);
}

fn check_impl_item(&mut self, cx: &LateContext, impl_item: &hir::ImplItem) {
self.perform_lint(cx, "item", impl_item.id, &impl_item.vis, impl_item.span, false);
}
}
1 change: 1 addition & 0 deletions src/librustc_lint/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) {
PluginAsLibrary,
MutableTransmutes,
UnionsWithDropFields,
UnreachablePub,
);

add_builtin_with_new!(sess,
Expand Down
74 changes: 74 additions & 0 deletions src/test/ui/lint/unreachable_pub-pub_crate.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
// 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.

// This is just like unreachable_pub.rs, but without the
// `crate_visibility_modifier` feature (so that we can test the suggestions to
// use `pub(crate)` that are given when that feature is off, as opposed to the
// suggestions to use `crate` given when it is on). When that feature becomes
// stable, this test can be deleted.

#![feature(macro_vis_matcher)]

#![allow(unused)]
#![warn(unreachable_pub)]

mod private_mod {
// non-leaked `pub` items in private module should be linted
pub use std::fmt;

pub struct Hydrogen {
// `pub` struct fields, too
pub neutrons: usize,
// (... but not more-restricted fields)
pub(crate) electrons: usize
}
impl Hydrogen {
// impls, too
pub fn count_neutrons(&self) -> usize { self.neutrons }
pub(crate) fn count_electrons(&self) -> usize { self.electrons }
}

pub enum Helium {}
pub union Lithium { c1: usize, c2: u8 }
pub fn beryllium() {}
pub trait Boron {}
pub const CARBON: usize = 1;
pub static NITROGEN: usize = 2;
pub type Oxygen = bool;

macro_rules! define_empty_struct_with_visibility {
($visibility: vis, $name: ident) => { $visibility struct $name {} }
}
define_empty_struct_with_visibility!(pub, Fluorine);

extern {
pub fn catalyze() -> bool;
}

// items leaked through signatures (see `get_neon` below) are OK
pub struct Neon {}

// crate-visible items are OK
pub(crate) struct Sodium {}
}

pub mod public_mod {
// module is public: these are OK, too
pub struct Magnesium {}
pub(crate) struct Aluminum {}
}

pub fn get_neon() -> private_mod::Neon {
private_mod::Neon {}
}

fn main() {
let _ = get_neon();
}
134 changes: 134 additions & 0 deletions src/test/ui/lint/unreachable_pub-pub_crate.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:24:5
|
24 | pub use std::fmt;
| ---^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
note: lint level defined here
--> $DIR/unreachable_pub-pub_crate.rs:20:9
|
20 | #![warn(unreachable_pub)]
| ^^^^^^^^^^^^^^^
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:26:5
|
26 | pub struct Hydrogen {
| ---^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` field
--> $DIR/unreachable_pub-pub_crate.rs:28:9
|
28 | pub neutrons: usize,
| ---^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:34:9
|
34 | pub fn count_neutrons(&self) -> usize { self.neutrons }
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:38:5
|
38 | pub enum Helium {}
| ---^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:39:5
|
39 | pub union Lithium { c1: usize, c2: u8 }
| ---^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:40:5
|
40 | pub fn beryllium() {}
| ---^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:41:5
|
41 | pub trait Boron {}
| ---^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:42:5
|
42 | pub const CARBON: usize = 1;
| ---^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:43:5
|
43 | pub static NITROGEN: usize = 2;
| ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:44:5
|
44 | pub type Oxygen = bool;
| ---^^^^^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:47:47
|
47 | ($visibility: vis, $name: ident) => { $visibility struct $name {} }
| -----------^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
48 | }
49 | define_empty_struct_with_visibility!(pub, Fluorine);
| ---------------------------------------------------- in this macro invocation
|
= help: or consider exporting it for use by other crates

warning: unreachable `pub` item
--> $DIR/unreachable_pub-pub_crate.rs:52:9
|
52 | pub fn catalyze() -> bool;
| ---^^^^^^^^^^^^^^^^^^^^^^^
| |
| help: consider restricting its visibility: `pub(crate)`
|
= help: or consider exporting it for use by other crates

69 changes: 69 additions & 0 deletions src/test/ui/lint/unreachable_pub.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
// 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)]
#![feature(macro_vis_matcher)]

#![allow(unused)]
#![warn(unreachable_pub)]

mod private_mod {
// non-leaked `pub` items in private module should be linted
pub use std::fmt;

pub struct Hydrogen {
// `pub` struct fields, too
pub neutrons: usize,
// (... but not more-restricted fields)
crate electrons: usize
}
impl Hydrogen {
// impls, too
pub fn count_neutrons(&self) -> usize { self.neutrons }
crate fn count_electrons(&self) -> usize { self.electrons }
}

pub enum Helium {}
pub union Lithium { c1: usize, c2: u8 }
pub fn beryllium() {}
pub trait Boron {}
pub const CARBON: usize = 1;
pub static NITROGEN: usize = 2;
pub type Oxygen = bool;

macro_rules! define_empty_struct_with_visibility {
($visibility: vis, $name: ident) => { $visibility struct $name {} }
}
define_empty_struct_with_visibility!(pub, Fluorine);

extern {
pub fn catalyze() -> bool;
}

// items leaked through signatures (see `get_neon` below) are OK
pub struct Neon {}

// crate-visible items are OK
crate struct Sodium {}
}

pub mod public_mod {
// module is public: these are OK, too
pub struct Magnesium {}
crate struct Aluminum {}
}

pub fn get_neon() -> private_mod::Neon {
private_mod::Neon {}
}

fn main() {
let _ = get_neon();
}
Loading

0 comments on commit 085ec6d

Please sign in to comment.