Auto merge of #5319 - 1tgr:master, r=flip1995

Lint for `pub(crate)` items that are not crate visible due to the visibility of the module that contains them

changelog: Add `redundant_pub_crate` lint

Closes #5274.
This commit is contained in:
bors 2020-03-23 20:35:49 +00:00
commit d3989eef2d
13 changed files with 458 additions and 21 deletions

View File

@ -1439,6 +1439,7 @@ Released 2018-09-13
[`redundant_field_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_field_names
[`redundant_pattern`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern
[`redundant_pattern_matching`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pattern_matching
[`redundant_pub_crate`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_pub_crate
[`redundant_static_lifetimes`]: https://rust-lang.github.io/rust-clippy/master/index.html#redundant_static_lifetimes
[`ref_in_deref`]: https://rust-lang.github.io/rust-clippy/master/index.html#ref_in_deref
[`regex_macro`]: https://rust-lang.github.io/rust-clippy/master/index.html#regex_macro

View File

@ -5,7 +5,7 @@
A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.
[There are 361 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 362 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

View File

@ -285,6 +285,7 @@ pub mod ranges;
pub mod redundant_clone;
pub mod redundant_field_names;
pub mod redundant_pattern_matching;
pub mod redundant_pub_crate;
pub mod redundant_static_lifetimes;
pub mod reference;
pub mod regex;
@ -323,7 +324,7 @@ pub mod zero_div_zero;
pub use crate::utils::conf::Conf;
mod reexport {
crate use rustc_ast::ast::Name;
pub use rustc_ast::ast::Name;
}
/// Register all pre expansion lints
@ -745,6 +746,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
&redundant_clone::REDUNDANT_CLONE,
&redundant_field_names::REDUNDANT_FIELD_NAMES,
&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING,
&redundant_pub_crate::REDUNDANT_PUB_CRATE,
&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES,
&reference::DEREF_ADDROF,
&reference::REF_IN_DEREF,
@ -1021,6 +1023,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
store.register_late_pass(|| box wildcard_imports::WildcardImports);
store.register_early_pass(|| box macro_use::MacroUseImports);
store.register_late_pass(|| box verbose_file_reads::VerboseFileReads);
store.register_late_pass(|| box redundant_pub_crate::RedundantPubCrate::default());
store.register_group(true, "clippy::restriction", Some("clippy_restriction"), vec![
LintId::of(&arithmetic::FLOAT_ARITHMETIC),
@ -1322,6 +1325,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&redundant_clone::REDUNDANT_CLONE),
LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING),
LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(&reference::DEREF_ADDROF),
LintId::of(&reference::REF_IN_DEREF),
@ -1463,6 +1467,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
LintId::of(&question_mark::QUESTION_MARK),
LintId::of(&redundant_field_names::REDUNDANT_FIELD_NAMES),
LintId::of(&redundant_pattern_matching::REDUNDANT_PATTERN_MATCHING),
LintId::of(&redundant_pub_crate::REDUNDANT_PUB_CRATE),
LintId::of(&redundant_static_lifetimes::REDUNDANT_STATIC_LIFETIMES),
LintId::of(&regex::REGEX_MACRO),
LintId::of(&regex::TRIVIAL_REGEX),

View File

@ -0,0 +1,76 @@
use crate::utils::span_lint_and_then;
use rustc_errors::Applicability;
use rustc_hir::{Item, ItemKind, VisibilityKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_session::{declare_tool_lint, impl_lint_pass};
declare_clippy_lint! {
/// **What it does:** Checks for items declared `pub(crate)` that are not crate visible because they
/// are inside a private module.
///
/// **Why is this bad?** Writing `pub(crate)` is misleading when it's redundant due to the parent
/// module's visibility.
///
/// **Known problems:** None.
///
/// **Example:**
///
/// ```rust
/// mod internal {
/// pub(crate) fn internal_fn() { }
/// }
/// ```
/// This function is not visible outside the module and it can be declared with `pub` or
/// private visibility
/// ```rust
/// mod internal {
/// pub fn internal_fn() { }
/// }
/// ```
pub REDUNDANT_PUB_CRATE,
style,
"Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them."
}
#[derive(Default)]
pub struct RedundantPubCrate {
is_exported: Vec<bool>,
}
impl_lint_pass!(RedundantPubCrate => [REDUNDANT_PUB_CRATE]);
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for RedundantPubCrate {
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
if let VisibilityKind::Crate { .. } = item.vis.node {
if !cx.access_levels.is_exported(item.hir_id) {
if let Some(false) = self.is_exported.last() {
let span = item.span.with_hi(item.ident.span.hi());
span_lint_and_then(
cx,
REDUNDANT_PUB_CRATE,
span,
&format!("pub(crate) {} inside private module", item.kind.descr()),
|db| {
db.span_suggestion(
item.vis.span,
"consider using",
"pub".to_string(),
Applicability::MachineApplicable,
);
},
)
}
}
}
if let ItemKind::Mod { .. } = item.kind {
self.is_exported.push(cx.access_levels.is_exported(item.hir_id));
}
}
fn check_item_post(&mut self, _cx: &LateContext<'a, 'tcx>, item: &'tcx Item<'tcx>) {
if let ItemKind::Mod { .. } = item.kind {
self.is_exported.pop().expect("unbalanced check_item/check_item_post");
}
}
}

View File

@ -80,7 +80,7 @@ macro_rules! define_Conf {
$(
mod $config {
use serde::Deserialize;
crate fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<$Ty, D::Error> {
pub fn deserialize<'de, D: serde::Deserializer<'de>>(deserializer: D) -> Result<$Ty, D::Error> {
use super::super::{ERRORS, Error};
Ok(
<$Ty>::deserialize(deserializer).unwrap_or_else(|e| {

View File

@ -1,7 +1,7 @@
use rustc_ast::ast::{Lit, LitFloatType, LitIntType, LitKind};
#[derive(Debug, PartialEq)]
pub(crate) enum Radix {
pub enum Radix {
Binary,
Octal,
Decimal,
@ -26,7 +26,7 @@ pub fn format(lit: &str, type_suffix: Option<&str>, float: bool) -> String {
}
#[derive(Debug)]
pub(crate) struct NumericLiteral<'a> {
pub struct NumericLiteral<'a> {
/// Which radix the literal was represented in.
pub radix: Radix,
/// The radix prefix, if present.

View File

@ -6,7 +6,7 @@ pub use lint::Lint;
pub use lint::LINT_LEVELS;
// begin lint list, do not remove this comment, its used in `update_lints`
pub const ALL_LINTS: [Lint; 361] = [
pub const ALL_LINTS: [Lint; 362] = [
Lint {
name: "absurd_extreme_comparisons",
group: "correctness",
@ -1764,6 +1764,13 @@ pub const ALL_LINTS: [Lint; 361] = [
deprecation: None,
module: "redundant_pattern_matching",
},
Lint {
name: "redundant_pub_crate",
group: "style",
desc: "Using `pub(crate)` visibility on items that are not crate visible due to the visibility of the module that contains them.",
deprecation: None,
module: "redundant_pub_crate",
},
Lint {
name: "redundant_static_lifetimes",
group: "style",

View File

@ -0,0 +1,107 @@
// run-rustfix
#![allow(dead_code)]
#![warn(clippy::redundant_pub_crate)]
mod m1 {
fn f() {}
pub fn g() {} // private due to m1
pub fn h() {}
mod m1_1 {
fn f() {}
pub fn g() {} // private due to m1_1 and m1
pub fn h() {}
}
pub mod m1_2 {
// ^ private due to m1
fn f() {}
pub fn g() {} // private due to m1_2 and m1
pub fn h() {}
}
pub mod m1_3 {
fn f() {}
pub fn g() {} // private due to m1
pub fn h() {}
}
}
pub(crate) mod m2 {
fn f() {}
pub fn g() {} // already crate visible due to m2
pub fn h() {}
mod m2_1 {
fn f() {}
pub fn g() {} // private due to m2_1
pub fn h() {}
}
pub mod m2_2 {
// ^ already crate visible due to m2
fn f() {}
pub fn g() {} // already crate visible due to m2_2 and m2
pub fn h() {}
}
pub mod m2_3 {
fn f() {}
pub fn g() {} // already crate visible due to m2
pub fn h() {}
}
}
pub mod m3 {
fn f() {}
pub(crate) fn g() {} // ok: m3 is exported
pub fn h() {}
mod m3_1 {
fn f() {}
pub fn g() {} // private due to m3_1
pub fn h() {}
}
pub(crate) mod m3_2 {
// ^ ok
fn f() {}
pub fn g() {} // already crate visible due to m3_2
pub fn h() {}
}
pub mod m3_3 {
fn f() {}
pub(crate) fn g() {} // ok: m3 and m3_3 are exported
pub fn h() {}
}
}
mod m4 {
fn f() {}
pub fn g() {} // private: not re-exported by `pub use m4::*`
pub fn h() {}
mod m4_1 {
fn f() {}
pub fn g() {} // private due to m4_1
pub fn h() {}
}
pub mod m4_2 {
// ^ private: not re-exported by `pub use m4::*`
fn f() {}
pub fn g() {} // private due to m4_2
pub fn h() {}
}
pub mod m4_3 {
fn f() {}
pub(crate) fn g() {} // ok: m4_3 is re-exported by `pub use m4::*`
pub fn h() {}
}
}
pub use m4::*;
fn main() {}

View File

@ -0,0 +1,107 @@
// run-rustfix
#![allow(dead_code)]
#![warn(clippy::redundant_pub_crate)]
mod m1 {
fn f() {}
pub(crate) fn g() {} // private due to m1
pub fn h() {}
mod m1_1 {
fn f() {}
pub(crate) fn g() {} // private due to m1_1 and m1
pub fn h() {}
}
pub(crate) mod m1_2 {
// ^ private due to m1
fn f() {}
pub(crate) fn g() {} // private due to m1_2 and m1
pub fn h() {}
}
pub mod m1_3 {
fn f() {}
pub(crate) fn g() {} // private due to m1
pub fn h() {}
}
}
pub(crate) mod m2 {
fn f() {}
pub(crate) fn g() {} // already crate visible due to m2
pub fn h() {}
mod m2_1 {
fn f() {}
pub(crate) fn g() {} // private due to m2_1
pub fn h() {}
}
pub(crate) mod m2_2 {
// ^ already crate visible due to m2
fn f() {}
pub(crate) fn g() {} // already crate visible due to m2_2 and m2
pub fn h() {}
}
pub mod m2_3 {
fn f() {}
pub(crate) fn g() {} // already crate visible due to m2
pub fn h() {}
}
}
pub mod m3 {
fn f() {}
pub(crate) fn g() {} // ok: m3 is exported
pub fn h() {}
mod m3_1 {
fn f() {}
pub(crate) fn g() {} // private due to m3_1
pub fn h() {}
}
pub(crate) mod m3_2 {
// ^ ok
fn f() {}
pub(crate) fn g() {} // already crate visible due to m3_2
pub fn h() {}
}
pub mod m3_3 {
fn f() {}
pub(crate) fn g() {} // ok: m3 and m3_3 are exported
pub fn h() {}
}
}
mod m4 {
fn f() {}
pub(crate) fn g() {} // private: not re-exported by `pub use m4::*`
pub fn h() {}
mod m4_1 {
fn f() {}
pub(crate) fn g() {} // private due to m4_1
pub fn h() {}
}
pub(crate) mod m4_2 {
// ^ private: not re-exported by `pub use m4::*`
fn f() {}
pub(crate) fn g() {} // private due to m4_2
pub fn h() {}
}
pub mod m4_3 {
fn f() {}
pub(crate) fn g() {} // ok: m4_3 is re-exported by `pub use m4::*`
pub fn h() {}
}
}
pub use m4::*;
fn main() {}

View File

@ -0,0 +1,132 @@
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:7:5
|
LL | pub(crate) fn g() {} // private due to m1
| ----------^^^^^
| |
| help: consider using: `pub`
|
= note: `-D clippy::redundant-pub-crate` implied by `-D warnings`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:12:9
|
LL | pub(crate) fn g() {} // private due to m1_1 and m1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) module inside private module
--> $DIR/redundant_pub_crate.rs:16:5
|
LL | pub(crate) mod m1_2 {
| ----------^^^^^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:19:9
|
LL | pub(crate) fn g() {} // private due to m1_2 and m1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:25:9
|
LL | pub(crate) fn g() {} // private due to m1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:32:5
|
LL | pub(crate) fn g() {} // already crate visible due to m2
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:37:9
|
LL | pub(crate) fn g() {} // private due to m2_1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) module inside private module
--> $DIR/redundant_pub_crate.rs:41:5
|
LL | pub(crate) mod m2_2 {
| ----------^^^^^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:44:9
|
LL | pub(crate) fn g() {} // already crate visible due to m2_2 and m2
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:50:9
|
LL | pub(crate) fn g() {} // already crate visible due to m2
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:62:9
|
LL | pub(crate) fn g() {} // private due to m3_1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:69:9
|
LL | pub(crate) fn g() {} // already crate visible due to m3_2
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:82:5
|
LL | pub(crate) fn g() {} // private: not re-exported by `pub use m4::*`
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:87:9
|
LL | pub(crate) fn g() {} // private due to m4_1
| ----------^^^^^
| |
| help: consider using: `pub`
error: pub(crate) module inside private module
--> $DIR/redundant_pub_crate.rs:91:5
|
LL | pub(crate) mod m4_2 {
| ----------^^^^^^^^^
| |
| help: consider using: `pub`
error: pub(crate) function inside private module
--> $DIR/redundant_pub_crate.rs:94:9
|
LL | pub(crate) fn g() {} // private due to m4_2
| ----------^^^^^
| |
| help: consider using: `pub`
error: aborting due to 16 previous errors

View File

@ -2,6 +2,7 @@
// aux-build:wildcard_imports_helper.rs
#![warn(clippy::wildcard_imports)]
#![allow(clippy::redundant_pub_crate)]
#![allow(unused)]
#![warn(unused_imports)]

View File

@ -2,6 +2,7 @@
// aux-build:wildcard_imports_helper.rs
#![warn(clippy::wildcard_imports)]
#![allow(clippy::redundant_pub_crate)]
#![allow(unused)]
#![warn(unused_imports)]

View File

@ -1,5 +1,5 @@
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:10:5
--> $DIR/wildcard_imports.rs:11:5
|
LL | use crate::fn_mod::*;
| ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo`
@ -7,85 +7,85 @@ LL | use crate::fn_mod::*;
= note: `-D clippy::wildcard-imports` implied by `-D warnings`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:11:5
--> $DIR/wildcard_imports.rs:12:5
|
LL | use crate::mod_mod::*;
| ^^^^^^^^^^^^^^^^^ help: try: `crate::mod_mod::inner_mod`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:12:5
--> $DIR/wildcard_imports.rs:13:5
|
LL | use crate::multi_fn_mod::*;
| ^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate::multi_fn_mod::{multi_bar, multi_foo, multi_inner_mod}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:14:5
--> $DIR/wildcard_imports.rs:15:5
|
LL | use crate::struct_mod::*;
| ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::struct_mod::{A, inner_struct_mod}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:18:5
--> $DIR/wildcard_imports.rs:19:5
|
LL | use wildcard_imports_helper::inner::inner_for_self_import::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::inner::inner_for_self_import::inner_extern_bar`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:19:5
--> $DIR/wildcard_imports.rs:20:5
|
LL | use wildcard_imports_helper::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:88:13
--> $DIR/wildcard_imports.rs:89:13
|
LL | use crate::fn_mod::*;
| ^^^^^^^^^^^^^^^^ help: try: `crate::fn_mod::foo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:94:75
--> $DIR/wildcard_imports.rs:95:75
|
LL | use wildcard_imports_helper::inner::inner_for_self_import::{self, *};
| ^ help: try: `inner_extern_foo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:95:13
--> $DIR/wildcard_imports.rs:96:13
|
LL | use wildcard_imports_helper::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternA, extern_foo}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:106:20
--> $DIR/wildcard_imports.rs:107:20
|
LL | use self::{inner::*, inner2::*};
| ^^^^^^^^ help: try: `inner::inner_foo`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:106:30
--> $DIR/wildcard_imports.rs:107:30
|
LL | use self::{inner::*, inner2::*};
| ^^^^^^^^^ help: try: `inner2::inner_bar`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:113:13
--> $DIR/wildcard_imports.rs:114:13
|
LL | use wildcard_imports_helper::*;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `wildcard_imports_helper::{ExternExportedEnum, ExternExportedStruct, extern_exported}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:142:9
--> $DIR/wildcard_imports.rs:143:9
|
LL | use crate::in_fn_test::*;
| ^^^^^^^^^^^^^^^^^^^^ help: try: `crate::in_fn_test::{ExportedEnum, ExportedStruct, exported}`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:151:9
--> $DIR/wildcard_imports.rs:152:9
|
LL | use crate:: in_fn_test:: * ;
| ^^^^^^^^^^^^^^^^^^^^^^^^ help: try: `crate:: in_fn_test::exported`
error: usage of wildcard import
--> $DIR/wildcard_imports.rs:152:9
--> $DIR/wildcard_imports.rs:153:9
|
LL | use crate:: fn_mod::
| _________^