]> git.lizzy.rs Git - rust.git/blob - src/tools/clippy/clippy_lints/src/needless_question_mark.rs
Auto merge of #82864 - jyn514:short-circuit, r=GuillaumeGomez
[rust.git] / src / tools / clippy / clippy_lints / src / needless_question_mark.rs
1 use clippy_utils::diagnostics::span_lint_and_sugg;
2 use clippy_utils::source::snippet;
3 use clippy_utils::ty::is_type_diagnostic_item;
4 use clippy_utils::{differing_macro_contexts, is_ok_ctor, is_some_ctor, meets_msrv};
5 use if_chain::if_chain;
6 use rustc_errors::Applicability;
7 use rustc_hir::{Body, Expr, ExprKind, LangItem, MatchSource, QPath};
8 use rustc_lint::{LateContext, LateLintPass, LintContext};
9 use rustc_semver::RustcVersion;
10 use rustc_session::{declare_tool_lint, impl_lint_pass};
11 use rustc_span::sym;
12
13 declare_clippy_lint! {
14     /// **What it does:**
15     /// Suggests alternatives for useless applications of `?` in terminating expressions
16     ///
17     /// **Why is this bad?** There's no reason to use `?` to short-circuit when execution of the body will end there anyway.
18     ///
19     /// **Known problems:** None.
20     ///
21     /// **Example:**
22     ///
23     /// ```rust
24     /// struct TO {
25     ///     magic: Option<usize>,
26     /// }
27     ///
28     /// fn f(to: TO) -> Option<usize> {
29     ///     Some(to.magic?)
30     /// }
31     ///
32     /// struct TR {
33     ///     magic: Result<usize, bool>,
34     /// }
35     ///
36     /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
37     ///     tr.and_then(|t| Ok(t.magic?))
38     /// }
39     ///
40     /// ```
41     /// Use instead:
42     /// ```rust
43     /// struct TO {
44     ///     magic: Option<usize>,
45     /// }
46     ///
47     /// fn f(to: TO) -> Option<usize> {
48     ///    to.magic
49     /// }
50     ///
51     /// struct TR {
52     ///     magic: Result<usize, bool>,
53     /// }
54     ///
55     /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> {
56     ///     tr.and_then(|t| t.magic)
57     /// }
58     /// ```
59     pub NEEDLESS_QUESTION_MARK,
60     complexity,
61     "Suggest `value.inner_option` instead of `Some(value.inner_option?)`. The same goes for `Result<T, E>`."
62 }
63
64 const NEEDLESS_QUESTION_MARK_RESULT_MSRV: RustcVersion = RustcVersion::new(1, 13, 0);
65 const NEEDLESS_QUESTION_MARK_OPTION_MSRV: RustcVersion = RustcVersion::new(1, 22, 0);
66
67 pub struct NeedlessQuestionMark {
68     msrv: Option<RustcVersion>,
69 }
70
71 impl NeedlessQuestionMark {
72     #[must_use]
73     pub fn new(msrv: Option<RustcVersion>) -> Self {
74         Self { msrv }
75     }
76 }
77
78 impl_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]);
79
80 #[derive(Debug)]
81 enum SomeOkCall<'a> {
82     SomeCall(&'a Expr<'a>, &'a Expr<'a>),
83     OkCall(&'a Expr<'a>, &'a Expr<'a>),
84 }
85
86 impl LateLintPass<'_> for NeedlessQuestionMark {
87     /*
88      * The question mark operator is compatible with both Result<T, E> and Option<T>,
89      * from Rust 1.13 and 1.22 respectively.
90      */
91
92     /*
93      * What do we match:
94      * Expressions that look like this:
95      * Some(option?), Ok(result?)
96      *
97      * Where do we match:
98      *      Last expression of a body
99      *      Return statement
100      *      A body's value (single line closure)
101      *
102      * What do we not match:
103      *      Implicit calls to `from(..)` on the error value
104      */
105
106     fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
107         let e = match &expr.kind {
108             ExprKind::Ret(Some(e)) => e,
109             _ => return,
110         };
111
112         if let Some(ok_some_call) = is_some_or_ok_call(self, cx, e) {
113             emit_lint(cx, &ok_some_call);
114         }
115     }
116
117     fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) {
118         // Function / Closure block
119         let expr_opt = if let ExprKind::Block(block, _) = &body.value.kind {
120             block.expr
121         } else {
122             // Single line closure
123             Some(&body.value)
124         };
125
126         if_chain! {
127             if let Some(expr) = expr_opt;
128             if let Some(ok_some_call) = is_some_or_ok_call(self, cx, expr);
129             then {
130                 emit_lint(cx, &ok_some_call);
131             }
132         };
133     }
134
135     extract_msrv_attr!(LateContext);
136 }
137
138 fn emit_lint(cx: &LateContext<'_>, expr: &SomeOkCall<'_>) {
139     let (entire_expr, inner_expr) = match expr {
140         SomeOkCall::OkCall(outer, inner) | SomeOkCall::SomeCall(outer, inner) => (outer, inner),
141     };
142
143     span_lint_and_sugg(
144         cx,
145         NEEDLESS_QUESTION_MARK,
146         entire_expr.span,
147         "question mark operator is useless here",
148         "try",
149         format!("{}", snippet(cx, inner_expr.span, r#""...""#)),
150         Applicability::MachineApplicable,
151     );
152 }
153
154 fn is_some_or_ok_call<'a>(
155     nqml: &NeedlessQuestionMark,
156     cx: &'a LateContext<'_>,
157     expr: &'a Expr<'_>,
158 ) -> Option<SomeOkCall<'a>> {
159     if_chain! {
160         // Check outer expression matches CALL_IDENT(ARGUMENT) format
161         if let ExprKind::Call(path, args) = &expr.kind;
162         if let ExprKind::Path(QPath::Resolved(None, path)) = &path.kind;
163         if is_some_ctor(cx, path.res) || is_ok_ctor(cx, path.res);
164
165         // Extract inner expression from ARGUMENT
166         if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &args[0].kind;
167         if let ExprKind::Call(called, args) = &inner_expr_with_q.kind;
168         if args.len() == 1;
169
170         if let ExprKind::Path(QPath::LangItem(LangItem::TryIntoResult, _)) = &called.kind;
171         then {
172             // Extract inner expr type from match argument generated by
173             // question mark operator
174             let inner_expr = &args[0];
175
176             // if the inner expr is inside macro but the outer one is not, do not lint (#6921)
177             if  differing_macro_contexts(expr.span, inner_expr.span) {
178                 return None;
179             }
180
181             let inner_ty = cx.typeck_results().expr_ty(inner_expr);
182             let outer_ty = cx.typeck_results().expr_ty(expr);
183
184             // Check if outer and inner type are Option
185             let outer_is_some = is_type_diagnostic_item(cx, outer_ty, sym::option_type);
186             let inner_is_some = is_type_diagnostic_item(cx, inner_ty, sym::option_type);
187
188             // Check for Option MSRV
189             let meets_option_msrv = meets_msrv(nqml.msrv.as_ref(), &NEEDLESS_QUESTION_MARK_OPTION_MSRV);
190             if outer_is_some && inner_is_some && meets_option_msrv {
191                 return Some(SomeOkCall::SomeCall(expr, inner_expr));
192             }
193
194             // Check if outer and inner type are Result
195             let outer_is_result = is_type_diagnostic_item(cx, outer_ty, sym::result_type);
196             let inner_is_result = is_type_diagnostic_item(cx, inner_ty, sym::result_type);
197
198             // Additional check: if the error type of the Result can be converted
199             // via the From trait, then don't match
200             let does_not_call_from = !has_implicit_error_from(cx, expr, inner_expr);
201
202             // Must meet Result MSRV
203             let meets_result_msrv = meets_msrv(nqml.msrv.as_ref(), &NEEDLESS_QUESTION_MARK_RESULT_MSRV);
204             if outer_is_result && inner_is_result && does_not_call_from && meets_result_msrv {
205                 return Some(SomeOkCall::OkCall(expr, inner_expr));
206             }
207         }
208     }
209
210     None
211 }
212
213 fn has_implicit_error_from(cx: &LateContext<'_>, entire_expr: &Expr<'_>, inner_result_expr: &Expr<'_>) -> bool {
214     return cx.typeck_results().expr_ty(entire_expr) != cx.typeck_results().expr_ty(inner_result_expr);
215 }