Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unreachable-pub lint (as authorized by RFC 2126) #45569

Merged
merged 1 commit into from
Nov 3, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a test without this feature enabled to make sure there're no regressions going forward with the pub(crate) suggestion?

#![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