intellij-rust / intellij-rust

Rust plugin for the IntelliJ Platform
https://intellij-rust.github.io
MIT License
4.54k stars 380 forks source link

Adding parameter when changing signature ignores impl #7078

Open krojew opened 3 years ago

krojew commented 3 years ago

Environment

Problem description

After adding a parameter using "change signature", corresponding impls are not updated. It's possible this only happens when adding non-trivial parameters.

Steps to reproduce

  1. Create a trait with a fn.
  2. Add an implementation (hard to tell if placing it in another file makes a difference).
  3. Change signature of the fn and add a paramter.
  4. No parameter is added in the impl.
Kobzol commented 3 years ago

Hi, I could not reproduce it. Could you please post a code snippet where this happens? Thank you.

krojew commented 3 years ago

Unfortunately, I can't since it's both complex and proprietary. I can give more detailed steps of what exactly happened in my case:

  1. A struct was added in crate A.
  2. Crate B, which depends on A, had a trait with some fn.
  3. Without importing the new struct, I used "change signature" on the fn and added a parameter of that struct type.
  4. CLion complained the fn will cease being visible and pointed to call sites.
  5. I ignored the error, since it was obviously false, and clicked "do refactor" to proceed.
  6. The new parameter was added to the fn (as expected), call sites got an empty comma added (as expected?), but one actual impl (residing in crate B) was not updated.

This process was a bit more convoluted than my trivial example, but it might provide some more insight.

Kobzol commented 3 years ago

I see. If you click on the green arrow in the left column next to the method that you have refactored inside the trait in crate B (it should invoke find implementations), does it find the method impl that was missed by the refactoring?

krojew commented 3 years ago

Yes, it jumps to the impl without problems.

Kobzol commented 3 years ago

I tried a few situations where the impl was in another file and I added an unresolved type from a different crate, but so far I couldn't reproduce it.

I wonder where that error about the function ceasing to be visible came from, that might be the source of the problem. So it might be related to visibility conflict detection.

Another reason might be that if the impl had a wrong signature before the refactoring (for example a different amount of parameters), the refactoring would not touch it.

I'm not sure how to reproduce the error though. @Undin any ideas?

krojew commented 3 years ago

That's unfortunate. I am able to reproduce it 100% of times.

image

krojew commented 3 years ago

Seems like I can replicate the problem also with primitive parameters, and in various places. In fact, each time I use "change signature" I get this error. This makes even more confusing why you can't replicate it.

Kobzol commented 3 years ago

Do you see any IDE errors when you run the refactoring? (a red exclamation mark at the bottom right corner).

It would be great if you could find a minimal reproducible example that would demonstrate the error.

krojew commented 3 years ago

I don't see a red exclamation mark, but I found something interesting - after I open the refactoring window, change nothing and click preview, I get the visibility problems error. Maybe there are some logs I can attach?