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 5 pull requests #5868

Merged
merged 31 commits into from
Aug 4, 2020
Merged

Rollup of 5 pull requests #5868

merged 31 commits into from
Aug 4, 2020

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`?
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
Collaborator

bors commented Aug 4, 2020

📌 Commit fb7ad95 has been approved by flip1995

@bors
Copy link
Collaborator

bors commented Aug 4, 2020

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

@bors
Copy link
Collaborator

bors commented Aug 4, 2020

⌛ Testing commit fb7ad95 with merge f02c65e...

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

flip1995 commented Aug 4, 2020

@bors retry (forgot changlog)

@bors
Copy link
Collaborator

bors commented Aug 4, 2020

⌛ Testing commit fb7ad95 with merge 8012178...

@bors
Copy link
Collaborator

bors commented Aug 4, 2020

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

@bors bors merged commit 8012178 into rust-lang:master Aug 4, 2020
@flip1995 flip1995 deleted the rollup-5g8vft5 branch August 4, 2020 10:36
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.

7 participants