Do not lint reachable enums and enum variants used as functions in the same crate

Author: ARandomDev99

clippy_lints/src/empty_with_brackets.rs

@@ -1,10 +1,15 @@
 use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::source::snippet_opt;
-use rustc_ast::ast::{Item, ItemKind, Variant, VariantData};
+use rustc_data_structures::fx::FxHashSet;
 use rustc_errors::Applicability;
+use rustc_hir::def::CtorOf;
+use rustc_hir::def::DefKind::Ctor;
+use rustc_hir::def::Res::Def;
+use rustc_hir::def_id::DefId;
+use rustc_hir::{Expr, ExprKind, Item, ItemKind, Path, QPath, Variant, VariantData};
 use rustc_lexer::TokenKind;
-use rustc_lint::{EarlyContext, EarlyLintPass};
-use rustc_session::declare_lint_pass;
+use rustc_lint::{LateContext, LateLintPass};
+use rustc_session::impl_lint_pass;
 use rustc_span::Span;
 
 declare_clippy_lint! {
@@ -70,10 +75,16 @@ declare_clippy_lint! {
     "finds enum variants with empty brackets"
 }
 
-declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
+#[derive(Default)]
+pub struct EmptyWithBrackets {
+    empty_tuple_enum_variants: FxHashSet<(DefId, Span)>,
+    enum_variants_used_as_functions: FxHashSet<DefId>,
+}
+
+impl_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
 
-impl EarlyLintPass for EmptyWithBrackets {
-    fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
+impl LateLintPass<'_> for EmptyWithBrackets {
+    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
         let span_after_ident = item.span.with_lo(item.ident.span.hi());
 
         if let ItemKind::Struct(var_data, _) = &item.kind
@@ -97,22 +108,62 @@ impl EarlyLintPass for EmptyWithBrackets {
         }
     }
 
-    fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
+    fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
+        // Don't lint pub enums
+        if cx.effective_visibilities.is_reachable(variant.def_id) {
+            return;
+        }
+
         let span_after_ident = variant.span.with_lo(variant.ident.span.hi());
 
-        if has_brackets(&variant.data) && has_no_fields(cx, &variant.data, span_after_ident) {
+        if has_no_fields(cx, &variant.data, span_after_ident) {
+            match variant.data {
+                VariantData::Struct { .. } => {
+                    span_lint_and_then(
+                        cx,
+                        EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
+                        span_after_ident,
+                        "enum variant has empty brackets",
+                        |diagnostic| {
+                            diagnostic.span_suggestion_hidden(
+                                span_after_ident,
+                                "remove the brackets",
+                                "",
+                                Applicability::MaybeIncorrect,
+                            );
+                        },
+                    );
+                },
+                VariantData::Tuple(..) => {
+                    if let Some(local_def_id) = variant.data.ctor_def_id() {
+                        self.empty_tuple_enum_variants
+                            .insert((local_def_id.to_def_id(), span_after_ident));
+                    }
+                },
+                VariantData::Unit(..) => {},
+            }
+        }
+    }
+
+    fn check_expr(&mut self, _cx: &LateContext<'_>, expr: &Expr<'_>) {
+        if let Some(def_id) = check_expr_for_enum_as_function(expr) {
+            self.enum_variants_used_as_functions.insert(def_id);
+        }
+    }
+
+    fn check_crate_post(&mut self, cx: &LateContext<'_>) {
+        for &(_, span) in self
+            .empty_tuple_enum_variants
+            .iter()
+            .filter(|(variant, _)| !self.enum_variants_used_as_functions.contains(variant))
+        {
             span_lint_and_then(
                 cx,
                 EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
-                span_after_ident,
+                span,
                 "enum variant has empty brackets",
                 |diagnostic| {
-                    diagnostic.span_suggestion_hidden(
-                        span_after_ident,
-                        "remove the brackets",
-                        "",
-                        Applicability::MaybeIncorrect,
-                    );
+                    diagnostic.span_suggestion_hidden(span, "remove the brackets", "", Applicability::MaybeIncorrect);
                 },
             );
         }
@@ -123,11 +174,11 @@ fn has_no_ident_token(braces_span_str: &str) -> bool {
     !rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
 }
 
-fn has_brackets(var_data: &VariantData) -> bool {
-    !matches!(var_data, VariantData::Unit(_))
+fn has_brackets(var_data: &VariantData<'_>) -> bool {
+    !matches!(var_data, VariantData::Unit(..))
 }
 
-fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Span) -> bool {
+fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
     if !var_data.fields().is_empty() {
         return false;
     }
@@ -142,6 +193,20 @@ fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Spa
     has_no_ident_token(braces_span_str.as_ref())
 }
 
+fn check_expr_for_enum_as_function(expr: &Expr<'_>) -> Option<DefId> {
+    let ExprKind::Path(QPath::Resolved(
+        _,
+        Path {
+            res: Def(Ctor(CtorOf::Variant, _), def_id),
+            ..
+        },
+    )) = expr.kind
+    else {
+        return None;
+    };
+    Some(*def_id)
+}
+
 #[cfg(test)]
 mod unit_test {
     use super::*;

clippy_lints/src/lib.rs

@@ -1007,7 +1007,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
         })
     });
     store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
-    store.register_early_pass(|| Box::new(empty_with_brackets::EmptyWithBrackets));
+    store.register_late_pass(|_| Box::new(empty_with_brackets::EmptyWithBrackets::default()));
     store.register_late_pass(|_| Box::new(unnecessary_owned_empty_strings::UnnecessaryOwnedEmptyStrings));
     store.register_early_pass(|| Box::new(pub_use::PubUse));
     store.register_late_pass(|_| Box::new(format_push_string::FormatPushString));

tests/ui/empty_enum_variants_with_brackets.fixed

@@ -2,10 +2,11 @@
 #![allow(dead_code)]
 
 pub enum PublicTestEnum {
-    NonEmptyBraces { x: i32, y: i32 }, // No error
-    NonEmptyParentheses(i32, i32),     // No error
-    EmptyBraces,                    //~ ERROR: enum variant has empty brackets
-    EmptyParentheses,                //~ ERROR: enum variant has empty brackets
+    // No errors as enum is reachable
+    NonEmptyBraces { x: i32, y: i32 },
+    NonEmptyParentheses(i32, i32),
+    EmptyBraces {},
+    EmptyParentheses(),
 }
 
 enum TestEnum {
@@ -16,6 +17,20 @@ enum TestEnum {
     AnotherEnum,                       // No error
 }
 
+mod issue12551 {
+    enum EvenOdd {
+        // Used as functions -> no error
+        Even(),
+        Odd(),
+        // Not used as a function
+        Unknown, //~ ERROR: enum variant has empty brackets
+    }
+
+    fn even_odd(x: i32) -> EvenOdd {
+        (x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
+    }
+}
+
 enum TestEnumWithFeatures {
     NonEmptyBraces {
         #[cfg(feature = "thisisneverenabled")]

tests/ui/empty_enum_variants_with_brackets.rs

@@ -2,10 +2,11 @@
 #![allow(dead_code)]
 
 pub enum PublicTestEnum {
-    NonEmptyBraces { x: i32, y: i32 }, // No error
-    NonEmptyParentheses(i32, i32),     // No error
-    EmptyBraces {},                    //~ ERROR: enum variant has empty brackets
-    EmptyParentheses(),                //~ ERROR: enum variant has empty brackets
+    // No errors as enum is reachable
+    NonEmptyBraces { x: i32, y: i32 },
+    NonEmptyParentheses(i32, i32),
+    EmptyBraces {},
+    EmptyParentheses(),
 }
 
 enum TestEnum {
@@ -16,6 +17,20 @@ enum TestEnum {
     AnotherEnum,                       // No error
 }
 
+mod issue12551 {
+    enum EvenOdd {
+        // Used as functions -> no error
+        Even(),
+        Odd(),
+        // Not used as a function
+        Unknown(), //~ ERROR: enum variant has empty brackets
+    }
+
+    fn even_odd(x: i32) -> EvenOdd {
+        (x % 2 == 0).then(EvenOdd::Even).unwrap_or_else(EvenOdd::Odd)
+    }
+}
+
 enum TestEnumWithFeatures {
     NonEmptyBraces {
         #[cfg(feature = "thisisneverenabled")]

tests/ui/empty_enum_variants_with_brackets.stderr

@@ -1,5 +1,5 @@
 error: enum variant has empty brackets
-  --> tests/ui/empty_enum_variants_with_brackets.rs:7:16
+  --> tests/ui/empty_enum_variants_with_brackets.rs:15:16
    |
 LL |     EmptyBraces {},
    |                ^^^
@@ -9,28 +9,20 @@ LL |     EmptyBraces {},
    = help: remove the brackets
 
 error: enum variant has empty brackets
-  --> tests/ui/empty_enum_variants_with_brackets.rs:8:21
+  --> tests/ui/empty_enum_variants_with_brackets.rs:16:21
    |
 LL |     EmptyParentheses(),
    |                     ^^
    |
    = help: remove the brackets
 
 error: enum variant has empty brackets
-  --> tests/ui/empty_enum_variants_with_brackets.rs:14:16
+  --> tests/ui/empty_enum_variants_with_brackets.rs:26:16
    |
-LL |     EmptyBraces {},
-   |                ^^^
-   |
-   = help: remove the brackets
-
-error: enum variant has empty brackets
-  --> tests/ui/empty_enum_variants_with_brackets.rs:15:21
-   |
-LL |     EmptyParentheses(),
-   |                     ^^
+LL |         Unknown(),
+   |                ^^
    |
    = help: remove the brackets
 
-error: aborting due to 4 previous errors
+error: aborting due to 3 previous errors