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 Jul 23, 2024
1 parent 3e84ca8 commit cf507a9
Show file tree
Hide file tree
Showing 5 changed files with 245 additions and 38 deletions.
148 changes: 128 additions & 20 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,10 +76,22 @@ 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 `true` 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
Expand All @@ -97,22 +115,86 @@ impl EarlyLintPass for EmptyWithBrackets {
}
}

fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
// Don't lint pub enums
if cx.effective_visibilities.is_reachable(variant.def_id) {
return;
}

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, &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) => {
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)
&& let Some(Usage::Unused {
ref mut redundant_use_sites,
}) = self.empty_tuple_enum_variants.get_mut(&def_id)
{
redundant_use_sites.push(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,
);
diagnostic.span_suggestion_hidden(span, "remove the brackets", "", Applicability::MaybeIncorrect);
if !redundant_use_sites.is_empty() {
let parentheses_spans = redundant_use_sites.iter().map(|span| (*span, String::new())).collect();
diagnostic.multipart_suggestion(
"remove the brackets",
parentheses_spans,
Applicability::MaybeIncorrect,
);
}
},
);
}
Expand All @@ -123,11 +205,11 @@ fn has_no_ident_token(braces_span_str: &str) -> bool {
!rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
}

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 {
fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
if !var_data.fields().is_empty() {
return false;
}
Expand All @@ -142,6 +224,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 @@ -1007,7 +1007,7 @@ pub fn register_lints(store: &mut rustc_lint::LintStore, conf: &'static 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
51 changes: 47 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,11 @@
#![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
// No errors as enum is reachable
NonEmptyBraces { x: i32, y: i32 },
NonEmptyParentheses(i32, i32),
EmptyBraces {},
EmptyParentheses(),
}

enum TestEnum {
Expand All @@ -16,6 +17,48 @@ 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;
}
}

enum TestEnumWithFeatures {
NonEmptyBraces {
#[cfg(feature = "thisisneverenabled")]
Expand Down
51 changes: 47 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,11 @@
#![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
// No errors as enum is reachable
NonEmptyBraces { x: i32, y: i32 },
NonEmptyParentheses(i32, i32),
EmptyBraces {},
EmptyParentheses(),
}

enum TestEnum {
Expand All @@ -16,6 +17,48 @@ 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;
}
}

enum TestEnumWithFeatures {
NonEmptyBraces {
#[cfg(feature = "thisisneverenabled")]
Expand Down
31 changes: 22 additions & 9 deletions tests/ui/empty_enum_variants_with_brackets.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: enum variant has empty brackets
--> tests/ui/empty_enum_variants_with_brackets.rs:7:16
--> tests/ui/empty_enum_variants_with_brackets.rs:15:16
|
LL | EmptyBraces {},
| ^^^
Expand All @@ -9,28 +9,41 @@ LL | EmptyBraces {},
= help: remove the brackets

error: enum variant has empty brackets
--> tests/ui/empty_enum_variants_with_brackets.rs:8:21
--> tests/ui/empty_enum_variants_with_brackets.rs:16:21
|
LL | EmptyParentheses(),
| ^^
|
= help: remove the brackets

error: enum variant has empty brackets
--> tests/ui/empty_enum_variants_with_brackets.rs:14:16
--> tests/ui/empty_enum_variants_with_brackets.rs:26:16
|
LL | EmptyBraces {},
| ^^^
LL | Unknown(),
| ^^
|
= help: remove the brackets

error: enum variant has empty brackets
--> tests/ui/empty_enum_variants_with_brackets.rs:15:21
--> tests/ui/empty_enum_variants_with_brackets.rs:44:16
|
LL | EmptyParentheses(),
| ^^
LL | Unknown(),
| ^^
|
= help: remove the brackets

error: aborting due to 4 previous errors
error: enum variant has empty brackets
--> tests/ui/empty_enum_variants_with_brackets.rs:49:20
|
LL | Parentheses(),
| ^^
|
= help: remove the brackets
help: remove the brackets
|
LL - RedundantParenthesesFunctionCall::Parentheses();
LL + RedundantParenthesesFunctionCall::Parentheses;
|

error: aborting due to 5 previous errors

0 comments on commit cf507a9

Please sign in to comment.