[unnecessary_unwrap]: lint on .as_ref().unwrap()
#11387
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
r? @blyxyas (rustbot has picked a reviewer for you, use r? to override) |
rustbot
added
the
S-waiting-on-review
Wouldn't that also be... an unnecessary unwrap? |
|
Oh true 😄 that example might've been confusing... I suppose if option.is_some() {
option = calculate_option(); // some kind of expression that mutates option, we don't really know what this does
option.unwrap();
}would be more realistic. Suggesting to change that to an if let wouldnt preserve semantics since that mutation may change it to |
blyxyas
reviewed
Aug 25, 2023
blyxyas
approved these changes
Aug 28, 2023
|
@bors r+ |
bors
added a commit
that referenced
this pull request
Aug 28, 2023
[`unnecessary_unwrap`]: lint on `.as_ref().unwrap()` Closes #11371 This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option: ```rs if option.is_some() { option = None; option.unwrap(); // don't lint here } ``` ... which means that even if we taught this lint to recognize `.as_mut()`, it would *still* not lint because that would count as a mutation. So we need to allow `.as_mut()` calls but reject other kinds of mutations. Unfortunately it doesn't look like this is possible with `is_potentially_mutated` (seeing what kind of mutation happened). This replaces it with a custom little visitor that does basically what it did before, but also allows `.as_mut()`. changelog: [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()`
|
💔 Test failed - checks-action_test |
|
Looks like a spurious error? |
|
Looks like one :3 |
|
💡 This pull request was already approved, no need to approve it again.
|
|
☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test |
linyihai
pushed a commit
to linyihai/rust-clippy
that referenced
this pull request
Oct 23, 2023
Auto merge of rust-lang#11387 - y21:issue11371, r=blyxyas Created-by: chenshuo 00511327 Author-id: 1557 MR-id: 575518 Commit-by: chenshuo c00511327;bors;mojave2;Samuel Moelius;y21;Alex Macleod;Meng Huang;Philipp Krones;xFrednet;Caio;lengyijun;Red Rapious;Igor Aleksanov;Esteban Küber;Guillaume Gomez;Ralf Jung;Ethan Brierley;blyxyas;Matthias Krüger;J-ZhengLi;Nilstrieb;Deadbeef;unvalley;Samuel Tardieu;Oli Scherer;Max Niederman;Catherine Flores;Timo;Vadim Petrochenkov;León Orell Valerian Liehr;Manish Goregaokar;ouz-a;Morten Lohne;Your Name;yukang;Urgau;Samuel "Sam" Tardieu;Maybe Waffle;Jason Newcomb;Trevor Gross;Catherine;Michael Goulet;David Wood;Milo Moisson;Centri3;Thibaut Vandervelden;lcnr;Sylvestre Ledru;Mahdi Dibaiee;syvb;Mingwei Samuel;Panagiotis Foliadis;Eric Huss;Boxy;est31;darklyspaced;许杰友 Jieyou Xu (Joe);Renato Lochetti;Eric Mark Martin;hehaoqian;dswij;Martin Fischer;Kisaragi;Kisaragi Marine;Pavan Kumar Sunkara;rsdy;avborhanian;Andrew Xie;asquared31415;MarcusGrass;Alessio Cosenza;Brian Hetro;Raghul Nanth A;ihciah;beetrees;disco07;Charalampos Mitrodimas;Fridtjof Stoldt;Nicholas Nethercote;Kyle Matsuda;klensy;Takayuki Nakata;Yuri Astrakhan;adrianEffe Merged-by: lizheng 30020856 E2E-issues: issue16 Description: Auto merge of rust-lang#11387 - y21:issue11371, r=blyxyas [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()` Closes rust-lang#11371 This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option: ```rs if option.is_some() { option = None; option.unwrap(); // don't lint here } ``` ... which means that even if we taught this lint to recognize `.as_mut()`, it would *still* not lint because that would count as a mutation. So we need to allow `.as_mut()` calls but reject other kinds of mutations. Unfortunately it doesn't look like this is possible with `is_potentially_mutated` (seeing what kind of mutation happened). This replaces it with a custom little visitor that does basically what it did before, but also allows `.as_mut()`. changelog: [`unnecessary_unwrap`]: lint on `.as_ref().unwrap()` [rust-lang#16 同步社区master最新代码](https://open.codehub.huawei.com/innersource/rust/toolset/rust-clippy/issues/16) See merge request: innersource/rust/toolset/rust-clippy!18
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #11371
This turned out to be a little more code than I originally thought, because the lint also makes sure to not lint if the user tries to mutate the option:
... which means that even if we taught this lint to recognize
.as_mut(), it would still not lint because that would count as a mutation. So we need to allow.as_mut()calls but reject other kinds of mutations.Unfortunately it doesn't look like this is possible with
is_potentially_mutated(seeing what kind of mutation happened).This replaces it with a custom little visitor that does basically what it did before, but also allows
.as_mut().changelog: [
unnecessary_unwrap]: lint on.as_ref().unwrap()