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

Register redundant_field_names and non_expressive_names as early passes #5651

Merged
merged 4 commits into from
May 26, 2020

Conversation

ebroto
Copy link
Member

@ebroto ebroto commented May 26, 2020

Similar names was moved to a pre-expansion pass to solve #2927, so I'm avoiding linting on code from expansion, which makes the dogfood (mostly, see below) pass.

I had to change new_without_default though, and although I understand why it was not triggering before, TBH I don't see why the binding inside the nested if_chain is being linted now. Any ideas? (it seems legit though as the code can be changed by the user)

changelog: Register redundant_field_names and non_expressive_names as early passes

Fixes #5356
Fixes #5521

@rust-highfive
Copy link

r? @matthiaskrgr

(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 May 26, 2020
@ebroto ebroto changed the title Names as early passes Register redundant_field_names and non_expressive_names as early passes May 26, 2020
@flip1995
Copy link
Member

Thanks for finishing this!

The lint triggering in the if_chain is because the variables don't come from an expansion, but are written by the user. It's like they are macro arguments in e.g. assert_eq!(arg1, arg2).

@bors r+

@bors
Copy link
Collaborator

bors commented May 26, 2020

📌 Commit 4161823 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented May 26, 2020

⌛ Testing commit 4161823 with merge fca76de...

@bors
Copy link
Collaborator

bors commented May 26, 2020

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: flip1995
Pushing fca76de to master...

@bors bors merged commit fca76de into rust-lang:master May 26, 2020
This was referenced May 26, 2020
@ebroto ebroto deleted the names_as_early_passes branch May 27, 2020 06:37
@sfackler
Copy link
Member

@Mark-Simulacrum if the 1.44.1 window is still open, it might be worth backporting this to fix a stable-stable regression.

@Mark-Simulacrum
Copy link
Member

@rust-lang/clippy we do have time to rebuild artifacts, but I don't know enough here to be able to say whether it's worth it or not. If we feel it is, could someone prepare a 1.44.1 branch on the clippy repository that we can checkout on rust-lang/rust's stable branch? (subtree has not yet hit rust-lang/rust stable).

@flip1995
Copy link
Member

Today was another issue opened about this, so I would say, that it would be worth it, if possible. I try to push a working commit to a rust-1.44.1 branch.

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