Skip to content

Commit d4b388e

Browse files
committed
Add from_over_into replace for type in Self reference
1 parent 423f081 commit d4b388e

File tree

4 files changed

+108
-2
lines changed

4 files changed

+108
-2
lines changed

clippy_lints/src/from_over_into.rs

Lines changed: 74 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,8 @@ fn convert_to_from(
163163
let PatKind::Binding(.., self_ident, None) = input.pat.kind else { return None };
164164

165165
let from = snippet_opt(cx, self_ty.span)?;
166-
let into = snippet_opt(cx, target_ty.span)?;
166+
// If Self is used, it refers to `self_ty`, which is now out `from` snippet
167+
let into = replace_self(&snippet_opt(cx, target_ty.span)?, &from);
167168

168169
let mut suggestions = vec![
169170
// impl Into<T> for U -> impl From<T> for U
@@ -212,3 +213,75 @@ fn convert_to_from(
212213

213214
Some(suggestions)
214215
}
216+
217+
fn replace_self(input: &str, replace_with: &str) -> String {
218+
const SELF: &str = "Self";
219+
let mut chunks = input.split(SELF).peekable();
220+
if let Some(first) = chunks.next() {
221+
let mut last_ended_with_break = false;
222+
// Heuristic, we're making a guess that the expansion probably doesn't exceed `input.len() * 2`
223+
let mut output = String::with_capacity(input.len() * 2);
224+
if first.is_empty() || first.ends_with(word_break) {
225+
last_ended_with_break = true;
226+
}
227+
output.push_str(first);
228+
while let Some(val) = chunks.next() {
229+
let is_last = chunks.peek().is_none();
230+
if last_ended_with_break && is_last && val.is_empty() {
231+
output.push_str(replace_with);
232+
break;
233+
}
234+
let this_starts_with_break = val.starts_with(word_break);
235+
let this_ends_with_break = val.ends_with(word_break);
236+
if this_starts_with_break && last_ended_with_break {
237+
output.push_str(replace_with);
238+
} else {
239+
output.push_str(SELF);
240+
}
241+
output.push_str(val);
242+
last_ended_with_break = this_ends_with_break;
243+
}
244+
output
245+
} else {
246+
input.to_string()
247+
}
248+
}
249+
250+
#[inline]
251+
fn word_break(ch: char) -> bool {
252+
!ch.is_alphanumeric()
253+
}
254+
255+
#[cfg(test)]
256+
mod tests {
257+
use crate::from_over_into::replace_self;
258+
259+
#[test]
260+
fn replace_doesnt_touch_coincidental_self() {
261+
let input = "impl Into<SelfType> for String {";
262+
assert_eq!(input, &replace_self(input, "T"));
263+
}
264+
265+
#[test]
266+
fn replace_replaces_self() {
267+
let input = "impl Into<Self> for String {";
268+
assert_eq!("impl Into<String> for String {", &replace_self(input, "String"));
269+
}
270+
#[test]
271+
fn replace_replaces_self_many() {
272+
let input = "impl Into<Self<Self<SelfSelfSelfSelf>>> for Self {";
273+
assert_eq!(
274+
"impl Into<String<String<SelfSelfSelfSelf>>> for String {",
275+
&replace_self(input, "String")
276+
);
277+
}
278+
279+
#[test]
280+
fn replace_replaces_self_many_starts_ends_self() {
281+
let input = "Self impl Into<Self<Self<SelfSelf>>> for Self";
282+
assert_eq!(
283+
"String impl Into<String<String<SelfSelf>>> for String",
284+
&replace_self(input, "String")
285+
);
286+
}
287+
}

tests/ui/from_over_into.fixed

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,14 @@ impl Into<Opaque> for IntoOpaque {
8888
fn into(self) -> Opaque {}
8989
}
9090

91+
pub struct Lval<T>(T);
92+
93+
pub struct Rval<T>(T);
94+
95+
impl<T> From<Lval<T>> for Rval<Lval<T>> {
96+
fn from(val: Lval<T>) -> Self {
97+
Rval(val)
98+
}
99+
}
100+
91101
fn main() {}

tests/ui/from_over_into.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,4 +88,14 @@ impl Into<Opaque> for IntoOpaque {
8888
fn into(self) -> Opaque {}
8989
}
9090

91+
pub struct Lval<T>(T);
92+
93+
pub struct Rval<T>(T);
94+
95+
impl<T> Into<Rval<Self>> for Lval<T> {
96+
fn into(self) -> Rval<Self> {
97+
Rval(self)
98+
}
99+
}
100+
91101
fn main() {}

tests/ui/from_over_into.stderr

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,5 +71,18 @@ LL ~ fn from(val: Vec<T>) -> Self {
7171
LL ~ FromOverInto(val)
7272
|
7373

74-
error: aborting due to 5 previous errors
74+
error: an implementation of `From` is preferred since it gives you `Into<_>` for free where the reverse isn't true
75+
--> $DIR/from_over_into.rs:95:1
76+
|
77+
LL | impl<T> Into<Rval<Self>> for Lval<T> {
78+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
79+
|
80+
help: replace the `Into` implementation with `From<Lval<T>>`
81+
|
82+
LL ~ impl<T> From<Lval<T>> for Rval<Lval<T>> {
83+
LL ~ fn from(val: Lval<T>) -> Self {
84+
LL ~ Rval(val)
85+
|
86+
87+
error: aborting due to 6 previous errors
7588

0 commit comments

Comments
 (0)