Skip to content

Commit 36f4feb

Browse files
committed
Auto merge of rust-lang#10967 - Centri3:visibility_lints2, r=Manishearth
New lints ['`needless_pub_self`], [`pub_with_shorthand`] and [`pub_without_shorthand`] Closes rust-lang#10963 Closes rust-lang#10964 changelog: New lints ['`needless_pub_self`], [`pub_with_shorthand`] and [`pub_without_shorthand`]
2 parents 1df9110 + 46aa8ab commit 36f4feb

15 files changed

+431
-2
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -5047,6 +5047,7 @@ Released 2018-09-13
50475047
[`needless_option_take`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_option_take
50485048
[`needless_parens_on_range_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_parens_on_range_literals
50495049
[`needless_pass_by_value`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pass_by_value
5050+
[`needless_pub_self`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_pub_self
50505051
[`needless_question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_question_mark
50515052
[`needless_range_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#needless_range_loop
50525053
[`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
51215122
[`ptr_offset_with_cast`]: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_offset_with_cast
51225123
[`pub_enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_enum_variant_names
51235124
[`pub_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_use
5125+
[`pub_with_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_with_shorthand
5126+
[`pub_without_shorthand`]: https://rust-lang.github.io/rust-clippy/master/index.html#pub_without_shorthand
51245127
[`question_mark`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark
51255128
[`question_mark_used`]: https://rust-lang.github.io/rust-clippy/master/index.html#question_mark_used
51265129
[`range_minus_one`]: https://rust-lang.github.io/rust-clippy/master/index.html#range_minus_one

clippy_lints/src/declared_lints.rs

+3
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,9 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
666666
crate::useless_conversion::USELESS_CONVERSION_INFO,
667667
crate::vec::USELESS_VEC_INFO,
668668
crate::vec_init_then_push::VEC_INIT_THEN_PUSH_INFO,
669+
crate::visibility::NEEDLESS_PUB_SELF_INFO,
670+
crate::visibility::PUB_WITHOUT_SHORTHAND_INFO,
671+
crate::visibility::PUB_WITH_SHORTHAND_INFO,
669672
crate::wildcard_imports::ENUM_GLOB_USE_INFO,
670673
crate::wildcard_imports::WILDCARD_IMPORTS_INFO,
671674
crate::write::PRINTLN_EMPTY_STRING_INFO,

clippy_lints/src/lib.rs

+2
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,7 @@ mod use_self;
338338
mod useless_conversion;
339339
mod vec;
340340
mod vec_init_then_push;
341+
mod visibility;
341342
mod wildcard_imports;
342343
mod write;
343344
mod zero_div_zero;
@@ -1070,6 +1071,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
10701071
})
10711072
});
10721073
store.register_late_pass(|_| Box::new(manual_range_patterns::ManualRangePatterns));
1074+
store.register_early_pass(|| Box::new(visibility::Visibility));
10731075
// add lints here, do not remove this comment, it's used in `new_lint`
10741076
}
10751077

clippy_lints/src/visibility.rs

+131
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
use clippy_utils::{diagnostics::span_lint_and_sugg, source::snippet_opt};
2+
use rustc_ast::ast::{Item, VisibilityKind};
3+
use rustc_errors::Applicability;
4+
use rustc_lint::{EarlyContext, EarlyLintPass, LintContext};
5+
use rustc_middle::lint::in_external_macro;
6+
use rustc_session::{declare_lint_pass, declare_tool_lint};
7+
use rustc_span::{symbol::kw, Span};
8+
9+
declare_clippy_lint! {
10+
/// ### What it does
11+
/// Checks for usage of `pub(self)` and `pub(in self)`.
12+
///
13+
/// ### Why is this bad?
14+
/// It's unnecessary, omitting the `pub` entirely will give the same results.
15+
///
16+
/// ### Example
17+
/// ```rust,ignore
18+
/// pub(self) type OptBox<T> = Option<Box<T>>;
19+
/// ```
20+
/// Use instead:
21+
/// ```rust,ignore
22+
/// type OptBox<T> = Option<Box<T>>;
23+
/// ```
24+
#[clippy::version = "1.72.0"]
25+
pub NEEDLESS_PUB_SELF,
26+
style,
27+
"checks for usage of `pub(self)` and `pub(in self)`."
28+
}
29+
declare_clippy_lint! {
30+
/// ### What it does
31+
/// Checks for usage of `pub(<loc>)` with `in`.
32+
///
33+
/// ### Why is this bad?
34+
/// Consistency. Use it or don't, just be consistent about it.
35+
///
36+
/// Also see the `pub_without_shorthand` lint for an alternative.
37+
///
38+
/// ### Example
39+
/// ```rust,ignore
40+
/// pub(super) type OptBox<T> = Option<Box<T>>;
41+
/// ```
42+
/// Use instead:
43+
/// ```rust,ignore
44+
/// pub(in super) type OptBox<T> = Option<Box<T>>;
45+
/// ```
46+
#[clippy::version = "1.72.0"]
47+
pub PUB_WITH_SHORTHAND,
48+
restriction,
49+
"disallows usage of `pub(<loc>)`, without `in`"
50+
}
51+
declare_clippy_lint! {
52+
/// ### What it does
53+
/// Checks for usage of `pub(<loc>)` without `in`.
54+
///
55+
/// Note: As you cannot write a module's path in `pub(<loc>)`, this will only trigger on
56+
/// `pub(super)` and the like.
57+
///
58+
/// ### Why is this bad?
59+
/// Consistency. Use it or don't, just be consistent about it.
60+
///
61+
/// Also see the `pub_with_shorthand` lint for an alternative.
62+
///
63+
/// ### Example
64+
/// ```rust,ignore
65+
/// pub(in super) type OptBox<T> = Option<Box<T>>;
66+
/// ```
67+
/// Use instead:
68+
/// ```rust,ignore
69+
/// pub(super) type OptBox<T> = Option<Box<T>>;
70+
/// ```
71+
#[clippy::version = "1.72.0"]
72+
pub PUB_WITHOUT_SHORTHAND,
73+
restriction,
74+
"disallows usage of `pub(in <loc>)` with `in`"
75+
}
76+
declare_lint_pass!(Visibility => [NEEDLESS_PUB_SELF, PUB_WITH_SHORTHAND, PUB_WITHOUT_SHORTHAND]);
77+
78+
impl EarlyLintPass for Visibility {
79+
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
80+
if !in_external_macro(cx.sess(), item.span)
81+
&& let VisibilityKind::Restricted { path, shorthand, .. } = &item.vis.kind
82+
{
83+
if **path == kw::SelfLower && let Some(false) = is_from_proc_macro(cx, item.vis.span) {
84+
span_lint_and_sugg(
85+
cx,
86+
NEEDLESS_PUB_SELF,
87+
item.vis.span,
88+
&format!("unnecessary `pub({}self)`", if *shorthand { "" } else { "in " }),
89+
"remove it",
90+
String::new(),
91+
Applicability::MachineApplicable,
92+
);
93+
}
94+
95+
if (**path == kw::Super || **path == kw::SelfLower || **path == kw::Crate)
96+
&& !*shorthand
97+
&& let [.., last] = &*path.segments
98+
&& let Some(false) = is_from_proc_macro(cx, item.vis.span)
99+
{
100+
span_lint_and_sugg(
101+
cx,
102+
PUB_WITHOUT_SHORTHAND,
103+
item.vis.span,
104+
"usage of `pub` with `in`",
105+
"remove it",
106+
format!("pub({})", last.ident),
107+
Applicability::MachineApplicable,
108+
);
109+
}
110+
111+
if *shorthand
112+
&& let [.., last] = &*path.segments
113+
&& let Some(false) = is_from_proc_macro(cx, item.vis.span)
114+
{
115+
span_lint_and_sugg(
116+
cx,
117+
PUB_WITH_SHORTHAND,
118+
item.vis.span,
119+
"usage of `pub` without `in`",
120+
"add it",
121+
format!("pub(in {})", last.ident),
122+
Applicability::MachineApplicable,
123+
);
124+
}
125+
}
126+
}
127+
}
128+
129+
fn is_from_proc_macro(cx: &EarlyContext<'_>, span: Span) -> Option<bool> {
130+
snippet_opt(cx, span).map(|s| !s.starts_with("pub"))
131+
}

tests/ui/manual_async_fn.fixed

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@run-rustfix
22
#![warn(clippy::manual_async_fn)]
3-
#![allow(unused)]
3+
#![allow(clippy::needless_pub_self, unused)]
44

55
use std::future::Future;
66

tests/ui/manual_async_fn.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//@run-rustfix
22
#![warn(clippy::manual_async_fn)]
3-
#![allow(unused)]
3+
#![allow(clippy::needless_pub_self, unused)]
44

55
use std::future::Future;
66

tests/ui/needless_pub_self.fixed

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![feature(custom_inner_attributes)]
4+
#![allow(unused)]
5+
#![warn(clippy::needless_pub_self)]
6+
#![no_main]
7+
#![rustfmt::skip] // rustfmt will remove `in`, understandable
8+
// but very annoying for our purposes!
9+
10+
#[macro_use]
11+
extern crate proc_macros;
12+
13+
fn a() {}
14+
fn b() {}
15+
16+
pub fn c() {}
17+
mod a {
18+
pub(in super) fn d() {}
19+
pub(super) fn e() {}
20+
fn f() {}
21+
}
22+
23+
external! {
24+
pub(self) fn g() {}
25+
pub(in self) fn h() {}
26+
}
27+
with_span! {
28+
span
29+
pub(self) fn i() {}
30+
pub(in self) fn j() {}
31+
}
32+
33+
// not really anything more to test. just a really simple lint overall

tests/ui/needless_pub_self.rs

+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![feature(custom_inner_attributes)]
4+
#![allow(unused)]
5+
#![warn(clippy::needless_pub_self)]
6+
#![no_main]
7+
#![rustfmt::skip] // rustfmt will remove `in`, understandable
8+
// but very annoying for our purposes!
9+
10+
#[macro_use]
11+
extern crate proc_macros;
12+
13+
pub(self) fn a() {}
14+
pub(in self) fn b() {}
15+
16+
pub fn c() {}
17+
mod a {
18+
pub(in super) fn d() {}
19+
pub(super) fn e() {}
20+
pub(self) fn f() {}
21+
}
22+
23+
external! {
24+
pub(self) fn g() {}
25+
pub(in self) fn h() {}
26+
}
27+
with_span! {
28+
span
29+
pub(self) fn i() {}
30+
pub(in self) fn j() {}
31+
}
32+
33+
// not really anything more to test. just a really simple lint overall

tests/ui/needless_pub_self.stderr

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
error: unnecessary `pub(self)`
2+
--> $DIR/needless_pub_self.rs:13:1
3+
|
4+
LL | pub(self) fn a() {}
5+
| ^^^^^^^^^ help: remove it
6+
|
7+
= note: `-D clippy::needless-pub-self` implied by `-D warnings`
8+
9+
error: unnecessary `pub(in self)`
10+
--> $DIR/needless_pub_self.rs:14:1
11+
|
12+
LL | pub(in self) fn b() {}
13+
| ^^^^^^^^^^^^ help: remove it
14+
15+
error: unnecessary `pub(self)`
16+
--> $DIR/needless_pub_self.rs:20:5
17+
|
18+
LL | pub(self) fn f() {}
19+
| ^^^^^^^^^ help: remove it
20+
21+
error: aborting due to 3 previous errors
22+

tests/ui/pub_with_shorthand.fixed

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![feature(custom_inner_attributes)]
4+
#![allow(clippy::needless_pub_self, unused)]
5+
#![warn(clippy::pub_with_shorthand)]
6+
#![no_main]
7+
#![rustfmt::skip] // rustfmt will remove `in`, understandable
8+
// but very annoying for our purposes!
9+
10+
#[macro_use]
11+
extern crate proc_macros;
12+
13+
pub(in self) fn a() {}
14+
pub(in self) fn b() {}
15+
16+
pub fn c() {}
17+
mod a {
18+
pub(in super) fn d() {}
19+
pub(in super) fn e() {}
20+
pub(in self) fn f() {}
21+
pub(in crate) fn k() {}
22+
pub(in crate) fn m() {}
23+
mod b {
24+
pub(in crate::a) fn l() {}
25+
}
26+
}
27+
28+
external! {
29+
pub(self) fn g() {}
30+
pub(in self) fn h() {}
31+
}
32+
with_span! {
33+
span
34+
pub(self) fn i() {}
35+
pub(in self) fn j() {}
36+
}
37+
38+
// not really anything more to test. just a really simple lint overall

tests/ui/pub_with_shorthand.rs

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
//@run-rustfix
2+
//@aux-build:proc_macros.rs:proc-macro
3+
#![feature(custom_inner_attributes)]
4+
#![allow(clippy::needless_pub_self, unused)]
5+
#![warn(clippy::pub_with_shorthand)]
6+
#![no_main]
7+
#![rustfmt::skip] // rustfmt will remove `in`, understandable
8+
// but very annoying for our purposes!
9+
10+
#[macro_use]
11+
extern crate proc_macros;
12+
13+
pub(self) fn a() {}
14+
pub(in self) fn b() {}
15+
16+
pub fn c() {}
17+
mod a {
18+
pub(in super) fn d() {}
19+
pub(super) fn e() {}
20+
pub(self) fn f() {}
21+
pub(crate) fn k() {}
22+
pub(in crate) fn m() {}
23+
mod b {
24+
pub(in crate::a) fn l() {}
25+
}
26+
}
27+
28+
external! {
29+
pub(self) fn g() {}
30+
pub(in self) fn h() {}
31+
}
32+
with_span! {
33+
span
34+
pub(self) fn i() {}
35+
pub(in self) fn j() {}
36+
}
37+
38+
// not really anything more to test. just a really simple lint overall

tests/ui/pub_with_shorthand.stderr

+28
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
error: usage of `pub` without `in`
2+
--> $DIR/pub_with_shorthand.rs:13:1
3+
|
4+
LL | pub(self) fn a() {}
5+
| ^^^^^^^^^ help: add it: `pub(in self)`
6+
|
7+
= note: `-D clippy::pub-with-shorthand` implied by `-D warnings`
8+
9+
error: usage of `pub` without `in`
10+
--> $DIR/pub_with_shorthand.rs:19:5
11+
|
12+
LL | pub(super) fn e() {}
13+
| ^^^^^^^^^^ help: add it: `pub(in super)`
14+
15+
error: usage of `pub` without `in`
16+
--> $DIR/pub_with_shorthand.rs:20:5
17+
|
18+
LL | pub(self) fn f() {}
19+
| ^^^^^^^^^ help: add it: `pub(in self)`
20+
21+
error: usage of `pub` without `in`
22+
--> $DIR/pub_with_shorthand.rs:21:5
23+
|
24+
LL | pub(crate) fn k() {}
25+
| ^^^^^^^^^^ help: add it: `pub(in crate)`
26+
27+
error: aborting due to 4 previous errors
28+

0 commit comments

Comments
 (0)