From 085ec6d5286151fb06416965eb5ebaaf947b5ec8 Mon Sep 17 00:00:00 2001 From: "Zack M. Davis" Date: Thu, 2 Nov 2017 20:50:17 -0700 Subject: [PATCH] unreachable-pub lint for `pub` items not reachable from crate root This is with deepest thanks to Vadim Petrochenkov for thorough review, and resolves #45521. --- src/librustc_lint/builtin.rs | 57 ++++++++ src/librustc_lint/lib.rs | 1 + src/test/ui/lint/unreachable_pub-pub_crate.rs | 74 ++++++++++ .../ui/lint/unreachable_pub-pub_crate.stderr | 134 ++++++++++++++++++ src/test/ui/lint/unreachable_pub.rs | 69 +++++++++ src/test/ui/lint/unreachable_pub.stderr | 134 ++++++++++++++++++ 6 files changed, 469 insertions(+) create mode 100644 src/test/ui/lint/unreachable_pub-pub_crate.rs create mode 100644 src/test/ui/lint/unreachable_pub-pub_crate.stderr create mode 100644 src/test/ui/lint/unreachable_pub.rs create mode 100644 src/test/ui/lint/unreachable_pub.stderr diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 70cac419648..daee2b783ef 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -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); + } +} diff --git a/src/librustc_lint/lib.rs b/src/librustc_lint/lib.rs index 42fcf377d65..5ff29189b2c 100644 --- a/src/librustc_lint/lib.rs +++ b/src/librustc_lint/lib.rs @@ -137,6 +137,7 @@ pub fn register_builtins(store: &mut lint::LintStore, sess: Option<&Session>) { PluginAsLibrary, MutableTransmutes, UnionsWithDropFields, + UnreachablePub, ); add_builtin_with_new!(sess, diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.rs b/src/test/ui/lint/unreachable_pub-pub_crate.rs new file mode 100644 index 00000000000..b794f6c9517 --- /dev/null +++ b/src/test/ui/lint/unreachable_pub-pub_crate.rs @@ -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 or the MIT license +// , 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(); +} diff --git a/src/test/ui/lint/unreachable_pub-pub_crate.stderr b/src/test/ui/lint/unreachable_pub-pub_crate.stderr new file mode 100644 index 00000000000..84cbf87c1a1 --- /dev/null +++ b/src/test/ui/lint/unreachable_pub-pub_crate.stderr @@ -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 + diff --git a/src/test/ui/lint/unreachable_pub.rs b/src/test/ui/lint/unreachable_pub.rs new file mode 100644 index 00000000000..5812061dfdb --- /dev/null +++ b/src/test/ui/lint/unreachable_pub.rs @@ -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 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)] +#![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(); +} diff --git a/src/test/ui/lint/unreachable_pub.stderr b/src/test/ui/lint/unreachable_pub.stderr new file mode 100644 index 00000000000..bdd016ff2df --- /dev/null +++ b/src/test/ui/lint/unreachable_pub.stderr @@ -0,0 +1,134 @@ +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:19:5 + | +19 | pub use std::fmt; + | ---^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | +note: lint level defined here + --> $DIR/unreachable_pub.rs:15:9 + | +15 | #![warn(unreachable_pub)] + | ^^^^^^^^^^^^^^^ + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:21:5 + | +21 | pub struct Hydrogen { + | ---^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` field + --> $DIR/unreachable_pub.rs:23:9 + | +23 | pub neutrons: usize, + | ---^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:29:9 + | +29 | pub fn count_neutrons(&self) -> usize { self.neutrons } + | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:33:5 + | +33 | pub enum Helium {} + | ---^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:34:5 + | +34 | pub union Lithium { c1: usize, c2: u8 } + | ---^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:35:5 + | +35 | pub fn beryllium() {} + | ---^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:36:5 + | +36 | pub trait Boron {} + | ---^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:37:5 + | +37 | pub const CARBON: usize = 1; + | ---^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:38:5 + | +38 | pub static NITROGEN: usize = 2; + | ---^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:39:5 + | +39 | pub type Oxygen = bool; + | ---^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates + +warning: unreachable `pub` item + --> $DIR/unreachable_pub.rs:42:47 + | +42 | ($visibility: vis, $name: ident) => { $visibility struct $name {} } + | -----------^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` +43 | } +44 | 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.rs:47:9 + | +47 | pub fn catalyze() -> bool; + | ---^^^^^^^^^^^^^^^^^^^^^^^ + | | + | help: consider restricting its visibility: `crate` + | + = help: or consider exporting it for use by other crates +