Fix empty_enum_variants_with_brackets misses removing brackets in patterns (#16160)

Closes #16157

changelog: [`empty_enum_variants_with_brackets`] fix missing to remove
brackets in patterns

Author: samueltardieu

clippy_lints/src/empty_with_brackets.rs

@@ -1,16 +1,18 @@
 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 clippy_utils::source::SpanRangeExt;
+use clippy_utils::span_contains_non_whitespace;
+use rustc_data_structures::fx::{FxIndexMap, IndexEntry};
 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::{CtorOf, DefKind};
 use rustc_hir::def_id::LocalDefId;
-use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node, Path, QPath, Variant, VariantData};
+use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node, Pat, PatKind, Path, QPath, Variant, VariantData};
 use rustc_lint::{LateContext, LateLintPass};
-use rustc_middle::ty::TyCtxt;
+use rustc_middle::ty::{self, TyCtxt};
 use rustc_session::impl_lint_pass;
-use rustc_span::Span;
+use rustc_span::{BytePos, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -118,109 +120,80 @@ 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;
                 }
-                match self.empty_tuple_enum_variants.get_mut(&def_id) {
-                    Some(
-                        &mut (Usage::Unused {
-                            ref mut redundant_use_sites,
-                        }
-                        | Usage::NoDefinition {
-                            ref mut redundant_use_sites,
-                        }),
-                    ) => {
-                        redundant_use_sites.push(parentheses_span);
-                    },
-                    None => {
-                        // 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],
-                            },
-                        );
-                    },
-                    _ => {},
-                }
-            } 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 !pat.span.from_expansion()
+            && let Some((def_id, span)) = check_pat_for_enum_as_function(cx, pat)
+        {
+            self.update_enum_variant_usage(def_id, span);
+        }
+    }
+
     fn check_crate_post(&mut self, cx: &LateContext<'_>) {
-        for (local_def_id, usage) in &self.empty_tuple_enum_variants {
+        for (&local_def_id, usage) in &self.empty_tuple_enum_variants {
             // Ignore all variants with Usage::Used or Usage::NoDefinition
             let Usage::Unused { redundant_use_sites } = usage else {
                 continue;
             };
+
             // Attempt to fetch the Variant from LocalDefId.
-            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 {
+            let variant = if let Node::Variant(variant) = cx.tcx.hir_node_by_def_id(local_def_id) {
+                variant
+            } else if let Node::Variant(variant) = cx.tcx.hir_node_by_def_id(cx.tcx.local_parent(local_def_id)) {
+                variant
+            } else {
                 continue;
             };
+
             // Span of the parentheses in variant definition
             let span = variant.span.with_lo(variant.ident.span.hi());
+            let span_inner = span
+                .with_lo(SpanRangeExt::trim_start(span, cx).start + BytePos(1))
+                .with_hi(span.hi() - BytePos(1));
+            if span_contains_non_whitespace(cx, span_inner, false) {
+                continue;
+            }
             span_lint_hir_and_then(
                 cx,
                 EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
@@ -252,6 +225,43 @@ 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.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);
+                }
+            },
+            IndexEntry::Vacant(e) => {
+                // The variant isn't in the IndexMap which means its definition wasn't encountered yet.
+                e.insert(Usage::NoDefinition {
+                    redundant_use_sites: vec![parentheses_span],
+                });
+            },
+        }
+    }
+}
+
 fn has_brackets(var_data: &VariantData<'_>) -> bool {
     !matches!(var_data, VariantData::Unit(..))
 }
@@ -277,17 +287,47 @@ 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,
+    }
+}
+
+fn check_pat_for_enum_as_function(cx: &LateContext<'_>, pat: &Pat<'_>) -> Option<(LocalDefId, Span)> {
+    match pat.kind {
+        PatKind::TupleStruct(qpath, ..)
+            if let Def(Ctor(CtorOf::Variant, _), def_id) = cx.typeck_results().qpath_res(&qpath, pat.hir_id) =>
+        {
+            def_id.as_local().map(|id| (id, qpath.span().with_lo(pat.span.hi())))
+        },
+        PatKind::Struct(qpath, ..)
+            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

@@ -1,5 +1,6 @@
 #![warn(clippy::empty_enum_variants_with_brackets)]
 #![allow(dead_code)]
+#![feature(more_qualified_paths)]
 
 pub enum PublicTestEnum {
     NonEmptyBraces { x: i32, y: i32 }, // No error
@@ -102,4 +103,62 @@ pub enum PubFoo {
     Variant3(),
 }
 
+fn issue16157() {
+    enum E {
+        V,
+        //~^ empty_enum_variants_with_brackets
+    }
+
+    let E::V = E::V;
+
+    <E>::V = E::V;
+    <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 variant_with_comments_and_cfg() {
+    enum E {
+        V(
+            // This is a comment
+        ),
+    }
+    E::V() = E::V();
+
+    enum F {
+        U {
+            // This is a comment
+        },
+    }
+    F::U {} = F::U {};
+
+    enum G {
+        V(#[cfg(target_os = "cuda")] String),
+    }
+    G::V() = G::V();
+
+    enum H {
+        U {
+            #[cfg(target_os = "cuda")]
+            value: String,
+        },
+    }
+    H::U {} = H::U {};
+}
+
 fn main() {}