diff --git a/clippy_lints/src/empty_with_brackets.rs b/clippy_lints/src/empty_with_brackets.rs index 745599b0e57a..e3210110101a 100644 --- a/clippy_lints/src/empty_with_brackets.rs +++ b/clippy_lints/src/empty_with_brackets.rs @@ -1,9 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_then; -use clippy_utils::source::snippet_opt; +use clippy_utils::source::{IntoSpan, SpanRangeExt}; use rustc_ast::ast::{Item, ItemKind, Variant, VariantData}; use rustc_errors::Applicability; use rustc_lexer::TokenKind; -use rustc_lint::{EarlyContext, EarlyLintPass}; +use rustc_lint::{EarlyContext, EarlyLintPass, Lint}; use rustc_session::declare_lint_pass; use rustc_span::Span; @@ -74,93 +74,78 @@ declare_lint_pass!(EmptyWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS, EMPTY_ENUM impl EarlyLintPass for EmptyWithBrackets { fn check_item(&mut self, cx: &EarlyContext<'_>, 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) - { - span_lint_and_then( + if let ItemKind::Struct(var_data, _) = &item.kind { + check( cx, + var_data, + item.span, + item.ident.span, EMPTY_STRUCTS_WITH_BRACKETS, - span_after_ident, - "found empty brackets on struct declaration", - |diagnostic| { - diagnostic.span_suggestion_hidden( - span_after_ident, - "remove the brackets", - ";", - Applicability::Unspecified, - ); - }, + "non-unit struct contains no fields", + true, ); } } fn check_variant(&mut self, cx: &EarlyContext<'_>, 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( - 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, - ); - }, - ); - } + check( + cx, + &variant.data, + variant.span, + variant.ident.span, + EMPTY_ENUM_VARIANTS_WITH_BRACKETS, + "non-unit variant contains no fields", + false, + ); } } -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_no_fields(cx: &EarlyContext<'_>, var_data: &VariantData, braces_span: Span) -> bool { - if !var_data.fields().is_empty() { - return false; - } - - // 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; +fn check( + cx: &EarlyContext<'_>, + data: &VariantData, + item_sp: Span, + name_sp: Span, + lint: &'static Lint, + msg: &'static str, + needs_semi: bool, +) { + let (fields, has_semi, start_char, end_char, help_msg) = match &data { + VariantData::Struct { fields, .. } => (fields, false, '{', '}', "remove the braces"), + VariantData::Tuple(fields, _) => (fields, needs_semi, '(', ')', "remove the parentheses"), + VariantData::Unit(_) => return, }; - - has_no_ident_token(braces_span_str.as_ref()) -} - -#[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 fields.is_empty() + && !item_sp.from_expansion() + && !name_sp.from_expansion() + && let name_hi = name_sp.hi() + && let Some(err_range) = (name_hi..item_sp.hi()).clone().map_range(cx, |src, range| { + let src = src.get(range.clone())?; + let (src, end) = if has_semi { + (src.strip_suffix(';')?, range.end - 1) + } else { + (src, range.end) + }; + let trimmed = src.trim_start(); + let start = range.start + (src.len() - trimmed.len()); + // Proc-macro check. + let trimmed = trimmed.strip_prefix(start_char)?.strip_suffix(end_char)?; + // Check for anything inside the brackets, including comments. + rustc_lexer::tokenize(trimmed) + .all(|tt| matches!(tt.kind, TokenKind::Whitespace)) + .then_some(start..end) + }) + { + span_lint_and_then(cx, lint, err_range.clone().into_span(), msg, |diagnostic| { + diagnostic.span_suggestion_hidden( + (name_hi..err_range.end).into_span(), + help_msg, + if has_semi || !needs_semi { + String::new() + } else { + ";".into() + }, + Applicability::MaybeIncorrect, + ); + }); } } diff --git a/tests/ui/empty_enum_variants_with_brackets.fixed b/tests/ui/empty_enum_variants_with_brackets.fixed index 1a5e78dd47fe..00d74576c9a8 100644 --- a/tests/ui/empty_enum_variants_with_brackets.fixed +++ b/tests/ui/empty_enum_variants_with_brackets.fixed @@ -1,27 +1,61 @@ -#![warn(clippy::empty_enum_variants_with_brackets)] +//@aux-build:proc_macros.rs +#![deny(clippy::empty_enum_variants_with_brackets)] #![allow(dead_code)] +extern crate proc_macros; +use proc_macros::{external, with_span}; + 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, //~ empty_enum_variants_with_brackets + EmptyParentheses, //~ empty_enum_variants_with_brackets } enum TestEnum { - 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 - AnotherEnum, // No error + NonEmptyBraces { + x: i32, + y: i32, + }, + NonEmptyParentheses(i32, i32), + EmptyBraces, //~ empty_enum_variants_with_brackets + EmptyParentheses, //~ empty_enum_variants_with_brackets + AnotherEnum, + #[rustfmt::skip] + WithWhitespace, //~ empty_enum_variants_with_brackets + WithComment { + // Some long explanation here + }, } enum TestEnumWithFeatures { NonEmptyBraces { #[cfg(feature = "thisisneverenabled")] x: i32, - }, // No error - NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), // No error + }, + NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), +} + +external! { + enum External { + Foo {}, + } +} + +with_span! { + span + enum ProcMacro { + Foo(), + } +} + +macro_rules! m { + ($($ty:ty),*) => { + enum Macro { + Foo($($ty),*), + } + } } +m! {} fn main() {} diff --git a/tests/ui/empty_enum_variants_with_brackets.rs b/tests/ui/empty_enum_variants_with_brackets.rs index ca20b969a240..1a3641c3e304 100644 --- a/tests/ui/empty_enum_variants_with_brackets.rs +++ b/tests/ui/empty_enum_variants_with_brackets.rs @@ -1,27 +1,61 @@ -#![warn(clippy::empty_enum_variants_with_brackets)] +//@aux-build:proc_macros.rs +#![deny(clippy::empty_enum_variants_with_brackets)] #![allow(dead_code)] +extern crate proc_macros; +use proc_macros::{external, with_span}; + 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 {}, //~ empty_enum_variants_with_brackets + EmptyParentheses(), //~ empty_enum_variants_with_brackets } enum TestEnum { - 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 - AnotherEnum, // No error + NonEmptyBraces { + x: i32, + y: i32, + }, + NonEmptyParentheses(i32, i32), + EmptyBraces {}, //~ empty_enum_variants_with_brackets + EmptyParentheses(), //~ empty_enum_variants_with_brackets + AnotherEnum, + #[rustfmt::skip] + WithWhitespace { }, //~ empty_enum_variants_with_brackets + WithComment { + // Some long explanation here + }, } enum TestEnumWithFeatures { NonEmptyBraces { #[cfg(feature = "thisisneverenabled")] x: i32, - }, // No error - NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), // No error + }, + NonEmptyParentheses(#[cfg(feature = "thisisneverenabled")] i32), +} + +external! { + enum External { + Foo {}, + } +} + +with_span! { + span + enum ProcMacro { + Foo(), + } +} + +macro_rules! m { + ($($ty:ty),*) => { + enum Macro { + Foo($($ty),*), + } + } } +m! {} fn main() {} diff --git a/tests/ui/empty_enum_variants_with_brackets.stderr b/tests/ui/empty_enum_variants_with_brackets.stderr index 2b187b8f755b..ebbf4d08aa48 100644 --- a/tests/ui/empty_enum_variants_with_brackets.stderr +++ b/tests/ui/empty_enum_variants_with_brackets.stderr @@ -1,36 +1,47 @@ -error: enum variant has empty brackets - --> tests/ui/empty_enum_variants_with_brackets.rs:7:16 +error: non-unit variant contains no fields + --> tests/ui/empty_enum_variants_with_brackets.rs:11:17 | LL | EmptyBraces {}, - | ^^^ + | ^^ | - = note: `-D clippy::empty-enum-variants-with-brackets` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::empty_enum_variants_with_brackets)]` - = help: remove the brackets +note: the lint level is defined here + --> tests/ui/empty_enum_variants_with_brackets.rs:2:9 + | +LL | #![deny(clippy::empty_enum_variants_with_brackets)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: remove the braces -error: enum variant has empty brackets - --> tests/ui/empty_enum_variants_with_brackets.rs:8:21 +error: non-unit variant contains no fields + --> tests/ui/empty_enum_variants_with_brackets.rs:12:21 | LL | EmptyParentheses(), | ^^ | - = help: remove the brackets + = help: remove the parentheses -error: enum variant has empty brackets - --> tests/ui/empty_enum_variants_with_brackets.rs:14:16 +error: non-unit variant contains no fields + --> tests/ui/empty_enum_variants_with_brackets.rs:21:17 | LL | EmptyBraces {}, - | ^^^ + | ^^ | - = help: remove the brackets + = help: remove the braces -error: enum variant has empty brackets - --> tests/ui/empty_enum_variants_with_brackets.rs:15:21 +error: non-unit variant contains no fields + --> tests/ui/empty_enum_variants_with_brackets.rs:22:21 | LL | EmptyParentheses(), | ^^ | - = help: remove the brackets + = help: remove the parentheses + +error: non-unit variant contains no fields + --> tests/ui/empty_enum_variants_with_brackets.rs:25:20 + | +LL | WithWhitespace { }, + | ^^^^ + | + = help: remove the braces -error: aborting due to 4 previous errors +error: aborting due to 5 previous errors diff --git a/tests/ui/empty_structs_with_brackets.fixed b/tests/ui/empty_structs_with_brackets.fixed index 80572645f5d1..8436cdee6522 100644 --- a/tests/ui/empty_structs_with_brackets.fixed +++ b/tests/ui/empty_structs_with_brackets.fixed @@ -1,24 +1,48 @@ -#![warn(clippy::empty_structs_with_brackets)] +//@aux-build:proc_macros.rs +#![deny(clippy::empty_structs_with_brackets)] #![allow(dead_code)] -pub struct MyEmptyStruct; // should trigger lint -struct MyEmptyTupleStruct; // should trigger lint +extern crate proc_macros; +use proc_macros::{external, with_span}; + +pub struct MyEmptyStruct; //~ empty_structs_with_brackets +struct MyEmptyTupleStruct; //~ empty_structs_with_brackets + +#[rustfmt::skip] +struct WithWhitespace; //~ empty_structs_with_brackets + +struct WithComment { + // Some long explanation here +} -// should not trigger lint struct MyCfgStruct { #[cfg(feature = "thisisneverenabled")] field: u8, } -// should not trigger lint struct MyCfgTupleStruct(#[cfg(feature = "thisisneverenabled")] u8); -// should not trigger lint struct MyStruct { field: u8, } -struct MyTupleStruct(usize, String); // should not trigger lint -struct MySingleTupleStruct(usize); // should not trigger lint -struct MyUnitLikeStruct; // should not trigger lint +struct MyTupleStruct(usize, String); +struct MySingleTupleStruct(usize); +struct MyUnitLikeStruct; + +external! { + struct External {} +} + +with_span! { + span + struct ProcMacro {} +} + +macro_rules! m { + ($($name:ident: $ty:ty),*) => { + struct Macro { $($name: $ty),* } + } +} +m! {} fn main() {} diff --git a/tests/ui/empty_structs_with_brackets.rs b/tests/ui/empty_structs_with_brackets.rs index 8fb3e247a419..467760051293 100644 --- a/tests/ui/empty_structs_with_brackets.rs +++ b/tests/ui/empty_structs_with_brackets.rs @@ -1,24 +1,48 @@ -#![warn(clippy::empty_structs_with_brackets)] +//@aux-build:proc_macros.rs +#![deny(clippy::empty_structs_with_brackets)] #![allow(dead_code)] -pub struct MyEmptyStruct {} // should trigger lint -struct MyEmptyTupleStruct(); // should trigger lint +extern crate proc_macros; +use proc_macros::{external, with_span}; + +pub struct MyEmptyStruct {} //~ empty_structs_with_brackets +struct MyEmptyTupleStruct(); //~ empty_structs_with_brackets + +#[rustfmt::skip] +struct WithWhitespace { } //~ empty_structs_with_brackets + +struct WithComment { + // Some long explanation here +} -// should not trigger lint struct MyCfgStruct { #[cfg(feature = "thisisneverenabled")] field: u8, } -// should not trigger lint struct MyCfgTupleStruct(#[cfg(feature = "thisisneverenabled")] u8); -// should not trigger lint struct MyStruct { field: u8, } -struct MyTupleStruct(usize, String); // should not trigger lint -struct MySingleTupleStruct(usize); // should not trigger lint -struct MyUnitLikeStruct; // should not trigger lint +struct MyTupleStruct(usize, String); +struct MySingleTupleStruct(usize); +struct MyUnitLikeStruct; + +external! { + struct External {} +} + +with_span! { + span + struct ProcMacro {} +} + +macro_rules! m { + ($($name:ident: $ty:ty),*) => { + struct Macro { $($name: $ty),* } + } +} +m! {} fn main() {} diff --git a/tests/ui/empty_structs_with_brackets.stderr b/tests/ui/empty_structs_with_brackets.stderr index e57249aec023..febbff1fa143 100644 --- a/tests/ui/empty_structs_with_brackets.stderr +++ b/tests/ui/empty_structs_with_brackets.stderr @@ -1,20 +1,31 @@ -error: found empty brackets on struct declaration - --> tests/ui/empty_structs_with_brackets.rs:4:25 +error: non-unit struct contains no fields + --> tests/ui/empty_structs_with_brackets.rs:8:26 | -LL | pub struct MyEmptyStruct {} // should trigger lint - | ^^^ +LL | pub struct MyEmptyStruct {} + | ^^ | - = note: `-D clippy::empty-structs-with-brackets` implied by `-D warnings` - = help: to override `-D warnings` add `#[allow(clippy::empty_structs_with_brackets)]` - = help: remove the brackets +note: the lint level is defined here + --> tests/ui/empty_structs_with_brackets.rs:2:9 + | +LL | #![deny(clippy::empty_structs_with_brackets)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = help: remove the braces + +error: non-unit struct contains no fields + --> tests/ui/empty_structs_with_brackets.rs:9:26 + | +LL | struct MyEmptyTupleStruct(); + | ^^ + | + = help: remove the parentheses -error: found empty brackets on struct declaration - --> tests/ui/empty_structs_with_brackets.rs:5:26 +error: non-unit struct contains no fields + --> tests/ui/empty_structs_with_brackets.rs:12:23 | -LL | struct MyEmptyTupleStruct(); // should trigger lint - | ^^^ +LL | struct WithWhitespace { } + | ^^^^ | - = help: remove the brackets + = help: remove the braces -error: aborting due to 2 previous errors +error: aborting due to 3 previous errors