]>
Commit | Line | Data |
---|---|---|
c295e0f8 | 1 | use clippy_utils::diagnostics::span_lint_and_note; |
cdc7bbd5 | 2 | use clippy_utils::source::snippet; |
c295e0f8 XL |
3 | use clippy_utils::visitors::is_local_used; |
4 | use rustc_data_structures::fx::FxHashMap; | |
5 | use rustc_hir::def::Res; | |
6 | use rustc_hir::def_id::LocalDefId; | |
7 | use rustc_hir::hir_id::ItemLocalId; | |
a2a8927a | 8 | use rustc_hir::{Block, Body, BodyOwnerKind, Expr, ExprKind, HirId, Let, Node, Pat, PatKind, QPath, UnOp}; |
c295e0f8 XL |
9 | use rustc_lint::{LateContext, LateLintPass}; |
10 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
11 | use rustc_span::{Span, Symbol}; | |
f20569fa XL |
12 | |
13 | declare_clippy_lint! { | |
94222f64 XL |
14 | /// ### What it does |
15 | /// Checks for bindings that shadow other bindings already in | |
f20569fa XL |
16 | /// scope, while just changing reference level or mutability. |
17 | /// | |
94222f64 XL |
18 | /// ### Why is this bad? |
19 | /// Not much, in fact it's a very common pattern in Rust | |
f20569fa XL |
20 | /// code. Still, some may opt to avoid it in their code base, they can set this |
21 | /// lint to `Warn`. | |
22 | /// | |
94222f64 | 23 | /// ### Example |
f20569fa XL |
24 | /// ```rust |
25 | /// # let x = 1; | |
f20569fa | 26 | /// let x = &x; |
923072b8 | 27 | /// ``` |
f20569fa | 28 | /// |
923072b8 FG |
29 | /// Use instead: |
30 | /// ```rust | |
31 | /// # let x = 1; | |
f20569fa XL |
32 | /// let y = &x; // use different variable name |
33 | /// ``` | |
a2a8927a | 34 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
35 | pub SHADOW_SAME, |
36 | restriction, | |
37 | "rebinding a name to itself, e.g., `let mut x = &mut x`" | |
38 | } | |
39 | ||
40 | declare_clippy_lint! { | |
94222f64 XL |
41 | /// ### What it does |
42 | /// Checks for bindings that shadow other bindings already in | |
f20569fa XL |
43 | /// scope, while reusing the original value. |
44 | /// | |
94222f64 XL |
45 | /// ### Why is this bad? |
46 | /// Not too much, in fact it's a common pattern in Rust | |
f20569fa XL |
47 | /// code. Still, some argue that name shadowing like this hurts readability, |
48 | /// because a value may be bound to different things depending on position in | |
49 | /// the code. | |
50 | /// | |
94222f64 | 51 | /// ### Example |
f20569fa XL |
52 | /// ```rust |
53 | /// let x = 2; | |
54 | /// let x = x + 1; | |
55 | /// ``` | |
56 | /// use different variable name: | |
57 | /// ```rust | |
58 | /// let x = 2; | |
59 | /// let y = x + 1; | |
60 | /// ``` | |
a2a8927a | 61 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
62 | pub SHADOW_REUSE, |
63 | restriction, | |
64 | "rebinding a name to an expression that re-uses the original value, e.g., `let x = x + 1`" | |
65 | } | |
66 | ||
67 | declare_clippy_lint! { | |
94222f64 XL |
68 | /// ### What it does |
69 | /// Checks for bindings that shadow other bindings already in | |
70 | /// scope, either without an initialization or with one that does not even use | |
f20569fa XL |
71 | /// the original value. |
72 | /// | |
94222f64 XL |
73 | /// ### Why is this bad? |
74 | /// Name shadowing can hurt readability, especially in | |
f20569fa XL |
75 | /// large code bases, because it is easy to lose track of the active binding at |
76 | /// any place in the code. This can be alleviated by either giving more specific | |
77 | /// names to bindings or introducing more scopes to contain the bindings. | |
78 | /// | |
94222f64 | 79 | /// ### Example |
f20569fa XL |
80 | /// ```rust |
81 | /// # let y = 1; | |
82 | /// # let z = 2; | |
83 | /// let x = y; | |
f20569fa | 84 | /// let x = z; // shadows the earlier binding |
923072b8 | 85 | /// ``` |
f20569fa | 86 | /// |
923072b8 FG |
87 | /// Use instead: |
88 | /// ```rust | |
89 | /// # let y = 1; | |
90 | /// # let z = 2; | |
91 | /// let x = y; | |
f20569fa XL |
92 | /// let w = z; // use different variable name |
93 | /// ``` | |
a2a8927a | 94 | #[clippy::version = "pre 1.29.0"] |
f20569fa | 95 | pub SHADOW_UNRELATED, |
c295e0f8 | 96 | restriction, |
f20569fa XL |
97 | "rebinding a name without even using the original value" |
98 | } | |
99 | ||
c295e0f8 XL |
100 | #[derive(Default)] |
101 | pub(crate) struct Shadow { | |
064997fb | 102 | bindings: Vec<(FxHashMap<Symbol, Vec<ItemLocalId>>, LocalDefId)>, |
c295e0f8 XL |
103 | } |
104 | ||
105 | impl_lint_pass!(Shadow => [SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED]); | |
f20569fa XL |
106 | |
107 | impl<'tcx> LateLintPass<'tcx> for Shadow { | |
c295e0f8 | 108 | fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) { |
2b03887a | 109 | let PatKind::Binding(_, id, ident, _) = pat.kind else { return }; |
a2a8927a | 110 | |
49aad941 | 111 | if pat.span.desugaring_kind().is_some() || pat.span.from_expansion() { |
a2a8927a XL |
112 | return; |
113 | } | |
114 | ||
c295e0f8 | 115 | if ident.span.from_expansion() || ident.span.is_dummy() { |
f20569fa XL |
116 | return; |
117 | } | |
f20569fa | 118 | |
a2a8927a | 119 | let HirId { owner, local_id } = id; |
c295e0f8 | 120 | // get (or insert) the list of items for this owner and symbol |
064997fb | 121 | let (ref mut data, scope_owner) = *self.bindings.last_mut().unwrap(); |
c295e0f8 XL |
122 | let items_with_name = data.entry(ident.name).or_default(); |
123 | ||
124 | // check other bindings with the same name, most recently seen first | |
125 | for &prev in items_with_name.iter().rev() { | |
126 | if prev == local_id { | |
127 | // repeated binding in an `Or` pattern | |
128 | return; | |
129 | } | |
130 | ||
064997fb | 131 | if is_shadow(cx, scope_owner, prev, local_id) { |
c295e0f8 XL |
132 | let prev_hir_id = HirId { owner, local_id: prev }; |
133 | lint_shadow(cx, pat, prev_hir_id, ident.span); | |
134 | // only lint against the "nearest" shadowed binding | |
135 | break; | |
136 | } | |
f20569fa | 137 | } |
c295e0f8 XL |
138 | // store the binding |
139 | items_with_name.push(local_id); | |
f20569fa | 140 | } |
f20569fa | 141 | |
c295e0f8 XL |
142 | fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) { |
143 | let hir = cx.tcx.hir(); | |
064997fb FG |
144 | let owner_id = hir.body_owner_def_id(body.id()); |
145 | if !matches!(hir.body_owner_kind(owner_id), BodyOwnerKind::Closure) { | |
146 | self.bindings.push((FxHashMap::default(), owner_id)); | |
f20569fa XL |
147 | } |
148 | } | |
f20569fa | 149 | |
c295e0f8 XL |
150 | fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) { |
151 | let hir = cx.tcx.hir(); | |
5e7ed085 FG |
152 | if !matches!( |
153 | hir.body_owner_kind(hir.body_owner_def_id(body.id())), | |
154 | BodyOwnerKind::Closure | |
155 | ) { | |
c295e0f8 XL |
156 | self.bindings.pop(); |
157 | } | |
f20569fa XL |
158 | } |
159 | } | |
160 | ||
c295e0f8 XL |
161 | fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool { |
162 | let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id()); | |
923072b8 FG |
163 | if let Some(first_scope) = scope_tree.var_scope(first) { |
164 | if let Some(second_scope) = scope_tree.var_scope(second) { | |
165 | return scope_tree.is_subscope_of(second_scope, first_scope); | |
166 | } | |
167 | } | |
168 | ||
169 | false | |
f20569fa XL |
170 | } |
171 | ||
c295e0f8 XL |
172 | fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) { |
173 | let (lint, msg) = match find_init(cx, pat.hir_id) { | |
174 | Some(expr) if is_self_shadow(cx, pat, expr, shadowed) => { | |
175 | let msg = format!( | |
176 | "`{}` is shadowed by itself in `{}`", | |
177 | snippet(cx, pat.span, "_"), | |
178 | snippet(cx, expr.span, "..") | |
179 | ); | |
180 | (SHADOW_SAME, msg) | |
f20569fa | 181 | }, |
c295e0f8 | 182 | Some(expr) if is_local_used(cx, expr, shadowed) => { |
3c0e092e | 183 | let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_")); |
c295e0f8 | 184 | (SHADOW_REUSE, msg) |
f20569fa | 185 | }, |
c295e0f8 XL |
186 | _ => { |
187 | let msg = format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_")); | |
188 | (SHADOW_UNRELATED, msg) | |
f20569fa | 189 | }, |
c295e0f8 XL |
190 | }; |
191 | span_lint_and_note( | |
192 | cx, | |
193 | lint, | |
194 | span, | |
195 | &msg, | |
196 | Some(cx.tcx.hir().span(shadowed)), | |
197 | "previous binding is here", | |
198 | ); | |
f20569fa XL |
199 | } |
200 | ||
c295e0f8 XL |
201 | /// Returns true if the expression is a simple transformation of a local binding such as `&x` |
202 | fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_id: HirId) -> bool { | |
203 | let hir = cx.tcx.hir(); | |
204 | let is_direct_binding = hir | |
205 | .parent_iter(pat.hir_id) | |
206 | .map_while(|(_id, node)| match node { | |
207 | Node::Pat(pat) => Some(pat), | |
208 | _ => None, | |
209 | }) | |
210 | .all(|pat| matches!(pat.kind, PatKind::Ref(..) | PatKind::Or(_))); | |
211 | if !is_direct_binding { | |
212 | return false; | |
213 | } | |
214 | loop { | |
215 | expr = match expr.kind { | |
353b0b11 | 216 | ExprKind::AddrOf(_, _, e) |
c295e0f8 XL |
217 | | ExprKind::Block( |
218 | &Block { | |
219 | stmts: [], | |
220 | expr: Some(e), | |
221 | .. | |
f20569fa | 222 | }, |
c295e0f8 XL |
223 | _, |
224 | ) | |
225 | | ExprKind::Unary(UnOp::Deref, e) => e, | |
226 | ExprKind::Path(QPath::Resolved(None, path)) => break path.res == Res::Local(hir_id), | |
227 | _ => break false, | |
f20569fa | 228 | } |
f20569fa XL |
229 | } |
230 | } | |
231 | ||
a2a8927a | 232 | /// Finds the "init" expression for a pattern: `let <pat> = <init>;` (or `if let`) or |
c295e0f8 XL |
233 | /// `match <init> { .., <pat> => .., .. }` |
234 | fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> { | |
235 | for (_, node) in cx.tcx.hir().parent_iter(hir_id) { | |
236 | let init = match node { | |
237 | Node::Arm(_) | Node::Pat(_) => continue, | |
238 | Node::Expr(expr) => match expr.kind { | |
a2a8927a | 239 | ExprKind::Match(e, _, _) | ExprKind::Let(&Let { init: e, .. }) => Some(e), |
c295e0f8 XL |
240 | _ => None, |
241 | }, | |
242 | Node::Local(local) => local.init, | |
243 | _ => None, | |
244 | }; | |
245 | return init; | |
f20569fa | 246 | } |
c295e0f8 | 247 | None |
f20569fa | 248 | } |