Make empty_enum_variants_with_brackets to support empty variant with braces

Author: profetia

clippy_lints/src/empty_with_brackets.rs

@@ -1,6 +1,6 @@
 use clippy_utils::attrs::span_contains_cfg;
 use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
-use rustc_data_structures::fx::FxIndexMap;
+use rustc_data_structures::fx::{FxIndexMap, IndexEntry};
 use rustc_errors::Applicability;
 use rustc_hir::def::DefKind::Ctor;
 use rustc_hir::def::Res::Def;
@@ -118,80 +118,55 @@ impl LateLintPass<'_> for EmptyWithBrackets {
     }
 
     fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
-        // FIXME: handle `$name {}`
         if !variant.span.from_expansion()
             && !variant.ident.span.from_expansion()
             && let span_after_ident = variant.span.with_lo(variant.ident.span.hi())
             && has_no_fields(cx, &variant.data, span_after_ident)
         {
             match variant.data {
                 VariantData::Struct { .. } => {
-                    // Empty struct variants can be linted immediately
-                    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,
-                            );
-                        },
-                    );
+                    self.add_enum_variant(variant.def_id);
                 },
                 VariantData::Tuple(.., local_def_id) => {
                     // Don't lint reachable tuple enums
                     if cx.effective_visibilities.is_reachable(variant.def_id) {
                         return;
                     }
-                    if let Some(entry) = self.empty_tuple_enum_variants.get_mut(&local_def_id) {
-                        // empty_tuple_enum_variants contains Usage::NoDefinition if the variant was called before the
-                        // definition was encountered. Now that there's a definition, convert it
-                        // to Usage::Unused.
-                        if let Usage::NoDefinition { redundant_use_sites } = entry {
-                            *entry = Usage::Unused {
-                                redundant_use_sites: redundant_use_sites.clone(),
-                            };
-                        }
-                    } else {
-                        self.empty_tuple_enum_variants.insert(
-                            local_def_id,
-                            Usage::Unused {
-                                redundant_use_sites: vec![],
-                            },
-                        );
-                    }
+                    self.add_enum_variant(local_def_id);
                 },
                 VariantData::Unit(..) => {},
             }
         }
     }
 
     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
-        if let Some(def_id) = check_expr_for_enum_as_function(expr) {
-            if let Some(parentheses_span) = call_parentheses_span(cx.tcx, expr) {
+        if let Some((def_id, mut span)) = check_expr_for_enum_as_function(cx, expr) {
+            if span.is_empty()
+                && let Some(parentheses_span) = call_parentheses_span(cx.tcx, expr)
+            {
+                span = parentheses_span;
+            }
+
+            if span.is_empty() {
+                // The parentheses are not redundant.
+                self.empty_tuple_enum_variants.insert(def_id, Usage::Used);
+            } else {
                 // Do not count expressions from macro expansion as a redundant use site.
                 if expr.span.from_expansion() {
                     return;
                 }
-                self.update_enum_variant_usage(def_id, parentheses_span);
-            } else {
-                // The parentheses are not redundant.
-                self.empty_tuple_enum_variants.insert(def_id, Usage::Used);
+                self.update_enum_variant_usage(def_id, span);
             }
         }
     }
 
     fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) {
-        if let Some((def_id, parentheses_span)) = check_pat_for_enum_as_function(cx, pat) {
+        if let Some((def_id, span)) = check_pat_for_enum_as_function(cx, pat) {
             if pat.span.from_expansion() {
                 return;
             }
 
-            self.update_enum_variant_usage(def_id, parentheses_span);
+            self.update_enum_variant_usage(def_id, span);
         }
     }
 
@@ -201,13 +176,19 @@ impl LateLintPass<'_> for EmptyWithBrackets {
             let Usage::Unused { redundant_use_sites } = usage else {
                 continue;
             };
+
             // Attempt to fetch the Variant from LocalDefId.
-            let Node::Variant(variant) = cx.tcx.hir_node(
+            let variant = if let Node::Variant(variant) = cx.tcx.hir_node(cx.tcx.local_def_id_to_hir_id(local_def_id)) {
+                variant
+            } else if let Node::Variant(variant) = cx.tcx.hir_node(
                 cx.tcx
                     .local_def_id_to_hir_id(cx.tcx.parent(local_def_id.to_def_id()).expect_local()),
-            ) else {
+            ) {
+                variant
+            } else {
                 continue;
             };
+
             // Span of the parentheses in variant definition
             let span = variant.span.with_lo(variant.ident.span.hi());
             span_lint_hir_and_then(
@@ -242,28 +223,38 @@ impl LateLintPass<'_> for EmptyWithBrackets {
 }
 
 impl EmptyWithBrackets {
+    fn add_enum_variant(&mut self, local_def_id: LocalDefId) {
+        self.empty_tuple_enum_variants
+            .entry(local_def_id)
+            .and_modify(|entry| {
+                // empty_tuple_enum_variants contains Usage::NoDefinition if the variant was called before
+                // the definition was encountered. Now that there's a
+                // definition, convert it to Usage::Unused.
+                if let Usage::NoDefinition { redundant_use_sites } = entry {
+                    *entry = Usage::Unused {
+                        redundant_use_sites: redundant_use_sites.clone(),
+                    };
+                }
+            })
+            .or_insert_with(|| Usage::Unused {
+                redundant_use_sites: vec![],
+            });
+    }
+
     fn update_enum_variant_usage(&mut self, def_id: LocalDefId, parentheses_span: Span) {
-        match self.empty_tuple_enum_variants.get_mut(&def_id) {
-            Some(
-                &mut (Usage::Unused {
-                    ref mut redundant_use_sites,
+        match self.empty_tuple_enum_variants.entry(def_id) {
+            IndexEntry::Occupied(mut e) => {
+                if let Usage::Unused { redundant_use_sites } | Usage::NoDefinition { redundant_use_sites } = e.get_mut()
+                {
+                    redundant_use_sites.push(parentheses_span);
                 }
-                | Usage::NoDefinition {
-                    ref mut redundant_use_sites,
-                }),
-            ) => {
-                redundant_use_sites.push(parentheses_span);
             },
-            None => {
+            IndexEntry::Vacant(e) => {
                 // The variant isn't in the IndexMap which means its definition wasn't encountered yet.
-                self.empty_tuple_enum_variants.insert(
-                    def_id,
-                    Usage::NoDefinition {
-                        redundant_use_sites: vec![parentheses_span],
-                    },
-                );
+                e.insert(Usage::NoDefinition {
+                    redundant_use_sites: vec![parentheses_span],
+                });
             },
-            _ => {},
         }
     }
 }
@@ -293,18 +284,27 @@ fn call_parentheses_span(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option<Span> {
 }
 
 // Returns the LocalDefId of the variant being called as a function if it exists.
-fn check_expr_for_enum_as_function(expr: &Expr<'_>) -> Option<LocalDefId> {
-    if let ExprKind::Path(QPath::Resolved(
-        _,
-        Path {
-            res: Def(Ctor(CtorOf::Variant, _), def_id),
-            ..
+fn check_expr_for_enum_as_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<(LocalDefId, Span)> {
+    match expr.kind {
+        ExprKind::Path(QPath::Resolved(
+            _,
+            Path {
+                res: Def(Ctor(CtorOf::Variant, _), def_id),
+                span,
+                ..
+            },
+        )) => def_id.as_local().map(|id| (id, span.with_lo(expr.span.hi()))),
+        ExprKind::Struct(qpath, ..)
+            if let Def(DefKind::Variant, mut def_id) = cx.typeck_results().qpath_res(qpath, expr.hir_id) =>
+        {
+            let ty = cx.tcx.type_of(def_id).instantiate_identity();
+            if let ty::FnDef(ctor_def_id, _) = ty.kind() {
+                def_id = *ctor_def_id;
+            }
+
+            def_id.as_local().map(|id| (id, qpath.span().with_lo(expr.span.hi())))
         },
-    )) = expr.kind
-    {
-        def_id.as_local()
-    } else {
-        None
+        _ => None,
     }
 }
 
@@ -316,10 +316,13 @@ fn check_pat_for_enum_as_function(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option
             def_id.as_local().map(|id| (id, qpath.span().with_lo(pat.span.hi())))
         },
         PatKind::Struct(qpath, ..)
-            if let Def(DefKind::Variant, def_id) = cx.typeck_results().qpath_res(&qpath, pat.hir_id)
-                && let ty = cx.tcx.type_of(def_id).instantiate_identity()
-                && let ty::FnDef(def_id, _) = ty.kind() =>
+            if let Def(DefKind::Variant, mut def_id) = cx.typeck_results().qpath_res(&qpath, pat.hir_id) =>
         {
+            let ty = cx.tcx.type_of(def_id).instantiate_identity();
+            if let ty::FnDef(ctor_def_id, _) = ty.kind() {
+                def_id = *ctor_def_id;
+            }
+
             def_id.as_local().map(|id| (id, qpath.span().with_lo(pat.span.hi())))
         },
         _ => None,

tests/ui/empty_enum_variants_with_brackets.fixed

@@ -115,4 +115,21 @@ fn issue16157() {
     <E>::V = E::V;
 }
 
+fn variant_with_braces() {
+    enum E {
+        V,
+        //~^ empty_enum_variants_with_brackets
+    }
+    E::V = E::V;
+    E::V = E::V;
+    <E>::V = <E>::V;
+
+    enum F {
+        U,
+        //~^ empty_enum_variants_with_brackets
+    }
+    F::U = F::U;
+    <F>::U = F::U;
+}
+
 fn main() {}

tests/ui/empty_enum_variants_with_brackets.rs

@@ -115,4 +115,21 @@ fn issue16157() {
     <E>::V {} = E::V();
 }
 
+fn variant_with_braces() {
+    enum E {
+        V(),
+        //~^ empty_enum_variants_with_brackets
+    }
+    E::V() = E::V();
+    E::V() = E::V {};
+    <E>::V {} = <E>::V {};
+
+    enum F {
+        U {},
+        //~^ empty_enum_variants_with_brackets
+    }
+    F::U {} = F::U {};
+    <F>::U {} = F::U {};
+}
+
 fn main() {}

tests/ui/empty_enum_variants_with_brackets.stderr

@@ -96,5 +96,36 @@ LL ~     <E>::V = E::V;
 LL ~     <E>::V = E::V;
    |
 
-error: aborting due to 9 previous errors
+error: enum variant has empty brackets
+  --> tests/ui/empty_enum_variants_with_brackets.rs:120:10
+   |
+LL |         V(),
+   |          ^^
+   |
+help: remove the brackets
+   |
+LL ~         V,
+LL |
+LL |     }
+LL ~     E::V = E::V;
+LL ~     E::V = E::V;
+LL ~     <E>::V = <E>::V;
+   |
+
+error: enum variant has empty brackets
+  --> tests/ui/empty_enum_variants_with_brackets.rs:128:10
+   |
+LL |         U {},
+   |          ^^^
+   |
+help: remove the brackets
+   |
+LL ~         U,
+LL |
+LL |     }
+LL ~     F::U = F::U;
+LL ~     <F>::U = F::U;
+   |
+
+error: aborting due to 11 previous errors