Skip to content

Commit f4c3d47

Browse files
tnielenstnielens
committed
use_self - fix issue with hir_ty_to_ty 
1 parent fc7ee75 commit f4c3d47

File tree

6 files changed

+112
-167
lines changed

6 files changed

+112
-167
lines changed

‎clippy_lints/src/use_self.rs

Lines changed: 56 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ declare_lint_pass!(UseSelf => [USE_SELF]);
5757

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

60-
fn span_lint<'tcx>(cx: &LateContext<'tcx>, span: Span) {
60+
fn span_lint(cx: &LateContext<'_>, span: Span) {
6161
span_lint_and_sugg(
6262
cx,
6363
USE_SELF,
@@ -99,12 +99,12 @@ fn span_lint_on_qpath_resolved<'tcx>(cx: &LateContext<'tcx>, qpath: &'tcx QPath<
9999
}
100100
}
101101

102-
struct ImplVisitor<'a, 'tcx> {
102+
struct BodyVisitor<'a, 'tcx> {
103103
cx: &'a LateContext<'tcx>,
104104
self_ty: Ty<'tcx>,
105105
}
106106

107-
impl<'a, 'tcx> ImplVisitor<'a, 'tcx> {
107+
impl<'a, 'tcx> BodyVisitor<'a, 'tcx> {
108108
fn check_trait_method_impl_decl(
109109
&mut self,
110110
impl_item: &ImplItem<'tcx>,
@@ -151,46 +151,13 @@ impl<'a, 'tcx> ImplVisitor<'a, 'tcx> {
151151
}
152152
}
153153

154-
impl<'a, 'tcx> Visitor<'tcx> for ImplVisitor<'a, 'tcx> {
154+
impl<'a, 'tcx> Visitor<'tcx> for BodyVisitor<'a, 'tcx> {
155155
type Map = Map<'tcx>;
156156

157157
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
158158
NestedVisitorMap::OnlyBodies(self.cx.tcx.hir())
159159
}
160160

161-
fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
162-
if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind {
163-
match path.res {
164-
def::Res::SelfTy(..) => {},
165-
_ => {
166-
match self.cx.tcx.hir().find(self.cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
167-
Some(Node::Expr(Expr {
168-
kind: ExprKind::Path(QPath::TypeRelative(_, _segment)),
169-
..
170-
})) => {
171-
// The following block correctly identifies applicable lint locations
172-
// but `hir_ty_to_ty` calls cause odd ICEs.
173-
//
174-
// if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
175-
// // FIXME: this span manipulation should not be necessary
176-
// // @flip1995 found an ast lowering issue in
177-
// // https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#L142-L162
178-
// span_lint_until_last_segment(self.cx, hir_ty.span, segment);
179-
// }
180-
},
181-
_ => {
182-
if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
183-
span_lint(self.cx, hir_ty.span)
184-
}
185-
},
186-
}
187-
},
188-
}
189-
}
190-
191-
walk_ty(self, hir_ty);
192-
}
193-
194161
fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) {
195162
fn expr_ty_matches<'tcx>(expr: &'tcx Expr<'tcx>, self_ty: Ty<'tcx>, cx: &LateContext<'tcx>) -> bool {
196163
let def_id = expr.hir_id.owner;
@@ -247,6 +214,52 @@ impl<'a, 'tcx> Visitor<'tcx> for ImplVisitor<'a, 'tcx> {
247214
}
248215
}
249216

217+
struct FnSigVisitor<'a, 'tcx> {
218+
cx: &'a LateContext<'tcx>,
219+
self_ty: Ty<'tcx>,
220+
}
221+
222+
impl<'a, 'tcx> Visitor<'tcx> for FnSigVisitor<'a, 'tcx> {
223+
type Map = Map<'tcx>;
224+
225+
fn nested_visit_map(&mut self) -> NestedVisitorMap<Self::Map> {
226+
NestedVisitorMap::None
227+
}
228+
229+
fn visit_ty(&mut self, hir_ty: &'tcx hir::Ty<'tcx>) {
230+
if let TyKind::Path(QPath::Resolved(_, path)) = hir_ty.kind {
231+
match path.res {
232+
def::Res::SelfTy(..) => {},
233+
_ => {
234+
match self.cx.tcx.hir().find(self.cx.tcx.hir().get_parent_node(hir_ty.hir_id)) {
235+
Some(Node::Expr(Expr {
236+
kind: ExprKind::Path(QPath::TypeRelative(_, segment)),
237+
..
238+
})) => {
239+
// The following block correctly identifies applicable lint locations
240+
// but `hir_ty_to_ty` calls cause odd ICEs.
241+
//
242+
if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
243+
// fixme: this span manipulation should not be necessary
244+
// @flip1995 found an ast lowering issue in
245+
// https://github.com/rust-lang/rust/blob/master/src/librustc_ast_lowering/path.rs#l142-l162
246+
span_lint_until_last_segment(self.cx, hir_ty.span, segment);
247+
}
248+
},
249+
_ => {
250+
if hir_ty_to_ty(self.cx.tcx, hir_ty) == self.self_ty {
251+
span_lint(self.cx, hir_ty.span)
252+
}
253+
},
254+
}
255+
},
256+
}
257+
}
258+
259+
walk_ty(self, hir_ty);
260+
}
261+
}
262+
250263
impl<'tcx> LateLintPass<'tcx> for UseSelf {
251264
fn check_impl_item(&mut self, cx: &LateContext<'tcx>, impl_item: &'tcx ImplItem<'_>) {
252265
if in_external_macro(cx.sess(), impl_item.span) {
@@ -270,7 +283,8 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
270283
// TODO: don't short-circuit upon lifetime parameters
271284
if should_check {
272285
let self_ty = hir_ty_to_ty(cx.tcx, hir_self_ty);
273-
let visitor = &mut ImplVisitor { cx, self_ty };
286+
let body_visitor = &mut BodyVisitor { cx, self_ty };
287+
let fn_sig_visitor = &mut FnSigVisitor { cx, self_ty };
274288

275289
let tcx = cx.tcx;
276290
let impl_def_id = tcx.hir().local_def_id(imp.hir_id);
@@ -279,11 +293,12 @@ impl<'tcx> LateLintPass<'tcx> for UseSelf {
279293
if let Some(impl_trait_ref) = impl_trait_ref;
280294
if let ImplItemKind::Fn(FnSig { decl: impl_decl, .. }, impl_body_id) = &impl_item.kind;
281295
then {
282-
visitor.check_trait_method_impl_decl(impl_item, impl_decl, impl_trait_ref);
296+
body_visitor.check_trait_method_impl_decl(impl_item, impl_decl, impl_trait_ref);
283297
let body = tcx.hir().body(*impl_body_id);
284-
visitor.visit_body(body);
298+
body_visitor.visit_body(body);
285299
} else {
286-
walk_impl_item(visitor, impl_item)
300+
walk_impl_item(body_visitor, impl_item);
301+
walk_impl_item(fn_sig_visitor, impl_item);
287302
}
288303
}
289304
}

‎tests/ui/use_self.fixed

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,14 @@ mod use_self {
1515
Self {}
1616
}
1717
fn test() -> Self {
18+
// FIXME: applicable here
1819
Foo::new()
1920
}
2021
}
2122

2223
impl Default for Foo {
23-
fn default() -> Self {
24+
// FIXME: applicable here
25+
fn default() -> Foo {
2426
// FIXME: applicable here
2527
Foo::new()
2628
}
@@ -212,7 +214,9 @@ mod rustfix {
212214
fn fun_1() {}
213215

214216
fn fun_2() {
217+
// FIXME: applicable here
215218
nested::A::fun_1();
219+
// FIXME: applicable here
216220
nested::A::A;
217221

218222
Self {};

‎tests/ui/use_self.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,13 @@ mod use_self {
1515
Foo {}
1616
}
1717
fn test() -> Foo {
18+
// FIXME: applicable here
1819
Foo::new()
1920
}
2021
}
2122

2223
impl Default for Foo {
24+
// FIXME: applicable here
2325
fn default() -> Foo {
2426
// FIXME: applicable here
2527
Foo::new()
@@ -212,7 +214,9 @@ mod rustfix {
212214
fn fun_1() {}
213215

214216
fn fun_2() {
217+
// FIXME: applicable here
215218
nested::A::fun_1();
219+
// FIXME: applicable here
216220
nested::A::A;
217221

218222
nested::A {};

‎tests/ui/use_self.stderr

Lines changed: 39 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
11
error: unnecessary structure name repetition
2-
--> $DIR/use_self.rs:14:21
2+
--> $DIR/use_self.rs:15:13
33
|
4-
LL | fn new() -> Foo {
5-
| ^^^ help: use the applicable keyword: `Self`
4+
LL | Foo {}
5+
| ^^^ help: use the applicable keyword: `Self`
66
|
77
= note: `-D clippy::use-self` implied by `-D warnings`
88

99
error: unnecessary structure name repetition
10-
--> $DIR/use_self.rs:15:13
10+
--> $DIR/use_self.rs:14:21
1111
|
12-
LL | Foo {}
13-
| ^^^ help: use the applicable keyword: `Self`
12+
LL | fn new() -> Foo {
13+
| ^^^ help: use the applicable keyword: `Self`
1414

1515
error: unnecessary structure name repetition
1616
--> $DIR/use_self.rs:17:22
@@ -19,122 +19,116 @@ LL | fn test() -> Foo {
1919
| ^^^ help: use the applicable keyword: `Self`
2020

2121
error: unnecessary structure name repetition
22-
--> $DIR/use_self.rs:23:25
23-
|
24-
LL | fn default() -> Foo {
25-
| ^^^ help: use the applicable keyword: `Self`
26-
27-
error: unnecessary structure name repetition
28-
--> $DIR/use_self.rs:94:24
22+
--> $DIR/use_self.rs:96:24
2923
|
3024
LL | fn bad(foos: &[Foo]) -> impl Iterator<Item = &Foo> {
3125
| ^^^ help: use the applicable keyword: `Self`
3226

3327
error: unnecessary structure name repetition
34-
--> $DIR/use_self.rs:109:13
28+
--> $DIR/use_self.rs:111:13
3529
|
3630
LL | TS(0)
3731
| ^^ help: use the applicable keyword: `Self`
3832

3933
error: unnecessary structure name repetition
40-
--> $DIR/use_self.rs:117:25
34+
--> $DIR/use_self.rs:120:17
4135
|
42-
LL | fn new() -> Foo {
43-
| ^^^ help: use the applicable keyword: `Self`
36+
LL | Foo {}
37+
| ^^^ help: use the applicable keyword: `Self`
4438
...
4539
LL | use_self_expand!(); // Should lint in local macros
4640
| ------------------- in this macro invocation
4741
|
4842
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
4943

5044
error: unnecessary structure name repetition
51-
--> $DIR/use_self.rs:118:17
45+
--> $DIR/use_self.rs:119:25
5246
|
53-
LL | Foo {}
54-
| ^^^ help: use the applicable keyword: `Self`
47+
LL | fn new() -> Foo {
48+
| ^^^ help: use the applicable keyword: `Self`
5549
...
5650
LL | use_self_expand!(); // Should lint in local macros
5751
| ------------------- in this macro invocation
5852
|
5953
= note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info)
6054

6155
error: unnecessary structure name repetition
62-
--> $DIR/use_self.rs:141:29
63-
|
64-
LL | fn bar() -> Bar {
65-
| ^^^ help: use the applicable keyword: `Self`
66-
67-
error: unnecessary structure name repetition
68-
--> $DIR/use_self.rs:142:21
56+
--> $DIR/use_self.rs:144:21
6957
|
7058
LL | Bar { foo: Foo {} }
7159
| ^^^ help: use the applicable keyword: `Self`
7260

7361
error: unnecessary structure name repetition
74-
--> $DIR/use_self.rs:153:21
62+
--> $DIR/use_self.rs:143:29
7563
|
76-
LL | fn baz() -> Foo {
77-
| ^^^ help: use the applicable keyword: `Self`
64+
LL | fn bar() -> Bar {
65+
| ^^^ help: use the applicable keyword: `Self`
7866

7967
error: unnecessary structure name repetition
80-
--> $DIR/use_self.rs:154:13
68+
--> $DIR/use_self.rs:156:13
8169
|
8270
LL | Foo {}
8371
| ^^^ help: use the applicable keyword: `Self`
8472

8573
error: unnecessary structure name repetition
86-
--> $DIR/use_self.rs:171:21
74+
--> $DIR/use_self.rs:155:21
75+
|
76+
LL | fn baz() -> Foo {
77+
| ^^^ help: use the applicable keyword: `Self`
78+
79+
error: unnecessary structure name repetition
80+
--> $DIR/use_self.rs:173:21
8781
|
8882
LL | let _ = Enum::B(42);
8983
| ^^^^ help: use the applicable keyword: `Self`
9084

9185
error: unnecessary structure name repetition
92-
--> $DIR/use_self.rs:172:21
86+
--> $DIR/use_self.rs:174:21
9387
|
9488
LL | let _ = Enum::C { field: true };
9589
| ^^^^ help: use the applicable keyword: `Self`
9690

9791
error: unnecessary structure name repetition
98-
--> $DIR/use_self.rs:173:21
92+
--> $DIR/use_self.rs:175:21
9993
|
10094
LL | let _ = Enum::A;
10195
| ^^^^ help: use the applicable keyword: `Self`
10296

10397
error: unnecessary structure name repetition
104-
--> $DIR/use_self.rs:218:13
98+
--> $DIR/use_self.rs:222:13
10599
|
106100
LL | nested::A {};
107101
| ^^^^^^^^^ help: use the applicable keyword: `Self`
108102

109103
error: unnecessary structure name repetition
110-
--> $DIR/use_self.rs:254:13
104+
--> $DIR/use_self.rs:258:13
111105
|
112106
LL | S {}
113107
| ^ help: use the applicable keyword: `Self`
114108

115109
error: unnecessary structure name repetition
116-
--> $DIR/use_self.rs:282:29
110+
--> $DIR/use_self.rs:287:13
117111
|
118-
LL | fn foo(value: T) -> Foo<T> {
119-
| ^^^^^^ help: use the applicable keyword: `Self`
112+
LL | Foo { value }
113+
| ^^^ help: use the applicable keyword: `Self`
120114

121115
error: unnecessary structure name repetition
122-
--> $DIR/use_self.rs:283:13
116+
--> $DIR/use_self.rs:286:29
123117
|
124-
LL | Foo { value }
125-
| ^^^ help: use the applicable keyword: `Self`
118+
LL | fn foo(value: T) -> Foo<T> {
119+
| ^^^^^^ help: use the applicable keyword: `Self`
126120

127121
error: unnecessary structure name repetition
128-
--> $DIR/use_self.rs:320:21
122+
--> $DIR/use_self.rs:324:21
129123
|
130124
LL | type From = T::From;
131125
| ^^^^^^^ help: use the applicable keyword: `Self`
132126

133127
error: unnecessary structure name repetition
134-
--> $DIR/use_self.rs:321:19
128+
--> $DIR/use_self.rs:325:19
135129
|
136130
LL | type To = T::To;
137131
| ^^^^^ help: use the applicable keyword: `Self`
138132

139-
error: aborting due to 21 previous errors
133+
error: aborting due to 20 previous errors
140134

0 commit comments

Comments
 (0)