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