Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add empty_structs_with_brackets #8594

Merged
merged 12 commits into from Apr 4, 2022

Conversation

FoseFx
Copy link
Contributor

@FoseFx FoseFx commented Mar 27, 2022

Closes #8591

I'm already sorry for the massive diff 😅

changelog: New lint [empty_structs_with_brackets]

@rust-highfive
Copy link

r? @giraffate

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 27, 2022
@FoseFx FoseFx changed the title Unit like struct brackets Unit-like struct brackets Mar 27, 2022
clippy_lints/src/unit_like_struct_brackets.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_like_struct_brackets.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_like_struct_brackets.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_like_struct_brackets.rs Outdated Show resolved Hide resolved
clippy_lints/src/unit_like_struct_brackets.rs Outdated Show resolved Hide resolved
@FoseFx FoseFx changed the title Unit-like struct brackets add empty_structs_with_brackets Mar 28, 2022
@bors
Copy link
Collaborator

bors commented Mar 30, 2022

☔ The latest upstream changes (presumably #8576) made this pull request unmergeable. Please resolve the merge conflicts.

@giraffate
Copy link
Contributor

I'm going through it last one time and review this, but it's going to be next week.

Copy link
Contributor

@giraffate giraffate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good.

!rustc_lexer::tokenize(braces_span_str).any(|t| t.kind == TokenKind::Ident)
}

fn is_unit_like_struct(var_data: &VariantData) -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nits

Suggested change
fn is_unit_like_struct(var_data: &VariantData) -> bool {
fn is_empty_struct(var_data: &VariantData) -> bool {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_unit_like_struct checks if the struct at hand already has no brackets (i.e. struct Cookie;). is_empty_struct sounds like we would check whether the struct is empty or not. struct Cookie {} is an empty struct, but is_unit_like_struct/is_empty_struct will return false on it.

If we want to go way from the "unit-like" term maybe has_no_brackets is a better function name.

@giraffate
Copy link
Contributor

@bors r+

Thanks!

@bors
Copy link
Collaborator

bors commented Apr 4, 2022

📌 Commit 58833e5 has been approved by giraffate

@bors
Copy link
Collaborator

bors commented Apr 4, 2022

⌛ Testing commit 58833e5 with merge 1cec8b3...

@bors
Copy link
Collaborator

bors commented Apr 4, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 1cec8b3 to master...

@bors bors merged commit 1cec8b3 into rust-lang:master Apr 4, 2022
@FoseFx FoseFx deleted the unit_like_struct_brackets branch April 4, 2022 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Suggest removing {} on structs with empty "bodies"
6 participants