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

needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ... #5837

Merged
merged 7 commits into from
Aug 4, 2020

Conversation

JarredAllen
Copy link
Contributor

changelog: Expand the needless_collect lint as suggested in #5627 (WIP).

This PR is WIP because I can't figure out how to make the multi-part suggestion include its changes in the source code (the fixed is identical to the source, despite the lint making suggestions). Aside from that one issue, I think this should be good.

@rust-highfive
Copy link

r? @phansch

(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 23, 2020
Copy link
Member

@phansch phansch left a comment

Choose a reason for hiding this comment

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

how to make the multi-part suggestion include its changes in the source code

Multi-part suggestions are not yet supported by rustfix, so that's not something we could fix here.

Minus the nits, this looks good to me, thanks!

@@ -0,0 +1,22 @@
// run-rustfix

#[allow(unused)]
Copy link
Member

Choose a reason for hiding this comment

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

Did you mean this to be #![allow(unused)]?

let is_empty = sym!(is_empty);
let contains = sym!(contains);
match method_name.ident.name {
name if name == into_iter => self.uses.push(
Copy link
Member

Choose a reason for hiding this comment

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

Ah.. function calls are not allowed in patterns. At least for into_iter we can use the pre-interned symbol from rustc (will need a use rustc_span::symbol::sym):

https://github.com/rust-lang/rust/blob/46cf80dc1a75ad27f67e79f73fec371a16762494/src/librustc_span/symbol.rs#L594

Suggested change
name if name == into_iter => self.uses.push(
sym::into_iter => self.uses.push(

if let Some(ref generic_args) = method_name.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
if let ty = cx.typeck_results().node_type(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
if is_type_diagnostic_item(cx, ty, sym::vec_type) ||

@JarredAllen JarredAllen changed the title WIP needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ... needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ... Aug 3, 2020
@JarredAllen
Copy link
Contributor Author

@phansch I think I've added those suggestions.

Copy link
Member

@phansch phansch left a comment

Choose a reason for hiding this comment

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

LGTM, thanks again!

@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 3, 2020
@flip1995
Copy link
Member

flip1995 commented Aug 4, 2020

@bors r=phansch rollup=always

@bors
Copy link
Contributor

bors commented Aug 4, 2020

📌 Commit 5e10b03 has been approved by phansch

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Aug 4, 2020
needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...

changelog: Expand the needless_collect lint as suggested in rust-lang#5627 (WIP).

This PR is WIP because I can't figure out how to make the multi-part suggestion include its changes in the source code (the fixed is identical to the source, despite the lint making suggestions). Aside from that one issue, I think this should be good.
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 6 pull requests

Successful merges:

 - #5725 (should_impl_trait - ignore methods with lifetime params)
 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 6 pull requests

Successful merges:

 - #5725 (should_impl_trait - ignore methods with lifetime params)
 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 5 pull requests

Successful merges:

 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 5 pull requests

Successful merges:

 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
@bors bors merged commit ca2a25d into rust-lang:master Aug 4, 2020
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