Skip to content

Commit 26ff4a7

Browse files
committed
Simplify the code by avoiding, instead of correcting, corner cases
1 parent a0927f9 commit 26ff4a7

File tree

3 files changed

+20
-82
lines changed

3 files changed

+20
-82
lines changed

Diff for: clippy_lints/src/question_mark.rs

+4-36
Original file line numberDiff line numberDiff line change
@@ -142,45 +142,13 @@ fn check_let_some_else_return_none(cx: &LateContext<'_>, stmt: &Stmt<'_>) {
142142
&& let Some(ret) = find_let_else_ret_expression(els)
143143
&& let Some(inner_pat) = pat_and_expr_can_be_question_mark(cx, pat, ret)
144144
&& !span_contains_comment(cx.tcx.sess.source_map(), els.span)
145+
// https://github.com/rust-lang/rust-clippy/issues/13417
146+
&& !matches!(inner_pat.kind, PatKind::Binding(BindingMode(ByRef::Yes(_), _mut), _hir_id, _ident, _subpat))
145147
{
146148
let mut applicability = Applicability::MaybeIncorrect;
147149
let init_expr_str = snippet_with_applicability(cx, init_expr.span, "..", &mut applicability);
148-
let sugg = if let PatKind::Binding(
149-
BindingMode(ByRef::Yes(ref_mutability), binding_mutability),
150-
_hir_id,
151-
ident,
152-
subpattern,
153-
) = inner_pat.kind
154-
{
155-
let (method_str, subpattern_unpack_str) = match ref_mutability {
156-
Mutability::Mut => (".as_mut()", "&mut "),
157-
Mutability::Not => (".as_ref()", "&"),
158-
};
159-
let mutability_str = match binding_mutability {
160-
Mutability::Mut => "mut ",
161-
Mutability::Not => "",
162-
};
163-
let subpattern_str = match subpattern {
164-
Some(Pat {
165-
kind: PatKind::Binding(BindingMode(ByRef::Yes(_), _), _, subident, None),
166-
..
167-
}) => {
168-
// avoid `&ref`
169-
// note that, because you can't have aliased, mutable references, we don't have to worry about
170-
// the outer and inner mutability being different
171-
format!(" @ {subident}")
172-
},
173-
Some(subpattern) => {
174-
let substr = snippet_with_applicability(cx, subpattern.span, "..", &mut applicability);
175-
format!(" @ {subpattern_unpack_str}{substr}")
176-
},
177-
None => String::new(),
178-
};
179-
format!("let {mutability_str}{ident}{subpattern_str} = {init_expr_str}{method_str}?;")
180-
} else {
181-
let receiver_str = snippet_with_applicability(cx, inner_pat.span, "..", &mut applicability);
182-
format!("let {receiver_str} = {init_expr_str}?;")
183-
};
150+
let receiver_str = snippet_with_applicability(cx, inner_pat.span, "..", &mut applicability);
151+
let sugg = format!("let {receiver_str} = {init_expr_str}?;",);
184152
span_lint_and_sugg(
185153
cx,
186154
QUESTION_MARK,

Diff for: tests/ui/question_mark.fixed

+15-5
Original file line numberDiff line numberDiff line change
@@ -382,23 +382,33 @@ struct WrapperStructWithString(String);
382382

383383
#[allow(clippy::disallowed_names)]
384384
fn issue_13417(foo: &mut StructWithOptionString) -> Option<String> {
385-
let x = foo.opt_x.as_ref()?;
385+
let Some(ref x) = foo.opt_x else {
386+
return None;
387+
};
386388
let opt_y = Some(x.clone());
387389
std::mem::replace(&mut foo.opt_x, opt_y)
388390
}
389391

390392
#[allow(clippy::disallowed_names)]
391393
fn issue_13417_mut(foo: &mut StructWithOptionString) -> Option<String> {
392-
let x = foo.opt_x.as_mut()?;
394+
let Some(ref mut x) = foo.opt_x else {
395+
return None;
396+
};
393397
let opt_y = Some(x.clone());
394398
std::mem::replace(&mut foo.opt_x, opt_y)
395399
}
396400

397401
#[allow(clippy::disallowed_names)]
398402
#[allow(unused)]
399403
fn issue_13417_weirder(foo: &mut StructWithOptionString, mut bar: Option<WrapperStructWithString>) -> Option<()> {
400-
let x @ y = foo.opt_x.as_ref()?;
401-
let x @ &WrapperStructWithString(_) = bar.as_ref()?;
402-
let x @ &mut WrapperStructWithString(_) = bar.as_mut()?;
404+
let Some(ref x @ ref y) = foo.opt_x else {
405+
return None;
406+
};
407+
let Some(ref x @ WrapperStructWithString(_)) = bar else {
408+
return None;
409+
};
410+
let Some(ref mut x @ WrapperStructWithString(_)) = bar else {
411+
return None;
412+
};
403413
Some(())
404414
}

Diff for: tests/ui/question_mark.stderr

+1-41
Original file line numberDiff line numberDiff line change
@@ -195,45 +195,5 @@ LL | | return None;
195195
LL | | };
196196
| |______^ help: replace it with: `let v = bar.foo.owned.clone()?;`
197197

198-
error: this `let...else` may be rewritten with the `?` operator
199-
--> tests/ui/question_mark.rs:442:5
200-
|
201-
LL | / let Some(ref x) = foo.opt_x else {
202-
LL | | return None;
203-
LL | | };
204-
| |______^ help: replace it with: `let x = foo.opt_x.as_ref()?;`
205-
206-
error: this `let...else` may be rewritten with the `?` operator
207-
--> tests/ui/question_mark.rs:451:5
208-
|
209-
LL | / let Some(ref mut x) = foo.opt_x else {
210-
LL | | return None;
211-
LL | | };
212-
| |______^ help: replace it with: `let x = foo.opt_x.as_mut()?;`
213-
214-
error: this `let...else` may be rewritten with the `?` operator
215-
--> tests/ui/question_mark.rs:461:5
216-
|
217-
LL | / let Some(ref x @ ref y) = foo.opt_x else {
218-
LL | | return None;
219-
LL | | };
220-
| |______^ help: replace it with: `let x @ y = foo.opt_x.as_ref()?;`
221-
222-
error: this `let...else` may be rewritten with the `?` operator
223-
--> tests/ui/question_mark.rs:464:5
224-
|
225-
LL | / let Some(ref x @ WrapperStructWithString(_)) = bar else {
226-
LL | | return None;
227-
LL | | };
228-
| |______^ help: replace it with: `let x @ &WrapperStructWithString(_) = bar.as_ref()?;`
229-
230-
error: this `let...else` may be rewritten with the `?` operator
231-
--> tests/ui/question_mark.rs:467:5
232-
|
233-
LL | / let Some(ref mut x @ WrapperStructWithString(_)) = bar else {
234-
LL | | return None;
235-
LL | | };
236-
| |______^ help: replace it with: `let x @ &mut WrapperStructWithString(_) = bar.as_mut()?;`
237-
238-
error: aborting due to 27 previous errors
198+
error: aborting due to 22 previous errors
239199

0 commit comments

Comments
 (0)