New lints ['needless_pub_self], [pub_with_shorthand] and [pub_without_shorthand]

CHANGELOG.md

@@ -5047,6 +5047,7 @@ Released 2018-09-13
 [`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take
 [`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals
 [`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
+[`needless_pub_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pub_self
 [`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
 [`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
 [`needless_raw_string_hashes`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_raw_string_hashes
@@ -5121,6 +5122,8 @@ Released 2018-09-13
 [`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
 [`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
 [`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use
+[`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand
+[`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand
 [`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark
 [`question_mark_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark_used
 [`range_minus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_minus_one

clippy_lints/src/declared_lints.rs

@@ -666,6 +666,9 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
     crate::useless_conversion::USELESS_CONVERSION_INFO,
     crate::vec::USELESS_VEC_INFO,
     crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO,
+    crate::visibility::NEEDLESS_PUB_SELF_INFO,
+    crate::visibility::PUB_WITHOUT_SHORTHAND_INFO,
+    crate::visibility::PUB_WITH_SHORTHAND_INFO,
     crate::wildcard_imports::ENUM_GLOB_USE_INFO,
     crate::wildcard_imports::WILDCARD_IMPORTS_INFO,
     crate::write::PRINTLN_EMPTY_STRING_INFO,

clippy_lints/src/lib.rs

@@ -338,6 +338,7 @@ mod use_self;
 mod useless_conversion;
 mod vec;
 mod vec_init_then_push;
+mod visibility;
 mod wildcard_imports;
 mod write;
 mod zero_div_zero;
@@ -1070,6 +1071,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         })
     });
     store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
+    store.register_early_pass(|| Box::new(visibility::Visibility));
     // add lints here, do not remove this comment, it's used in `new_lint`
 }
 

clippy_lints/src/visibility.rs

@@ -0,0 +1,131 @@
+use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt};
+use rustc_ast::ast::{Item, VisibilityKind};
+use rustc_errors::Applicability;
+use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
+use rustc_middle::lint::in_external_macro;
+use rustc_session::{declare_lint_pass, declare_tool_lint};
+use rustc_span::{symbol::kw, Span};
+
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `pub(self)` and `pub(in self)`.
+    ///
+    /// ### Why is this bad?
+    /// It's unnecessary, omitting the `pub` entirely will give the same results.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// pub(self) type OptBox<T> = Option<Box<T>>;
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// type OptBox<T> = Option<Box<T>>;
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub NEEDLESS_PUB_SELF,
+    style,
+    "checks for usage of `pub(self)` and `pub(in self)`."
+}
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `pub(<loc>)` with `in`.
+    ///
+    /// ### Why is this bad?
+    /// Consistency. Use it or don't, just be consistent about it.
+    ///
+    /// Also see the `pub_without_shorthand` lint for an alternative.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// pub(super) type OptBox<T> = Option<Box<T>>;
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// pub(in super) type OptBox<T> = Option<Box<T>>;
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub PUB_WITH_SHORTHAND,
+    restriction,
+    "disallows usage of `pub(<loc>)`, without `in`"
+}
+declare_clippy_lint! {
+    /// ### What it does
+    /// Checks for usage of `pub(<loc>)` without `in`.
+    ///
+    /// Note: As you cannot write a module's path in `pub(<loc>)`, this will only trigger on
+    /// `pub(super)` and the like.
+    ///
+    /// ### Why is this bad?
+    /// Consistency. Use it or don't, just be consistent about it.
+    ///
+    /// Also see the `pub_with_shorthand` lint for an alternative.
+    ///
+    /// ### Example
+    /// ```rust,ignore
+    /// pub(in super) type OptBox<T> = Option<Box<T>>;
+    /// ```
+    /// Use instead:
+    /// ```rust,ignore
+    /// pub(super) type OptBox<T> = Option<Box<T>>;
+    /// ```
+    #[clippy::version = "1.72.0"]
+    pub PUB_WITHOUT_SHORTHAND,
+    restriction,
+    "disallows usage of `pub(in <loc>)` with `in`"
+}
+declare_lint_pass!(Visibility => [NEEDLESS_PUB_SELF, PUB_WITH_SHORTHAND, PUB_WITHOUT_SHORTHAND]);
+
+impl EarlyLintPass for Visibility {
+    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+        if !in_external_macro(cx.sess(), item.span)
+            && let VisibilityKind::Restricted { path, shorthand, .. } = &item.vis.kind
+        {
+            if **path == kw::SelfLower && let Some(false) = is_from_proc_macro(cx, item.vis.span) {
+                span_lint_and_sugg(
+                    cx,
+                    NEEDLESS_PUB_SELF,
+                    item.vis.span,
+                    &format!("unnecessary `pub({}self)`", if *shorthand { "" } else { "in " }),
+                    "remove it",
+                    String::new(),
+                    Applicability::MachineApplicable,
+                );
+            }
+
+            if (**path == kw::Super || **path == kw::SelfLower || **path == kw::Crate)
+                && !*shorthand
+                && let [.., last] = &*path.segments
+                && let Some(false) = is_from_proc_macro(cx, item.vis.span)
+            {
+                span_lint_and_sugg(
+                    cx,
+                    PUB_WITHOUT_SHORTHAND,
+                    item.vis.span,
+                    "usage of `pub` with `in`",
+                    "remove it",
+                    format!("pub({})", last.ident),
+                    Applicability::MachineApplicable,
+                );
+            }
+
+            if *shorthand
+                && let [.., last] = &*path.segments
+                && let Some(false) = is_from_proc_macro(cx, item.vis.span)
+            {
+                span_lint_and_sugg(
+                    cx,
+                    PUB_WITH_SHORTHAND,
+                    item.vis.span,
+                    "usage of `pub` without `in`",
+                    "add it",
+                    format!("pub(in {})", last.ident),
+                    Applicability::MachineApplicable,
+                );
+            }
+        }
+    }
+}
+
+fn is_from_proc_macro(cx: &EarlyContext<'_>, span: Span) -> Option<bool> {
+    snippet_opt(cx, span).map(|s| !s.starts_with("pub"))
+}

tests/ui/manual_async_fn.fixed

@@ -1,6 +1,6 @@
 //@run-rustfix
 #![warn(clippy::manual_async_fn)]
-#![allow(unused)]
+#![allow(clippy::needless_pub_self, unused)]
 
 use std::future::Future;
 

tests/ui/manual_async_fn.rs

@@ -1,6 +1,6 @@
 //@run-rustfix
 #![warn(clippy::manual_async_fn)]
-#![allow(unused)]
+#![allow(clippy::needless_pub_self, unused)]
 
 use std::future::Future;
 

tests/ui/needless_pub_self.fixed

@@ -0,0 +1,33 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![feature(custom_inner_attributes)]
+#![allow(unused)]
+#![warn(clippy::needless_pub_self)]
+#![no_main]
+#![rustfmt::skip] // rustfmt will remove `in`, understandable
+                  // but very annoying for our purposes!
+
+#[macro_use]
+extern crate proc_macros;
+
+ fn a() {}
+ fn b() {}
+
+pub fn c() {}
+mod a {
+    pub(in super) fn d() {}
+    pub(super) fn e() {}
+     fn f() {}
+}
+
+external! {
+    pub(self) fn g() {}
+    pub(in self) fn h() {}
+}
+with_span! {
+    span
+    pub(self) fn i() {}
+    pub(in self) fn j() {}
+}
+
+// not really anything more to test. just a really simple lint overall

tests/ui/needless_pub_self.rs

@@ -0,0 +1,33 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![feature(custom_inner_attributes)]
+#![allow(unused)]
+#![warn(clippy::needless_pub_self)]
+#![no_main]
+#![rustfmt::skip] // rustfmt will remove `in`, understandable
+                  // but very annoying for our purposes!
+
+#[macro_use]
+extern crate proc_macros;
+
+pub(self) fn a() {}
+pub(in self) fn b() {}
+
+pub fn c() {}
+mod a {
+    pub(in super) fn d() {}
+    pub(super) fn e() {}
+    pub(self) fn f() {}
+}
+
+external! {
+    pub(self) fn g() {}
+    pub(in self) fn h() {}
+}
+with_span! {
+    span
+    pub(self) fn i() {}
+    pub(in self) fn j() {}
+}
+
+// not really anything more to test. just a really simple lint overall

tests/ui/needless_pub_self.stderr

@@ -0,0 +1,22 @@
+error: unnecessary `pub(self)`
+  --> $DIR/needless_pub_self.rs:13:1
+   |
+LL | pub(self) fn a() {}
+   | ^^^^^^^^^ help: remove it
+   |
+   = note: `-D clippy::needless-pub-self` implied by `-D warnings`
+
+error: unnecessary `pub(in self)`
+  --> $DIR/needless_pub_self.rs:14:1
+   |
+LL | pub(in self) fn b() {}
+   | ^^^^^^^^^^^^ help: remove it
+
+error: unnecessary `pub(self)`
+  --> $DIR/needless_pub_self.rs:20:5
+   |
+LL |     pub(self) fn f() {}
+   |     ^^^^^^^^^ help: remove it
+
+error: aborting due to 3 previous errors
+

tests/ui/pub_with_shorthand.fixed

@@ -0,0 +1,38 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![feature(custom_inner_attributes)]
+#![allow(clippy::needless_pub_self, unused)]
+#![warn(clippy::pub_with_shorthand)]
+#![no_main]
+#![rustfmt::skip] // rustfmt will remove `in`, understandable
+                  // but very annoying for our purposes!
+
+#[macro_use]
+extern crate proc_macros;
+
+pub(in self) fn a() {}
+pub(in self) fn b() {}
+
+pub fn c() {}
+mod a {
+    pub(in super) fn d() {}
+    pub(in super) fn e() {}
+    pub(in self) fn f() {}
+    pub(in crate) fn k() {}
+    pub(in crate) fn m() {}
+    mod b {
+        pub(in crate::a) fn l() {}
+    }
+}
+
+external! {
+    pub(self) fn g() {}
+    pub(in self) fn h() {}
+}
+with_span! {
+    span
+    pub(self) fn i() {}
+    pub(in self) fn j() {}
+}
+
+// not really anything more to test. just a really simple lint overall

tests/ui/pub_with_shorthand.rs

@@ -0,0 +1,38 @@
+//@run-rustfix
+//@aux-build:proc_macros.rs:proc-macro
+#![feature(custom_inner_attributes)]
+#![allow(clippy::needless_pub_self, unused)]
+#![warn(clippy::pub_with_shorthand)]
+#![no_main]
+#![rustfmt::skip] // rustfmt will remove `in`, understandable
+                  // but very annoying for our purposes!
+
+#[macro_use]
+extern crate proc_macros;
+
+pub(self) fn a() {}
+pub(in self) fn b() {}
+
+pub fn c() {}
+mod a {
+    pub(in super) fn d() {}
+    pub(super) fn e() {}
+    pub(self) fn f() {}
+    pub(crate) fn k() {}
+    pub(in crate) fn m() {}
+    mod b {
+        pub(in crate::a) fn l() {}
+    }
+}
+
+external! {
+    pub(self) fn g() {}
+    pub(in self) fn h() {}
+}
+with_span! {
+    span
+    pub(self) fn i() {}
+    pub(in self) fn j() {}
+}
+
+// not really anything more to test. just a really simple lint overall

tests/ui/pub_with_shorthand.stderr

@@ -0,0 +1,28 @@
+error: usage of `pub` without `in`
+  --> $DIR/pub_with_shorthand.rs:13:1
+   |
+LL | pub(self) fn a() {}
+   | ^^^^^^^^^ help: add it: `pub(in self)`
+   |
+   = note: `-D clippy::pub-with-shorthand` implied by `-D warnings`
+
+error: usage of `pub` without `in`
+  --> $DIR/pub_with_shorthand.rs:19:5
+   |
+LL |     pub(super) fn e() {}
+   |     ^^^^^^^^^^ help: add it: `pub(in super)`
+
+error: usage of `pub` without `in`
+  --> $DIR/pub_with_shorthand.rs:20:5
+   |
+LL |     pub(self) fn f() {}
+   |     ^^^^^^^^^ help: add it: `pub(in self)`
+
+error: usage of `pub` without `in`
+  --> $DIR/pub_with_shorthand.rs:21:5
+   |
+LL |     pub(crate) fn k() {}
+   |     ^^^^^^^^^^ help: add it: `pub(in crate)`
+
+error: aborting due to 4 previous errors
+