Skip to content

unnecessary_sort_by: avoid linting if key borrows #5756

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 13, 2020
Merged

unnecessary_sort_by: avoid linting if key borrows #5756

merged 1 commit into from
Jul 13, 2020

Conversation

ebroto
Copy link
Contributor

@ebroto ebroto commented Jun 28, 2020 •
edited
Loading

changelog: Avoid linting if key borrows in [unnecessary_sort_by]

Fixes #5754
Closes #2313

@rust-highfive
Copy link

r? @flip1995

(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 Jun 28, 2020
@ebroto
Copy link
Contributor Author

ebroto commented Jun 28, 2020 •
edited
Loading

While working on this fix I've seen something that could backfire.

In the sort_by_key cases, it's assumed that the involved types are Copy because in the suggestion the parameter of the closure it's unconditionally being dereferenced. I think it should work without it (e.g. in the tests, arithmetic operations and std::ops::Neg can work with references).

Am I missing something?

@bors
Copy link
Contributor

bors commented Jun 30, 2020

☔ The latest upstream changes (presumably #5751) made this pull request unmergeable. Please resolve the merge conflicts.

@ebroto
Copy link
Contributor Author

ebroto commented Jun 30, 2020 •
edited
Loading

Rebased to handle the rustup.

I've no idea why the UI test for multiple_crate_versions had to be changed, especially since the manifest uses the "=X.X.X" syntax to specify the versions 🤔

EDIT: Nevermind, it has been solved by mikerite in #5759. It's ansi_term's manifest which caused the issue.

@flip1995
Copy link
Member

flip1995 commented Jul 3, 2020

@bors r+

Thanks!

@bors
Copy link
Contributor

bors commented Jul 3, 2020

📌 Commit 18d09c5 has been approved by flip1995

bors added a commit that referenced this pull request Jul 3, 2020
@bors
Auto merge of #5756 - ebroto:5754_sort_by, r=flip1995
98d1d45 
unnecessary_sort_by: avoid linting if key borrows

changelog: Avoid linting if key borrows in [`unnecessary_sort_by`]

Fixes #5754
Closes #2313
@bors
Copy link
Contributor

bors commented Jul 3, 2020

⌛ Testing commit 18d09c5 with merge 98d1d45...

@bors
Copy link
Contributor

bors commented Jul 3, 2020

💔 Test failed - checks-action_test

@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 Jul 3, 2020
@bors
Copy link
Contributor

bors commented Jul 3, 2020

☔ The latest upstream changes (presumably #5763) made this pull request unmergeable. Please resolve the merge conflicts.

@flip1995
Copy link
Member

@bors r+ rollup=always

@bors
Copy link
Contributor

bors commented Jul 13, 2020

📌 Commit db1c946 has been approved by flip1995

@flip1995 flip1995 mentioned this pull request Jul 13, 2020
Merged
bors added a commit that referenced this pull request Jul 13, 2020
@bors
Auto merge of #5792 - flip1995:rollup-torc1we, r=flip1995
4b87008 
Rollup of 5 pull requests

Successful merges:

 - #5443 (Some accuracy lints for floating point operations)
 - #5752 (Move range_minus_one to pedantic)
 - #5756 (unnecessary_sort_by: avoid linting if key borrows)
 - #5784 (Fix out of bounds access by checking length equality BEFORE accessing by index.)
 - #5786 (fix phrase in new_lint issue template)

Failed merges:

r? @ghost

changelog: rollup
@bors bors merged commit 32ef448 into rust-lang:master Jul 13, 2020
@ebroto ebroto deleted the 5754_sort_by branch July 13, 2020 14:52
@ebroto ebroto mentioned this pull request Sep 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@flip1995 flip1995

Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

False positive for unnecessary_sort_by lint: Suggestion does not compile Redundant x_by and x_by_key
4 participants
@ebroto @rust-highfive @bors @flip1995