]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
2b03887a | 2 | use clippy_utils::path_res; |
cdc7bbd5 | 3 | use clippy_utils::source::snippet; |
cdc7bbd5 | 4 | use if_chain::if_chain; |
f20569fa | 5 | use rustc_errors::Applicability; |
2b03887a | 6 | use rustc_hir::def::{DefKind, Res}; |
5099ac24 | 7 | use rustc_hir::{AsyncGeneratorKind, Block, Body, Expr, ExprKind, GeneratorKind, LangItem, MatchSource, QPath}; |
cdc7bbd5 | 8 | use rustc_lint::{LateContext, LateLintPass}; |
2b03887a | 9 | use rustc_middle::ty::DefIdTree; |
cdc7bbd5 | 10 | use rustc_session::{declare_lint_pass, declare_tool_lint}; |
f20569fa | 11 | |
f20569fa | 12 | declare_clippy_lint! { |
94222f64 | 13 | /// ### What it does |
f20569fa XL |
14 | /// Suggests alternatives for useless applications of `?` in terminating expressions |
15 | /// | |
94222f64 XL |
16 | /// ### Why is this bad? |
17 | /// There's no reason to use `?` to short-circuit when execution of the body will end there anyway. | |
f20569fa | 18 | /// |
94222f64 | 19 | /// ### Example |
f20569fa XL |
20 | /// ```rust |
21 | /// struct TO { | |
22 | /// magic: Option<usize>, | |
23 | /// } | |
24 | /// | |
25 | /// fn f(to: TO) -> Option<usize> { | |
26 | /// Some(to.magic?) | |
27 | /// } | |
28 | /// | |
29 | /// struct TR { | |
30 | /// magic: Result<usize, bool>, | |
31 | /// } | |
32 | /// | |
33 | /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> { | |
34 | /// tr.and_then(|t| Ok(t.magic?)) | |
35 | /// } | |
36 | /// | |
37 | /// ``` | |
38 | /// Use instead: | |
39 | /// ```rust | |
40 | /// struct TO { | |
41 | /// magic: Option<usize>, | |
42 | /// } | |
43 | /// | |
44 | /// fn f(to: TO) -> Option<usize> { | |
45 | /// to.magic | |
46 | /// } | |
47 | /// | |
48 | /// struct TR { | |
49 | /// magic: Result<usize, bool>, | |
50 | /// } | |
51 | /// | |
52 | /// fn g(tr: Result<TR, bool>) -> Result<usize, bool> { | |
53 | /// tr.and_then(|t| t.magic) | |
54 | /// } | |
55 | /// ``` | |
a2a8927a | 56 | #[clippy::version = "1.51.0"] |
f20569fa XL |
57 | pub NEEDLESS_QUESTION_MARK, |
58 | complexity, | |
59 | "Suggest `value.inner_option` instead of `Some(value.inner_option?)`. The same goes for `Result<T, E>`." | |
60 | } | |
61 | ||
cdc7bbd5 | 62 | declare_lint_pass!(NeedlessQuestionMark => [NEEDLESS_QUESTION_MARK]); |
f20569fa | 63 | |
f20569fa XL |
64 | impl LateLintPass<'_> for NeedlessQuestionMark { |
65 | /* | |
66 | * The question mark operator is compatible with both Result<T, E> and Option<T>, | |
67 | * from Rust 1.13 and 1.22 respectively. | |
68 | */ | |
69 | ||
70 | /* | |
71 | * What do we match: | |
72 | * Expressions that look like this: | |
73 | * Some(option?), Ok(result?) | |
74 | * | |
75 | * Where do we match: | |
76 | * Last expression of a body | |
77 | * Return statement | |
78 | * A body's value (single line closure) | |
79 | * | |
80 | * What do we not match: | |
81 | * Implicit calls to `from(..)` on the error value | |
82 | */ | |
83 | ||
84 | fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) { | |
17df50a5 XL |
85 | if let ExprKind::Ret(Some(e)) = expr.kind { |
86 | check(cx, e); | |
f20569fa XL |
87 | } |
88 | } | |
89 | ||
90 | fn check_body(&mut self, cx: &LateContext<'_>, body: &'_ Body<'_>) { | |
5099ac24 FG |
91 | if let Some(GeneratorKind::Async(AsyncGeneratorKind::Fn)) = body.generator_kind { |
92 | if let ExprKind::Block( | |
93 | Block { | |
94 | expr: | |
95 | Some(Expr { | |
96 | kind: ExprKind::DropTemps(async_body), | |
97 | .. | |
98 | }), | |
99 | .. | |
100 | }, | |
101 | _, | |
102 | ) = body.value.kind | |
103 | { | |
104 | if let ExprKind::Block(Block { expr: Some(expr), .. }, ..) = async_body.kind { | |
105 | check(cx, expr); | |
106 | } | |
107 | } | |
108 | } else { | |
109 | check(cx, body.value.peel_blocks()); | |
110 | } | |
f20569fa | 111 | } |
f20569fa XL |
112 | } |
113 | ||
17df50a5 | 114 | fn check(cx: &LateContext<'_>, expr: &Expr<'_>) { |
a2a8927a | 115 | if_chain! { |
2b03887a FG |
116 | if let ExprKind::Call(path, [arg]) = expr.kind; |
117 | if let Res::Def(DefKind::Ctor(..), ctor_id) = path_res(cx, path); | |
118 | if let Some(variant_id) = cx.tcx.opt_parent(ctor_id); | |
119 | let sugg_remove = if cx.tcx.lang_items().option_some_variant() == Some(variant_id) { | |
a2a8927a | 120 | "Some()" |
2b03887a | 121 | } else if cx.tcx.lang_items().result_ok_variant() == Some(variant_id) { |
a2a8927a XL |
122 | "Ok()" |
123 | } else { | |
124 | return; | |
125 | }; | |
17df50a5 XL |
126 | if let ExprKind::Match(inner_expr_with_q, _, MatchSource::TryDesugar) = &arg.kind; |
127 | if let ExprKind::Call(called, [inner_expr]) = &inner_expr_with_q.kind; | |
a2a8927a | 128 | if let ExprKind::Path(QPath::LangItem(LangItem::TryTraitBranch, ..)) = &called.kind; |
17df50a5 XL |
129 | if expr.span.ctxt() == inner_expr.span.ctxt(); |
130 | let expr_ty = cx.typeck_results().expr_ty(expr); | |
131 | let inner_ty = cx.typeck_results().expr_ty(inner_expr); | |
5099ac24 | 132 | if expr_ty == inner_ty; |
a2a8927a XL |
133 | then { |
134 | span_lint_and_sugg( | |
135 | cx, | |
136 | NEEDLESS_QUESTION_MARK, | |
137 | expr.span, | |
138 | "question mark operator is useless here", | |
2b03887a | 139 | &format!("try removing question mark and `{sugg_remove}`"), |
a2a8927a XL |
140 | format!("{}", snippet(cx, inner_expr.span, r#""...""#)), |
141 | Applicability::MachineApplicable, | |
142 | ); | |
143 | } | |
144 | } | |
f20569fa | 145 | } |