Do not lint reachable enums and enum variants used as functions in the same crate
This commit is contained in:
Aneesh K
parent
26f43ff346
commit
fa9254feaf
5 changed files with 389 additions and 72 deletions
|
|
@ -1,10 +1,15 @@
|
|||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use rustc_ast::ast::{Item, ItemKind, Variant, VariantData};
|
||||
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_errors::Applicability;
|
||||
use rustc_lexer::TokenKind;
|
||||
use rustc_lint::{EarlyContext, EarlyLintPass};
|
||||
use rustc_session::declare_lint_pass;
|
||||
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_lint::{LateContext, LateLintPass};
|
||||
use rustc_middle::ty::TyCtxt;
|
||||
use rustc_session::impl_lint_pass;
|
||||
use rustc_span::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
|
|
@ -70,10 +75,23 @@ 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,
|
||||
NoDefinition { redundant_use_sites: Vec<Span> },
|
||||
}
|
||||
|
||||
impl EarlyLintPass for EmptyWithBrackets {
|
||||
fn check_item(&mut self, cx: &EarlyContext<'_>, item: &Item) {
|
||||
#[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 LateLintPass<'_> for EmptyWithBrackets {
|
||||
fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
|
||||
if let ItemKind::Struct(ident, var_data, _) = &item.kind
|
||||
&& has_brackets(var_data)
|
||||
&& let span_after_ident = item.span.with_lo(ident.span.hi())
|
||||
|
|
@ -96,70 +114,175 @@ impl EarlyLintPass for EmptyWithBrackets {
|
|||
}
|
||||
}
|
||||
|
||||
fn check_variant(&mut self, cx: &EarlyContext<'_>, variant: &Variant) {
|
||||
fn check_variant(&mut self, cx: &LateContext<'_>, variant: &Variant<'_>) {
|
||||
// the span of the parentheses/braces
|
||||
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 { .. } => {
|
||||
// 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,
|
||||
);
|
||||
},
|
||||
);
|
||||
},
|
||||
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![],
|
||||
},
|
||||
);
|
||||
}
|
||||
},
|
||||
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) {
|
||||
// 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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
fn check_crate_post(&mut self, cx: &LateContext<'_>) {
|
||||
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 {
|
||||
continue;
|
||||
};
|
||||
// Span of the parentheses in variant definition
|
||||
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() {
|
||||
// If there's no redundant use sites, the definition is the only place to modify.
|
||||
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)
|
||||
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() {
|
||||
return false;
|
||||
}
|
||||
|
||||
fn has_no_fields(cx: &LateContext<'_>, var_data: &VariantData<'_>, braces_span: Span) -> bool {
|
||||
var_data.fields().is_empty() &&
|
||||
// there might still be field declarations hidden from the AST
|
||||
// (conditionally compiled code using #[cfg(..)])
|
||||
|
||||
let Some(braces_span_str) = snippet_opt(cx, braces_span) else {
|
||||
return false;
|
||||
};
|
||||
|
||||
has_no_ident_token(braces_span_str.as_ref())
|
||||
!span_contains_cfg(cx, braces_span)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod unit_test {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn test_has_no_ident_token() {
|
||||
let input = "{ field: u8 }";
|
||||
assert!(!has_no_ident_token(input));
|
||||
|
||||
let input = "(u8, String);";
|
||||
assert!(!has_no_ident_token(input));
|
||||
|
||||
let input = " {
|
||||
// test = 5
|
||||
}
|
||||
";
|
||||
assert!(has_no_ident_token(input));
|
||||
|
||||
let input = " ();";
|
||||
assert!(has_no_ident_token(input));
|
||||
// If expression HIR ID and callee HIR ID are same, returns the span of the parentheses, else,
|
||||
// returns None.
|
||||
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
|
||||
}
|
||||
}
|
||||
|
||||
// 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),
|
||||
..
|
||||
},
|
||||
)) = expr.kind
|
||||
{
|
||||
def_id.as_local()
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -858,7 +858,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));
|
||||
|
|
|
|||
|
|
@ -6,8 +6,7 @@ pub enum PublicTestEnum {
|
|||
NonEmptyParentheses(i32, i32), // No error
|
||||
EmptyBraces,
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
EmptyParentheses,
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
EmptyParentheses(), // No error as enum is pub
|
||||
}
|
||||
|
||||
enum TestEnum {
|
||||
|
|
@ -20,6 +19,67 @@ enum TestEnum {
|
|||
AnotherEnum, // No error
|
||||
}
|
||||
|
||||
mod issue12551 {
|
||||
enum EvenOdd {
|
||||
// Used as functions -> no error
|
||||
Even(),
|
||||
Odd(),
|
||||
// Not used as a function
|
||||
Unknown,
|
||||
//~^ empty_enum_variants_with_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,
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
}
|
||||
|
||||
enum RedundantParenthesesFunctionCall {
|
||||
// Used as a function call but with redundant parentheses
|
||||
Parentheses,
|
||||
//~^ empty_enum_variants_with_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,
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
// Not used as a function
|
||||
NoParentheses,
|
||||
}
|
||||
}
|
||||
|
||||
enum TestEnumWithFeatures {
|
||||
NonEmptyBraces {
|
||||
#[cfg(feature = "thisisneverenabled")]
|
||||
|
|
@ -28,4 +88,18 @@ enum TestEnumWithFeatures {
|
|||
NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), // No error
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum Foo {
|
||||
Variant1(i32),
|
||||
Variant2,
|
||||
Variant3, //~ ERROR: enum variant has empty brackets
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub enum PubFoo {
|
||||
Variant1(i32),
|
||||
Variant2,
|
||||
Variant3(),
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -6,8 +6,7 @@ pub enum PublicTestEnum {
|
|||
NonEmptyParentheses(i32, i32), // No error
|
||||
EmptyBraces {},
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
EmptyParentheses(),
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
EmptyParentheses(), // No error as enum is pub
|
||||
}
|
||||
|
||||
enum TestEnum {
|
||||
|
|
@ -20,6 +19,67 @@ enum TestEnum {
|
|||
AnotherEnum, // No error
|
||||
}
|
||||
|
||||
mod issue12551 {
|
||||
enum EvenOdd {
|
||||
// Used as functions -> no error
|
||||
Even(),
|
||||
Odd(),
|
||||
// Not used as a function
|
||||
Unknown(),
|
||||
//~^ empty_enum_variants_with_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(),
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
}
|
||||
|
||||
enum RedundantParenthesesFunctionCall {
|
||||
// Used as a function call but with redundant parentheses
|
||||
Parentheses(),
|
||||
//~^ empty_enum_variants_with_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(),
|
||||
//~^ empty_enum_variants_with_brackets
|
||||
// Not used as a function
|
||||
NoParentheses,
|
||||
}
|
||||
}
|
||||
|
||||
enum TestEnumWithFeatures {
|
||||
NonEmptyBraces {
|
||||
#[cfg(feature = "thisisneverenabled")]
|
||||
|
|
@ -28,4 +88,18 @@ enum TestEnumWithFeatures {
|
|||
NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), // No error
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
enum Foo {
|
||||
Variant1(i32),
|
||||
Variant2,
|
||||
Variant3(), //~ ERROR: enum variant has empty brackets
|
||||
}
|
||||
|
||||
#[derive(Clone)]
|
||||
pub enum PubFoo {
|
||||
Variant1(i32),
|
||||
Variant2,
|
||||
Variant3(),
|
||||
}
|
||||
|
||||
fn main() {}
|
||||
|
|
|
|||
|
|
@ -9,15 +9,7 @@ LL | EmptyBraces {},
|
|||
= help: remove the brackets
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:9:21
|
||||
|
|
||||
LL | EmptyParentheses(),
|
||||
| ^^
|
||||
|
|
||||
= help: remove the brackets
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:16:16
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:15:16
|
||||
|
|
||||
LL | EmptyBraces {},
|
||||
| ^^^
|
||||
|
|
@ -25,12 +17,66 @@ LL | EmptyBraces {},
|
|||
= help: remove the brackets
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:18:21
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:17:21
|
||||
|
|
||||
LL | EmptyParentheses(),
|
||||
| ^^
|
||||
|
|
||||
= 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:28:16
|
||||
|
|
||||
LL | Unknown(),
|
||||
| ^^
|
||||
|
|
||||
= help: remove the brackets
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:47:16
|
||||
|
|
||||
LL | Unknown(),
|
||||
| ^^
|
||||
|
|
||||
= help: remove the brackets
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:53:20
|
||||
|
|
||||
LL | Parentheses(),
|
||||
| ^^
|
||||
|
|
||||
help: remove the brackets
|
||||
|
|
||||
LL ~ Parentheses,
|
||||
LL |
|
||||
...
|
||||
LL | // The parentheses in the below line are redundant.
|
||||
LL ~ RedundantParenthesesFunctionCall::Parentheses;
|
||||
|
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:76:20
|
||||
|
|
||||
LL | Parentheses(),
|
||||
| ^^
|
||||
|
|
||||
help: remove the brackets
|
||||
|
|
||||
LL ~ RedundantParenthesesFunctionCall2::Parentheses;
|
||||
LL | RedundantParenthesesFunctionCall2::NoParentheses;
|
||||
...
|
||||
LL | // Used as a function call but with redundant parentheses
|
||||
LL ~ Parentheses,
|
||||
|
|
||||
|
||||
error: enum variant has empty brackets
|
||||
--> tests/ui/empty_enum_variants_with_brackets.rs:95:13
|
||||
|
|
||||
LL | Variant3(),
|
||||
| ^^
|
||||
|
|
||||
= help: remove the brackets
|
||||
|
||||
error: aborting due to 8 previous errors
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue