Skip to content

Commit 895e669

Browse files
committed
Do not lint reachable enums and enum variants used as functions in the same crate
1 parent 3e84ca8 commit 895e669

5 files changed

+134
-41
lines changed

clippy_lints/src/empty_with_brackets.rs

+89-18
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,15 @@
11
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::source::snippet_opt;
3-
use rustc_ast::ast::{Item, ItemKind, Variant, VariantData};
3+
use rustc_data_structures::fx::FxIndexMap;
44
use rustc_errors::Applicability;
5+
use rustc_hir::def::CtorOf;
6+
use rustc_hir::def::DefKind::Ctor;
7+
use rustc_hir::def::Res::Def;
8+
use rustc_hir::def_id::DefId;
9+
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Path, QPath, Variant, VariantData};
510
use rustc_lexer::TokenKind;
6-
use rustc_lint::{EarlyContext, EarlyLintPass};
7-
use rustc_session::declare_lint_pass;
11+
use rustc_lint::{LateContext, LateLintPass};
12+
use rustc_session::impl_lint_pass;
813
use rustc_span::Span;
914

1015
declare_clippy_lint! {
@@ -70,10 +75,21 @@ declare_clippy_lint! {
7075
"finds enum variants with empty brackets"
7176
}
7277

73-
declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
78+
enum Usage {
79+
Unused(Span),
80+
Used,
81+
}
82+
83+
#[derive(Default)]
84+
pub struct EmptyWithBrackets {
85+
// Value holds `true` if the empty tuple variant was used as a function
86+
empty_tuple_enum_variants: FxIndexMap<DefId, Usage>,
87+
}
7488

75-
impl EarlyLintPass for EmptyWithBrackets {
76-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
89+
impl_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
90+
91+
impl LateLintPass<'_> for EmptyWithBrackets {
92+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
7793
let span_after_ident = item.span.with_lo(item.ident.span.hi());
7894

7995
if let ItemKind::Struct(var_data, _) = &item.kind
@@ -97,22 +113,63 @@ impl EarlyLintPass for EmptyWithBrackets {
97113
}
98114
}
99115

100-
fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
116+
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
117+
// Don't lint pub enums
118+
if cx.effective_visibilities.is_reachable(variant.def_id) {
119+
return;
120+
}
121+
101122
let span_after_ident = variant.span.with_lo(variant.ident.span.hi());
102123

103-
if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) {
124+
if has_no_fields(cx, &variant.data, span_after_ident) {
125+
match variant.data {
126+
VariantData::Struct { .. } => {
127+
span_lint_and_then(
128+
cx,
129+
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
130+
span_after_ident,
131+
"enum variant has empty brackets",
132+
|diagnostic| {
133+
diagnostic.span_suggestion_hidden(
134+
span_after_ident,
135+
"remove the brackets",
136+
"",
137+
Applicability::MaybeIncorrect,
138+
);
139+
},
140+
);
141+
},
142+
VariantData::Tuple(..) => {
143+
if let Some(local_def_id) = variant.data.ctor_def_id()
144+
&& !self.empty_tuple_enum_variants.contains_key(&local_def_id.to_def_id())
145+
{
146+
self.empty_tuple_enum_variants
147+
.insert(local_def_id.to_def_id(), Usage::Unused(span_after_ident));
148+
}
149+
},
150+
VariantData::Unit(..) => {},
151+
}
152+
}
153+
}
154+
155+
fn check_expr(&mut self, _cx: &LateContext<'_>, expr: &Expr<'_>) {
156+
if let Some(def_id) = check_expr_for_enum_as_function(expr) {
157+
self.empty_tuple_enum_variants.insert(def_id, Usage::Used);
158+
}
159+
}
160+
161+
fn check_crate_post(&mut self, cx: &LateContext<'_>) {
162+
for &span in self.empty_tuple_enum_variants.values().filter_map(|v| match v {
163+
Usage::Unused(span) => Some(span),
164+
Usage::Used => None,
165+
}) {
104166
span_lint_and_then(
105167
cx,
106168
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
107-
span_after_ident,
169+
span,
108170
"enum variant has empty brackets",
109171
|diagnostic| {
110-
diagnostic.span_suggestion_hidden(
111-
span_after_ident,
112-
"remove the brackets",
113-
"",
114-
Applicability::MaybeIncorrect,
115-
);
172+
diagnostic.span_suggestion_hidden(span, "remove the brackets", "", Applicability::MaybeIncorrect);
116173
},
117174
);
118175
}
@@ -123,11 +180,11 @@ fn has_no_ident_token(braces_span_str: &str) -> bool {
123180
!rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
124181
}
125182

126-
fn has_brackets(var_data: &VariantData) -> bool {
127-
!matches!(var_data, VariantData::Unit(_))
183+
fn has_brackets(var_data: &VariantData<'_>) -> bool {
184+
!matches!(var_data, VariantData::Unit(..))
128185
}
129186

130-
fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Span) -> bool {
187+
fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
131188
if !var_data.fields().is_empty() {
132189
return false;
133190
}
@@ -142,6 +199,20 @@ fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Spa
142199
has_no_ident_token(braces_span_str.as_ref())
143200
}
144201

202+
fn check_expr_for_enum_as_function(expr: &Expr<'_>) -> Option<DefId> {
203+
let ExprKind::Path(QPath::Resolved(
204+
_,
205+
Path {
206+
res: Def(Ctor(CtorOf::Variant, _), def_id),
207+
..
208+
},
209+
)) = expr.kind
210+
else {
211+
return None;
212+
};
213+
Some(*def_id)
214+
}
215+
145216
#[cfg(test)]
146217
mod unit_test {
147218
use super::*;

clippy_lints/src/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1007,7 +1007,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
10071007
})
10081008
});
10091009
store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
1010-
store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets));
1010+
store.register_late_pass(|_| Box::new(empty_with_brackets::EmptyWithBrackets::default()));
10111011
store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
10121012
store.register_early_pass(|| Box::new(pub_use::PubUse));
10131013
store.register_late_pass(|_| Box::new(format_push_string::FormatPushString));

tests/ui/empty_enum_variants_with_brackets.fixed

+19-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
#![allow(dead_code)]
33

44
pub enum PublicTestEnum {
5-
NonEmptyBraces { x: i32, y: i32 }, // No error
6-
NonEmptyParentheses(i32, i32), // No error
7-
EmptyBraces, //~ ERROR: enum variant has empty brackets
8-
EmptyParentheses, //~ ERROR: enum variant has empty brackets
5+
// No errors as enum is reachable
6+
NonEmptyBraces { x: i32, y: i32 },
7+
NonEmptyParentheses(i32, i32),
8+
EmptyBraces {},
9+
EmptyParentheses(),
910
}
1011

1112
enum TestEnum {
@@ -16,6 +17,20 @@ enum TestEnum {
1617
AnotherEnum, // No error
1718
}
1819

20+
mod issue12551 {
21+
enum EvenOdd {
22+
// Used as functions -> no error
23+
Even(),
24+
Odd(),
25+
// Not used as a function
26+
Unknown, //~ ERROR: enum variant has empty brackets
27+
}
28+
29+
fn even_odd(x: i32) -> EvenOdd {
30+
(x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
31+
}
32+
}
33+
1934
enum TestEnumWithFeatures {
2035
NonEmptyBraces {
2136
#[cfg(feature = "thisisneverenabled")]

tests/ui/empty_enum_variants_with_brackets.rs

+19-4
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,11 @@
22
#![allow(dead_code)]
33

44
pub enum PublicTestEnum {
5-
NonEmptyBraces { x: i32, y: i32 }, // No error
6-
NonEmptyParentheses(i32, i32), // No error
7-
EmptyBraces {}, //~ ERROR: enum variant has empty brackets
8-
EmptyParentheses(), //~ ERROR: enum variant has empty brackets
5+
// No errors as enum is reachable
6+
NonEmptyBraces { x: i32, y: i32 },
7+
NonEmptyParentheses(i32, i32),
8+
EmptyBraces {},
9+
EmptyParentheses(),
910
}
1011

1112
enum TestEnum {
@@ -16,6 +17,20 @@ enum TestEnum {
1617
AnotherEnum, // No error
1718
}
1819

20+
mod issue12551 {
21+
enum EvenOdd {
22+
// Used as functions -> no error
23+
Even(),
24+
Odd(),
25+
// Not used as a function
26+
Unknown(), //~ ERROR: enum variant has empty brackets
27+
}
28+
29+
fn even_odd(x: i32) -> EvenOdd {
30+
(x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
31+
}
32+
}
33+
1934
enum TestEnumWithFeatures {
2035
NonEmptyBraces {
2136
#[cfg(feature = "thisisneverenabled")]
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: enum variant has empty brackets
2-
--> tests/ui/empty_enum_variants_with_brackets.rs:7:16
2+
--> tests/ui/empty_enum_variants_with_brackets.rs:15:16
33
|
44
LL | EmptyBraces {},
55
| ^^^
@@ -9,28 +9,20 @@ LL | EmptyBraces {},
99
= help: remove the brackets
1010

1111
error: enum variant has empty brackets
12-
--> tests/ui/empty_enum_variants_with_brackets.rs:8:21
12+
--> tests/ui/empty_enum_variants_with_brackets.rs:16:21
1313
|
1414
LL | EmptyParentheses(),
1515
| ^^
1616
|
1717
= help: remove the brackets
1818

1919
error: enum variant has empty brackets
20-
--> tests/ui/empty_enum_variants_with_brackets.rs:14:16
20+
--> tests/ui/empty_enum_variants_with_brackets.rs:26:16
2121
|
22-
LL | EmptyBraces {},
23-
| ^^^
24-
|
25-
= help: remove the brackets
26-
27-
error: enum variant has empty brackets
28-
--> tests/ui/empty_enum_variants_with_brackets.rs:15:21
29-
|
30-
LL | EmptyParentheses(),
31-
| ^^
22+
LL | Unknown(),
23+
| ^^
3224
|
3325
= help: remove the brackets
3426

35-
error: aborting due to 4 previous errors
27+
error: aborting due to 3 previous errors
3628

0 commit comments

Comments
 (0)