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

Rollup of 6 pull requests #5866

Closed
wants to merge 37 commits into from
Closed

Conversation

flip1995
Copy link
Member

@flip1995 flip1995 commented Aug 4, 2020

Successful merges:

Failed merges:

r? @ghost

changelog: rollup

Ryan1729 and others added 30 commits July 26, 2020 20:40
specifically:
cargo dev new_lint --name derive_ord_xor_partial_ord --category correctness --pass late
… which is showing one error when there should be four
I couldn't really tell what it was meant to improve. It seems more clear
without the renaming to `Name`?
wiomoc and others added 7 commits August 3, 2020 12:32
…95,yaahc

should_impl_trait - ignore methods with lifetime params

Fixes: rust-lang#5617

changelog: don't lint should_implement_trait when an `Iterator::next` case has explicit parameters
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.
…flip1995

Handle mapping to Option in `map_flatten` lint

Fixes rust-lang#4496

The existing [`map_flatten`](https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten) lint suggests changing `expr.map(...).flatten()` to `expr.flat_map(...)` when `expr` is `Iterator`. This PR changes suggestion to `filter_map` instead of `flat_map` when mapping to `Option`, because it is more natural

Also here are some questions:
* If expression has type which implements `Iterator` trait (`match_trait_method(cx, expr, &paths::ITERATOR) == true`), how can I get type of iterator elements? Currently I use return type of closure inside `map`, but probably it is not good way
* I would like to change suggestion range to cover only `.map(...).flatten()`, that is from:
```
    let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `vec![5_i8; 6].into_iter().flat_map
```
to
```
    let _: Vec<_> = vec![5_i8; 6].into_iter().map(|x| 0..x).flatten().collect();
                                             ^^^^^^^^^^^^^^^^^^^^^^^^ help: try using `flat_map` instead: `.flat_map(|x| 0..x)`
```
Is it ok?
* Is `map_flatten` lint intentionally in `pedantic` category, or could it be moved to `complexity`?

changelog: Handle mapping to Option in [`map_flatten`](https://rust-lang.github.io/rust-clippy/master/index.html#map_flatten) lint
…ord-lint, r=matthiaskrgr

Add derive_ord_xor_partial_ord lint

Fix rust-lang#1621

Some remarks:
This PR follows the example of the analogous derive_hash_xor_partial_eq lint where possible.
I initially tried using the `match_path` function to identify `Ord` implementation like the derive_hash_xor_partial_eq lint currently does, for `Hash` implementations but that didn't work.

Specifically, the structs at the top level were getting paths that matched `&["$crate", "cmp", "Ord"]` instead of `&["std", "cmp", "Ord"]`. While trying to figure out what to do instead I saw the comment at the top of [clippy_lints/src/utils/paths.rs](https://github.com/rust-lang/rust-clippy/blob/f5d429cd762423901f946abd800dc2cd91366ccf/clippy_lints/src/utils/paths.rs#L5) that mentioned [this issue](rust-lang#5393) and suggested to use diagnostic items instead of hardcoded paths whenever possible. I looked for a way to identify `Ord` implementations with diagnostic items, but (possibly because this was the first time I had heard of diagnostic items,) I was unable to find one.

Eventually I tried using `get_trait_def_id` and comparing `DefId` values directly and that seems to work as expected. Maybe there's a better approach however?

changelog: new lint: derive_ord_xor_partial_ord
… r=Manishearth

Add lint for duplicate methods of trait bounds

rel: rust-lang#5777

changelog: Add [`trait_duplication_in_bounds`] lint
…ip1995

Remove old Symbol reexport

I couldn't really tell what it was meant to improve. It seems more clear
without the renaming to `Name`?

changelog: none
@flip1995
Copy link
Member Author

flip1995 commented Aug 4, 2020

@bors r+ p=6

@bors
Copy link
Contributor

bors commented Aug 4, 2020

📌 Commit 59e901c has been approved by flip1995

@bors
Copy link
Contributor

bors commented Aug 4, 2020

⌛ Testing commit 59e901c with merge 1358d4d...

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
@flip1995
Copy link
Member Author

flip1995 commented Aug 4, 2020

@bors treeclosed=6

@flip1995
Copy link
Member Author

flip1995 commented Aug 4, 2020

@bors retry (yeet)

@bors
Copy link
Contributor

bors commented Aug 4, 2020

⌛ Testing commit 59e901c with merge 1f798e8...

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
@flip1995 flip1995 closed this Aug 4, 2020
@flip1995 flip1995 deleted the rollup-ovtjqh4 branch August 4, 2020 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants