Fix `map_unwrap_or` fail to cover `Result::unwrap_or` #15718

profetia · 2025-09-19T15:52:58Z

Closes #15713
Closes #15714
Closes #15752
Closes #16258

changelog: [map_unwrap_or] add cover for Result::unwrap_or

rustbot · 2025-09-19T15:53:03Z

rustbot has assigned @blyxyas.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

github-actions · 2025-09-19T16:02:38Z

Lintcheck changes for 0db25db

Lint	Added	Removed	Changed
`clippy::map_unwrap_or`	15	0	5

This comment will be updated if you push new changes

samueltardieu · 2025-09-19T16:09:03Z

Unless you have a good reason not to, please use map_unwrap_or_fixable.rs for the tests.

profetia · 2025-09-19T16:17:12Z

Moved. Thank you!

profetia · 2025-09-23T16:05:58Z

Also fixes #15713

profetia · 2025-09-24T01:26:59Z

Also closes #15752

samueltardieu

If possible, you should squash together the two commits "Apply map_unwrap to Clippy itself" and make it the first commit). This way, after every commit, Clippy tests still pass (including dogfood), and it makes it easy to bisect should something go wrong. Then each "fix" commit can be put on top.

View changes since this review

clippy_lints/src/methods/map_unwrap_or.rs

clippy_lints/src/derivable_impls.rs

profetia · 2025-09-29T18:21:06Z

Updated. Thank you

clippy_lints/src/methods/map_unwrap_or.rs

profetia · 2025-10-21T21:01:08Z

r? clippy

samueltardieu

Could you also squash the commits please? The current separation makes it harder to read and review.

View changes since this review

clippy_lints/src/methods/map_unwrap_or.rs

profetia · 2025-11-25T20:29:54Z

r? clippy

profetia · 2025-12-07T01:58:03Z

r? clippy

tests/ui/map_unwrap_or.rs

clippy_lints/src/methods/map_unwrap_or_else.rs

clippy_lints/src/methods/map_unwrap_or.rs

tests/ui/map_unwrap_or_fixable.stderr

rustbot · 2025-12-18T18:18:01Z

This PR was rebased onto a different master commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

profetia · 2025-12-18T18:28:39Z

Also closes #16258

clippy_lints/src/methods/map_unwrap_or.rs

Centri3 · 2026-01-07T17:21:36Z

clippy_lints/src/methods/map_unwrap_or.rs

+        // So, we have to check that `a` is not referenced anywhere (even outside of the `.map` closure!)
+        // before the call to `unwrap_or`.
+
+        let mut unwrap_visitor = UnwrapVisitor {


I think these two are simple enough that we can just use the for_each_expr helper. And, can we add a comment here?

Centri3 · 2026-01-07T17:22:16Z

clippy_lints/src/methods/map_unwrap_or.rs

+            && let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
+            && let PatKind::Binding(_, local_id, ..) = pat.kind


What does this part fix? When will this not be a PatKind::Binding, and why?

I'm not sure about this too.

If you look into the issues this PR is addressing, all the changes in map_unwrap_or.rs, wrongly showed as additions, are actually the implementation from option_map_unwrap_or.rs, which is renamed to this file now.

For this reason, I prefer to leave these implementation details as is, while just focusing reorganizing the map_unwrap_or and map_unwrap_or_else two lints in this PR.

Centri3 · 2026-01-07T17:23:24Z

clippy_lints/src/methods/map_unwrap_or.rs

+    type NestedFilter = nested_filter::All;
+    type Result = ControlFlow<()>;
+    fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> {
+        // If we haven't found a reference yet, check if this references


Suggested change

// If we haven't found a reference yet, check if this references

// Check if this references

This more so implies that the previous UnwrapVisitor checks for references, not this code. I was a bit confused last review.

Centri3 · 2026-01-07T17:25:39Z

tests/ui/map_unwrap_or_fixable.rs

I think we just need more tests in general. To be specific, cases where it doesn't lint, like for when unwrap_arg is referenced. Same with the mutable borrows case.

Ookay, looking at it again, that's not all that's in this file. So +1 from me.

Centri3 · 2026-01-07T17:25:57Z

clippy_lints/src/methods/map_unwrap_or.rs

+    fn visit_expr(&mut self, expr: &'tcx rustc_hir::Expr<'_>) -> ControlFlow<()> {
+        // If we haven't found a reference yet, check if this references
+        // one of the locals that was moved in the `unwrap_or` argument.
+        // We are only interested in exprs that appear before the `unwrap_or` call.


We are only interested in exprs that appear before the unwrap_or call.

Why is that? Does the borrowck error not happen in that case?

Centri3 · 2026-01-08T00:53:00Z

Ok, hadn't noticed, that's fair. I'll do one final look tomorrow and then probably merge 👍

Centri3 · 2026-01-11T01:37:16Z

tests/ui/map_unwrap_or_fixable.rs

Ookay, looking at it again, that's not all that's in this file. So +1 from me.

Centri3 · 2026-01-11T01:38:33Z

cc @samueltardieu, this look good to you? You previously requested changes.

Addressed

samueltardieu · 2026-01-11T08:42:52Z

cc @samueltardieu, this look good to you? You previously requested changes.

Yes, my points have been addressed. I'll merge since you +1'ed.

rustbot assigned blyxyas Sep 19, 2025

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Sep 19, 2025

profetia force-pushed the issue15714 branch from d89877a to b08fb14 Compare September 19, 2025 16:16

profetia force-pushed the issue15714 branch from b08fb14 to 6375703 Compare September 23, 2025 16:05

This comment has been minimized.

Sign in to view

profetia force-pushed the issue15714 branch from 0b7431e to 48b8dd9 Compare September 24, 2025 01:26

samueltardieu requested changes Sep 24, 2025

View reviewed changes

clippy_lints/src/methods/map_unwrap_or.rs Outdated Show resolved Hide resolved

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

profetia force-pushed the issue15714 branch from 48b8dd9 to 761ecf0 Compare September 29, 2025 18:19

profetia requested a review from samueltardieu September 29, 2025 18:21

ada4a reviewed Oct 1, 2025

View reviewed changes

clippy_lints/src/methods/map_unwrap_or.rs Outdated Show resolved Hide resolved

clippy_lints/src/methods/map_unwrap_or.rs Outdated Show resolved Hide resolved

profetia force-pushed the issue15714 branch from 761ecf0 to 1e16651 Compare October 1, 2025 15:19

This comment has been minimized.

Sign in to view

profetia force-pushed the issue15714 branch from 18f686a to f94aeaf Compare October 14, 2025 00:02

This comment has been minimized.

Sign in to view

profetia force-pushed the issue15714 branch from f94aeaf to 185ac09 Compare October 14, 2025 00:14

rustbot assigned samueltardieu and unassigned blyxyas Oct 21, 2025

samueltardieu requested changes Oct 23, 2025

View reviewed changes

rustbot 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 Oct 23, 2025

profetia force-pushed the issue15714 branch 2 times, most recently from 0a176fc to 081f15f Compare October 23, 2025 21:32

rustbot assigned Jarcho and unassigned dswij Nov 25, 2025

rustbot assigned Centri3 and unassigned Jarcho Dec 7, 2025

Centri3 reviewed Dec 17, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

profetia force-pushed the issue15714 branch from dcace10 to 102e2e7 Compare December 18, 2025 18:08

This comment has been minimized.

Sign in to view

profetia added 2 commits December 18, 2025 18:14

fix: map_unwrap_or fail to cover Result::unwrap_or

f2283e2

fix: map_unwrap_or FN on references

9565b28

profetia force-pushed the issue15714 branch from 102e2e7 to a0f09e9 Compare December 18, 2025 18:17

profetia requested a review from Centri3 December 18, 2025 18:24

profetia mentioned this pull request Dec 18, 2025

map_unwrap_or should suggest is_ok_and(<f>) on a result #16258

Closed

Centri3 reviewed Jan 7, 2026

View reviewed changes

fix: map_unwrap_or suggests wrongly for empty slice

0db25db

profetia force-pushed the issue15714 branch from a0f09e9 to 0db25db Compare January 7, 2026 22:06

Centri3 approved these changes Jan 11, 2026

View reviewed changes

tests/ui/map_unwrap_or_fixable.rs

Copy link

Member

Centri3 Jan 11, 2026

Choose a reason for hiding this comment

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

Ookay, looking at it again, that's not all that's in this file. So +1 from me.

samueltardieu added this pull request to the merge queue Jan 11, 2026

Merged via the queue into rust-lang:master with commit 669fbcb Jan 11, 2026
11 checks passed

rustbot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jan 11, 2026

		&& let Node::Pat(pat) = self.cx.tcx.hir_node(local_id)
		&& let PatKind::Binding(_, local_id, ..) = pat.kind

	// If we haven't found a reference yet, check if this references
	// Check if this references

Fix map_unwrap_or fail to cover Result::unwrap_or #15718

Fix map_unwrap_or fail to cover Result::unwrap_or #15718

Conversation

profetia commented Sep 19, 2025 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 19, 2025

Uh oh!

github-actions bot commented Sep 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samueltardieu commented Sep 19, 2025

Uh oh!

profetia commented Sep 19, 2025

Uh oh!

This comment has been minimized.

profetia commented Sep 23, 2025

Uh oh!

profetia commented Sep 24, 2025

Uh oh!

samueltardieu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

profetia commented Sep 29, 2025

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

profetia commented Oct 21, 2025

Uh oh!

samueltardieu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

profetia commented Nov 25, 2025

Uh oh!

profetia commented Dec 7, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Dec 18, 2025

Uh oh!

profetia commented Dec 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Fix `map_unwrap_or` fail to cover `Result::unwrap_or` #15718

Fix `map_unwrap_or` fail to cover `Result::unwrap_or` #15718

profetia commented Sep 19, 2025 •

edited by rustbot

Loading

github-actions bot commented Sep 19, 2025 •

edited

Loading

samueltardieu left a comment •

edited by rustbot

Loading

samueltardieu left a comment •

edited by rustbot

Loading