Skip to content

Commit d8726ca

Browse files
fix false negative for unnecessary_unwrap (#14758)
changelog: Fix [`unnecessary_unwrap`] false negative when any assignment occurs in `if` branch (regardless of any variable). Fixes: #14725
2 parents 33519b7 + b73d3b4 commit d8726ca

File tree

3 files changed

+92
-3
lines changed

3 files changed

+92
-3
lines changed

clippy_lints/src/unwrap.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,10 @@ impl<'tcx> Delegate<'tcx> for MutationVisitor<'tcx> {
224224
}
225225
}
226226

227-
fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {
228-
self.is_mutated = true;
227+
fn mutate(&mut self, cat: &PlaceWithHirId<'tcx>, _: HirId) {
228+
if is_potentially_local_place(self.local_id, &cat.place) {
229+
self.is_mutated = true;
230+
}
229231
}
230232

231233
fn consume(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}

tests/ui/checked_unwrap/simple_conditionals.rs

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -188,6 +188,58 @@ fn issue11371() {
188188
}
189189
}
190190

191+
fn gen_option() -> Option<()> {
192+
Some(())
193+
// Or None
194+
}
195+
196+
fn gen_result() -> Result<(), ()> {
197+
Ok(())
198+
// Or Err(())
199+
}
200+
201+
fn issue14725() {
202+
let option = Some(());
203+
204+
if option.is_some() {
205+
let _ = option.as_ref().unwrap();
206+
//~^ unnecessary_unwrap
207+
} else {
208+
let _ = option.as_ref().unwrap();
209+
//~^ panicking_unwrap
210+
}
211+
212+
let result = Ok::<(), ()>(());
213+
214+
if result.is_ok() {
215+
let _y = 1;
216+
result.as_ref().unwrap();
217+
//~^ unnecessary_unwrap
218+
} else {
219+
let _y = 1;
220+
result.as_ref().unwrap();
221+
//~^ panicking_unwrap
222+
}
223+
224+
let mut option = Some(());
225+
if option.is_some() {
226+
option = gen_option();
227+
option.as_mut().unwrap();
228+
} else {
229+
option = gen_option();
230+
option.as_mut().unwrap();
231+
}
232+
233+
let mut result = Ok::<(), ()>(());
234+
if result.is_ok() {
235+
result = gen_result();
236+
result.as_mut().unwrap();
237+
} else {
238+
result = gen_result();
239+
result.as_mut().unwrap();
240+
}
241+
}
242+
191243
fn check_expect() {
192244
let x = Some(());
193245
if x.is_some() {

tests/ui/checked_unwrap/simple_conditionals.stderr

Lines changed: 36 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,41 @@ LL | if result.is_ok() {
236236
LL | result.as_mut().unwrap();
237237
| ^^^^^^^^^^^^^^^^^^^^^^^^
238238

239+
error: called `unwrap` on `option` after checking its variant with `is_some`
240+
--> tests/ui/checked_unwrap/simple_conditionals.rs:205:17
241+
|
242+
LL | if option.is_some() {
243+
| ------------------- help: try: `if let Some(<item>) = &option`
244+
LL | let _ = option.as_ref().unwrap();
245+
| ^^^^^^^^^^^^^^^^^^^^^^^^
246+
247+
error: this call to `unwrap()` will always panic
248+
--> tests/ui/checked_unwrap/simple_conditionals.rs:208:17
249+
|
250+
LL | if option.is_some() {
251+
| ---------------- because of this check
252+
...
253+
LL | let _ = option.as_ref().unwrap();
254+
| ^^^^^^^^^^^^^^^^^^^^^^^^
255+
256+
error: called `unwrap` on `result` after checking its variant with `is_ok`
257+
--> tests/ui/checked_unwrap/simple_conditionals.rs:216:9
258+
|
259+
LL | if result.is_ok() {
260+
| ----------------- help: try: `if let Ok(<item>) = &result`
261+
LL | let _y = 1;
262+
LL | result.as_ref().unwrap();
263+
| ^^^^^^^^^^^^^^^^^^^^^^^^
264+
265+
error: this call to `unwrap()` will always panic
266+
--> tests/ui/checked_unwrap/simple_conditionals.rs:220:9
267+
|
268+
LL | if result.is_ok() {
269+
| -------------- because of this check
270+
...
271+
LL | result.as_ref().unwrap();
272+
| ^^^^^^^^^^^^^^^^^^^^^^^^
273+
239274
error: creating a shared reference to mutable static
240275
--> tests/ui/checked_unwrap/simple_conditionals.rs:183:12
241276
|
@@ -246,5 +281,5 @@ LL | if X.is_some() {
246281
= note: shared references to mutable statics are dangerous; it's undefined behavior if the static is mutated or if a mutable reference is created for it while the shared reference lives
247282
= note: `#[deny(static_mut_refs)]` on by default
248283

249-
error: aborting due to 26 previous errors
284+
error: aborting due to 30 previous errors
250285

0 commit comments

Comments
 (0)