]>
Commit | Line | Data |
---|---|---|
cdc7bbd5 | 1 | use clippy_utils::diagnostics::span_lint_and_sugg; |
17df50a5 | 2 | use clippy_utils::ty::same_type_and_consts; |
a2a8927a | 3 | use clippy_utils::{meets_msrv, msrvs}; |
f20569fa | 4 | use if_chain::if_chain; |
136023e0 | 5 | use rustc_data_structures::fx::FxHashSet; |
f20569fa | 6 | use rustc_errors::Applicability; |
f20569fa | 7 | use rustc_hir::{ |
17df50a5 | 8 | self as hir, |
136023e0 | 9 | def::{CtorOf, DefKind, Res}, |
f20569fa | 10 | def_id::LocalDefId, |
5099ac24 | 11 | intravisit::{walk_inf, walk_ty, Visitor}, |
5e7ed085 FG |
12 | Expr, ExprKind, FnRetTy, FnSig, GenericArg, HirId, Impl, ImplItemKind, Item, ItemKind, Pat, PatKind, Path, QPath, |
13 | TyKind, | |
f20569fa | 14 | }; |
5099ac24 | 15 | use rustc_lint::{LateContext, LateLintPass}; |
f20569fa XL |
16 | use rustc_semver::RustcVersion; |
17 | use rustc_session::{declare_tool_lint, impl_lint_pass}; | |
136023e0 | 18 | use rustc_span::Span; |
f20569fa XL |
19 | use rustc_typeck::hir_ty_to_ty; |
20 | ||
21 | declare_clippy_lint! { | |
94222f64 XL |
22 | /// ### What it does |
23 | /// Checks for unnecessary repetition of structure name when a | |
f20569fa XL |
24 | /// replacement with `Self` is applicable. |
25 | /// | |
94222f64 XL |
26 | /// ### Why is this bad? |
27 | /// Unnecessary repetition. Mixed use of `Self` and struct | |
f20569fa XL |
28 | /// name |
29 | /// feels inconsistent. | |
30 | /// | |
94222f64 | 31 | /// ### Known problems |
f20569fa XL |
32 | /// - Unaddressed false negative in fn bodies of trait implementations |
33 | /// - False positive with assotiated types in traits (#4140) | |
34 | /// | |
94222f64 | 35 | /// ### Example |
f20569fa XL |
36 | /// ```rust |
37 | /// struct Foo {} | |
38 | /// impl Foo { | |
39 | /// fn new() -> Foo { | |
40 | /// Foo {} | |
41 | /// } | |
42 | /// } | |
43 | /// ``` | |
44 | /// could be | |
45 | /// ```rust | |
46 | /// struct Foo {} | |
47 | /// impl Foo { | |
48 | /// fn new() -> Self { | |
49 | /// Self {} | |
50 | /// } | |
51 | /// } | |
52 | /// ``` | |
a2a8927a | 53 | #[clippy::version = "pre 1.29.0"] |
f20569fa XL |
54 | pub USE_SELF, |
55 | nursery, | |
56 | "unnecessary structure name repetition whereas `Self` is applicable" | |
57 | } | |
58 | ||
59 | #[derive(Default)] | |
60 | pub struct UseSelf { | |
61 | msrv: Option<RustcVersion>, | |
62 | stack: Vec<StackItem>, | |
63 | } | |
64 | ||
f20569fa XL |
65 | impl UseSelf { |
66 | #[must_use] | |
67 | pub fn new(msrv: Option<RustcVersion>) -> Self { | |
68 | Self { | |
69 | msrv, | |
70 | ..Self::default() | |
71 | } | |
72 | } | |
73 | } | |
74 | ||
75 | #[derive(Debug)] | |
76 | enum StackItem { | |
77 | Check { | |
136023e0 XL |
78 | impl_id: LocalDefId, |
79 | in_body: u32, | |
80 | types_to_skip: FxHashSet<HirId>, | |
f20569fa XL |
81 | }, |
82 | NoCheck, | |
83 | } | |
84 | ||
85 | impl_lint_pass!(UseSelf => [USE_SELF]); | |
86 | ||
87 | const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element"; | |
88 | ||
89 | impl<'tcx> LateLintPass<'tcx> for UseSelf { | |
136023e0 XL |
90 | fn check_item(&mut self, _cx: &LateContext<'_>, item: &Item<'_>) { |
91 | if matches!(item.kind, ItemKind::OpaqueTy(_)) { | |
92 | // skip over `ItemKind::OpaqueTy` in order to lint `foo() -> impl <..>` | |
93 | return; | |
94 | } | |
f20569fa XL |
95 | // We push the self types of `impl`s on a stack here. Only the top type on the stack is |
96 | // relevant for linting, since this is the self type of the `impl` we're currently in. To | |
97 | // avoid linting on nested items, we push `StackItem::NoCheck` on the stack to signal, that | |
98 | // we're in an `impl` or nested item, that we don't want to lint | |
136023e0 XL |
99 | let stack_item = if_chain! { |
100 | if let ItemKind::Impl(Impl { self_ty, .. }) = item.kind; | |
101 | if let TyKind::Path(QPath::Resolved(_, item_path)) = self_ty.kind; | |
102 | let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args; | |
103 | if parameters.as_ref().map_or(true, |params| { | |
104 | !params.parenthesized && !params.args.iter().any(|arg| matches!(arg, GenericArg::Lifetime(_))) | |
105 | }); | |
106 | then { | |
107 | StackItem::Check { | |
108 | impl_id: item.def_id, | |
109 | in_body: 0, | |
110 | types_to_skip: std::iter::once(self_ty.hir_id).collect(), | |
f20569fa | 111 | } |
136023e0 XL |
112 | } else { |
113 | StackItem::NoCheck | |
114 | } | |
115 | }; | |
116 | self.stack.push(stack_item); | |
f20569fa XL |
117 | } |
118 | ||
119 | fn check_item_post(&mut self, _: &LateContext<'_>, item: &Item<'_>) { | |
136023e0 XL |
120 | if !matches!(item.kind, ItemKind::OpaqueTy(_)) { |
121 | self.stack.pop(); | |
f20569fa XL |
122 | } |
123 | } | |
124 | ||
125 | fn check_impl_item(&mut self, cx: &LateContext<'_>, impl_item: &hir::ImplItem<'_>) { | |
126 | // We want to skip types in trait `impl`s that aren't declared as `Self` in the trait | |
127 | // declaration. The collection of those types is all this method implementation does. | |
128 | if_chain! { | |
129 | if let ImplItemKind::Fn(FnSig { decl, .. }, ..) = impl_item.kind; | |
130 | if let Some(&mut StackItem::Check { | |
136023e0 | 131 | impl_id, |
f20569fa XL |
132 | ref mut types_to_skip, |
133 | .. | |
134 | }) = self.stack.last_mut(); | |
136023e0 | 135 | if let Some(impl_trait_ref) = cx.tcx.impl_trait_ref(impl_id); |
f20569fa XL |
136 | then { |
137 | // `self_ty` is the semantic self type of `impl <trait> for <type>`. This cannot be | |
138 | // `Self`. | |
139 | let self_ty = impl_trait_ref.self_ty(); | |
140 | ||
141 | // `trait_method_sig` is the signature of the function, how it is declared in the | |
142 | // trait, not in the impl of the trait. | |
143 | let trait_method = cx | |
144 | .tcx | |
5099ac24 FG |
145 | .associated_item(impl_item.def_id) |
146 | .trait_item_def_id | |
f20569fa | 147 | .expect("impl method matches a trait method"); |
5099ac24 | 148 | let trait_method_sig = cx.tcx.fn_sig(trait_method); |
f20569fa XL |
149 | let trait_method_sig = cx.tcx.erase_late_bound_regions(trait_method_sig); |
150 | ||
151 | // `impl_inputs_outputs` is an iterator over the types (`hir::Ty`) declared in the | |
152 | // implementation of the trait. | |
153 | let output_hir_ty = if let FnRetTy::Return(ty) = &decl.output { | |
154 | Some(&**ty) | |
155 | } else { | |
156 | None | |
157 | }; | |
158 | let impl_inputs_outputs = decl.inputs.iter().chain(output_hir_ty); | |
159 | ||
160 | // `impl_hir_ty` (of type `hir::Ty`) represents the type written in the signature. | |
161 | // | |
162 | // `trait_sem_ty` (of type `ty::Ty`) is the semantic type for the signature in the | |
163 | // trait declaration. This is used to check if `Self` was used in the trait | |
164 | // declaration. | |
165 | // | |
166 | // If `any`where in the `trait_sem_ty` the `self_ty` was used verbatim (as opposed | |
167 | // to `Self`), we want to skip linting that type and all subtypes of it. This | |
168 | // avoids suggestions to e.g. replace `Vec<u8>` with `Vec<Self>`, in an `impl Trait | |
169 | // for u8`, when the trait always uses `Vec<u8>`. | |
170 | // | |
171 | // See also https://github.com/rust-lang/rust-clippy/issues/2894. | |
172 | for (impl_hir_ty, trait_sem_ty) in impl_inputs_outputs.zip(trait_method_sig.inputs_and_output) { | |
5099ac24 | 173 | if trait_sem_ty.walk().any(|inner| inner == self_ty.into()) { |
f20569fa | 174 | let mut visitor = SkipTyCollector::default(); |
cdc7bbd5 | 175 | visitor.visit_ty(impl_hir_ty); |
f20569fa XL |
176 | types_to_skip.extend(visitor.types_to_skip); |
177 | } | |
178 | } | |
179 | } | |
180 | } | |
181 | } | |
182 | ||
136023e0 | 183 | fn check_body(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) { |
f20569fa XL |
184 | // `hir_ty_to_ty` cannot be called in `Body`s or it will panic (sometimes). But in bodies |
185 | // we can use `cx.typeck_results.node_type(..)` to get the `ty::Ty` from a `hir::Ty`. | |
186 | // However the `node_type()` method can *only* be called in bodies. | |
136023e0 XL |
187 | if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() { |
188 | *in_body = in_body.saturating_add(1); | |
f20569fa XL |
189 | } |
190 | } | |
191 | ||
136023e0 XL |
192 | fn check_body_post(&mut self, _: &LateContext<'_>, _: &hir::Body<'_>) { |
193 | if let Some(&mut StackItem::Check { ref mut in_body, .. }) = self.stack.last_mut() { | |
194 | *in_body = in_body.saturating_sub(1); | |
f20569fa | 195 | } |
136023e0 | 196 | } |
f20569fa | 197 | |
136023e0 XL |
198 | fn check_ty(&mut self, cx: &LateContext<'_>, hir_ty: &hir::Ty<'_>) { |
199 | if_chain! { | |
a2a8927a | 200 | if !hir_ty.span.from_expansion(); |
136023e0 XL |
201 | if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); |
202 | if let Some(&StackItem::Check { | |
203 | impl_id, | |
204 | in_body, | |
205 | ref types_to_skip, | |
206 | }) = self.stack.last(); | |
207 | if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind; | |
5099ac24 | 208 | if !matches!(path.res, Res::SelfTy { .. } | Res::Def(DefKind::TyParam, _)); |
136023e0 XL |
209 | if !types_to_skip.contains(&hir_ty.hir_id); |
210 | let ty = if in_body > 0 { | |
211 | cx.typeck_results().node_type(hir_ty.hir_id) | |
f20569fa | 212 | } else { |
136023e0 XL |
213 | hir_ty_to_ty(cx.tcx, hir_ty) |
214 | }; | |
215 | if same_type_and_consts(ty, cx.tcx.type_of(impl_id)); | |
f20569fa | 216 | let hir = cx.tcx.hir(); |
a2a8927a XL |
217 | // prevents false positive on `#[derive(serde::Deserialize)]` |
218 | if !hir.span(hir.get_parent_node(hir_ty.hir_id)).in_derive_expansion(); | |
136023e0 XL |
219 | then { |
220 | span_lint(cx, hir_ty.span); | |
f20569fa XL |
221 | } |
222 | } | |
223 | } | |
224 | ||
225 | fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) { | |
136023e0 | 226 | if_chain! { |
a2a8927a | 227 | if !expr.span.from_expansion(); |
136023e0 XL |
228 | if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); |
229 | if let Some(&StackItem::Check { impl_id, .. }) = self.stack.last(); | |
230 | if cx.typeck_results().expr_ty(expr) == cx.tcx.type_of(impl_id); | |
231 | then {} else { return; } | |
f20569fa | 232 | } |
136023e0 XL |
233 | match expr.kind { |
234 | ExprKind::Struct(QPath::Resolved(_, path), ..) => match path.res { | |
5099ac24 | 235 | Res::SelfTy { .. } => (), |
136023e0 XL |
236 | Res::Def(DefKind::Variant, _) => lint_path_to_variant(cx, path), |
237 | _ => span_lint(cx, path.span), | |
238 | }, | |
239 | // tuple struct instantiation (`Foo(arg)` or `Enum::Foo(arg)`) | |
240 | ExprKind::Call(fun, _) => { | |
241 | if let ExprKind::Path(QPath::Resolved(_, path)) = fun.kind { | |
242 | if let Res::Def(DefKind::Ctor(ctor_of, _), ..) = path.res { | |
243 | match ctor_of { | |
244 | CtorOf::Variant => lint_path_to_variant(cx, path), | |
245 | CtorOf::Struct => span_lint(cx, path.span), | |
f20569fa XL |
246 | } |
247 | } | |
136023e0 XL |
248 | } |
249 | }, | |
250 | // unit enum variants (`Enum::A`) | |
251 | ExprKind::Path(QPath::Resolved(_, path)) => lint_path_to_variant(cx, path), | |
252 | _ => (), | |
5e7ed085 FG |
253 | } |
254 | } | |
255 | ||
256 | fn check_pat(&mut self, cx: &LateContext<'_>, pat: &Pat<'_>) { | |
257 | if_chain! { | |
258 | if !pat.span.from_expansion(); | |
259 | if meets_msrv(self.msrv.as_ref(), &msrvs::TYPE_ALIAS_ENUM_VARIANTS); | |
260 | if let Some(&StackItem::Check { impl_id, .. }) = self.stack.last(); | |
261 | if let PatKind::Path(QPath::Resolved(_, path)) = pat.kind; | |
262 | if !matches!(path.res, Res::SelfTy { .. } | Res::Def(DefKind::TyParam, _)); | |
263 | if cx.typeck_results().pat_ty(pat) == cx.tcx.type_of(impl_id); | |
264 | if let [first, ..] = path.segments; | |
265 | if let Some(hir_id) = first.hir_id; | |
266 | then { | |
267 | span_lint(cx, cx.tcx.hir().span(hir_id)); | |
268 | } | |
f20569fa XL |
269 | } |
270 | } | |
271 | ||
272 | extract_msrv_attr!(LateContext); | |
273 | } | |
274 | ||
275 | #[derive(Default)] | |
276 | struct SkipTyCollector { | |
277 | types_to_skip: Vec<HirId>, | |
278 | } | |
279 | ||
280 | impl<'tcx> Visitor<'tcx> for SkipTyCollector { | |
94222f64 XL |
281 | fn visit_infer(&mut self, inf: &hir::InferArg) { |
282 | self.types_to_skip.push(inf.hir_id); | |
283 | ||
284 | walk_inf(self, inf); | |
285 | } | |
f20569fa XL |
286 | fn visit_ty(&mut self, hir_ty: &hir::Ty<'_>) { |
287 | self.types_to_skip.push(hir_ty.hir_id); | |
288 | ||
17df50a5 | 289 | walk_ty(self, hir_ty); |
f20569fa | 290 | } |
f20569fa XL |
291 | } |
292 | ||
f20569fa XL |
293 | fn span_lint(cx: &LateContext<'_>, span: Span) { |
294 | span_lint_and_sugg( | |
295 | cx, | |
296 | USE_SELF, | |
297 | span, | |
298 | "unnecessary structure name repetition", | |
299 | "use the applicable keyword", | |
300 | "Self".to_owned(), | |
301 | Applicability::MachineApplicable, | |
302 | ); | |
303 | } | |
304 | ||
136023e0 XL |
305 | fn lint_path_to_variant(cx: &LateContext<'_>, path: &Path<'_>) { |
306 | if let [.., self_seg, _variant] = path.segments { | |
307 | let span = path | |
308 | .span | |
309 | .with_hi(self_seg.args().span_ext().unwrap_or(self_seg.ident.span).hi()); | |
310 | span_lint(cx, span); | |
f20569fa XL |
311 | } |
312 | } |