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.

Sign up for GitHub

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

Jump to bottom

Add new lint mismatching_type_param_order #8831

Merged
merged 1 commit into from
Jun 3, 2022
Merged

Add new lint mismatching_type_param_order #8831

merged 1 commit into from
Jun 3, 2022

Conversation

arieluy
Copy link
Contributor

@arieluy arieluy commented May 14, 2022 •
edited by dswij
Loading

changelog: Add new lint [mismatching_type_param_order] for checking if type parameters are consistent between type definitions and impl blocks.

fixes #7147

@rust-highfive
Copy link

r? @flip1995

(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 14, 2022
@arieluy arieluy changed the title Add new lint `[type_param_mismatch]` Add new lint [type_param_mismatch] May 14, 2022
@arieluy arieluy changed the title Add new lint [type_param_mismatch] Add new lint type_param_mismatch May 14, 2022
@arieluy arieluy force-pushed the type_params branch 2 times, most recently from 337ca0c to 478c410 Compare May 14, 2022 07:39
@bors
Copy link
Contributor

bors commented May 15, 2022

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

@xFrednet
Copy link
Member

r? @dswij Could you review this, once the merge conflicts are resolved? 🙃

@xFrednet xFrednet self-assigned this May 15, 2022
@arieluy
Copy link
Contributor Author

arieluy commented May 15, 2022

Merge conflicts are fixed

@bors
Copy link
Contributor

bors commented May 25, 2022

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

@dswij
Copy link
Member

dswij commented May 25, 2022

Sorry, I forgot that this was in my review queue. I've taken a brief look, but got distracted. Will review this soon

dswij
dswij requested changes May 25, 2022
View reviewed changes
Copy link
Member

@dswij dswij left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the PR (and waiting patiently for the review)! 👍

I'd like to apologize first for forgetting to review this for a while.

The PR looks great overall. But I think we should name the lint type_param_order_mismatch since we're only checking the order. type_param_mismatch is a little bit ambiguous. What do you think?

tests/ui/type_param_mismatch.stderr Outdated Show resolved Hide resolved
clippy_lints/src/type_param_mismatch.rs Outdated Show resolved Hide resolved
clippy_lints/src/type_param_mismatch.rs Outdated Show resolved Hide resolved
clippy_lints/src/type_param_mismatch.rs Outdated Show resolved Hide resolved
@xFrednet
Copy link
Member

I think we should name the lint type_param_order_mismatch since we're only checking the order.

Agreed, good catch! The name should ideally be plural to align with rust's lint naming guidelines. So, type_param_order_mismatches. I would maybe suggest the name mismatching_type_param_order as it would work a bit better with lint attributes. 🙃

@arieluy arieluy force-pushed the type_params branch 2 times, most recently from 7261ee2 to d606288 Compare May 28, 2022 06:08
@arieluy
Copy link
Contributor Author

arieluy commented May 28, 2022

Renamed lint, thanks for the suggestion. Also resolved merge conflicts and a few other things.

@arieluy arieluy changed the title Add new lint type_param_mismatch Add new lint mismatching_type_param_order May 28, 2022
@bors
Copy link
Contributor

bors commented May 31, 2022

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

@arieluy
Add new lint mismatching_type_param_order
58cd01c 
Add new lint for checking if type parameters are consistent between type
definitions and impl blocks.
@arieluy
Copy link
Contributor Author

arieluy commented Jun 3, 2022

Added hashmap, changed the help message, resolved merge conflicts

@dswij
Copy link
Member

dswij commented Jun 3, 2022

This looks good, thanks for this!

@bors r+

@bors
Copy link
Contributor

bors commented Jun 3, 2022

📌 Commit 58cd01c has been approved by dswij

@bors
Copy link
Contributor

bors commented Jun 3, 2022

⌛ Testing commit 58cd01c with merge baddc03...

bors added a commit that referenced this pull request Jun 3, 2022
@bors
Auto merge of #8831 - arieluy:type_params, r=dswij
baddc03 
Add new lint `mismatching_type_param_order`

changelog:
Add new lint `mismatching_type_param_order` for checking if type parameters
are consistent between type definitions and impl blocks.

fixes #7147
@bors
Copy link
Contributor

bors commented Jun 3, 2022

💔 Test failed - checks-action_test

@dswij
Copy link
Member

dswij commented Jun 3, 2022

Whoops, seems like it doesn't like the changelog.
@arieluy I edited the changelog in PR description

@dswij
Copy link
Member

dswij commented Jun 3, 2022

@bors retry

@bors
Copy link
Contributor

bors commented Jun 3, 2022

⌛ Testing commit 58cd01c with merge 7c0d649...

@bors
Copy link
Contributor

bors commented Jun 3, 2022

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: dswij
Pushing 7c0d649 to master...

@bors bors merged commit 7c0d649 into rust-lang:master Jun 3, 2022
This was referenced Jun 3, 2022
Closed
Merged
@dtolnay dtolnay mentioned this pull request Jun 7, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dswij dswij dswij approved these changes

Assignees

@flip1995 flip1995

@xFrednet xFrednet

@dswij dswij

Labels
S-waiting-on-review Status: Awaiting review from the assignee but also interested parties
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Inconsistent type-parameter order
6 participants
@arieluy @rust-highfive @bors @xFrednet @dswij @flip1995