]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 XL |
1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
2 | use clippy_utils::source::snippet_with_context; | |
3 | use clippy_utils::ty::peel_mid_ty_refs; | |
136023e0 | 4 | use clippy_utils::{get_parent_node, in_macro, is_lint_allowed}; |
cdc7bbd5 | 5 | use rustc_ast::util::parser::PREC_PREFIX; |
f20569fa | 6 | use rustc_errors::Applicability; |
cdc7bbd5 | 7 | use rustc_hir::{BorrowKind, Expr, ExprKind, HirId, MatchSource, Mutability, Node, UnOp}; |
f20569fa | 8 | use rustc_lint::{LateContext, LateLintPass}; |
cdc7bbd5 XL |
9 | use rustc_middle::ty::{self, Ty, TyCtxt, TyS, TypeckResults}; |
10 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
11 | use rustc_span::{symbol::sym, Span}; | |
f20569fa XL |
12 | |
13 | declare_clippy_lint! { | |
94222f64 XL |
14 | /// ### What it does |
15 | /// Checks for explicit `deref()` or `deref_mut()` method calls. | |
f20569fa | 16 | /// |
94222f64 XL |
17 | /// ### Why is this bad? |
18 | /// Dereferencing by `&*x` or `&mut *x` is clearer and more concise, | |
f20569fa XL |
19 | /// when not part of a method chain. |
20 | /// | |
94222f64 | 21 | /// ### Example |
f20569fa XL |
22 | /// ```rust |
23 | /// use std::ops::Deref; | |
24 | /// let a: &mut String = &mut String::from("foo"); | |
25 | /// let b: &str = a.deref(); | |
26 | /// ``` | |
27 | /// Could be written as: | |
28 | /// ```rust | |
29 | /// let a: &mut String = &mut String::from("foo"); | |
30 | /// let b = &*a; | |
31 | /// ``` | |
32 | /// | |
33 | /// This lint excludes | |
34 | /// ```rust,ignore | |
35 | /// let _ = d.unwrap().deref(); | |
36 | /// ``` | |
37 | pub EXPLICIT_DEREF_METHODS, | |
38 | pedantic, | |
39 | "Explicit use of deref or deref_mut method while not in a method chain." | |
40 | } | |
41 | ||
cdc7bbd5 XL |
42 | impl_lint_pass!(Dereferencing => [ |
43 | EXPLICIT_DEREF_METHODS, | |
f20569fa XL |
44 | ]); |
45 | ||
cdc7bbd5 XL |
46 | #[derive(Default)] |
47 | pub struct Dereferencing { | |
48 | state: Option<(State, StateData)>, | |
49 | ||
50 | // While parsing a `deref` method call in ufcs form, the path to the function is itself an | |
51 | // expression. This is to store the id of that expression so it can be skipped when | |
52 | // `check_expr` is called for it. | |
53 | skip_expr: Option<HirId>, | |
54 | } | |
55 | ||
56 | struct StateData { | |
57 | /// Span of the top level expression | |
58 | span: Span, | |
59 | /// The required mutability | |
60 | target_mut: Mutability, | |
61 | } | |
62 | ||
63 | enum State { | |
64 | // Any number of deref method calls. | |
65 | DerefMethod { | |
66 | // The number of calls in a sequence which changed the referenced type | |
67 | ty_changed_count: usize, | |
68 | is_final_ufcs: bool, | |
69 | }, | |
70 | } | |
71 | ||
72 | // A reference operation considered by this lint pass | |
73 | enum RefOp { | |
74 | Method(Mutability), | |
75 | Deref, | |
76 | AddrOf, | |
77 | } | |
78 | ||
f20569fa XL |
79 | impl<'tcx> LateLintPass<'tcx> for Dereferencing { |
80 | fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { | |
cdc7bbd5 XL |
81 | // Skip path expressions from deref calls. e.g. `Deref::deref(e)` |
82 | if Some(expr.hir_id) == self.skip_expr.take() { | |
83 | return; | |
84 | } | |
85 | ||
86 | // Stop processing sub expressions when a macro call is seen | |
87 | if in_macro(expr.span) { | |
88 | if let Some((state, data)) = self.state.take() { | |
89 | report(cx, expr, state, data); | |
90 | } | |
91 | return; | |
92 | } | |
93 | ||
94 | let typeck = cx.typeck_results(); | |
95 | let (kind, sub_expr) = if let Some(x) = try_parse_ref_op(cx.tcx, typeck, expr) { | |
96 | x | |
97 | } else { | |
98 | // The whole chain of reference operations has been seen | |
99 | if let Some((state, data)) = self.state.take() { | |
100 | report(cx, expr, state, data); | |
101 | } | |
102 | return; | |
103 | }; | |
104 | ||
105 | match (self.state.take(), kind) { | |
106 | (None, kind) => { | |
107 | let parent = get_parent_node(cx.tcx, expr.hir_id); | |
108 | let expr_ty = typeck.expr_ty(expr); | |
109 | ||
110 | match kind { | |
111 | RefOp::Method(target_mut) | |
136023e0 | 112 | if !is_lint_allowed(cx, EXPLICIT_DEREF_METHODS, expr.hir_id) |
cdc7bbd5 XL |
113 | && is_linted_explicit_deref_position(parent, expr.hir_id, expr.span) => |
114 | { | |
115 | self.state = Some(( | |
116 | State::DerefMethod { | |
117 | ty_changed_count: if deref_method_same_type(expr_ty, typeck.expr_ty(sub_expr)) { | |
118 | 0 | |
119 | } else { | |
120 | 1 | |
121 | }, | |
122 | is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), | |
123 | }, | |
124 | StateData { | |
125 | span: expr.span, | |
126 | target_mut, | |
127 | }, | |
128 | )); | |
c295e0f8 | 129 | }, |
cdc7bbd5 | 130 | _ => (), |
f20569fa | 131 | } |
cdc7bbd5 XL |
132 | }, |
133 | (Some((State::DerefMethod { ty_changed_count, .. }, data)), RefOp::Method(_)) => { | |
134 | self.state = Some(( | |
135 | State::DerefMethod { | |
136 | ty_changed_count: if deref_method_same_type(typeck.expr_ty(expr), typeck.expr_ty(sub_expr)) { | |
137 | ty_changed_count | |
138 | } else { | |
139 | ty_changed_count + 1 | |
140 | }, | |
141 | is_final_ufcs: matches!(expr.kind, ExprKind::Call(..)), | |
142 | }, | |
143 | data, | |
144 | )); | |
145 | }, | |
146 | ||
147 | (Some((state, data)), _) => report(cx, expr, state, data), | |
f20569fa XL |
148 | } |
149 | } | |
150 | } | |
151 | ||
cdc7bbd5 XL |
152 | fn try_parse_ref_op( |
153 | tcx: TyCtxt<'tcx>, | |
154 | typeck: &'tcx TypeckResults<'_>, | |
155 | expr: &'tcx Expr<'_>, | |
156 | ) -> Option<(RefOp, &'tcx Expr<'tcx>)> { | |
157 | let (def_id, arg) = match expr.kind { | |
158 | ExprKind::MethodCall(_, _, [arg], _) => (typeck.type_dependent_def_id(expr.hir_id)?, arg), | |
159 | ExprKind::Call( | |
160 | Expr { | |
161 | kind: ExprKind::Path(path), | |
162 | hir_id, | |
163 | .. | |
164 | }, | |
165 | [arg], | |
166 | ) => (typeck.qpath_res(path, *hir_id).opt_def_id()?, arg), | |
167 | ExprKind::Unary(UnOp::Deref, sub_expr) if !typeck.expr_ty(sub_expr).is_unsafe_ptr() => { | |
168 | return Some((RefOp::Deref, sub_expr)); | |
f20569fa | 169 | }, |
cdc7bbd5 XL |
170 | ExprKind::AddrOf(BorrowKind::Ref, _, sub_expr) => return Some((RefOp::AddrOf, sub_expr)), |
171 | _ => return None, | |
172 | }; | |
173 | if tcx.is_diagnostic_item(sym::deref_method, def_id) { | |
174 | Some((RefOp::Method(Mutability::Not), arg)) | |
175 | } else if tcx.trait_of_item(def_id)? == tcx.lang_items().deref_mut_trait()? { | |
176 | Some((RefOp::Method(Mutability::Mut), arg)) | |
177 | } else { | |
178 | None | |
179 | } | |
180 | } | |
181 | ||
182 | // Checks whether the type for a deref call actually changed the type, not just the mutability of | |
183 | // the reference. | |
184 | fn deref_method_same_type(result_ty: Ty<'tcx>, arg_ty: Ty<'tcx>) -> bool { | |
185 | match (result_ty.kind(), arg_ty.kind()) { | |
186 | (ty::Ref(_, result_ty, _), ty::Ref(_, arg_ty, _)) => TyS::same_type(result_ty, arg_ty), | |
187 | ||
188 | // The result type for a deref method is always a reference | |
189 | // Not matching the previous pattern means the argument type is not a reference | |
190 | // This means that the type did change | |
191 | _ => false, | |
192 | } | |
193 | } | |
194 | ||
195 | // Checks whether the parent node is a suitable context for switching from a deref method to the | |
196 | // deref operator. | |
197 | fn is_linted_explicit_deref_position(parent: Option<Node<'_>>, child_id: HirId, child_span: Span) -> bool { | |
198 | let parent = match parent { | |
199 | Some(Node::Expr(e)) if e.span.ctxt() == child_span.ctxt() => e, | |
200 | _ => return true, | |
201 | }; | |
202 | match parent.kind { | |
203 | // Leave deref calls in the middle of a method chain. | |
204 | // e.g. x.deref().foo() | |
205 | ExprKind::MethodCall(_, _, [self_arg, ..], _) if self_arg.hir_id == child_id => false, | |
206 | ||
207 | // Leave deref calls resulting in a called function | |
208 | // e.g. (x.deref())() | |
209 | ExprKind::Call(func_expr, _) if func_expr.hir_id == child_id => false, | |
210 | ||
211 | // Makes an ugly suggestion | |
212 | // e.g. *x.deref() => *&*x | |
213 | ExprKind::Unary(UnOp::Deref, _) | |
214 | // Postfix expressions would require parens | |
215 | | ExprKind::Match(_, _, MatchSource::TryDesugar | MatchSource::AwaitDesugar) | |
216 | | ExprKind::Field(..) | |
217 | | ExprKind::Index(..) | |
218 | | ExprKind::Err => false, | |
219 | ||
220 | ExprKind::Box(..) | |
221 | | ExprKind::ConstBlock(..) | |
222 | | ExprKind::Array(_) | |
223 | | ExprKind::Call(..) | |
224 | | ExprKind::MethodCall(..) | |
225 | | ExprKind::Tup(..) | |
226 | | ExprKind::Binary(..) | |
227 | | ExprKind::Unary(..) | |
228 | | ExprKind::Lit(..) | |
229 | | ExprKind::Cast(..) | |
230 | | ExprKind::Type(..) | |
231 | | ExprKind::DropTemps(..) | |
232 | | ExprKind::If(..) | |
233 | | ExprKind::Loop(..) | |
234 | | ExprKind::Match(..) | |
94222f64 | 235 | | ExprKind::Let(..) |
cdc7bbd5 XL |
236 | | ExprKind::Closure(..) |
237 | | ExprKind::Block(..) | |
238 | | ExprKind::Assign(..) | |
239 | | ExprKind::AssignOp(..) | |
240 | | ExprKind::Path(..) | |
241 | | ExprKind::AddrOf(..) | |
242 | | ExprKind::Break(..) | |
243 | | ExprKind::Continue(..) | |
244 | | ExprKind::Ret(..) | |
245 | | ExprKind::InlineAsm(..) | |
246 | | ExprKind::LlvmInlineAsm(..) | |
247 | | ExprKind::Struct(..) | |
248 | | ExprKind::Repeat(..) | |
249 | | ExprKind::Yield(..) => true, | |
250 | } | |
251 | } | |
252 | ||
253 | #[allow(clippy::needless_pass_by_value)] | |
254 | fn report(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, state: State, data: StateData) { | |
255 | match state { | |
256 | State::DerefMethod { | |
257 | ty_changed_count, | |
258 | is_final_ufcs, | |
259 | } => { | |
260 | let mut app = Applicability::MachineApplicable; | |
261 | let (expr_str, expr_is_macro_call) = snippet_with_context(cx, expr.span, data.span.ctxt(), "..", &mut app); | |
262 | let ty = cx.typeck_results().expr_ty(expr); | |
263 | let (_, ref_count) = peel_mid_ty_refs(ty); | |
264 | let deref_str = if ty_changed_count >= ref_count && ref_count != 0 { | |
265 | // a deref call changing &T -> &U requires two deref operators the first time | |
266 | // this occurs. One to remove the reference, a second to call the deref impl. | |
267 | "*".repeat(ty_changed_count + 1) | |
268 | } else { | |
269 | "*".repeat(ty_changed_count) | |
270 | }; | |
271 | let addr_of_str = if ty_changed_count < ref_count { | |
272 | // Check if a reborrow from &mut T -> &T is required. | |
273 | if data.target_mut == Mutability::Not && matches!(ty.kind(), ty::Ref(_, _, Mutability::Mut)) { | |
274 | "&*" | |
275 | } else { | |
276 | "" | |
277 | } | |
278 | } else if data.target_mut == Mutability::Mut { | |
279 | "&mut " | |
280 | } else { | |
281 | "&" | |
282 | }; | |
283 | ||
284 | let expr_str = if !expr_is_macro_call && is_final_ufcs && expr.precedence().order() < PREC_PREFIX { | |
285 | format!("({})", expr_str) | |
286 | } else { | |
287 | expr_str.into_owned() | |
288 | }; | |
289 | ||
290 | span_lint_and_sugg( | |
291 | cx, | |
292 | EXPLICIT_DEREF_METHODS, | |
293 | data.span, | |
294 | match data.target_mut { | |
295 | Mutability::Not => "explicit `deref` method call", | |
296 | Mutability::Mut => "explicit `deref_mut` method call", | |
297 | }, | |
298 | "try this", | |
299 | format!("{}{}{}", addr_of_str, deref_str, expr_str), | |
300 | app, | |
301 | ); | |
f20569fa | 302 | }, |
f20569fa XL |
303 | } |
304 | } |