Skip to content

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