Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Class renaming is broken when you have other tab opened with import #6478

Open
brndt opened this issue Jun 3, 2024 · 9 comments
Open

Class renaming is broken when you have other tab opened with import #6478

brndt opened this issue Jun 3, 2024 · 9 comments
Assignees
Labels
bloop Bloop related tickets bug Something that is making a piece of functionality unusable

Comments

@brndt
Copy link

brndt commented Jun 3, 2024

Describe the bug

  1. Rename class A name to B
  2. Everything works
  3. Open class C that have imported class B
  4. Try to rename class B to class D
  5. Sometimes is broken as you can see in the video
    https://github.com/scalameta/metals/assets/21007364/49fe71de-7583-4ee9-b2a1-0cc7aaffa339

Maybe related to: #5759

Expected behavior

No response

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

v1.3.1

Extra context or search terms

No response

@kasiaMarek kasiaMarek added bug Something that is making a piece of functionality unusable investigation needed labels Jun 4, 2024
@tgodzik tgodzik moved this to Triage in Metals Issue Board Jun 12, 2024
@tgodzik
Copy link
Contributor

tgodzik commented Oct 31, 2024

Getting back to it think the issue here is that sometimes the newly renamed class is not compiled and I see old classfiles together with new semanticdb, which contains old information.

Not sure yet if the problem is Bloop or Zinc. But the workaround is to force the compilation for the file again by adding some lines etc.

tgodzik added a commit to tgodzik/bloop that referenced this issue Oct 31, 2024
Previously, we would ask zinc to compile even a file that was removed and I think this was causing scalameta/metals#6478

Now, we invalidated all changed sources, but only compile new ones.
@tgodzik
Copy link
Contributor

tgodzik commented Oct 31, 2024

Looks like scalacenter/bloop#2490 could help from my testing.

@brndt
Copy link
Author

brndt commented Oct 31, 2024

@tgodzik thanks for your reply!
How can I test it by myself?

@brndt
Copy link
Author

brndt commented Oct 31, 2024

Also, I have a question: Does renaming a class produce the same effect as moving it to another folder? Because I have the same experience for both cases, if I make one or another change (renaming or repackaging) in the main folder, only the references to this class from the test are starting to break. That's why I think it could be related to this [issue]. (#5759)

tgodzik added a commit to tgodzik/bloop that referenced this issue Nov 1, 2024
Previously, we would ask zinc to compile even a file that was removed and I think this was causing scalameta/metals#6478

Now, we invalidated all changed sources, but only compile new ones.
tgodzik added a commit to tgodzik/bloop that referenced this issue Nov 1, 2024
Previously, we would ask zinc to compile even a file that was removed and I think this was causing scalameta/metals#6478

Now, we invalidated all changed sources, but only compile new ones.
@tgodzik tgodzik moved this from Triage to In progress in Metals Issue Board Nov 1, 2024
@tgodzik
Copy link
Contributor

tgodzik commented Nov 1, 2024

@tgodzik thanks for your reply! How can I test it by myself?

You would need to checkout my branch, do sbt publishLocal, change metals.bloopVersion in metals settings and restart the build server (you should be prompted automatically)

Also, I have a question: Does renaming a class produce the same effect as moving it to another folder? Because I have the same experience for both cases, if I make one or another change (renaming or repackaging) in the main folder, only the references to this class from the test are starting to break. That's why I think it could be related to this [issue]. (#5759)

I think so, but might be worth checking out. If we try to compile a file that no longer exist then something weird might happen. My hope is that this is the cause of many weird metals bugs, but I will need to see.

tgodzik added a commit to tgodzik/bloop that referenced this issue Nov 1, 2024
Previously, we would ask zinc to compile even a file that was removed and I think this was causing scalameta/metals#6478

Now, we invalidated all changed sources, but only compile new ones.
@brndt
Copy link
Author

brndt commented Nov 1, 2024

renaming_file.mp4

I didn't work for me :(

You can reproduce my case by opening CreateArticle and CreateArticleSpec classes and then renaming (o moving to another folder) CreateArticle class. I gave you an access to spursarmy-main repo.

@tgodzik
Copy link
Contributor

tgodzik commented Nov 1, 2024

That looks like another issue than the one I found. It would stop renaming altogether for me after a couple of renames, here it looks like it looses a couple of references. I might have misunderstood the issue in your first video then. I will take a look at what you posted now.

@tgodzik
Copy link
Contributor

tgodzik commented Nov 1, 2024

Curiously, I see a different problem in my case:

Screencast.from.2024-11-01.18-52-58.mp4

Things are correctly renamed, but badly recompiled and it take several tries for it to compiled correctly.

I will see what that is about, but we might need to revisit this once the full release it out.

@brndt
Copy link
Author

brndt commented Nov 1, 2024

Yeah, sometimes it happens too - class is renamed, bad badly recompiled. But normally it's not renamed and badly recompiled.
I guess maybe if you try to rename it several times (or try different class from the main) you'll get my case too.

PS: I've tried renaming several times and can confirm that both cases happen. Correct renaming with bad recompilation and incorrect renaming with bad recompilation.

@tgodzik tgodzik added bloop Bloop related tickets and removed investigation needed labels Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bloop Bloop related tickets bug Something that is making a piece of functionality unusable
Projects
Status: In progress
Development

No branches or pull requests

3 participants