Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
202 changes: 121 additions & 81 deletions clippy_lints/src/empty_with_brackets.rs
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(..))
}
Expand All @@ -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,
}
}
59 changes: 59 additions & 0 deletions tests/ui/empty_enum_variants_with_brackets.fixed
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,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() {}
Loading