Skip to content

Commit e6c4a28

Browse files
committed
unified type extraction
1 parent f59e084 commit e6c4a28

File tree

1 file changed

+29
-29
lines changed

1 file changed

+29
-29
lines changed

clippy_lints/src/question_mark.rs

Lines changed: 29 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ use rustc_errors::Applicability;
1010
use rustc_hir::LangItem::{OptionNone, OptionSome, ResultErr, ResultOk};
1111
use rustc_hir::{BindingAnnotation, Expr, ExprKind, PatKind, PathSegment, QPath};
1212
use rustc_lint::{LateContext, LateLintPass};
13+
use rustc_middle::ty::Ty;
1314
use rustc_session::{declare_lint_pass, declare_tool_lint};
1415
use rustc_span::{sym, symbol::Symbol};
1516

@@ -43,13 +44,21 @@ declare_lint_pass!(QuestionMark => [QUESTION_MARK]);
4344
enum IfBlockType<'hir> {
4445
/// An `if x.is_xxx() { a } else { b } ` expression.
4546
///
46-
/// Contains: caller (x), call_sym (is_xxx), if_then (a), if_else (b)
47-
IfIs(&'hir Expr<'hir>, Symbol, &'hir Expr<'hir>, Option<&'hir Expr<'hir>>),
47+
/// Contains: caller (x), caller_type, call_sym (is_xxx), if_then (a), if_else (b)
48+
IfIs(
49+
&'hir Expr<'hir>,
50+
Ty<'hir>,
51+
Symbol,
52+
&'hir Expr<'hir>,
53+
Option<&'hir Expr<'hir>>,
54+
),
4855
/// An `if let Xxx(a) = b { c } else { d }` expression.
4956
///
50-
/// Contains: let_pat_qpath (Xxx), let_pat_sym (a), let_expr (b), if_then (c), if_else (d)
57+
/// Contains: let_pat_qpath (Xxx), let_pat_type, let_pat_sym (a), let_expr (b), if_then (c),
58+
/// if_else (d)
5159
IfLet(
5260
&'hir QPath<'hir>,
61+
Ty<'hir>,
5362
Symbol,
5463
&'hir Expr<'hir>,
5564
&'hir Expr<'hir>,
@@ -77,17 +86,18 @@ fn check_is_none_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>)
7786
if let Some(higher::If { cond, then, r#else }) = higher::If::hir(expr);
7887
if !is_else_clause(cx.tcx, expr);
7988
if let ExprKind::MethodCall(segment, args, _) = &cond.kind;
80-
if let Some(subject) = args.get(0);
81-
let if_block = IfBlockType::IfIs(subject, segment.ident.name, then, r#else);
89+
if let Some(caller) = args.get(0);
90+
let caller_ty = cx.typeck_results().expr_ty(caller);
91+
let if_block = IfBlockType::IfIs(caller, caller_ty, segment.ident.name, then, r#else);
8292
if is_early_return(sym::Option, cx, &if_block) || is_early_return(sym::Result, cx, &if_block);
8393
then {
8494
let mut applicability = Applicability::MachineApplicable;
85-
let receiver_str = snippet_with_applicability(cx, subject.span, "..", &mut applicability);
86-
let by_ref = move_by_default(cx, subject) &&
87-
!matches!(subject.kind, ExprKind::Call(..) | ExprKind::MethodCall(..));
95+
let receiver_str = snippet_with_applicability(cx, caller.span, "..", &mut applicability);
96+
let by_ref = !caller_ty.is_copy_modulo_regions(cx.tcx.at(caller.span), cx.param_env) &&
97+
!matches!(caller.kind, ExprKind::Call(..) | ExprKind::MethodCall(..));
8898
let mut suggestion = String::new();
8999
if let Some(else_inner) = r#else {
90-
if eq_expr_value(cx, subject, peel_blocks(else_inner)) {
100+
if eq_expr_value(cx, caller, peel_blocks(else_inner)) {
91101
suggestion = format!("Some({}?)", receiver_str);
92102
}
93103
} else {
@@ -98,18 +108,14 @@ fn check_is_none_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>)
98108
}
99109
}
100110

101-
fn move_by_default(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
102-
let expr_ty = cx.typeck_results().expr_ty(expr);
103-
!expr_ty.is_copy_modulo_regions(cx.tcx.at(expr.span), cx.param_env)
104-
}
105-
106111
fn check_if_let_some_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'_>) {
107112
if_chain! {
108113
if let Some(higher::IfLet { let_pat, let_expr, if_then, if_else }) = higher::IfLet::hir(cx, expr);
109114
if !is_else_clause(cx.tcx, expr);
110115
if let PatKind::TupleStruct(ref path1, fields, None) = let_pat.kind;
111116
if let PatKind::Binding(annot, bind_id, ident, _) = fields[0].kind;
112-
let if_block = IfBlockType::IfLet(path1, ident.name, let_expr, if_then, if_else);
117+
let caller_ty = cx.typeck_results().expr_ty(let_expr);
118+
let if_block = IfBlockType::IfLet(path1, caller_ty, ident.name, let_expr, if_then, if_else);
113119
if (is_early_return(sym::Option, cx, &if_block) && path_to_local_id(peel_blocks(if_then), bind_id))
114120
|| is_early_return(sym::Result, cx, &if_block);
115121
then {
@@ -129,8 +135,7 @@ fn check_if_let_some_or_err_and_early_return(cx: &LateContext<'_>, expr: &Expr<'
129135

130136
fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_>) -> bool {
131137
match *if_block {
132-
IfBlockType::IfIs(caller, call_sym, if_then, _) => {
133-
let caller_ty = cx.typeck_results().expr_ty(caller);
138+
IfBlockType::IfIs(caller, caller_ty, call_sym, if_then, _) => {
134139
// If the block could be identified as `if x.is_none()/is_err()`,
135140
// we then only need to check the if_then return to see if it is none/err.
136141
is_type_diagnostic_item(cx, caller_ty, smbl)
@@ -141,29 +146,24 @@ fn is_early_return(smbl: Symbol, cx: &LateContext<'_>, if_block: &IfBlockType<'_
141146
_ => false,
142147
}
143148
},
144-
IfBlockType::IfLet(qpath, let_pat_sym, let_expr, if_then, if_else) => {
145-
let let_expr_ty = cx.typeck_results().expr_ty(let_expr);
149+
IfBlockType::IfLet(qpath, let_expr_ty, let_pat_sym, let_expr, if_then, if_else) => {
146150
is_type_diagnostic_item(cx, let_expr_ty, smbl)
147151
&& match smbl {
148152
sym::Option => {
149-
// We only need to id `if let Some(x) = option` not `if let None = option`,
150-
// because the later one will be suggested as `if option.is_none()` thus causing conflict
151-
let else_expr = if let Some(e) = if_else {
152-
e
153-
} else {
153+
// We only need to check `if let Some(x) = option` not `if let None = option`,
154+
// because the later one will be suggested as `if option.is_none()` thus causing conflict.
155+
if if_else.is_none() {
154156
return false;
155157
};
156-
expr_return_none_or_err(smbl, cx, else_expr, let_expr, None)
158+
expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, None)
157159
&& is_lang_ctor(cx, qpath, OptionSome)
158160
},
159161
sym::Result => {
160162
if is_lang_ctor(cx, qpath, ResultOk) {
161-
let else_expr = if let Some(e) = if_else {
162-
e
163-
} else {
163+
if if_else.is_none() {
164164
return false;
165165
};
166-
expr_return_none_or_err(smbl, cx, else_expr, let_expr, Some(let_pat_sym))
166+
expr_return_none_or_err(smbl, cx, if_else.unwrap(), let_expr, Some(let_pat_sym))
167167
} else if is_lang_ctor(cx, qpath, ResultErr) {
168168
expr_return_none_or_err(smbl, cx, if_then, let_expr, Some(let_pat_sym))
169169
} else {

0 commit comments

Comments
 (0)