feat(unnecessary_unwrap_unchecked): new lint by ada4a · Pull Request #16252 · rust-lang/rust-clippy

ada4a · 2025-12-17T21:31:16Z

Closes #11108
Resurrects #11139

changelog: [unnecessary_unwrap_unchecked]: new lint

rustbot · 2025-12-17T21:31:22Z

rustbot has assigned @samueltardieu.
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

samueltardieu

I would restrict this lint to the following cases:

an _unchecked function exists
with the same inputs types (or even the same input names if they are available, I haven't checked if they are for non-local functions), and an unwrapped output type (unwrapping can be from Option or Result)
which is unsafe

And in this case, I would even suggest a replacement with MaybeIncorrect (since we can't be certain that the semantics will match), except maybe when it comes from the standard library from which we can expect a safe _unchecked meaning.

That should limit false positives a lot.

We could even later add a configuration option to check for .unwrap(), but that would probably be best to discuss this separately.

What do you think?

View changes since this review

ada4a · 2026-01-19T15:31:47Z

All very sensible suggestions.

One question about this part:

except maybe when [the function] comes from the standard library

How can I find that out? I see that DefId contains CrateNum, but I'm not sure where to go from there...

samueltardieu · 2026-01-19T16:30:34Z

except maybe when [the function] comes from the standard library

How can I find that out? I see that DefId contains CrateNum, but I'm not sure where to go from there...

I'm not sure this can be done easily, as the user can even come with their own version of core/alloc/std, or run in #![no_core] and reimport extern crate core as foobar. Hence the "maybe" here, meaning "if it can be done".

ada4a · 2026-01-20T23:20:42Z

This ended up requiring quite a lot of refactoring. Hopefully the commits are small enough to be reviewable but not too small as to be annoying to deal with. I think it makes sense to squash them after the review (apart from the same_types_modulo_regions fix probably)

ada4a · 2026-01-20T23:22:47Z

+                && let Some((unchecked, unchecked_ident)) =
+                    find_unchecked_sibling_method(cx, checked_def_id, checked_ident)
+                && same_functions_modulo_safety(cx, checked_def_id, unchecked.def_id, expected_ret_ty)
+                && (cx.tcx.visibility(unchecked.def_id)).is_at_least(cx.tcx.visibility(checked_def_id), cx.tcx) =>


This check immediately follows the "same functions" check in all three cases. I thought about moving it into the latter, but wasn't sure how clean that would be. WDYT?

You could import it into same_functions_modulo_safety(), maybe renaming the function, or just adding a doc comment saying that visibility is also checked.

There is little chance that the "unchecked" variant has a greater visibility than the "checked" one, you could just compare the visibility for equality, this would even better fit same_functions_modulo_safety().

ada4a · 2026-01-21T13:57:28Z

Oh, right, I also wanted to add a label pointing to the definition site of the unchecked counterpart – would be nice, no?

I'll mark the PR as draft for now so that I don't forget to address this

samueltardieu · 2026-01-21T14:00:12Z

Oh, right, I also wanted to add a label pointing to the definition site of the unchecked counterpart – would be nice, no?

Not always, as it might give different results depending on whether the rust-src package is installed or not. And in tests it leads to churn when the line of the standard library function changes.

ada4a · 2026-01-21T15:37:21Z

I'm surprised there isn't a Diag helper for emitting such notes, with conditional (on #[cfg(test)]) line info sanitization -- though I guess it wouldn't be able to solve the rust-src part of the problem.

Oh well, let's undraft this then

samueltardieu · 2026-02-21T13:34:40Z

+        // XXX: look into methods from other impls as well?
+        // would need to make sure that the impl generics etc. match


I agree that this is probably not worth it. Don't let it as a XXX, this is not a todo, if someone wants to add this later, they can.

You can maybe add a note in the lint definition saying that the unchecked methods are proposed only if they come from the same impl block.

samueltardieu · 2026-02-21T13:38:08Z

+                && let Some((unchecked, unchecked_ident)) =
+                    find_unchecked_sibling_method(cx, checked_def_id, checked_ident)
+                && same_functions_modulo_safety(cx, checked_def_id, unchecked.def_id, expected_ret_ty)
+                && (cx.tcx.visibility(unchecked.def_id)).is_at_least(cx.tcx.visibility(checked_def_id), cx.tcx) =>


You could import it into same_functions_modulo_safety(), maybe renaming the function, or just adding a doc comment saying that visibility is also checked.

There is little chance that the "unchecked" variant has a greater visibility than the "checked" one, you could just compare the visibility for equality, this would even better fit same_functions_modulo_safety().

samueltardieu · 2026-02-21T13:44:01Z

FCP thread created

ada4a · 2026-02-21T18:40:54Z

I've squashed the existing commits, addressed review comments in a new commit, and did some reordering in yet another commit, to hopefully ease review

EDIT: argh, no, accidentally left the visibility thing in the squashed commit... I don't think I'll bother digging it out now

..and add a bunch more cases

rustbot · 2026-03-06T19:10:00Z

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.

Co-authored-by: Catherine Flores <catherine.3.flores@gmail.com>

rustbot assigned samueltardieu Dec 17, 2025

rustbot added needs-fcp PRs that add, remove, or rename lints and need an FCP S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Dec 17, 2025

samueltardieu reviewed Jan 18, 2026

View reviewed changes

ada4a force-pushed the unnecessary_unwrap_unchecked branch from 068c26a to c88d136 Compare January 20, 2026 23:18

This comment has been minimized.

Sign in to view

ada4a commented Jan 20, 2026

View reviewed changes

ada4a force-pushed the unnecessary_unwrap_unchecked branch 3 times, most recently from 1155425 to 45c37c3 Compare January 21, 2026 09:34

ada4a marked this pull request as draft January 21, 2026 13:57

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

ada4a marked this pull request as ready for review January 21, 2026 15:37

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

samueltardieu reviewed Feb 21, 2026

View reviewed changes

ada4a force-pushed the unnecessary_unwrap_unchecked branch from 45c37c3 to c3dec12 Compare February 21, 2026 17:53

This comment has been minimized.

Sign in to view

kpreid suggested changes Feb 21, 2026

View reviewed changes

Comment thread clippy_lints/src/methods/mod.rs Outdated

ada4a force-pushed the unnecessary_unwrap_unchecked branch from c3dec12 to e9f927a Compare February 21, 2026 18:39

ada4a force-pushed the unnecessary_unwrap_unchecked branch from e9f927a to 7527242 Compare February 21, 2026 18:46

This comment has been minimized.

Sign in to view

fix(same_type_modulo_regions): also ignore regions in references

3e51853

..and add a bunch more cases

ada4a force-pushed the unnecessary_unwrap_unchecked branch from 7527242 to 3bdef2b Compare March 6, 2026 19:09

feat(unnecessary_unwrap_unchecked): new lint

c14f21e

Co-authored-by: Catherine Flores <catherine.3.flores@gmail.com>

ada4a force-pushed the unnecessary_unwrap_unchecked branch from 3bdef2b to c14f21e Compare March 7, 2026 09:55

		// XXX: look into methods from other impls as well?
		// would need to make sure that the impl generics etc. match

Conversation

ada4a commented Dec 17, 2025

Uh oh!

rustbot commented Dec 17, 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!

ada4a commented Jan 19, 2026

Uh oh!

samueltardieu commented Jan 19, 2026

Uh oh!

This comment has been minimized.

ada4a commented Jan 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ada4a Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

samueltardieu Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

ada4a commented Jan 21, 2026

Uh oh!

samueltardieu commented Jan 21, 2026

Uh oh!

ada4a commented Jan 21, 2026

Uh oh!

Uh oh!

samueltardieu Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

samueltardieu Feb 21, 2026

Choose a reason for hiding this comment

Uh oh!

samueltardieu commented Feb 21, 2026

Uh oh!

This comment has been minimized.

Uh oh!

ada4a commented Feb 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

samueltardieu left a comment •

edited by rustbot

Loading

ada4a commented Jan 20, 2026 •

edited

Loading

ada4a commented Feb 21, 2026 •

edited

Loading