rust-clippy icon indicating copy to clipboard operation
rust-clippy copied to clipboard

Fixes for `from_over_into` and `clone_on_copy` conflict and lead to compiler error

Open saites opened this issue 1 year ago • 2 comments

Summary

The fix for from_over_into fails if the Into implementation includes a self.<expr>.clone() if the cloned type implements Copy. The fix for clone_on_copy keeps self, but the fix for from_over_into means self is no longer valid. Using #[allow(...)] to apply the suggestions serially (in either order) works fine.

Reproducer

Here's a simple example that triggers the error:

struct Foo(i64); 
impl Into<i64> for Foo {
    fn into(self) -> i64 {
        self.0.clone()
    }
}

I expected to see the code transformed into this:

struct Foo(i64); 
impl From<Foo> for i64 {
    fn from(val: Foo) -> Self {
        val.0
    }
}

Instead, it produces invalid code that results in the following compiler error:

struct Foo(i64); 
impl From<Foo> for i64 {
    fn from(val: Foo) -> Self {
        self.0
    }
}
error[E0424]: expected value, found module `self`
    --> example/src/foo.rs:1000:9
     |
999  |     fn from(val: Foo) -> Self {
     |        ---- this function doesn't have a `self` parameter
1000 |         self.0
     |         ^^^^ `self` value is a keyword only available in methods with a `self` parameter
     |
help: add a `self` receiver parameter to make the associated `fn` a method
     |
999  |     fn from(&self, val: Foo) -> Self {
     |             ++++++

error: aborting due to 1 previous error

Version

rustc 1.81.0 (eeb90cda1 2024-09-04) binary: rustc commit-hash: eeb90cda1969383f56a2637cbd3037bdf598841c commit-date: 2024-09-04 host: x86_64-unknown-linux-gnu release: 1.81.0 LLVM version: 18.1.7

Additional Labels

@rustbot label +I-suggestion-causes-error

saites avatar Oct 14 '24 22:10 saites

This may be a bug that needs to be resolved by rustfix itself. I'll see if I can do a quick fix on the cargo repo, if that isn't possible we'll have to transfer this to their repo =^w^=

blyxyas avatar Oct 16 '24 15:10 blyxyas

We'll have to transfer this, as the Cargo bug fixing process requires an issue beforehand.

Seems that the issue is that the apply_suggestions function here only checks if the solution is the same as other applied (this includes the error message)

Thus, various solutions pertaining to the same range will overlap.

blyxyas avatar Oct 16 '24 17:10 blyxyas