Skip to content

Commit 5214689

Browse files
alex-semenyukalex-semenyuk
committed
Fix question_mark suggesting when type is behind Deref include parentheses
1 parent e0e2a93 commit 5214689

File tree

4 files changed

+58
-31
lines changed

4 files changed

+58
-31
lines changed

‎clippy_lints/src/question_mark.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ use clippy_config::types::MatchLintBehaviour;
55
use clippy_utils::diagnostics::span_lint_and_sugg;
66
use clippy_utils::msrvs::{self, Msrv};
77
use clippy_utils::source::snippet_with_applicability;
8+
use clippy_utils::sugg::Sugg;
89
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
910
use clippy_utils::{
1011
eq_expr_value, higher, is_else_clause, is_in_const_context, is_lint_allowed, is_path_lang_item, is_res_lang_ctor,
@@ -144,7 +145,7 @@ fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
144145
&& !span_contains_comment(cx.tcx.sess.source_map(), els.span)
145146
{
146147
let mut applicability = Applicability::MaybeIncorrect;
147-
let init_expr_str = snippet_with_applicability(cx, init_expr.span, "..", &mut applicability);
148+
let init_expr_str = Sugg::hir_with_applicability(cx, init_expr, "..", &mut applicability).maybe_paren();
148149
// Take care when binding is `ref`
149150
let sugg = if let PatKind::Binding(
150151
BindingMode(ByRef::Yes(ref_mutability), binding_mutability),

‎tests/ui/question_mark.fixed

+8
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#![allow(dead_code)]
44
#![allow(clippy::unnecessary_wraps)]
55

6+
use parking_lot::Mutex;
7+
68
fn some_func(a: Option<u32>) -> Option<u32> {
79
a?;
810

@@ -430,3 +432,9 @@ fn msrv_1_13(arg: Option<i32>) -> Option<i32> {
430432
println!("{}", val);
431433
Some(val)
432434
}
435+
436+
fn issue_14615(a: Mutex<Option<u32>>) -> Option<String> {
437+
let a = (*a.lock())?;
438+
//~^^^ question_mark
439+
Some(format!("{a}"))
440+
}

‎tests/ui/question_mark.rs

+10
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
#![allow(dead_code)]
44
#![allow(clippy::unnecessary_wraps)]
55

6+
use parking_lot::Mutex;
7+
68
fn some_func(a: Option<u32>) -> Option<u32> {
79
if a.is_none() {
810
//~^ question_mark
@@ -524,3 +526,11 @@ fn msrv_1_13(arg: Option<i32>) -> Option<i32> {
524526
println!("{}", val);
525527
Some(val)
526528
}
529+
530+
fn issue_14615(a: Mutex<Option<u32>>) -> Option<String> {
531+
let Some(a) = *a.lock() else {
532+
return None;
533+
};
534+
//~^^^ question_mark
535+
Some(format!("{a}"))
536+
}

‎tests/ui/question_mark.stderr

+38-30
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
error: this block may be rewritten with the `?` operator
2-
--> tests/ui/question_mark.rs:7:5
2+
--> tests/ui/question_mark.rs:9:5
33
|
44
LL | / if a.is_none() {
55
LL | |
@@ -11,7 +11,7 @@ LL | | }
1111
= help: to override `-D warnings` add `#[allow(clippy::question_mark)]`
1212

1313
error: this block may be rewritten with the `?` operator
14-
--> tests/ui/question_mark.rs:53:9
14+
--> tests/ui/question_mark.rs:55:9
1515
|
1616
LL | / if (self.opt).is_none() {
1717
LL | |
@@ -20,7 +20,7 @@ LL | | }
2020
| |_________^ help: replace it with: `(self.opt)?;`
2121

2222
error: this block may be rewritten with the `?` operator
23-
--> tests/ui/question_mark.rs:58:9
23+
--> tests/ui/question_mark.rs:60:9
2424
|
2525
LL | / if self.opt.is_none() {
2626
LL | |
@@ -29,7 +29,7 @@ LL | | }
2929
| |_________^ help: replace it with: `self.opt?;`
3030

3131
error: this block may be rewritten with the `?` operator
32-
--> tests/ui/question_mark.rs:63:17
32+
--> tests/ui/question_mark.rs:65:17
3333
|
3434
LL | let _ = if self.opt.is_none() {
3535
| _________________^
@@ -41,7 +41,7 @@ LL | | };
4141
| |_________^ help: replace it with: `Some(self.opt?)`
4242

4343
error: this block may be rewritten with the `?` operator
44-
--> tests/ui/question_mark.rs:70:17
44+
--> tests/ui/question_mark.rs:72:17
4545
|
4646
LL | let _ = if let Some(x) = self.opt {
4747
| _________________^
@@ -53,7 +53,7 @@ LL | | };
5353
| |_________^ help: replace it with: `self.opt?`
5454

5555
error: this block may be rewritten with the `?` operator
56-
--> tests/ui/question_mark.rs:88:9
56+
--> tests/ui/question_mark.rs:90:9
5757
|
5858
LL | / if self.opt.is_none() {
5959
LL | |
@@ -62,7 +62,7 @@ LL | | }
6262
| |_________^ help: replace it with: `self.opt.as_ref()?;`
6363

6464
error: this block may be rewritten with the `?` operator
65-
--> tests/ui/question_mark.rs:97:9
65+
--> tests/ui/question_mark.rs:99:9
6666
|
6767
LL | / if self.opt.is_none() {
6868
LL | |
@@ -71,7 +71,7 @@ LL | | }
7171
| |_________^ help: replace it with: `self.opt.as_ref()?;`
7272

7373
error: this block may be rewritten with the `?` operator
74-
--> tests/ui/question_mark.rs:106:9
74+
--> tests/ui/question_mark.rs:108:9
7575
|
7676
LL | / if self.opt.is_none() {
7777
LL | |
@@ -80,7 +80,7 @@ LL | | }
8080
| |_________^ help: replace it with: `self.opt.as_ref()?;`
8181

8282
error: this block may be rewritten with the `?` operator
83-
--> tests/ui/question_mark.rs:114:26
83+
--> tests/ui/question_mark.rs:116:26
8484
|
8585
LL | let v: &Vec<_> = if let Some(ref v) = self.opt {
8686
| __________________________^
@@ -92,7 +92,7 @@ LL | | };
9292
| |_________^ help: replace it with: `self.opt.as_ref()?`
9393

9494
error: this block may be rewritten with the `?` operator
95-
--> tests/ui/question_mark.rs:125:17
95+
--> tests/ui/question_mark.rs:127:17
9696
|
9797
LL | let v = if let Some(v) = self.opt {
9898
| _________________^
@@ -104,7 +104,7 @@ LL | | };
104104
| |_________^ help: replace it with: `self.opt?`
105105

106106
error: this block may be rewritten with the `?` operator
107-
--> tests/ui/question_mark.rs:147:5
107+
--> tests/ui/question_mark.rs:149:5
108108
|
109109
LL | / if f().is_none() {
110110
LL | |
@@ -113,7 +113,7 @@ LL | | }
113113
| |_____^ help: replace it with: `f()?;`
114114

115115
error: this `match` expression can be replaced with `?`
116-
--> tests/ui/question_mark.rs:152:16
116+
--> tests/ui/question_mark.rs:154:16
117117
|
118118
LL | let _val = match f() {
119119
| ________________^
@@ -124,7 +124,7 @@ LL | | };
124124
| |_____^ help: try instead: `f()?`
125125

126126
error: this `match` expression can be replaced with `?`
127-
--> tests/ui/question_mark.rs:163:5
127+
--> tests/ui/question_mark.rs:165:5
128128
|
129129
LL | / match f() {
130130
LL | |
@@ -134,7 +134,7 @@ LL | | };
134134
| |_____^ help: try instead: `f()?`
135135

136136
error: this `match` expression can be replaced with `?`
137-
--> tests/ui/question_mark.rs:169:5
137+
--> tests/ui/question_mark.rs:171:5
138138
|
139139
LL | / match opt_none!() {
140140
LL | |
@@ -144,13 +144,13 @@ LL | | };
144144
| |_____^ help: try instead: `opt_none!()?`
145145

146146
error: this block may be rewritten with the `?` operator
147-
--> tests/ui/question_mark.rs:196:13
147+
--> tests/ui/question_mark.rs:198:13
148148
|
149149
LL | let _ = if let Ok(x) = x { x } else { return x };
150150
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace it with: `x?`
151151

152152
error: this block may be rewritten with the `?` operator
153-
--> tests/ui/question_mark.rs:199:5
153+
--> tests/ui/question_mark.rs:201:5
154154
|
155155
LL | / if x.is_err() {
156156
LL | |
@@ -159,7 +159,7 @@ LL | | }
159159
| |_____^ help: replace it with: `x?;`
160160

161161
error: this `match` expression can be replaced with `?`
162-
--> tests/ui/question_mark.rs:204:16
162+
--> tests/ui/question_mark.rs:206:16
163163
|
164164
LL | let _val = match func_returning_result() {
165165
| ________________^
@@ -170,7 +170,7 @@ LL | | };
170170
| |_____^ help: try instead: `func_returning_result()?`
171171

172172
error: this `match` expression can be replaced with `?`
173-
--> tests/ui/question_mark.rs:210:5
173+
--> tests/ui/question_mark.rs:212:5
174174
|
175175
LL | / match func_returning_result() {
176176
LL | |
@@ -180,7 +180,7 @@ LL | | };
180180
| |_____^ help: try instead: `func_returning_result()?`
181181

182182
error: this block may be rewritten with the `?` operator
183-
--> tests/ui/question_mark.rs:302:5
183+
--> tests/ui/question_mark.rs:304:5
184184
|
185185
LL | / if let Err(err) = func_returning_result() {
186186
LL | |
@@ -189,7 +189,7 @@ LL | | }
189189
| |_____^ help: replace it with: `func_returning_result()?;`
190190

191191
error: this block may be rewritten with the `?` operator
192-
--> tests/ui/question_mark.rs:310:5
192+
--> tests/ui/question_mark.rs:312:5
193193
|
194194
LL | / if let Err(err) = func_returning_result() {
195195
LL | |
@@ -198,7 +198,7 @@ LL | | }
198198
| |_____^ help: replace it with: `func_returning_result()?;`
199199

200200
error: this block may be rewritten with the `?` operator
201-
--> tests/ui/question_mark.rs:388:13
201+
--> tests/ui/question_mark.rs:390:13
202202
|
203203
LL | / if a.is_none() {
204204
LL | |
@@ -208,55 +208,55 @@ LL | | }
208208
| |_____________^ help: replace it with: `a?;`
209209

210210
error: this `let...else` may be rewritten with the `?` operator
211-
--> tests/ui/question_mark.rs:449:5
211+
--> tests/ui/question_mark.rs:451:5
212212
|
213213
LL | / let Some(v) = bar.foo.owned.clone() else {
214214
LL | | return None;
215215
LL | | };
216216
| |______^ help: replace it with: `let v = bar.foo.owned.clone()?;`
217217

218218
error: this `let...else` may be rewritten with the `?` operator
219-
--> tests/ui/question_mark.rs:464:5
219+
--> tests/ui/question_mark.rs:466:5
220220
|
221221
LL | / let Some(ref x) = foo.opt_x else {
222222
LL | | return None;
223223
LL | | };
224224
| |______^ help: replace it with: `let x = foo.opt_x.as_ref()?;`
225225

226226
error: this `let...else` may be rewritten with the `?` operator
227-
--> tests/ui/question_mark.rs:474:5
227+
--> tests/ui/question_mark.rs:476:5
228228
|
229229
LL | / let Some(ref mut x) = foo.opt_x else {
230230
LL | | return None;
231231
LL | | };
232232
| |______^ help: replace it with: `let x = foo.opt_x.as_mut()?;`
233233

234234
error: this `let...else` may be rewritten with the `?` operator
235-
--> tests/ui/question_mark.rs:485:5
235+
--> tests/ui/question_mark.rs:487:5
236236
|
237237
LL | / let Some(ref x @ ref y) = foo.opt_x else {
238238
LL | | return None;
239239
LL | | };
240240
| |______^ help: replace it with: `let x @ y = foo.opt_x.as_ref()?;`
241241

242242
error: this `let...else` may be rewritten with the `?` operator
243-
--> tests/ui/question_mark.rs:489:5
243+
--> tests/ui/question_mark.rs:491:5
244244
|
245245
LL | / let Some(ref x @ WrapperStructWithString(_)) = bar else {
246246
LL | | return None;
247247
LL | | };
248248
| |______^ help: replace it with: `let x @ &WrapperStructWithString(_) = bar.as_ref()?;`
249249

250250
error: this `let...else` may be rewritten with the `?` operator
251-
--> tests/ui/question_mark.rs:493:5
251+
--> tests/ui/question_mark.rs:495:5
252252
|
253253
LL | / let Some(ref mut x @ WrapperStructWithString(_)) = bar else {
254254
LL | | return None;
255255
LL | | };
256256
| |______^ help: replace it with: `let x @ &mut WrapperStructWithString(_) = bar.as_mut()?;`
257257

258258
error: this block may be rewritten with the `?` operator
259-
--> tests/ui/question_mark.rs:515:5
259+
--> tests/ui/question_mark.rs:517:5
260260
|
261261
LL | / if arg.is_none() {
262262
LL | |
@@ -265,7 +265,7 @@ LL | | }
265265
| |_____^ help: replace it with: `arg?;`
266266

267267
error: this `match` expression can be replaced with `?`
268-
--> tests/ui/question_mark.rs:519:15
268+
--> tests/ui/question_mark.rs:521:15
269269
|
270270
LL | let val = match arg {
271271
| _______________^
@@ -275,5 +275,13 @@ LL | | None => return None,
275275
LL | | };
276276
| |_____^ help: try instead: `arg?`
277277

278-
error: aborting due to 29 previous errors
278+
error: this `let...else` may be rewritten with the `?` operator
279+
--> tests/ui/question_mark.rs:531:5
280+
|
281+
LL | / let Some(a) = *a.lock() else {
282+
LL | | return None;
283+
LL | | };
284+
| |______^ help: replace it with: `let a = (*a.lock())?;`
285+
286+
error: aborting due to 30 previous errors
279287

0 commit comments

Comments
 (0)