Skip to content

Commit

Permalink
Auto merge of #8594 - FoseFx:unit_like_struct_brackets, r=giraffate
Browse files Browse the repository at this point in the history
add `empty_structs_with_brackets`

<!-- Thank you for making Clippy better!

We're collecting our changelog from pull request descriptions.
If your PR only includes internal changes, you can just write
`changelog: none`. Otherwise, please write a short comment
explaining your change. Also, it's helpful for us that
the lint name is put into brackets `[]` and backticks `` ` ` ``,
e.g. ``[`lint_name`]``.

If your PR fixes an issue, you can add "fixes #issue_number" into this
PR description. This way the issue will be automatically closed when
your PR is merged.

If you added a new lint, here's a checklist for things that will be
checked during review or continuous integration.

- \[ ] Followed [lint naming conventions][lint_naming]
- \[ ] Added passing UI tests (including committed `.stderr` file)
- \[ ] `cargo test` passes locally
- \[ ] Executed `cargo dev update_lints`
- \[ ] Added lint documentation
- \[ ] Run `cargo dev fmt`

[lint_naming]: https://rust-lang.github.io/rfcs/0344-conventions-galore.html#lints

Note that you can skip the above if you are just opening a WIP PR in
order to get feedback.

Delete this line and everything above before opening your PR.

--

*Please write a short comment explaining your change (or "none" for internal only changes)*
-->
Closes #8591

I'm already sorry for the massive diff 😅

changelog: New lint [`empty_structs_with_brackets`]
  • Loading branch information
bors committed Apr 4, 2022
2 parents 85b88be + 58833e5 commit 1cec8b3
Show file tree
Hide file tree
Showing 58 changed files with 274 additions and 101 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -3263,6 +3263,7 @@ Released 2018-09-13
[`empty_enum`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_enum
[`empty_line_after_outer_attr`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_line_after_outer_attr
[`empty_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_loop
[`empty_structs_with_brackets`]: https://rust-lang.github.io/rust-clippy/master/index.html#empty_structs_with_brackets
[`enum_clike_unportable_variant`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_clike_unportable_variant
[`enum_glob_use`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_glob_use
[`enum_variant_names`]: https://rust-lang.github.io/rust-clippy/master/index.html#enum_variant_names
Expand Down
99 changes: 99 additions & 0 deletions clippy_lints/src/empty_structs_with_brackets.rs
@@ -0,0 +1,99 @@
use clippy_utils::{diagnostics::span_lint_and_then, source::snippet_opt};
use rustc_ast::ast::{Item, ItemKind, VariantData};
use rustc_errors::Applicability;
use rustc_lexer::TokenKind;
use rustc_lint::{EarlyContext, EarlyLintPass};
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::Span;

declare_clippy_lint! {
/// ### What it does
/// Finds structs without fields (a so-called "empty struct") that are declared with brackets.
///
/// ### Why is this bad?
/// Empty brackets after a struct declaration can be omitted.
///
/// ### Example
/// ```rust
/// struct Cookie {}
/// ```
/// Use instead:
/// ```rust
/// struct Cookie;
/// ```
#[clippy::version = "1.62.0"]
pub EMPTY_STRUCTS_WITH_BRACKETS,
restriction,
"finds struct declarations with empty brackets"
}
declare_lint_pass!(EmptyStructsWithBrackets => [EMPTY_STRUCTS_WITH_BRACKETS]);

impl EarlyLintPass for EmptyStructsWithBrackets {
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(
cx,
EMPTY_STRUCTS_WITH_BRACKETS,
span_after_ident,
"found empty brackets on struct declaration",
|diagnostic| {
diagnostic.span_suggestion_hidden(
span_after_ident,
"remove the brackets",
";".to_string(),
Applicability::MachineApplicable);
},
);
}
}
}

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
// (conditionaly 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())
}

#[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));
}
}
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Expand Up @@ -129,6 +129,7 @@ store.register_lints(&[
duration_subsec::DURATION_SUBSEC,
else_if_without_else::ELSE_IF_WITHOUT_ELSE,
empty_enum::EMPTY_ENUM,
empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS,
entry::MAP_ENTRY,
enum_clike::ENUM_CLIKE_UNPORTABLE_VARIANT,
enum_variants::ENUM_VARIANT_NAMES,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_restriction.rs
Expand Up @@ -16,6 +16,7 @@ store.register_group(true, "clippy::restriction", Some("clippy_restriction"), ve
LintId::of(default_union_representation::DEFAULT_UNION_REPRESENTATION),
LintId::of(disallowed_script_idents::DISALLOWED_SCRIPT_IDENTS),
LintId::of(else_if_without_else::ELSE_IF_WITHOUT_ELSE),
LintId::of(empty_structs_with_brackets::EMPTY_STRUCTS_WITH_BRACKETS),
LintId::of(exhaustive_items::EXHAUSTIVE_ENUMS),
LintId::of(exhaustive_items::EXHAUSTIVE_STRUCTS),
LintId::of(exit::EXIT),
Expand Down
2 changes: 2 additions & 0 deletions clippy_lints/src/lib.rs
Expand Up @@ -209,6 +209,7 @@ mod drop_forget_ref;
mod duration_subsec;
mod else_if_without_else;
mod empty_enum;
mod empty_structs_with_brackets;
mod entry;
mod enum_clike;
mod enum_variants;
Expand Down Expand Up @@ -869,6 +870,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
})
});
store.register_early_pass(|| Box::new(crate_in_macro_def::CrateInMacroDef));
store.register_early_pass(|| Box::new(empty_structs_with_brackets::EmptyStructsWithBrackets));
// add lints here, do not remove this comment, it's used in `new_lint`
}

Expand Down
4 changes: 2 additions & 2 deletions clippy_lints/src/use_self.rs
Expand Up @@ -34,7 +34,7 @@ declare_clippy_lint! {
///
/// ### Example
/// ```rust
/// struct Foo {}
/// struct Foo;
/// impl Foo {
/// fn new() -> Foo {
/// Foo {}
Expand All @@ -43,7 +43,7 @@ declare_clippy_lint! {
/// ```
/// could be
/// ```rust
/// struct Foo {}
/// struct Foo;
/// impl Foo {
/// fn new() -> Self {
/// Self {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui-toml/struct_excessive_bools/test.rs
Expand Up @@ -4,6 +4,6 @@ struct S {
a: bool,
}

struct Foo {}
struct Foo;

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui/case_sensitive_file_extension_comparisons.rs
Expand Up @@ -2,7 +2,7 @@

use std::string::String;

struct TestStruct {}
struct TestStruct;

impl TestStruct {
fn ends_with(self, arg: &str) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-2774.rs
Expand Up @@ -8,7 +8,7 @@ pub struct Bar {
}

#[derive(Eq, PartialEq, Debug, Hash)]
pub struct Foo {}
pub struct Foo;

#[allow(clippy::implicit_hasher)]
// This should not cause a "cannot relate bound region" ICE.
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-6179.rs
Expand Up @@ -4,7 +4,7 @@
#![warn(clippy::use_self)]
#![allow(dead_code)]

struct Foo {}
struct Foo;

impl Foo {
fn new() -> Self {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-6792.rs
Expand Up @@ -7,7 +7,7 @@ trait Trait {
fn broken() -> Self::Ty;
}

struct Foo {}
struct Foo;

impl Trait for Foo {
type Ty = Foo;
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/needless_lifetimes_impl_trait.rs
Expand Up @@ -3,7 +3,7 @@

trait Foo {}

struct Bar {}
struct Bar;

struct Baz<'a> {
bar: &'a Bar,
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/crashes/regressions.rs
Expand Up @@ -6,6 +6,6 @@ pub fn foo(bar: *const u8) {

// Regression test for https://github.com/rust-lang/rust-clippy/issues/4917
/// <foo
struct A {}
struct A;

fn main() {}
2 changes: 1 addition & 1 deletion tests/ui/default_numeric_fallback_f64.fixed
Expand Up @@ -134,7 +134,7 @@ mod enum_ctor {
}

mod method_calls {
struct StructForMethodCallTest {}
struct StructForMethodCallTest;

impl StructForMethodCallTest {
fn concrete_arg(&self, f: f64) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/default_numeric_fallback_f64.rs
Expand Up @@ -134,7 +134,7 @@ mod enum_ctor {
}

mod method_calls {
struct StructForMethodCallTest {}
struct StructForMethodCallTest;

impl StructForMethodCallTest {
fn concrete_arg(&self, f: f64) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/default_numeric_fallback_i32.fixed
Expand Up @@ -133,7 +133,7 @@ mod enum_ctor {
}

mod method_calls {
struct StructForMethodCallTest {}
struct StructForMethodCallTest;

impl StructForMethodCallTest {
fn concrete_arg(&self, x: i32) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/default_numeric_fallback_i32.rs
Expand Up @@ -133,7 +133,7 @@ mod enum_ctor {
}

mod method_calls {
struct StructForMethodCallTest {}
struct StructForMethodCallTest;

impl StructForMethodCallTest {
fn concrete_arg(&self, x: i32) {}
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/drop_forget_copy.rs
Expand Up @@ -5,7 +5,7 @@ use std::mem::{drop, forget};
use std::vec::Vec;

#[derive(Copy, Clone)]
struct SomeStruct {}
struct SomeStruct;

struct AnotherStruct {
x: u8,
Expand Down
25 changes: 25 additions & 0 deletions tests/ui/empty_structs_with_brackets.fixed
@@ -0,0 +1,25 @@
// run-rustfix
#![warn(clippy::empty_structs_with_brackets)]
#![allow(dead_code)]

pub struct MyEmptyStruct; // should trigger lint
struct MyEmptyTupleStruct; // should trigger lint

// 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

fn main() {}
25 changes: 25 additions & 0 deletions tests/ui/empty_structs_with_brackets.rs
@@ -0,0 +1,25 @@
// run-rustfix
#![warn(clippy::empty_structs_with_brackets)]
#![allow(dead_code)]

pub struct MyEmptyStruct {} // should trigger lint
struct MyEmptyTupleStruct(); // should trigger lint

// 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

fn main() {}
19 changes: 19 additions & 0 deletions tests/ui/empty_structs_with_brackets.stderr
@@ -0,0 +1,19 @@
error: found empty brackets on struct declaration
--> $DIR/empty_structs_with_brackets.rs:5:25
|
LL | pub struct MyEmptyStruct {} // should trigger lint
| ^^^
|
= note: `-D clippy::empty-structs-with-brackets` implied by `-D warnings`
= help: remove the brackets

error: found empty brackets on struct declaration
--> $DIR/empty_structs_with_brackets.rs:6:26
|
LL | struct MyEmptyTupleStruct(); // should trigger lint
| ^^^
|
= help: remove the brackets

error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion tests/ui/fn_params_excessive_bools.rs
Expand Up @@ -20,7 +20,7 @@ fn h(_: bool, _: bool, _: bool) {}
fn e(_: S, _: S, _: Box<S>, _: Vec<u32>) {}
fn t(_: S, _: S, _: Box<S>, _: Vec<u32>, _: bool, _: bool, _: bool, _: bool) {}

struct S {}
struct S;
trait Trait {
fn f(_: bool, _: bool, _: bool, _: bool);
fn g(_: bool, _: bool, _: bool, _: Vec<u32>);
Expand Down
4 changes: 2 additions & 2 deletions tests/ui/implicit_clone.rs
Expand Up @@ -30,7 +30,7 @@ where
}

#[derive(Copy, Clone)]
struct Kitten {}
struct Kitten;
impl Kitten {
// badly named method
fn to_vec(self) -> Kitten {
Expand All @@ -44,7 +44,7 @@ impl Borrow<BorrowedKitten> for Kitten {
}
}

struct BorrowedKitten {}
struct BorrowedKitten;
impl ToOwned for BorrowedKitten {
type Owned = Kitten;
fn to_owned(&self) -> Kitten {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/iter_nth_zero.fixed
Expand Up @@ -3,7 +3,7 @@
#![warn(clippy::iter_nth_zero)]
use std::collections::HashSet;

struct Foo {}
struct Foo;

impl Foo {
fn nth(&self, index: usize) -> usize {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/iter_nth_zero.rs
Expand Up @@ -3,7 +3,7 @@
#![warn(clippy::iter_nth_zero)]
use std::collections::HashSet;

struct Foo {}
struct Foo;

impl Foo {
fn nth(&self, index: usize) -> usize {
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/large_types_passed_by_value.rs
Expand Up @@ -37,7 +37,7 @@ pub trait PubLargeTypeDevourer {
fn devoure_array_in_public(&self, array: [u8; 6666]);
}

struct S {}
struct S;
impl LargeTypeDevourer for S {
fn devoure_array(&self, array: [u8; 6666]) {
todo!();
Expand Down
2 changes: 1 addition & 1 deletion tests/ui/let_and_return.rs
Expand Up @@ -88,7 +88,7 @@ mod no_lint_if_stmt_borrows {
ret
}

struct Bar {}
struct Bar;

impl Bar {
fn new() -> Self {
Expand Down

0 comments on commit 1cec8b3

Please sign in to comment.