Skip to content

Commit

Permalink
Do not lint reachable enums and enum variants used as functions in th…
Browse files Browse the repository at this point in the history
…e same crate
  • Loading branch information
ARandomDev99 committed Sep 1, 2024
1 parent 7381944 commit a33082c
Show file tree
Hide file tree
Showing 5 changed files with 320 additions and 40 deletions.
174 changes: 151 additions & 23 deletions clippy_lints/src/empty_with_brackets.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,16 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
use clippy_utils::source::snippet_opt;
use rustc_ast::ast::{Item, ItemKind, Variant, VariantData};
use rustc_data_structures::fx::FxIndexMap;
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::LocalDefId;
use rustc_hir::{Expr, ExprKind, Item, ItemKind, Node, 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_middle::ty::TyCtxt;
use rustc_session::impl_lint_pass;
use rustc_span::Span;

declare_clippy_lint! {
Expand Down Expand Up @@ -70,15 +76,27 @@ declare_clippy_lint! {
"finds enum variants with empty brackets"
}

declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM_VARIANTS_WITH_BRACKETS]);
#[derive(Debug)]
enum Usage {
Unused { redundant_use_sites: Vec<Span> },
Used,
}

#[derive(Default)]
pub struct EmptyWithBrackets {
// Value holds `Usage::Used` if the empty tuple variant was used as a function
empty_tuple_enum_variants: FxIndexMap<LocalDefId, Usage>,
}

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
&& has_brackets(var_data)
&& has_no_fields(cx, var_data, span_after_ident)
&& has_no_fields(cx, Some(var_data), span_after_ident)
{
span_lint_and_then(
cx,
Expand All @@ -97,38 +115,122 @@ impl EarlyLintPass for EmptyWithBrackets {
}
}

fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
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) {
span_lint_and_then(
if has_no_fields(cx, Some(&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(.., local_def_id) => {
// Don't lint reachable tuple enums
if cx.effective_visibilities.is_reachable(variant.def_id) {
return;
}
self.empty_tuple_enum_variants
.entry(local_def_id)
.or_insert(Usage::Unused {
redundant_use_sites: vec![],
});
},
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) {
match self.empty_tuple_enum_variants.get_mut(&def_id) {
Some(Usage::Unused {
ref mut redundant_use_sites,
}) => {
redundant_use_sites.push(parentheses_span);
},
None if has_no_fields(cx, None, parentheses_span) => {
self.empty_tuple_enum_variants.insert(
def_id,
Usage::Unused {
redundant_use_sites: vec![parentheses_span],
},
);
},
_ => {},
}
} else {
self.empty_tuple_enum_variants.insert(def_id, Usage::Used);
}
}
}

fn check_crate_post(&mut self, cx: &LateContext<'_>) {
for (local_def_id, usage) in &self.empty_tuple_enum_variants {
let Usage::Unused { redundant_use_sites } = usage else {
continue;
};
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 {
continue;
};
let span = variant.span.with_lo(variant.ident.span.hi());
span_lint_hir_and_then(
cx,
EMPTY_ENUM_VARIANTS_WITH_BRACKETS,
span_after_ident,
variant.hir_id,
span,
"enum variant has empty brackets",
|diagnostic| {
diagnostic.span_suggestion_hidden(
span_after_ident,
"remove the brackets",
"",
Applicability::MaybeIncorrect,
);
if redundant_use_sites.is_empty() {
diagnostic.span_suggestion_hidden(
span,
"remove the brackets",
"",
Applicability::MaybeIncorrect,
);
} else {
let mut parentheses_spans: Vec<_> =
redundant_use_sites.iter().map(|span| (*span, String::new())).collect();
parentheses_spans.push((span, String::new()));
diagnostic.multipart_suggestion(
"remove the brackets",
parentheses_spans,
Applicability::MaybeIncorrect,
);
}
},
);
}
}
}

fn has_no_ident_token(braces_span_str: &str) -> bool {
!rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
!rustc_lexer::tokenize(braces_span_str).any(|t| matches!(t.kind, TokenKind::Ident | TokenKind::Literal { .. }))
}

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 {
if !var_data.fields().is_empty() {
fn has_no_fields(cx: &LateContext<'_>, var_data_opt: Option<&VariantData<'_>>, braces_span: Span) -> bool {
if let Some(var_data) = var_data_opt
&& !var_data.fields().is_empty()
{
return false;
}

Expand All @@ -142,6 +244,32 @@ fn has_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Spa
has_no_ident_token(braces_span_str.as_ref())
}

fn call_parentheses_span(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option<Span> {
if let Node::Expr(parent) = tcx.parent_hir_node(expr.hir_id)
&& let ExprKind::Call(callee, ..) = parent.kind
&& callee.hir_id == expr.hir_id
{
Some(parent.span.with_lo(expr.span.hi()))
} else {
None
}
}

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),
..
},
)) = expr.kind
{
def_id.as_local()
} else {
None
}
}

#[cfg(test)]
mod unit_test {
use super::*;
Expand Down
2 changes: 1 addition & 1 deletion clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -823,7 +823,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static Conf) {
store.register_late_pass(move |_| Box::new(write::Write::new(conf, format_args.clone())));
store.register_late_pass(move |_| Box::new(cargo::Cargo::new(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));
Expand Down
65 changes: 61 additions & 4 deletions tests/ui/empty_enum_variants_with_brackets.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
#![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
NonEmptyBraces { x: i32, y: i32 },
NonEmptyParentheses(i32, i32),
EmptyBraces, //~ERROR: enum variant has empty brackets
EmptyParentheses(), // No error as enum is reachable
}

enum TestEnum {
Expand All @@ -16,6 +16,63 @@ 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)
}

fn natural_number(x: i32) -> NaturalOrNot {
(x > 0)
.then(NaturalOrNot::Natural)
.unwrap_or_else(NaturalOrNot::NotNatural)
}

enum NaturalOrNot {
// Used as functions -> no error
Natural(),
NotNatural(),
// Not used as a function
Unknown, //~ ERROR: enum variant has empty brackets
}

enum RedundantParenthesesFunctionCall {
// Used as a function call but with redundant parentheses
Parentheses, //~ ERROR: enum variant has empty brackets
// Not used as a function
NoParentheses,
}

#[allow(clippy::no_effect)]
fn redundant_parentheses_function_call() {
// The parentheses in the below line are redundant.
RedundantParenthesesFunctionCall::Parentheses;
RedundantParenthesesFunctionCall::NoParentheses;
}

// Same test as above but with usage of the enum occurring before the definition.
#[allow(clippy::no_effect)]
fn redundant_parentheses_function_call_2() {
// The parentheses in the below line are redundant.
RedundantParenthesesFunctionCall2::Parentheses;
RedundantParenthesesFunctionCall2::NoParentheses;
}

enum RedundantParenthesesFunctionCall2 {
// Used as a function call but with redundant parentheses
Parentheses, //~ ERROR: enum variant has empty brackets
// Not used as a function
NoParentheses,
}
}

enum TestEnumWithFeatures {
NonEmptyBraces {
#[cfg(feature = "thisisneverenabled")]
Expand Down
65 changes: 61 additions & 4 deletions tests/ui/empty_enum_variants_with_brackets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
#![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
NonEmptyBraces { x: i32, y: i32 },
NonEmptyParentheses(i32, i32),
EmptyBraces {}, //~ERROR: enum variant has empty brackets
EmptyParentheses(), // No error as enum is reachable
}

enum TestEnum {
Expand All @@ -16,6 +16,63 @@ 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)
}

fn natural_number(x: i32) -> NaturalOrNot {
(x > 0)
.then(NaturalOrNot::Natural)
.unwrap_or_else(NaturalOrNot::NotNatural)
}

enum NaturalOrNot {
// Used as functions -> no error
Natural(),
NotNatural(),
// Not used as a function
Unknown(), //~ ERROR: enum variant has empty brackets
}

enum RedundantParenthesesFunctionCall {
// Used as a function call but with redundant parentheses
Parentheses(), //~ ERROR: enum variant has empty brackets
// Not used as a function
NoParentheses,
}

#[allow(clippy::no_effect)]
fn redundant_parentheses_function_call() {
// The parentheses in the below line are redundant.
RedundantParenthesesFunctionCall::Parentheses();
RedundantParenthesesFunctionCall::NoParentheses;
}

// Same test as above but with usage of the enum occurring before the definition.
#[allow(clippy::no_effect)]
fn redundant_parentheses_function_call_2() {
// The parentheses in the below line are redundant.
RedundantParenthesesFunctionCall2::Parentheses();
RedundantParenthesesFunctionCall2::NoParentheses;
}

enum RedundantParenthesesFunctionCall2 {
// Used as a function call but with redundant parentheses
Parentheses(), //~ ERROR: enum variant has empty brackets
// Not used as a function
NoParentheses,
}
}

enum TestEnumWithFeatures {
NonEmptyBraces {
#[cfg(feature = "thisisneverenabled")]
Expand Down
Loading

0 comments on commit a33082c

Please sign in to comment.