From a239c8b52be0c1eed1a07be6bef558d3e62f9e44 Mon Sep 17 00:00:00 2001
From: y21 <30553356+y21@users.noreply.github.com>
Date: Fri, 2 Jun 2023 00:06:34 +0200
Subject: [PATCH] [`unnecessary_lazy_eval`]: don't lint on types with deref
impl
---
clippy_utils/src/eager_or_lazy.rs | 8 ++-
tests/ui/unnecessary_lazy_eval.fixed | 20 +++++++
tests/ui/unnecessary_lazy_eval.rs | 20 +++++++
tests/ui/unnecessary_lazy_eval.stderr | 86 ++++++++++++++++-----------
4 files changed, 97 insertions(+), 37 deletions(-)
diff --git a/clippy_utils/src/eager_or_lazy.rs b/clippy_utils/src/eager_or_lazy.rs
index 3df40942e7b..e05b88e4cd0 100644
--- a/clippy_utils/src/eager_or_lazy.rs
+++ b/clippy_utils/src/eager_or_lazy.rs
@@ -173,11 +173,15 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
self.eagerness = Lazy;
}
},
-
+ // Custom `Deref` impl might have side effects
+ ExprKind::Unary(UnOp::Deref, e)
+ if self.cx.typeck_results().expr_ty(e).builtin_deref(true).is_none() =>
+ {
+ self.eagerness |= NoChange;
+ },
// Dereferences should be cheap, but dereferencing a raw pointer earlier may not be safe.
ExprKind::Unary(UnOp::Deref, e) if !self.cx.typeck_results().expr_ty(e).is_unsafe_ptr() => (),
ExprKind::Unary(UnOp::Deref, _) => self.eagerness |= NoChange,
-
ExprKind::Unary(_, e)
if matches!(
self.cx.typeck_results().expr_ty(e).kind(),
diff --git a/tests/ui/unnecessary_lazy_eval.fixed b/tests/ui/unnecessary_lazy_eval.fixed
index c3728886ec9..f8d4c0bd912 100644
--- a/tests/ui/unnecessary_lazy_eval.fixed
+++ b/tests/ui/unnecessary_lazy_eval.fixed
@@ -5,6 +5,8 @@
#![allow(clippy::bind_instead_of_map)]
#![allow(clippy::map_identity)]
+use std::ops::Deref;
+
extern crate proc_macros;
use proc_macros::with_span;
@@ -41,6 +43,15 @@ impl Drop for Issue9427FollowUp {
}
}
+struct Issue10437;
+impl Deref for Issue10437 {
+ type Target = u32;
+ fn deref(&self) -> &Self::Target {
+ println!("side effect deref");
+ &0
+ }
+}
+
fn main() {
let astronomers_pi = 10;
let ext_arr: [usize; 1] = [2];
@@ -65,6 +76,12 @@ fn main() {
let _ = nested_tuple_opt.unwrap_or(Some((1, 2)));
let _ = cond.then_some(astronomers_pi);
+ // Should lint - Builtin deref
+ let r = &1;
+ let _ = Some(1).unwrap_or(*r);
+ let b = Box::new(1);
+ let _ = Some(1).unwrap_or(*b);
+
// Cases when unwrap is not called on a simple variable
let _ = Some(10).unwrap_or(2);
let _ = Some(10).and(ext_opt);
@@ -93,6 +110,9 @@ fn main() {
let _ = deep.0.or_else(|| some_call());
let _ = opt.ok_or_else(|| ext_arr[0]);
+ let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
+ let _ = Some(1).unwrap_or(*Issue10437);
+
// Should not lint - bool
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
diff --git a/tests/ui/unnecessary_lazy_eval.rs b/tests/ui/unnecessary_lazy_eval.rs
index 76e50fa5b03..b968894f870 100644
--- a/tests/ui/unnecessary_lazy_eval.rs
+++ b/tests/ui/unnecessary_lazy_eval.rs
@@ -5,6 +5,8 @@
#![allow(clippy::bind_instead_of_map)]
#![allow(clippy::map_identity)]
+use std::ops::Deref;
+
extern crate proc_macros;
use proc_macros::with_span;
@@ -41,6 +43,15 @@ impl Drop for Issue9427FollowUp {
}
}
+struct Issue10437;
+impl Deref for Issue10437 {
+ type Target = u32;
+ fn deref(&self) -> &Self::Target {
+ println!("side effect deref");
+ &0
+ }
+}
+
fn main() {
let astronomers_pi = 10;
let ext_arr: [usize; 1] = [2];
@@ -65,6 +76,12 @@ fn main() {
let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
let _ = cond.then(|| astronomers_pi);
+ // Should lint - Builtin deref
+ let r = &1;
+ let _ = Some(1).unwrap_or_else(|| *r);
+ let b = Box::new(1);
+ let _ = Some(1).unwrap_or_else(|| *b);
+
// Cases when unwrap is not called on a simple variable
let _ = Some(10).unwrap_or_else(|| 2);
let _ = Some(10).and_then(|_| ext_opt);
@@ -93,6 +110,9 @@ fn main() {
let _ = deep.0.or_else(|| some_call());
let _ = opt.ok_or_else(|| ext_arr[0]);
+ let _ = Some(1).unwrap_or_else(|| *Issue10437); // Issue10437 has a deref impl
+ let _ = Some(1).unwrap_or(*Issue10437);
+
// Should not lint - bool
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
diff --git a/tests/ui/unnecessary_lazy_eval.stderr b/tests/ui/unnecessary_lazy_eval.stderr
index 0339755442c..c3d106cd109 100644
--- a/tests/ui/unnecessary_lazy_eval.stderr
+++ b/tests/ui/unnecessary_lazy_eval.stderr
@@ -1,5 +1,5 @@
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:56:13
+ --> $DIR/unnecessary_lazy_eval.rs:67:13
|
LL | let _ = opt.unwrap_or_else(|| 2);
| ^^^^--------------------
@@ -9,7 +9,7 @@ LL | let _ = opt.unwrap_or_else(|| 2);
= note: `-D clippy::unnecessary-lazy-evaluations` implied by `-D warnings`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:57:13
+ --> $DIR/unnecessary_lazy_eval.rs:68:13
|
LL | let _ = opt.unwrap_or_else(|| astronomers_pi);
| ^^^^---------------------------------
@@ -17,7 +17,7 @@ LL | let _ = opt.unwrap_or_else(|| astronomers_pi);
| help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:58:13
+ --> $DIR/unnecessary_lazy_eval.rs:69:13
|
LL | let _ = opt.unwrap_or_else(|| ext_str.some_field);
| ^^^^-------------------------------------
@@ -25,7 +25,7 @@ LL | let _ = opt.unwrap_or_else(|| ext_str.some_field);
| help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:60:13
+ --> $DIR/unnecessary_lazy_eval.rs:71:13
|
LL | let _ = opt.and_then(|_| ext_opt);
| ^^^^---------------------
@@ -33,7 +33,7 @@ LL | let _ = opt.and_then(|_| ext_opt);
| help: use `and(..)` instead: `and(ext_opt)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:61:13
+ --> $DIR/unnecessary_lazy_eval.rs:72:13
|
LL | let _ = opt.or_else(|| ext_opt);
| ^^^^-------------------
@@ -41,7 +41,7 @@ LL | let _ = opt.or_else(|| ext_opt);
| help: use `or(..)` instead: `or(ext_opt)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:62:13
+ --> $DIR/unnecessary_lazy_eval.rs:73:13
|
LL | let _ = opt.or_else(|| None);
| ^^^^----------------
@@ -49,7 +49,7 @@ LL | let _ = opt.or_else(|| None);
| help: use `or(..)` instead: `or(None)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:63:13
+ --> $DIR/unnecessary_lazy_eval.rs:74:13
|
LL | let _ = opt.get_or_insert_with(|| 2);
| ^^^^------------------------
@@ -57,7 +57,7 @@ LL | let _ = opt.get_or_insert_with(|| 2);
| help: use `get_or_insert(..)` instead: `get_or_insert(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:64:13
+ --> $DIR/unnecessary_lazy_eval.rs:75:13
|
LL | let _ = opt.ok_or_else(|| 2);
| ^^^^----------------
@@ -65,7 +65,7 @@ LL | let _ = opt.ok_or_else(|| 2);
| help: use `ok_or(..)` instead: `ok_or(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:65:13
+ --> $DIR/unnecessary_lazy_eval.rs:76:13
|
LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
| ^^^^^^^^^^^^^^^^^-------------------------------
@@ -73,7 +73,7 @@ LL | let _ = nested_tuple_opt.unwrap_or_else(|| Some((1, 2)));
| help: use `unwrap_or(..)` instead: `unwrap_or(Some((1, 2)))`
error: unnecessary closure used with `bool::then`
- --> $DIR/unnecessary_lazy_eval.rs:66:13
+ --> $DIR/unnecessary_lazy_eval.rs:77:13
|
LL | let _ = cond.then(|| astronomers_pi);
| ^^^^^-----------------------
@@ -81,7 +81,23 @@ LL | let _ = cond.then(|| astronomers_pi);
| help: use `then_some(..)` instead: `then_some(astronomers_pi)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:69:13
+ --> $DIR/unnecessary_lazy_eval.rs:81:13
+ |
+LL | let _ = Some(1).unwrap_or_else(|| *r);
+ | ^^^^^^^^---------------------
+ | |
+ | help: use `unwrap_or(..)` instead: `unwrap_or(*r)`
+
+error: unnecessary closure used to substitute value for `Option::None`
+ --> $DIR/unnecessary_lazy_eval.rs:83:13
+ |
+LL | let _ = Some(1).unwrap_or_else(|| *b);
+ | ^^^^^^^^---------------------
+ | |
+ | help: use `unwrap_or(..)` instead: `unwrap_or(*b)`
+
+error: unnecessary closure used to substitute value for `Option::None`
+ --> $DIR/unnecessary_lazy_eval.rs:86:13
|
LL | let _ = Some(10).unwrap_or_else(|| 2);
| ^^^^^^^^^--------------------
@@ -89,7 +105,7 @@ LL | let _ = Some(10).unwrap_or_else(|| 2);
| help: use `unwrap_or(..)` instead: `unwrap_or(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:70:13
+ --> $DIR/unnecessary_lazy_eval.rs:87:13
|
LL | let _ = Some(10).and_then(|_| ext_opt);
| ^^^^^^^^^---------------------
@@ -97,7 +113,7 @@ LL | let _ = Some(10).and_then(|_| ext_opt);
| help: use `and(..)` instead: `and(ext_opt)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:71:28
+ --> $DIR/unnecessary_lazy_eval.rs:88:28
|
LL | let _: Option<usize> = None.or_else(|| ext_opt);
| ^^^^^-------------------
@@ -105,7 +121,7 @@ LL | let _: Option<usize> = None.or_else(|| ext_opt);
| help: use `or(..)` instead: `or(ext_opt)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:72:13
+ --> $DIR/unnecessary_lazy_eval.rs:89:13
|
LL | let _ = None.get_or_insert_with(|| 2);
| ^^^^^------------------------
@@ -113,7 +129,7 @@ LL | let _ = None.get_or_insert_with(|| 2);
| help: use `get_or_insert(..)` instead: `get_or_insert(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:73:35
+ --> $DIR/unnecessary_lazy_eval.rs:90:35
|
LL | let _: Result<usize, usize> = None.ok_or_else(|| 2);
| ^^^^^----------------
@@ -121,7 +137,7 @@ LL | let _: Result<usize, usize> = None.ok_or_else(|| 2);
| help: use `ok_or(..)` instead: `ok_or(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:74:28
+ --> $DIR/unnecessary_lazy_eval.rs:91:28
|
LL | let _: Option<usize> = None.or_else(|| None);
| ^^^^^----------------
@@ -129,7 +145,7 @@ LL | let _: Option<usize> = None.or_else(|| None);
| help: use `or(..)` instead: `or(None)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:77:13
+ --> $DIR/unnecessary_lazy_eval.rs:94:13
|
LL | let _ = deep.0.unwrap_or_else(|| 2);
| ^^^^^^^--------------------
@@ -137,7 +153,7 @@ LL | let _ = deep.0.unwrap_or_else(|| 2);
| help: use `unwrap_or(..)` instead: `unwrap_or(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:78:13
+ --> $DIR/unnecessary_lazy_eval.rs:95:13
|
LL | let _ = deep.0.and_then(|_| ext_opt);
| ^^^^^^^---------------------
@@ -145,7 +161,7 @@ LL | let _ = deep.0.and_then(|_| ext_opt);
| help: use `and(..)` instead: `and(ext_opt)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:79:13
+ --> $DIR/unnecessary_lazy_eval.rs:96:13
|
LL | let _ = deep.0.or_else(|| None);
| ^^^^^^^----------------
@@ -153,7 +169,7 @@ LL | let _ = deep.0.or_else(|| None);
| help: use `or(..)` instead: `or(None)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:80:13
+ --> $DIR/unnecessary_lazy_eval.rs:97:13
|
LL | let _ = deep.0.get_or_insert_with(|| 2);
| ^^^^^^^------------------------
@@ -161,7 +177,7 @@ LL | let _ = deep.0.get_or_insert_with(|| 2);
| help: use `get_or_insert(..)` instead: `get_or_insert(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:81:13
+ --> $DIR/unnecessary_lazy_eval.rs:98:13
|
LL | let _ = deep.0.ok_or_else(|| 2);
| ^^^^^^^----------------
@@ -169,7 +185,7 @@ LL | let _ = deep.0.ok_or_else(|| 2);
| help: use `ok_or(..)` instead: `ok_or(2)`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:105:28
+ --> $DIR/unnecessary_lazy_eval.rs:125:28
|
LL | let _: Option<usize> = None.or_else(|| Some(3));
| ^^^^^-------------------
@@ -177,7 +193,7 @@ LL | let _: Option<usize> = None.or_else(|| Some(3));
| help: use `or(..)` instead: `or(Some(3))`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:106:13
+ --> $DIR/unnecessary_lazy_eval.rs:126:13
|
LL | let _ = deep.0.or_else(|| Some(3));
| ^^^^^^^-------------------
@@ -185,7 +201,7 @@ LL | let _ = deep.0.or_else(|| Some(3));
| help: use `or(..)` instead: `or(Some(3))`
error: unnecessary closure used to substitute value for `Option::None`
- --> $DIR/unnecessary_lazy_eval.rs:107:13
+ --> $DIR/unnecessary_lazy_eval.rs:127:13
|
LL | let _ = opt.or_else(|| Some(3));
| ^^^^-------------------
@@ -193,7 +209,7 @@ LL | let _ = opt.or_else(|| Some(3));
| help: use `or(..)` instead: `or(Some(3))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:113:13
+ --> $DIR/unnecessary_lazy_eval.rs:133:13
|
LL | let _ = res2.unwrap_or_else(|_| 2);
| ^^^^^---------------------
@@ -201,7 +217,7 @@ LL | let _ = res2.unwrap_or_else(|_| 2);
| help: use `unwrap_or(..)` instead: `unwrap_or(2)`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:114:13
+ --> $DIR/unnecessary_lazy_eval.rs:134:13
|
LL | let _ = res2.unwrap_or_else(|_| astronomers_pi);
| ^^^^^----------------------------------
@@ -209,7 +225,7 @@ LL | let _ = res2.unwrap_or_else(|_| astronomers_pi);
| help: use `unwrap_or(..)` instead: `unwrap_or(astronomers_pi)`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:115:13
+ --> $DIR/unnecessary_lazy_eval.rs:135:13
|
LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field);
| ^^^^^--------------------------------------
@@ -217,7 +233,7 @@ LL | let _ = res2.unwrap_or_else(|_| ext_str.some_field);
| help: use `unwrap_or(..)` instead: `unwrap_or(ext_str.some_field)`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:137:35
+ --> $DIR/unnecessary_lazy_eval.rs:157:35
|
LL | let _: Result<usize, usize> = res.and_then(|_| Err(2));
| ^^^^--------------------
@@ -225,7 +241,7 @@ LL | let _: Result<usize, usize> = res.and_then(|_| Err(2));
| help: use `and(..)` instead: `and(Err(2))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:138:35
+ --> $DIR/unnecessary_lazy_eval.rs:158:35
|
LL | let _: Result<usize, usize> = res.and_then(|_| Err(astronomers_pi));
| ^^^^---------------------------------
@@ -233,7 +249,7 @@ LL | let _: Result<usize, usize> = res.and_then(|_| Err(astronomers_pi));
| help: use `and(..)` instead: `and(Err(astronomers_pi))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:139:35
+ --> $DIR/unnecessary_lazy_eval.rs:159:35
|
LL | let _: Result<usize, usize> = res.and_then(|_| Err(ext_str.some_field));
| ^^^^-------------------------------------
@@ -241,7 +257,7 @@ LL | let _: Result<usize, usize> = res.and_then(|_| Err(ext_str.some_field))
| help: use `and(..)` instead: `and(Err(ext_str.some_field))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:141:35
+ --> $DIR/unnecessary_lazy_eval.rs:161:35
|
LL | let _: Result<usize, usize> = res.or_else(|_| Ok(2));
| ^^^^------------------
@@ -249,7 +265,7 @@ LL | let _: Result<usize, usize> = res.or_else(|_| Ok(2));
| help: use `or(..)` instead: `or(Ok(2))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:142:35
+ --> $DIR/unnecessary_lazy_eval.rs:162:35
|
LL | let _: Result<usize, usize> = res.or_else(|_| Ok(astronomers_pi));
| ^^^^-------------------------------
@@ -257,7 +273,7 @@ LL | let _: Result<usize, usize> = res.or_else(|_| Ok(astronomers_pi));
| help: use `or(..)` instead: `or(Ok(astronomers_pi))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:143:35
+ --> $DIR/unnecessary_lazy_eval.rs:163:35
|
LL | let _: Result<usize, usize> = res.or_else(|_| Ok(ext_str.some_field));
| ^^^^-----------------------------------
@@ -265,7 +281,7 @@ LL | let _: Result<usize, usize> = res.or_else(|_| Ok(ext_str.some_field));
| help: use `or(..)` instead: `or(Ok(ext_str.some_field))`
error: unnecessary closure used to substitute value for `Result::Err`
- --> $DIR/unnecessary_lazy_eval.rs:144:35
+ --> $DIR/unnecessary_lazy_eval.rs:164:35
|
LL | let _: Result<usize, usize> = res.
| ___________________________________^
@@ -279,5 +295,5 @@ LL | | or_else(|_| Ok(ext_str.some_field));
| |
| help: use `or(..)` instead: `or(Ok(ext_str.some_field))`
-error: aborting due to 34 previous errors
+error: aborting due to 36 previous errors