Skip to content

Commit 1f2f50c

Browse files
committed
Fix many false negatives caused by autoderef
1 parent 0411edf commit 1f2f50c

File tree

3 files changed

+77
-39
lines changed

3 files changed

+77
-39
lines changed

clippy_lints/src/functions/misnamed_getters.rs

+21-23
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ use rustc_lint::LateContext;
66
use rustc_middle::ty;
77
use rustc_span::Span;
88

9+
use std::iter;
10+
911
use super::MISNAMED_GETTERS;
1012

1113
pub fn check_fn(
@@ -75,39 +77,35 @@ pub fn check_fn(
7577
}
7678
};
7779

78-
let ty = cx.typeck_results().expr_ty_adjusted(self_data);
79-
80-
let def = {
81-
let mut kind = ty.kind();
82-
loop {
83-
match kind {
84-
ty::Adt(def, _) => break def,
85-
ty::Ref(_, ty, _) => kind = ty.kind(),
86-
// We don't do tuples because the function name cannot be a number
87-
_ => return,
88-
}
89-
}
90-
};
91-
9280
let mut used_field = None;
9381
let mut correct_field = None;
94-
for f in def.all_fields() {
95-
if f.name.as_str() == name {
96-
correct_field = Some(f);
97-
}
98-
if f.name == used_ident.name {
99-
used_field = Some(f);
82+
let typeck_results = cx.typeck_results();
83+
for adjusted_type in iter::once(typeck_results.expr_ty(self_data))
84+
.chain(typeck_results.expr_adjustments(self_data).iter().map(|adj| adj.target))
85+
{
86+
let ty::Adt(def,_) = adjusted_type.kind() else {
87+
continue;
88+
};
89+
90+
for f in def.all_fields() {
91+
if f.name.as_str() == name {
92+
correct_field = Some(f);
93+
}
94+
if f.name == used_ident.name {
95+
used_field = Some(f);
96+
}
10097
}
10198
}
10299

103100
let Some(used_field) = used_field else {
104-
// FIXME: This can be reached if the field access uses autoderef.
105-
// `dec.all_fields()` should be replaced by something that uses autoderef on the unajusted type of `self_data`
101+
// Can happen if the field access is a tuple. We don't lint those because the getter name could not start with a number.
106102
return;
107103
};
108104

109105
let Some(correct_field) = correct_field else {
110-
return;
106+
// There is no field corresponding to the getter name.
107+
// FIXME: This can be a false positive if the correct field is reachable trought deeper autodereferences than used_field is
108+
return;
111109
};
112110

113111
if cx.tcx.type_of(used_field.did) == cx.tcx.type_of(correct_field.did) {

tests/ui/misnamed_getters.rs

+29-7
Original file line numberDiff line numberDiff line change
@@ -85,15 +85,37 @@ impl B {
8585
}
8686
}
8787

88-
struct C {
89-
inner: Box<A>,
88+
struct D {
89+
d: u8,
90+
inner: A,
9091
}
91-
impl C {
92-
unsafe fn a(&self) -> &u8 {
93-
&self.inner.b
92+
93+
impl core::ops::Deref for D {
94+
type Target = A;
95+
fn deref(&self) -> &A {
96+
&self.inner
9497
}
95-
unsafe fn a_mut(&mut self) -> &mut u8 {
96-
&mut self.inner.b
98+
}
99+
100+
impl core::ops::DerefMut for D {
101+
fn deref_mut(&mut self) -> &mut A {
102+
&mut self.inner
103+
}
104+
}
105+
106+
impl D {
107+
fn a(&self) -> &u8 {
108+
&self.b
109+
}
110+
fn a_mut(&mut self) -> &mut u8 {
111+
&mut self.b
112+
}
113+
114+
fn d(&self) -> &u8 {
115+
&self.b
116+
}
117+
fn d_mut(&mut self) -> &mut u8 {
118+
&mut self.b
97119
}
98120
}
99121

tests/ui/misnamed_getters.stderr

+27-9
Original file line numberDiff line numberDiff line change
@@ -127,22 +127,40 @@ LL | | }
127127
| |_____^
128128

129129
error: getter function appears to return the wrong field
130-
--> $DIR/misnamed_getters.rs:92:5
130+
--> $DIR/misnamed_getters.rs:107:5
131131
|
132-
LL | / unsafe fn a(&self) -> &u8 {
133-
LL | | &self.inner.b
134-
| | ------------- help: consider using: `&self.inner.a`
132+
LL | / fn a(&self) -> &u8 {
133+
LL | | &self.b
134+
| | ------- help: consider using: `&self.a`
135135
LL | | }
136136
| |_____^
137137

138138
error: getter function appears to return the wrong field
139-
--> $DIR/misnamed_getters.rs:95:5
139+
--> $DIR/misnamed_getters.rs:110:5
140140
|
141-
LL | / unsafe fn a_mut(&mut self) -> &mut u8 {
142-
LL | | &mut self.inner.b
143-
| | ----------------- help: consider using: `&mut self.inner.a`
141+
LL | / fn a_mut(&mut self) -> &mut u8 {
142+
LL | | &mut self.b
143+
| | ----------- help: consider using: `&mut self.a`
144+
LL | | }
145+
| |_____^
146+
147+
error: getter function appears to return the wrong field
148+
--> $DIR/misnamed_getters.rs:114:5
149+
|
150+
LL | / fn d(&self) -> &u8 {
151+
LL | | &self.b
152+
| | ------- help: consider using: `&self.d`
153+
LL | | }
154+
| |_____^
155+
156+
error: getter function appears to return the wrong field
157+
--> $DIR/misnamed_getters.rs:117:5
158+
|
159+
LL | / fn d_mut(&mut self) -> &mut u8 {
160+
LL | | &mut self.b
161+
| | ----------- help: consider using: `&mut self.d`
144162
LL | | }
145163
| |_____^
146164

147-
error: aborting due to 16 previous errors
165+
error: aborting due to 18 previous errors
148166

0 commit comments

Comments
 (0)