]> git.lizzy.rs Git - rust.git/blob - src/tools/clippy/clippy_lints/src/shadow.rs
Rollup merge of #89789 - jkugelman:must-use-thread-builder, r=joshtriplett
[rust.git] / src / tools / clippy / 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, 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     /// // Bad
27     /// let x = &x;
28     ///
29     /// // Good
30     /// let y = &x; // use different variable name
31     /// ```
32     pub SHADOW_SAME,
33     restriction,
34     "rebinding a name to itself, e.g., `let mut x = &mut x`"
35 }
36
37 declare_clippy_lint! {
38     /// ### What it does
39     /// Checks for bindings that shadow other bindings already in
40     /// scope, while reusing the original value.
41     ///
42     /// ### Why is this bad?
43     /// Not too much, in fact it's a common pattern in Rust
44     /// code. Still, some argue that name shadowing like this hurts readability,
45     /// because a value may be bound to different things depending on position in
46     /// the code.
47     ///
48     /// ### Example
49     /// ```rust
50     /// let x = 2;
51     /// let x = x + 1;
52     /// ```
53     /// use different variable name:
54     /// ```rust
55     /// let x = 2;
56     /// let y = x + 1;
57     /// ```
58     pub SHADOW_REUSE,
59     restriction,
60     "rebinding a name to an expression that re-uses the original value, e.g., `let x = x + 1`"
61 }
62
63 declare_clippy_lint! {
64     /// ### What it does
65     /// Checks for bindings that shadow other bindings already in
66     /// scope, either without an initialization or with one that does not even use
67     /// the original value.
68     ///
69     /// ### Why is this bad?
70     /// Name shadowing can hurt readability, especially in
71     /// large code bases, because it is easy to lose track of the active binding at
72     /// any place in the code. This can be alleviated by either giving more specific
73     /// names to bindings or introducing more scopes to contain the bindings.
74     ///
75     /// ### Example
76     /// ```rust
77     /// # let y = 1;
78     /// # let z = 2;
79     /// let x = y;
80     ///
81     /// // Bad
82     /// let x = z; // shadows the earlier binding
83     ///
84     /// // Good
85     /// let w = z; // use different variable name
86     /// ```
87     pub SHADOW_UNRELATED,
88     restriction,
89     "rebinding a name without even using the original value"
90 }
91
92 #[derive(Default)]
93 pub(crate) struct Shadow {
94     bindings: Vec<FxHashMap<Symbol, Vec<ItemLocalId>>>,
95 }
96
97 impl_lint_pass!(Shadow => [SHADOW_SAME, SHADOW_REUSE, SHADOW_UNRELATED]);
98
99 impl<'tcx> LateLintPass<'tcx> for Shadow {
100     fn check_pat(&mut self, cx: &LateContext<'tcx>, pat: &'tcx Pat<'_>) {
101         let (id, ident) = match pat.kind {
102             PatKind::Binding(_, hir_id, ident, _) => (hir_id, ident),
103             _ => return,
104         };
105         if ident.span.from_expansion() || ident.span.is_dummy() {
106             return;
107         }
108         let HirId { owner, local_id } = id;
109
110         // get (or insert) the list of items for this owner and symbol
111         let data = self.bindings.last_mut().unwrap();
112         let items_with_name = data.entry(ident.name).or_default();
113
114         // check other bindings with the same name, most recently seen first
115         for &prev in items_with_name.iter().rev() {
116             if prev == local_id {
117                 // repeated binding in an `Or` pattern
118                 return;
119             }
120
121             if is_shadow(cx, owner, prev, local_id) {
122                 let prev_hir_id = HirId { owner, local_id: prev };
123                 lint_shadow(cx, pat, prev_hir_id, ident.span);
124                 // only lint against the "nearest" shadowed binding
125                 break;
126             }
127         }
128         // store the binding
129         items_with_name.push(local_id);
130     }
131
132     fn check_body(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
133         let hir = cx.tcx.hir();
134         if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
135             self.bindings.push(FxHashMap::default());
136         }
137     }
138
139     fn check_body_post(&mut self, cx: &LateContext<'_>, body: &Body<'_>) {
140         let hir = cx.tcx.hir();
141         if !matches!(hir.body_owner_kind(hir.body_owner(body.id())), BodyOwnerKind::Closure) {
142             self.bindings.pop();
143         }
144     }
145 }
146
147 fn is_shadow(cx: &LateContext<'_>, owner: LocalDefId, first: ItemLocalId, second: ItemLocalId) -> bool {
148     let scope_tree = cx.tcx.region_scope_tree(owner.to_def_id());
149     let first_scope = scope_tree.var_scope(first);
150     let second_scope = scope_tree.var_scope(second);
151     scope_tree.is_subscope_of(second_scope, first_scope)
152 }
153
154 fn lint_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, shadowed: HirId, span: Span) {
155     let (lint, msg) = match find_init(cx, pat.hir_id) {
156         Some(expr) if is_self_shadow(cx, pat, expr, shadowed) => {
157             let msg = format!(
158                 "`{}` is shadowed by itself in `{}`",
159                 snippet(cx, pat.span, "_"),
160                 snippet(cx, expr.span, "..")
161             );
162             (SHADOW_SAME, msg)
163         },
164         Some(expr) if is_local_used(cx, expr, shadowed) => {
165             let msg = format!("`{}` is shadowed", snippet(cx, pat.span, "_"));
166             (SHADOW_REUSE, msg)
167         },
168         _ => {
169             let msg = format!("`{}` shadows a previous, unrelated binding", snippet(cx, pat.span, "_"));
170             (SHADOW_UNRELATED, msg)
171         },
172     };
173     span_lint_and_note(
174         cx,
175         lint,
176         span,
177         &msg,
178         Some(cx.tcx.hir().span(shadowed)),
179         "previous binding is here",
180     );
181 }
182
183 /// Returns true if the expression is a simple transformation of a local binding such as `&x`
184 fn is_self_shadow(cx: &LateContext<'_>, pat: &Pat<'_>, mut expr: &Expr<'_>, hir_id: HirId) -> bool {
185     let hir = cx.tcx.hir();
186     let is_direct_binding = hir
187         .parent_iter(pat.hir_id)
188         .map_while(|(_id, node)| match node {
189             Node::Pat(pat) => Some(pat),
190             _ => None,
191         })
192         .all(|pat| matches!(pat.kind, PatKind::Ref(..) | PatKind::Or(_)));
193     if !is_direct_binding {
194         return false;
195     }
196     loop {
197         expr = match expr.kind {
198             ExprKind::Box(e)
199             | ExprKind::AddrOf(_, _, e)
200             | ExprKind::Block(
201                 &Block {
202                     stmts: [],
203                     expr: Some(e),
204                     ..
205                 },
206                 _,
207             )
208             | ExprKind::Unary(UnOp::Deref, e) => e,
209             ExprKind::Path(QPath::Resolved(None, path)) => break path.res == Res::Local(hir_id),
210             _ => break false,
211         }
212     }
213 }
214
215 /// Finds the "init" expression for a pattern: `let <pat> = <init>;` or
216 /// `match <init> { .., <pat> => .., .. }`
217 fn find_init<'tcx>(cx: &LateContext<'tcx>, hir_id: HirId) -> Option<&'tcx Expr<'tcx>> {
218     for (_, node) in cx.tcx.hir().parent_iter(hir_id) {
219         let init = match node {
220             Node::Arm(_) | Node::Pat(_) => continue,
221             Node::Expr(expr) => match expr.kind {
222                 ExprKind::Match(e, _, _) => Some(e),
223                 _ => None,
224             },
225             Node::Local(local) => local.init,
226             _ => None,
227         };
228         return init;
229     }
230     None
231 }