Skip to content

Fix empty_enum_variants_with_brackets misses removing brackets in patterns #16160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
profetia wants to merge 2 commits into
base: master
Choose a base branch
from
+232 −86
Open

Fix empty_enum_variants_with_brackets misses removing brackets in patterns #16160

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bd0a5e4
fix: `empty_enum_variants_with_brackets` misses removing brackets in …
profetia Nov 30, 2025
ddd328b
Make `empty_enum_variants_with_brackets` to support empty variant wit…
profetia Dec 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
191 changes: 114 additions & 77 deletions clippy_lints/src/empty_with_brackets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
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::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;

Expand Down Expand Up @@ -118,91 +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;
}
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 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, span);
}
}

Expand All @@ -212,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(
Expand Down Expand Up @@ -252,6 +222,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(..))
}
Expand All @@ -277,17 +284,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,
}
}
30 changes: 30 additions & 0 deletions tests/ui/empty_enum_variants_with_brackets.fixed
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -102,4 +103,33 @@ 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 main() {}
30 changes: 30 additions & 0 deletions tests/ui/empty_enum_variants_with_brackets.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -102,4 +103,33 @@ 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 main() {}
Loading