Skip to content

Commit b90fc5e

Browse files
author
Michael Wright
committed
Fix #2894 
1 parent 06d6710 commit b90fc5e

File tree

5 files changed

+303
-116
lines changed

5 files changed

+303
-116
lines changed

‎clippy_lints/src/use_self.rs

Lines changed: 108 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
1-
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
2-
use rustc::hir::*;
3-
use rustc::hir::intravisit::{walk_path, NestedVisitorMap, Visitor};
41
use crate::utils::{in_macro, span_lint_and_then};
2+
use rustc::hir::intravisit::{walk_path, walk_ty, NestedVisitorMap, Visitor};
3+
use rustc::hir::*;
4+
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
5+
use rustc::ty;
56
use syntax::ast::NodeId;
7+
use syntax::symbol::keywords;
68
use syntax_pos::symbol::keywords::SelfType;
79

810
/// **What it does:** Checks for unnecessary repetition of structure name when a
@@ -49,13 +51,93 @@ impl LintPass for UseSelf {
4951

5052
const SEGMENTS_MSG: &str = "segments should be composed of at least 1 element";
5153

54+
fn span_use_self_lint(cx: &LateContext, path: &Path) {
55+
span_lint_and_then(cx, USE_SELF, path.span, "unnecessary structure name repetition", |db| {
56+
db.span_suggestion(path.span, "use the applicable keyword", "Self".to_owned());
57+
});
58+
}
59+
60+
struct TraitImplTyVisitor<'a, 'tcx: 'a> {
61+
cx: &'a LateContext<'a, 'tcx>,
62+
type_walker: ty::walk::TypeWalker<'tcx>,
63+
}
64+
65+
impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
66+
fn visit_ty(&mut self, t: &'tcx Ty) {
67+
let trait_ty = self.type_walker.next();
68+
if let TyPath(QPath::Resolved(_, path)) = &t.node {
69+
let impl_is_self_ty = if let def::Def::SelfTy(..) = path.def {
70+
true
71+
} else {
72+
false
73+
};
74+
if !impl_is_self_ty {
75+
let trait_is_self_ty = if let Some(ty::TyParam(ty::ParamTy { name, .. })) = trait_ty.map(|ty| &ty.sty) {
76+
*name == keywords::SelfType.name().as_str()
77+
} else {
78+
false
79+
};
80+
if trait_is_self_ty {
81+
span_use_self_lint(self.cx, path);
82+
}
83+
}
84+
}
85+
walk_ty(self, t)
86+
}
87+
88+
fn nested_visit_map<'this>(&'this mut self) -> NestedVisitorMap<'this, 'tcx> {
89+
NestedVisitorMap::None
90+
}
91+
}
92+
93+
fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
94+
cx: &'a LateContext<'a, 'tcx>,
95+
impl_item: &ImplItem,
96+
impl_decl: &'tcx FnDecl,
97+
impl_trait_ref: &ty::TraitRef,
98+
) {
99+
let trait_method = cx
100+
.tcx
101+
.associated_items(impl_trait_ref.def_id)
102+
.find(|assoc_item| {
103+
assoc_item.kind == ty::AssociatedKind::Method
104+
&& cx
105+
.tcx
106+
.hygienic_eq(impl_item.ident, assoc_item.ident, impl_trait_ref.def_id)
107+
})
108+
.expect("impl method matches a trait method");
109+
110+
let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
111+
let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);
112+
113+
let output_ty = if let FunctionRetTy::Return(ty) = &impl_decl.output {
114+
Some(&**ty)
115+
} else {
116+
None
117+
};
118+
119+
for (impl_ty, trait_ty) in impl_decl
120+
.inputs
121+
.iter()
122+
.chain(output_ty)
123+
.zip(trait_method_sig.inputs_and_output)
124+
{
125+
let mut visitor = TraitImplTyVisitor {
126+
cx,
127+
type_walker: trait_ty.walk(),
128+
};
129+
130+
visitor.visit_ty(&impl_ty);
131+
}
132+
}
133+
52134
impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
53135
fn check_item(&mut self, cx: &LateContext<'a, 'tcx>, item: &'tcx Item) {
54136
if in_macro(item.span) {
55137
return;
56138
}
57139
if_chain! {
58-
if let ItemImpl(.., ref item_type, ref refs) = item.node;
140+
if let ItemImpl(.., item_type, refs) = &item.node;
59141
if let Ty_::TyPath(QPath::Resolved(_, ref item_path)) = item_type.node;
60142
then {
61143
let parameters = &item_path.segments.last().expect(SEGMENTS_MSG).args;
@@ -67,13 +149,32 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
67149
} else {
68150
true
69151
};
152+
70153
if should_check {
71154
let visitor = &mut UseSelfVisitor {
72155
item_path,
73156
cx,
74157
};
75-
for impl_item_ref in refs {
76-
visitor.visit_impl_item(cx.tcx.hir.impl_item(impl_item_ref.id));
158+
let impl_def_id = cx.tcx.hir.local_def_id(item.id);
159+
let impl_trait_ref = cx.tcx.impl_trait_ref(impl_def_id);
160+
161+
if let Some(impl_trait_ref) = impl_trait_ref {
162+
for impl_item_ref in refs {
163+
let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
164+
if let ImplItemKind::Method(MethodSig{ decl: impl_decl, .. }, impl_body_id)
165+
= &impl_item.node {
166+
check_trait_method_impl_decl(cx, impl_item, impl_decl, &impl_trait_ref);
167+
let body = cx.tcx.hir.body(*impl_body_id);
168+
visitor.visit_body(body);
169+
} else {
170+
visitor.visit_impl_item(impl_item);
171+
}
172+
}
173+
} else {
174+
for impl_item_ref in refs {
175+
let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
176+
visitor.visit_impl_item(impl_item);
177+
}
77178
}
78179
}
79180
}
@@ -89,9 +190,7 @@ struct UseSelfVisitor<'a, 'tcx: 'a> {
89190
impl<'a, 'tcx> Visitor<'tcx> for UseSelfVisitor<'a, 'tcx> {
90191
fn visit_path(&mut self, path: &'tcx Path, _id: NodeId) {
91192
if self.item_path.def == path.def && path.segments.last().expect(SEGMENTS_MSG).ident.name != SelfType.name() {
92-
span_lint_and_then(self.cx, USE_SELF, path.span, "unnecessary structure name repetition", |db| {
93-
db.span_suggestion(path.span, "use the applicable keyword", "Self".to_owned());
94-
});
193+
span_use_self_lint(self.cx, path);
95194
}
96195

97196
walk_path(self, path);

‎tests/ui/methods.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
#![warn(clippy, clippy_pedantic, option_unwrap_used)]
55
#![allow(blacklisted_name, unused, print_stdout, non_ascii_literal, new_without_default,
66
new_without_default_derive, missing_docs_in_private_items, needless_pass_by_value,
7-
default_trait_access)]
7+
default_trait_access, use_self)]
88

99
use std::collections::BTreeMap;
1010
use std::collections::HashMap;

‎tests/ui/methods.stderr

Lines changed: 1 addition & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -1,47 +1,3 @@
1-
error: unnecessary structure name repetition
2-
--> $DIR/methods.rs:21:29
3-
|
4-
21 | pub fn add(self, other: T) -> T { self }
5-
| ^ help: use the applicable keyword: `Self`
6-
|
7-
= note: `-D use-self` implied by `-D warnings`
8-
9-
error: unnecessary structure name repetition
10-
--> $DIR/methods.rs:21:35
11-
|
12-
21 | pub fn add(self, other: T) -> T { self }
13-
| ^ help: use the applicable keyword: `Self`
14-
15-
error: unnecessary structure name repetition
16-
--> $DIR/methods.rs:25:25
17-
|
18-
25 | fn eq(&self, other: T) -> bool { true } // no error, private function
19-
| ^ help: use the applicable keyword: `Self`
20-
21-
error: unnecessary structure name repetition
22-
--> $DIR/methods.rs:27:26
23-
|
24-
27 | fn sub(&self, other: T) -> &T { self } // no error, self is a ref
25-
| ^ help: use the applicable keyword: `Self`
26-
27-
error: unnecessary structure name repetition
28-
--> $DIR/methods.rs:27:33
29-
|
30-
27 | fn sub(&self, other: T) -> &T { self } // no error, self is a ref
31-
| ^ help: use the applicable keyword: `Self`
32-
33-
error: unnecessary structure name repetition
34-
--> $DIR/methods.rs:28:21
35-
|
36-
28 | fn div(self) -> T { self } // no error, different #arguments
37-
| ^ help: use the applicable keyword: `Self`
38-
39-
error: unnecessary structure name repetition
40-
--> $DIR/methods.rs:29:25
41-
|
42-
29 | fn rem(self, other: T) { } // no error, wrong return type
43-
| ^ help: use the applicable keyword: `Self`
44-
451
error: defining a method called `add` on this type; consider implementing the `std::ops::Add` trait or choosing a less ambiguous name
462
--> $DIR/methods.rs:21:5
473
|
@@ -78,30 +34,6 @@ error: methods called `new` usually return `Self`
7834
|
7935
= note: `-D new-ret-no-self` implied by `-D warnings`
8036

81-
error: unnecessary structure name repetition
82-
--> $DIR/methods.rs:80:24
83-
|
84-
80 | fn new() -> Option<V<T>> { None }
85-
| ^^^^ help: use the applicable keyword: `Self`
86-
87-
error: unnecessary structure name repetition
88-
--> $DIR/methods.rs:84:19
89-
|
90-
84 | type Output = T;
91-
| ^ help: use the applicable keyword: `Self`
92-
93-
error: unnecessary structure name repetition
94-
--> $DIR/methods.rs:85:25
95-
|
96-
85 | fn mul(self, other: T) -> T { self } // no error, obviously
97-
| ^ help: use the applicable keyword: `Self`
98-
99-
error: unnecessary structure name repetition
100-
--> $DIR/methods.rs:85:31
101-
|
102-
85 | fn mul(self, other: T) -> T { self } // no error, obviously
103-
| ^ help: use the applicable keyword: `Self`
104-
10537
error: called `map(f).unwrap_or(a)` on an Option value. This can be done more directly by calling `map_or(a, f)` instead
10638
--> $DIR/methods.rs:104:13
10739
|
@@ -251,24 +183,6 @@ error: called `map(f).unwrap_or_else(g)` on a Result value. This can be done mor
251183
174 | | );
252184
| |_________________^
253185

254-
error: unnecessary structure name repetition
255-
--> $DIR/methods.rs:200:24
256-
|
257-
200 | fn filter(self) -> IteratorFalsePositives {
258-
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
259-
260-
error: unnecessary structure name repetition
261-
--> $DIR/methods.rs:204:22
262-
|
263-
204 | fn next(self) -> IteratorFalsePositives {
264-
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
265-
266-
error: unnecessary structure name repetition
267-
--> $DIR/methods.rs:224:32
268-
|
269-
224 | fn skip(self, _: usize) -> IteratorFalsePositives {
270-
| ^^^^^^^^^^^^^^^^^^^^^^ help: use the applicable keyword: `Self`
271-
272186
error: called `filter(p).next()` on an `Iterator`. This is more succinctly expressed by calling `.find(p)` instead.
273187
--> $DIR/methods.rs:234:13
274188
|
@@ -343,12 +257,6 @@ error: called `is_some()` after searching an `Iterator` with rposition. This is
343257
276 | | ).is_some();
344258
| |______________________________^
345259

346-
error: unnecessary structure name repetition
347-
--> $DIR/methods.rs:290:21
348-
|
349-
290 | fn new() -> Foo { Foo }
350-
| ^^^ help: use the applicable keyword: `Self`
351-
352260
error: use of `unwrap_or` followed by a function call
353261
--> $DIR/methods.rs:308:22
354262
|
@@ -527,5 +435,5 @@ error: used unwrap() on an Option value. If you don't want to handle the None ca
527435
|
528436
= note: `-D option-unwrap-used` implied by `-D warnings`
529437

530-
error: aborting due to 70 previous errors
438+
error: aborting due to 55 previous errors
531439

0 commit comments

Comments
 (0)