Skip to content

Commit 5bb52c4

Browse files
author
Michael Wright
committed
Fix use_self regressions
1 parent 847e4dc commit 5bb52c4

File tree

3 files changed

+105
-65
lines changed

3 files changed

+105
-65
lines changed

‎clippy_lints/src/use_self.rs

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use rustc::hir::*;
44
use rustc::lint::{LateContext, LateLintPass, LintArray, LintPass};
55
use rustc::ty;
66
use syntax::ast::NodeId;
7-
use syntax::symbol::keywords;
87
use syntax_pos::symbol::keywords::SelfType;
98

109
/// **What it does:** Checks for unnecessary repetition of structure name when a
@@ -58,26 +57,29 @@ fn span_use_self_lint(cx: &LateContext, path: &Path) {
5857
}
5958

6059
struct TraitImplTyVisitor<'a, 'tcx: 'a> {
60+
item_path: &'a Path,
6161
cx: &'a LateContext<'a, 'tcx>,
62-
type_walker: ty::walk::TypeWalker<'tcx>,
62+
trait_type_walker: ty::walk::TypeWalker<'tcx>,
63+
impl_type_walker: ty::walk::TypeWalker<'tcx>,
6364
}
6465

6566
impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
6667
fn visit_ty(&mut self, t: &'tcx Ty) {
67-
let trait_ty = self.type_walker.next();
68+
let trait_ty = self.trait_type_walker.next();
69+
let impl_ty = self.impl_type_walker.next();
70+
6871
if let TyKind::Path(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()
72+
if self.item_path.def == path.def {
73+
let is_self_ty = if let def::Def::SelfTy(..) = path.def {
74+
true
7775
} else {
7876
false
7977
};
80-
if trait_is_self_ty {
78+
79+
if !is_self_ty && impl_ty != trait_ty {
80+
// The implementation and trait types don't match which means that
81+
// the concrete type was specified by the implementation but
82+
// it didn't use `Self`
8183
span_use_self_lint(self.cx, path);
8284
}
8385
}
@@ -92,6 +94,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TraitImplTyVisitor<'a, 'tcx> {
9294

9395
fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
9496
cx: &'a LateContext<'a, 'tcx>,
97+
item_path: &'a Path,
9598
impl_item: &ImplItem,
9699
impl_decl: &'tcx FnDecl,
97100
impl_trait_ref: &ty::TraitRef,
@@ -110,24 +113,30 @@ fn check_trait_method_impl_decl<'a, 'tcx: 'a>(
110113
let trait_method_sig = cx.tcx.fn_sig(trait_method.def_id);
111114
let trait_method_sig = cx.tcx.erase_late_bound_regions(&trait_method_sig);
112115

116+
let impl_method_def_id = cx.tcx.hir.local_def_id(impl_item.id);
117+
let impl_method_sig = cx.tcx.fn_sig(impl_method_def_id);
118+
let impl_method_sig = cx.tcx.erase_late_bound_regions(&impl_method_sig);
119+
113120
let output_ty = if let FunctionRetTy::Return(ty) = &impl_decl.output {
114121
Some(&**ty)
115122
} else {
116123
None
117124
};
118125

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-
{
126+
for (impl_decl_ty, (impl_ty, trait_ty)) in impl_decl.inputs.iter().chain(output_ty).zip(
127+
impl_method_sig
128+
.inputs_and_output
129+
.iter()
130+
.zip(trait_method_sig.inputs_and_output),
131+
) {
125132
let mut visitor = TraitImplTyVisitor {
126133
cx,
127-
type_walker: trait_ty.walk(),
134+
item_path,
135+
trait_type_walker: trait_ty.walk(),
136+
impl_type_walker: impl_ty.walk(),
128137
};
129138

130-
visitor.visit_ty(&impl_ty);
139+
visitor.visit_ty(&impl_decl_ty);
131140
}
132141
}
133142

@@ -163,7 +172,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for UseSelf {
163172
let impl_item = cx.tcx.hir.impl_item(impl_item_ref.id);
164173
if let ImplItemKind::Method(MethodSig{ decl: impl_decl, .. }, impl_body_id)
165174
= &impl_item.node {
166-
check_trait_method_impl_decl(cx, impl_item, impl_decl, &impl_trait_ref);
175+
check_trait_method_impl_decl(cx, item_path, impl_item, impl_decl, &impl_trait_ref);
167176
let body = cx.tcx.hir.body(*impl_body_id);
168177
visitor.visit_body(body);
169178
} else {

‎tests/ui/use_self.rs

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
1-
2-
31
#![warn(use_self)]
42
#![allow(dead_code)]
53
#![allow(should_implement_trait)]
6-
#![allow(boxed_local)]
7-
84

95
fn main() {}
106

@@ -68,9 +64,10 @@ mod lifetimes {
6864
}
6965
}
7066

67+
#[allow(boxed_local)]
7168
mod traits {
7269

73-
#![cfg_attr(feature = "cargo-clippy", allow(boxed_local))]
70+
use std::ops::Mul;
7471

7572
trait SelfTrait {
7673
fn refs(p1: &Self) -> &Self;
@@ -104,6 +101,14 @@ mod traits {
104101
}
105102
}
106103

104+
impl Mul for Bad {
105+
type Output = Bad;
106+
107+
fn mul(self, rhs: Bad) -> Bad {
108+
rhs
109+
}
110+
}
111+
107112
#[derive(Default)]
108113
struct Good;
109114

@@ -128,6 +133,14 @@ mod traits {
128133
}
129134
}
130135

136+
impl Mul for Good {
137+
type Output = Self;
138+
139+
fn mul(self, rhs: Self) -> Self {
140+
rhs
141+
}
142+
}
143+
131144
trait NameTrait {
132145
fn refs(p1: &u8) -> &u8;
133146
fn ref_refs<'a>(p1: &'a &'a u8) -> &'a &'a u8;
@@ -162,7 +175,7 @@ mod traits {
162175
impl Clone for Good {
163176
fn clone(&self) -> Self {
164177
// Note: Not linted and it wouldn't be valid
165-
// because "can't use `Self` as a constructor`
178+
// because "can't use `Self` as a constructor`"
166179
Good
167180
}
168181
}

‎tests/ui/use_self.stderr

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

99
error: unnecessary structure name repetition
10-
--> $DIR/use_self.rs:16:13
10+
--> $DIR/use_self.rs:12:13
1111
|
12-
16 | Foo {}
12+
12 | Foo {}
1313
| ^^^ help: use the applicable keyword: `Self`
1414

1515
error: unnecessary structure name repetition
16-
--> $DIR/use_self.rs:18:22
16+
--> $DIR/use_self.rs:14:22
1717
|
18-
18 | fn test() -> Foo {
18+
14 | fn test() -> Foo {
1919
| ^^^ help: use the applicable keyword: `Self`
2020

2121
error: unnecessary structure name repetition
22-
--> $DIR/use_self.rs:19:13
22+
--> $DIR/use_self.rs:15:13
2323
|
24-
19 | Foo::new()
24+
15 | Foo::new()
2525
| ^^^^^^^^ help: use the applicable keyword: `Self`
2626

2727
error: unnecessary structure name repetition
28-
--> $DIR/use_self.rs:24:25
28+
--> $DIR/use_self.rs:20:25
2929
|
30-
24 | fn default() -> Foo {
30+
20 | fn default() -> Foo {
3131
| ^^^ help: use the applicable keyword: `Self`
3232

3333
error: unnecessary structure name repetition
34-
--> $DIR/use_self.rs:25:13
34+
--> $DIR/use_self.rs:21:13
3535
|
36-
25 | Foo::new()
36+
21 | Foo::new()
3737
| ^^^^^^^^ help: use the applicable keyword: `Self`
3838

3939
error: unnecessary structure name repetition
40-
--> $DIR/use_self.rs:87:22
40+
--> $DIR/use_self.rs:84:22
4141
|
42-
87 | fn refs(p1: &Bad) -> &Bad {
42+
84 | fn refs(p1: &Bad) -> &Bad {
4343
| ^^^ help: use the applicable keyword: `Self`
4444

4545
error: unnecessary structure name repetition
46-
--> $DIR/use_self.rs:87:31
46+
--> $DIR/use_self.rs:84:31
4747
|
48-
87 | fn refs(p1: &Bad) -> &Bad {
48+
84 | fn refs(p1: &Bad) -> &Bad {
4949
| ^^^ help: use the applicable keyword: `Self`
5050

5151
error: unnecessary structure name repetition
52-
--> $DIR/use_self.rs:91:37
52+
--> $DIR/use_self.rs:88:37
5353
|
54-
91 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
54+
88 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
5555
| ^^^ help: use the applicable keyword: `Self`
5656

5757
error: unnecessary structure name repetition
58-
--> $DIR/use_self.rs:91:53
58+
--> $DIR/use_self.rs:88:53
5959
|
60-
91 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
60+
88 | fn ref_refs<'a>(p1: &'a &'a Bad) -> &'a &'a Bad {
6161
| ^^^ help: use the applicable keyword: `Self`
6262

6363
error: unnecessary structure name repetition
64-
--> $DIR/use_self.rs:95:30
64+
--> $DIR/use_self.rs:92:30
6565
|
66-
95 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
66+
92 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
6767
| ^^^ help: use the applicable keyword: `Self`
6868

6969
error: unnecessary structure name repetition
70-
--> $DIR/use_self.rs:95:43
70+
--> $DIR/use_self.rs:92:43
7171
|
72-
95 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
72+
92 | fn mut_refs(p1: &mut Bad) -> &mut Bad {
7373
| ^^^ help: use the applicable keyword: `Self`
7474

7575
error: unnecessary structure name repetition
76-
--> $DIR/use_self.rs:99:28
76+
--> $DIR/use_self.rs:96:28
7777
|
78-
99 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
78+
96 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
7979
| ^^^ help: use the applicable keyword: `Self`
8080

8181
error: unnecessary structure name repetition
82-
--> $DIR/use_self.rs:99:46
82+
--> $DIR/use_self.rs:96:46
8383
|
84-
99 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
84+
96 | fn nested(_p1: Box<Bad>, _p2: (&u8, &Bad)) {
8585
| ^^^ help: use the applicable keyword: `Self`
8686

8787
error: unnecessary structure name repetition
88-
--> $DIR/use_self.rs:102:20
88+
--> $DIR/use_self.rs:99:20
89+
|
90+
99 | fn vals(_: Bad) -> Bad {
91+
| ^^^ help: use the applicable keyword: `Self`
92+
93+
error: unnecessary structure name repetition
94+
--> $DIR/use_self.rs:99:28
95+
|
96+
99 | fn vals(_: Bad) -> Bad {
97+
| ^^^ help: use the applicable keyword: `Self`
98+
99+
error: unnecessary structure name repetition
100+
--> $DIR/use_self.rs:100:13
89101
|
90-
102 | fn vals(_: Bad) -> Bad {
91-
| ^^^ help: use the applicable keyword: `Self`
102+
100 | Bad::default()
103+
| ^^^^^^^^^^^^ help: use the applicable keyword: `Self`
92104

93105
error: unnecessary structure name repetition
94-
--> $DIR/use_self.rs:102:28
106+
--> $DIR/use_self.rs:105:23
95107
|
96-
102 | fn vals(_: Bad) -> Bad {
97-
| ^^^ help: use the applicable keyword: `Self`
108+
105 | type Output = Bad;
109+
| ^^^ help: use the applicable keyword: `Self`
98110

99111
error: unnecessary structure name repetition
100-
--> $DIR/use_self.rs:103:13
112+
--> $DIR/use_self.rs:107:27
101113
|
102-
103 | Bad::default()
103-
| ^^^^^^^^^^^^ help: use the applicable keyword: `Self`
114+
107 | fn mul(self, rhs: Bad) -> Bad {
115+
| ^^^ help: use the applicable keyword: `Self`
116+
117+
error: unnecessary structure name repetition
118+
--> $DIR/use_self.rs:107:35
119+
|
120+
107 | fn mul(self, rhs: Bad) -> Bad {
121+
| ^^^ help: use the applicable keyword: `Self`
104122

105-
error: aborting due to 17 previous errors
123+
error: aborting due to 20 previous errors
106124

0 commit comments

Comments
 (0)