Improve `map_entry` suggestion #6937

Jarcho · 2021-03-19T18:13:06Z

fixes: #5176
fixes: #4674
fixes: #4664
fixes: #1450

Still need to handle the value returned by insert correctly.

changelog: Improve map_entry suggestion. Will now suggest or_insert, insert_with or match _.entry(_) as appopriate.
changelog: Fix map_entry false positives where the entry api can't be used. e.g. when the map is used for multiple things.

rust-highfive · 2021-03-19T18:13:09Z

r? @Manishearth

(rust-highfive has picked a reviewer for you, use r? to override)

Manishearth · 2021-03-23T00:20:42Z

@giraffate Woudl you like to review this PR? No worries if you can't.

giraffate · 2021-03-23T00:46:59Z

@Manishearth Yes, I'll do it. ( I think it will take a couple of days until first review, since the diff is so large. )

Manishearth · 2021-03-23T02:02:31Z

Totally fair, and feel free to pass it back to me if you need

r? @giraffate
@bors delegate=giraffate

bors · 2021-03-23T02:02:33Z

✌️ @giraffate can now approve this pull request

giraffate

@Jarcho Sorry, I'm just starting to read the code and still in the middle of review, but I made some comments.

clippy_lints/src/entry.rs

bors · 2021-03-25T18:08:00Z

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

giraffate

Sorry, I'm in the middle of review, but I made some review on this.

clippy_lints/src/entry.rs

tests/ui/entry_unfixable.rs

bors · 2021-04-06T15:44:02Z

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

bors · 2021-04-06T17:06:49Z

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

bors · 2021-04-12T09:05:47Z

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

Y-Nak · 2021-04-12T14:25:28Z

@Jarcho I'm still reviewing this and will finish in the next 24 hours.

Y-Nak

I looked through the changes and left some comments, but it takes more time to finish the review.

Btw, I ran lintcheck, this lint was triggered only 1 time and that seemed like true-positive:+1:

clippy_lints/src/entry.rs

clippy_utils/src/lib.rs

clippy_lints/src/entry.rs

giraffate · 2021-04-14T13:23:24Z

@Y-Nak Thanks for your review! Nice catches!

Y-Nak

I finished the review and added some comments.
@giraffate Could you have a final look?

clippy_lints/src/entry.rs

Fix false positives where the map is used before inserting into the map. Fix false positives where two insertions happen. Suggest using `if let Entry::Vacant(e) = _.entry(_)` when `or_insert` might be a semantic change

Lint `if _.[!]contains_key(&_) { .. } else { .. }` so long as one of the branches contains an insertion.

Suggest using `or_insert_with` when possible

giraffate · 2021-04-15T14:40:48Z

@Y-Nak Yes, thanks!

@Jarcho I think overall looks good. Just to be sure I will look through the changes again in the next 24 hour. Thanks for your patience!

Jarcho · 2021-04-15T20:20:26Z

No worries. It's a big change.

giraffate

It looks good to me, thanks for your work!

giraffate · 2021-04-16T13:23:18Z

@bors r+

bors · 2021-04-16T13:23:20Z

📌 Commit bcf3488 has been approved by giraffate

bors · 2021-04-16T13:23:27Z

⌛ Testing commit bcf3488 with merge 1e0a3ff...

bors · 2021-04-16T13:36:27Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: giraffate
Pushing 1e0a3ff to master...

rust-highfive assigned Manishearth Mar 19, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Mar 19, 2021

Jarcho force-pushed the map_entry_suggestion branch 3 times, most recently from 08d4f34 to c6feb9b Compare March 21, 2021 13:40

rust-highfive unassigned Manishearth Mar 23, 2021

rust-highfive assigned giraffate Mar 23, 2021

giraffate reviewed Mar 23, 2021

View reviewed changes

clippy_lints/src/entry.rs Show resolved Hide resolved

clippy_lints/src/entry.rs Outdated Show resolved Hide resolved

giraffate added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Mar 24, 2021

Jarcho force-pushed the map_entry_suggestion branch 2 times, most recently from f36a1fd to e3cd56a Compare March 25, 2021 13:49

giraffate requested changes Mar 26, 2021

View reviewed changes

Jarcho force-pushed the map_entry_suggestion branch 3 times, most recently from 2458362 to 0bfc9c9 Compare April 3, 2021 12:29

Jarcho force-pushed the map_entry_suggestion branch from 0bfc9c9 to 9ac6111 Compare April 6, 2021 16:16

Jarcho force-pushed the map_entry_suggestion branch from 9ac6111 to de18142 Compare April 6, 2021 20:35

giraffate added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties and removed S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) labels Apr 12, 2021

Y-Nak reviewed Apr 13, 2021

View reviewed changes

Jarcho force-pushed the map_entry_suggestion branch 2 times, most recently from 762651b to a0c4eca Compare April 14, 2021 14:36

Y-Nak approved these changes Apr 15, 2021

View reviewed changes

Jarcho force-pushed the map_entry_suggestion branch 2 times, most recently from ff78dd4 to 12e96e8 Compare April 15, 2021 12:09

Jarcho added 4 commits April 15, 2021 08:19

Improve map_entry lint

ce5e927

Fix false positives where the map is used before inserting into the map. Fix false positives where two insertions happen. Suggest using `if let Entry::Vacant(e) = _.entry(_)` when `or_insert` might be a semantic change

map_entry improvements

b63a5b5

Lint `if _.[!]contains_key(&_) { .. } else { .. }` so long as one of the branches contains an insertion.

map_entry improvements

3323ff7

Suggest using `or_insert_with` when possible

Minor cleanup of map_entry and a few additional tests.

bcf3488

Jarcho force-pushed the map_entry_suggestion branch from 12e96e8 to bcf3488 Compare April 15, 2021 12:25

giraffate approved these changes Apr 16, 2021

View reviewed changes

bors merged commit 1e0a3ff into rust-lang:master Apr 16, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve `map_entry` suggestion #6937

Improve `map_entry` suggestion #6937

Jarcho commented Mar 19, 2021

rust-highfive commented Mar 19, 2021

Manishearth commented Mar 23, 2021

giraffate commented Mar 23, 2021

Manishearth commented Mar 23, 2021

bors commented Mar 23, 2021

giraffate left a comment

bors commented Mar 25, 2021

giraffate left a comment

bors commented Apr 6, 2021

bors commented Apr 6, 2021

bors commented Apr 12, 2021

Y-Nak commented Apr 12, 2021

Y-Nak left a comment •

edited

Loading

giraffate commented Apr 14, 2021

Y-Nak left a comment •

edited

Loading

giraffate commented Apr 15, 2021

Jarcho commented Apr 15, 2021

giraffate left a comment

giraffate commented Apr 16, 2021

bors commented Apr 16, 2021

bors commented Apr 16, 2021

bors commented Apr 16, 2021

Improve map_entry suggestion #6937

Improve map_entry suggestion #6937

Conversation

Jarcho commented Mar 19, 2021

rust-highfive commented Mar 19, 2021

Manishearth commented Mar 23, 2021

giraffate commented Mar 23, 2021

Manishearth commented Mar 23, 2021

bors commented Mar 23, 2021

giraffate left a comment

Choose a reason for hiding this comment

bors commented Mar 25, 2021

giraffate left a comment

Choose a reason for hiding this comment

bors commented Apr 6, 2021

bors commented Apr 6, 2021

bors commented Apr 12, 2021

Y-Nak commented Apr 12, 2021

Y-Nak left a comment • edited Loading

Choose a reason for hiding this comment

giraffate commented Apr 14, 2021

Y-Nak left a comment • edited Loading

Choose a reason for hiding this comment

giraffate commented Apr 15, 2021

Jarcho commented Apr 15, 2021

giraffate left a comment

Choose a reason for hiding this comment

giraffate commented Apr 16, 2021

bors commented Apr 16, 2021

bors commented Apr 16, 2021

bors commented Apr 16, 2021

Improve `map_entry` suggestion #6937

Improve `map_entry` suggestion #6937

Y-Nak left a comment •

edited

Loading

Y-Nak left a comment •

edited

Loading