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

Downgrade new_ret_no_self to pedantic #5420

Merged
merged 1 commit into from
Apr 8, 2020
Merged

Conversation

dtolnay
Copy link
Member

@dtolnay dtolnay commented Apr 4, 2020

As motivated by #5418. This is the second most widely suppressed Clippy style lint, and this grep.app search shows a large number of diverse reasonable signatures for a new method.

changelog: Remove new_ret_no_self from default set of enabled lints

@flip1995 flip1995 added the S-waiting-on-bors Status: The marked PR was approved and is only waiting bors label Apr 6, 2020
@jplatte
Copy link
Contributor

jplatte commented Apr 7, 2020

A lot of these actually contain Self in the return type. This lint only fired for them because of #3313 and many people probably just haven't revisited the lint attribute, some may have CI that runs clippy on various releases including an old one that still has that bug.

@flip1995
Copy link
Member

flip1995 commented Apr 7, 2020

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Apr 7, 2020

📌 Commit 560c8c9 has been approved by flip1995

@bors
Copy link
Contributor

bors commented Apr 7, 2020

🌲 The tree is currently closed for pull requests below priority 2, this pull request will be tested once the tree is reopened

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Apr 7, 2020
Downgrade new_ret_no_self to pedantic

As motivated by rust-lang#5418. This is the second most widely suppressed Clippy style lint, and [this grep.app search](https://grep.app/search?q=%5C%5Ballow%5C%28.%2Aclippy%3A%3Anew_ret_no_self%5Cb&regexp=true&case=true&filter[lang][0]=Rust) shows a large number of diverse reasonable signatures for a `new` method.

changelog: Remove new_ret_no_self from default set of enabled lints
@joshtriplett
Copy link
Member

Given @jplatte's comment, I don't think this change should be merged. It sounds like this lint was blatantly wrong at some point in the past, and has since been fixed.

bors added a commit that referenced this pull request Apr 8, 2020
Rollup of 12 pull requests

Successful merges:

 - #5345 (Add lint for float in array comparison)
 - #5406 (Fix update_lints)
 - #5409 (Downgrade let_unit_value to pedantic)
 - #5410 (Downgrade trivially_copy_pass_by_ref to pedantic)
 - #5412 (Downgrade inefficient_to_string to pedantic)
 - #5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`)
 - #5417 (Update doc links and mentioned names in docs)
 - #5419 (Downgrade unreadable_literal to pedantic)
 - #5420 (Downgrade new_ret_no_self to pedantic)
 - #5422 (CONTRIBUTING.md: fix broken triage link)
 - #5424 (Incorrect suspicious_op_assign_impl)
 - #5425 (Ehance opt_as_ref_deref lint.)

Failed merges:

 - #5411 (Downgrade implicit_hasher to pedantic)
 - #5428 (Move cognitive_complexity to nursery)

r? @ghost

changelog: rollup
bors added a commit that referenced this pull request Apr 8, 2020
Rollup of 11 pull requests

Successful merges:

 - #5406 (Fix update_lints)
 - #5409 (Downgrade let_unit_value to pedantic)
 - #5410 (Downgrade trivially_copy_pass_by_ref to pedantic)
 - #5412 (Downgrade inefficient_to_string to pedantic)
 - #5415 (Add new lint for `Result<T, E>.map_or(None, Some(T))`)
 - #5417 (Update doc links and mentioned names in docs)
 - #5419 (Downgrade unreadable_literal to pedantic)
 - #5420 (Downgrade new_ret_no_self to pedantic)
 - #5422 (CONTRIBUTING.md: fix broken triage link)
 - #5424 (Incorrect suspicious_op_assign_impl)
 - #5425 (Ehance opt_as_ref_deref lint.)

Failed merges:

 - #5345 (Add lint for float in array comparison)
 - #5411 (Downgrade implicit_hasher to pedantic)
 - #5428 (Move cognitive_complexity to nursery)

r? @ghost

changelog: rollup
@bors bors merged commit 7cb5180 into rust-lang:master Apr 8, 2020
@dtolnay dtolnay deleted the newret branch April 8, 2020 17:58
bors added a commit that referenced this pull request Apr 9, 2020
Revert "Downgrade new_ret_no_self to pedantic"

Reverts #5420

This got through with the big rollup merge, where I didn't recheck every PR of the rollup. Reverting because of the reason given in #5420 (comment) by @joshtriplett

changelog: Move new_ret_no_self back to style category
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants