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

Fix mismatching_type_param_order false positive #9146

Merged
merged 1 commit into from
Jul 18, 2022
Merged

Fix mismatching_type_param_order false positive #9146

merged 1 commit into from
Jul 18, 2022

Conversation

arieluy
Copy link
Contributor

@arieluy arieluy commented Jul 10, 2022

changelog: Don't lint mismatching_type_param_order on complicated generic params

fixes #8962

@rust-highfive
Copy link

r? @llogiq

(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 Jul 10, 2022
@arieluy
Copy link
Contributor Author

arieluy commented Jul 10, 2022

r? @dswij sorry I took a while to get to this, would you mind taking a look?

@rust-highfive rust-highfive assigned dswij and unassigned llogiq Jul 10, 2022
@dswij
Copy link
Member

dswij commented Jul 11, 2022

sorry I took a while to get to this

No worries!

dswij
dswij reviewed Jul 11, 2022
View reviewed changes
Copy link
Member

@dswij dswij left a comment

Choose a reason for hiding this comment

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

Thanks for taking a look! The PR itself looks good to me, but I think we can add this info into the lint's docs in the "Limitation/Known Issue" section.

e.g. smth like this:

rust-clippy/clippy_lints/src/return_self_not_must_use.rs

Lines 24 to 26 in 4970527

/// ### Limitations
/// This lint is only applied on methods taking a `self` argument. It would be mostly noise
/// if it was added on constructors for example.

@arieluy
Fix mismatching_type_param_order false positive
8cf39a8 
Previously was giving false positive when an impl had a nontrivial
generic argument such as a tuple. Don't lint on these cases.
@arieluy
Copy link
Contributor Author

arieluy commented Jul 17, 2022

Added limitations section

@dswij
Copy link
Member

dswij commented Jul 18, 2022

Thanks for this! @bors r+

@bors
Copy link
Collaborator

bors commented Jul 18, 2022

📌 Commit 8cf39a8 has been approved by dswij

It is now in the queue for this repository.

@bors
Copy link
Collaborator

bors commented Jul 18, 2022

⌛ Testing commit 8cf39a8 with merge f4c9183...

@bors
Copy link
Collaborator

bors commented Jul 18, 2022

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

@bors bors merged commit f4c9183 into rust-lang:master Jul 18, 2022
@arieluy
Copy link
Contributor Author

arieluy commented Jul 28, 2022

Thanks for the review @dswij !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dswij dswij dswij left review comments

Assignees

@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.

mismatching_type_param_order false positive when preceded by nontrivial generic argument
5 participants
@arieluy @rust-highfive @dswij @bors @llogiq