Skip to content

Commit e46009d

Browse files
committed
Do not lint reachable enums and enum variants used as functions in the same crate
1 parent 7381944 commit e46009d

File tree

5 files changed

+314
-37
lines changed

5 files changed

+314
-37
lines changed

clippy_lints/src/empty_with_brackets.rs

Lines changed: 145 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,16 @@
1-
use clippy_utils::diagnostics::span_lint_and_then;
1+
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_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::LocalDefId;
9+
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node, 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_middle::ty::TyCtxt;
13+
use rustc_session::impl_lint_pass;
814
use rustc_span::Span;
915

1016
declare_clippy_lint! {
@@ -70,10 +76,22 @@ declare_clippy_lint! {
7076
"finds enum variants with empty brackets"
7177
}
7278

73-
declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
79+
#[derive(Debug)]
80+
enum Usage {
81+
Unused { redundant_use_sites: Vec<Span> },
82+
Used,
83+
}
84+
85+
#[derive(Default)]
86+
pub struct EmptyWithBrackets {
87+
// Value holds `Usage::Used` if the empty tuple variant was used as a function
88+
empty_tuple_enum_variants: FxIndexMap<LocalDefId, Usage>,
89+
}
90+
91+
impl_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
7492

75-
impl EarlyLintPass for EmptyWithBrackets {
76-
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
93+
impl LateLintPass<'_> for EmptyWithBrackets {
94+
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
7795
let span_after_ident = item.span.with_lo(item.ident.span.hi());
7896

7997
if let ItemKind::Struct(var_data, _) = &item.kind
@@ -97,22 +115,103 @@ impl EarlyLintPass for EmptyWithBrackets {
97115
}
98116
}
99117

100-
fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
118+
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
101119
let span_after_ident = variant.span.with_lo(variant.ident.span.hi());
102120

103-
if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) {
104-
span_lint_and_then(
121+
if has_no_fields(cx, &variant.data, span_after_ident) {
122+
match variant.data {
123+
VariantData::Struct { .. } => {
124+
span_lint_and_then(
125+
cx,
126+
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
127+
span_after_ident,
128+
"enum variant has empty brackets",
129+
|diagnostic| {
130+
diagnostic.span_suggestion_hidden(
131+
span_after_ident,
132+
"remove the brackets",
133+
"",
134+
Applicability::MaybeIncorrect,
135+
);
136+
},
137+
);
138+
},
139+
VariantData::Tuple(.., local_def_id) => {
140+
// Don't lint reachable tuple enums
141+
if cx.effective_visibilities.is_reachable(variant.def_id) {
142+
return;
143+
}
144+
self.empty_tuple_enum_variants
145+
.entry(local_def_id)
146+
.or_insert(Usage::Unused {
147+
redundant_use_sites: vec![],
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+
if let Some(parentheses_span) = call_parentheses_span(cx.tcx, expr)
158+
{
159+
match self.empty_tuple_enum_variants.get_mut(&def_id) {
160+
Some(Usage::Unused {
161+
ref mut redundant_use_sites,
162+
}) => {
163+
redundant_use_sites.push(parentheses_span);
164+
return;
165+
}
166+
None => {
167+
self.empty_tuple_enum_variants.insert(def_id, Usage::Unused { redundant_use_sites: vec![parentheses_span] });
168+
return;
169+
}
170+
_ => {}
171+
}
172+
173+
} else {
174+
self.empty_tuple_enum_variants.insert(def_id, Usage::Used);
175+
}
176+
}
177+
}
178+
179+
fn check_crate_post(&mut self, cx: &LateContext<'_>) {
180+
for (local_def_id, usage) in &self.empty_tuple_enum_variants {
181+
let Usage::Unused { redundant_use_sites } = usage else {
182+
continue;
183+
};
184+
let Node::Variant(variant) = cx.tcx.hir_node(
185+
cx.tcx
186+
.local_def_id_to_hir_id(cx.tcx.parent(local_def_id.to_def_id()).expect_local()),
187+
) else {
188+
continue;
189+
};
190+
let span = variant.span.with_lo(variant.ident.span.hi());
191+
span_lint_hir_and_then(
105192
cx,
106193
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
107-
span_after_ident,
194+
variant.hir_id,
195+
span,
108196
"enum variant has empty brackets",
109197
|diagnostic| {
110-
diagnostic.span_suggestion_hidden(
111-
span_after_ident,
112-
"remove the brackets",
113-
"",
114-
Applicability::MaybeIncorrect,
115-
);
198+
if redundant_use_sites.is_empty() {
199+
diagnostic.span_suggestion_hidden(
200+
span,
201+
"remove the brackets",
202+
"",
203+
Applicability::MaybeIncorrect,
204+
);
205+
} else {
206+
let mut parentheses_spans: Vec<_> =
207+
redundant_use_sites.iter().map(|span| (*span, String::new())).collect();
208+
parentheses_spans.push((span, String::new()));
209+
diagnostic.multipart_suggestion(
210+
"remove the brackets",
211+
parentheses_spans,
212+
Applicability::MaybeIncorrect,
213+
);
214+
}
116215
},
117216
);
118217
}
@@ -123,11 +222,11 @@ fn has_no_ident_token(braces_span_str: &str) -> bool {
123222
!rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
124223
}
125224

126-
fn has_brackets(var_data: &VariantData) -> bool {
127-
!matches!(var_data, VariantData::Unit(_))
225+
fn has_brackets(var_data: &VariantData<'_>) -> bool {
226+
!matches!(var_data, VariantData::Unit(..))
128227
}
129228

130-
fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Span) -> bool {
229+
fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
131230
if !var_data.fields().is_empty() {
132231
return false;
133232
}
@@ -142,6 +241,32 @@ fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Spa
142241
has_no_ident_token(braces_span_str.as_ref())
143242
}
144243

244+
fn call_parentheses_span(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option<Span> {
245+
if let Node::Expr(parent) = tcx.parent_hir_node(expr.hir_id)
246+
&& let ExprKind::Call(callee, ..) = parent.kind
247+
&& callee.hir_id == expr.hir_id
248+
{
249+
Some(parent.span.with_lo(expr.span.hi()))
250+
} else {
251+
None
252+
}
253+
}
254+
255+
fn check_expr_for_enum_as_function(expr: &Expr<'_>) -> Option<LocalDefId> {
256+
if let ExprKind::Path(QPath::Resolved(
257+
_,
258+
Path {
259+
res: Def(Ctor(CtorOf::Variant, _), def_id),
260+
..
261+
},
262+
)) = expr.kind
263+
{
264+
def_id.as_local()
265+
} else {
266+
None
267+
}
268+
}
269+
145270
#[cfg(test)]
146271
mod unit_test {
147272
use super::*;

clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -823,7 +823,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
823823
store.register_late_pass(move |_| Box::new(write::Write::new(conf, format_args.clone())));
824824
store.register_late_pass(move |_| Box::new(cargo::Cargo::new(conf)));
825825
store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
826-
store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets));
826+
store.register_late_pass(|_| Box::new(empty_with_brackets::EmptyWithBrackets::default()));
827827
store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
828828
store.register_early_pass(|| Box::new(pub_use::PubUse));
829829
store.register_late_pass(|_| Box::new(format_push_string::FormatPushString));

tests/ui/empty_enum_variants_with_brackets.fixed

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
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+
NonEmptyBraces { x: i32, y: i32 },
6+
NonEmptyParentheses(i32, i32),
7+
EmptyBraces, //~ERROR: enum variant has empty brackets
8+
EmptyParentheses(), // No error as enum is reachable
99
}
1010

1111
enum TestEnum {
@@ -16,6 +16,63 @@ enum TestEnum {
1616
AnotherEnum, // No error
1717
}
1818

19+
mod issue12551 {
20+
enum EvenOdd {
21+
// Used as functions -> no error
22+
Even(),
23+
Odd(),
24+
// Not used as a function
25+
Unknown, //~ ERROR: enum variant has empty brackets
26+
}
27+
28+
fn even_odd(x: i32) -> EvenOdd {
29+
(x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
30+
}
31+
32+
fn natural_number(x: i32) -> NaturalOrNot {
33+
(x > 0)
34+
.then(NaturalOrNot::Natural)
35+
.unwrap_or_else(NaturalOrNot::NotNatural)
36+
}
37+
38+
enum NaturalOrNot {
39+
// Used as functions -> no error
40+
Natural(),
41+
NotNatural(),
42+
// Not used as a function
43+
Unknown, //~ ERROR: enum variant has empty brackets
44+
}
45+
46+
enum RedundantParenthesesFunctionCall {
47+
// Used as a function call but with redundant parentheses
48+
Parentheses, //~ ERROR: enum variant has empty brackets
49+
// Not used as a function
50+
NoParentheses,
51+
}
52+
53+
#[allow(clippy::no_effect)]
54+
fn redundant_parentheses_function_call() {
55+
// The parentheses in the below line are redundant.
56+
RedundantParenthesesFunctionCall::Parentheses;
57+
RedundantParenthesesFunctionCall::NoParentheses;
58+
}
59+
60+
// Same test as above but with usage of the enum occurring before the definition.
61+
#[allow(clippy::no_effect)]
62+
fn redundant_parentheses_function_call_2() {
63+
// The parentheses in the below line are redundant.
64+
RedundantParenthesesFunctionCall2::Parentheses;
65+
RedundantParenthesesFunctionCall2::NoParentheses;
66+
}
67+
68+
enum RedundantParenthesesFunctionCall2 {
69+
// Used as a function call but with redundant parentheses
70+
Parentheses, //~ ERROR: enum variant has empty brackets
71+
// Not used as a function
72+
NoParentheses,
73+
}
74+
}
75+
1976
enum TestEnumWithFeatures {
2077
NonEmptyBraces {
2178
#[cfg(feature = "thisisneverenabled")]

tests/ui/empty_enum_variants_with_brackets.rs

Lines changed: 61 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@
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+
NonEmptyBraces { x: i32, y: i32 },
6+
NonEmptyParentheses(i32, i32),
7+
EmptyBraces {}, //~ERROR: enum variant has empty brackets
8+
EmptyParentheses(), // No error as enum is reachable
99
}
1010

1111
enum TestEnum {
@@ -16,6 +16,63 @@ enum TestEnum {
1616
AnotherEnum, // No error
1717
}
1818

19+
mod issue12551 {
20+
enum EvenOdd {
21+
// Used as functions -> no error
22+
Even(),
23+
Odd(),
24+
// Not used as a function
25+
Unknown(), //~ ERROR: enum variant has empty brackets
26+
}
27+
28+
fn even_odd(x: i32) -> EvenOdd {
29+
(x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
30+
}
31+
32+
fn natural_number(x: i32) -> NaturalOrNot {
33+
(x > 0)
34+
.then(NaturalOrNot::Natural)
35+
.unwrap_or_else(NaturalOrNot::NotNatural)
36+
}
37+
38+
enum NaturalOrNot {
39+
// Used as functions -> no error
40+
Natural(),
41+
NotNatural(),
42+
// Not used as a function
43+
Unknown(), //~ ERROR: enum variant has empty brackets
44+
}
45+
46+
enum RedundantParenthesesFunctionCall {
47+
// Used as a function call but with redundant parentheses
48+
Parentheses(), //~ ERROR: enum variant has empty brackets
49+
// Not used as a function
50+
NoParentheses,
51+
}
52+
53+
#[allow(clippy::no_effect)]
54+
fn redundant_parentheses_function_call() {
55+
// The parentheses in the below line are redundant.
56+
RedundantParenthesesFunctionCall::Parentheses();
57+
RedundantParenthesesFunctionCall::NoParentheses;
58+
}
59+
60+
// Same test as above but with usage of the enum occurring before the definition.
61+
#[allow(clippy::no_effect)]
62+
fn redundant_parentheses_function_call_2() {
63+
// The parentheses in the below line are redundant.
64+
RedundantParenthesesFunctionCall2::Parentheses();
65+
RedundantParenthesesFunctionCall2::NoParentheses;
66+
}
67+
68+
enum RedundantParenthesesFunctionCall2 {
69+
// Used as a function call but with redundant parentheses
70+
Parentheses(), //~ ERROR: enum variant has empty brackets
71+
// Not used as a function
72+
NoParentheses,
73+
}
74+
}
75+
1976
enum TestEnumWithFeatures {
2077
NonEmptyBraces {
2178
#[cfg(feature = "thisisneverenabled")]

0 commit comments

Comments
 (0)