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

Best effort compilation gives false errors #6628

Open
brndt opened this issue Jul 25, 2024 · 15 comments
Open

Best effort compilation gives false errors #6628

brndt opened this issue Jul 25, 2024 · 15 comments
Assignees
Labels
bug Something that is making a piece of functionality unusable

Comments

@brndt
Copy link

brndt commented Jul 25, 2024

Describe the bug

If I make any changes in the code and break compilation, the compiler gives random totally non-related errors.

For example:

be_test.mov

Expected behavior

No response

Operating system

macOS

Editor/Extension

VS Code

Version of Metals

1.3.4

Extra context or search terms

No response

@tgodzik
Copy link
Contributor

tgodzik commented Jul 26, 2024

Thanks for reporting! I wonder if this might be connected to the fact that we now compile all modules and sometimes this might lead to worse errors in the dependent modules.

Best effort itself will not produce any new diagnostics.

@tgodzik tgodzik self-assigned this Jul 29, 2024
@filipwiech
Copy link
Contributor

filipwiech commented Jul 30, 2024

I can confirm the same behaviour in my project. 👍

In fact, it also appears that even if I make the code compile successfully again (as confirmed by the SBT build), but not by reverting the file to the initial state (e.g. with undo), but by further modifying the implementation to fix it, then the bogus error diagnostics are still visible. 🤔

To add additional data point: I just had a case where I got weird (and incorrect) error diagnostics shown in the edited file (some imported types suddenly cannot be found), even though my changes did compile successfully from the get go, build was not broken at any point, I have only removed an unused implicit definition (although I did get an unused import warning from the compiler in the SBT afterwards if that matters, albeit I have no -Werror enabled, so the build was fine still). 😮 Had also a similar scenario after removing unused import from a completely different file (sudden errors about missing unrelated types).

Scala 3.5.0-RC6, Bloop 1.6.0, SBT 1.10.1, Metals 1.3.4+23-c744a47f-SNAPSHOT

@filipwiech
Copy link
Contributor

filipwiech commented Jul 31, 2024

One more update (sorry for a lot of messages, but I hope it will be useful info for you): while usually SBT build gives expected results (either a successful compilation or an error that is real and makes sense), I have regularly observed that after a while (of Metals compiling the project due to the editor changes) SBT seems to deteriorate as well and starts spewing weird messages (cannot find imports/types, broken build). After a clean build it starts working again and is good for some time. 🙁

At this point Metals is almost unusable for me with a Scala 3.5 project. @tgodzik Is there any way to disable the Best Effort Compilation for the time being? 😕

@tgodzik
Copy link
Contributor

tgodzik commented Jul 31, 2024

We might have to add a setting for that if it turns out to be the problem, but I can't imagine why would this happen.

I will check it out today.

@tgodzik tgodzik added the bug Something that is making a piece of functionality unusable label Jul 31, 2024
@tgodzik
Copy link
Contributor

tgodzik commented Jul 31, 2024

I will have to disable it, thanks for catching this before the actual release of 3.5.0! I will have to do a new release with this probably tomorrow.

@jchyb is on vacation till 16th, and I think I will have to ask him to take a look again. @brndt is it ok if I share your codebase with him once he's available?

@brndt
Copy link
Author

brndt commented Jul 31, 2024

I will have to disable it, thanks for catching this before the actual release of 3.5.0! I will have to do a new release with this probably tomorrow.

@jchyb is on vacation till 16th, and I think I will have to ask him to take a look again. @brndt is it ok if I share your codebase with him once he's available?

Yes of course.

@gchudnov
Copy link
Contributor

gchudnov commented Aug 25, 2024

wondering if there is any update?

with the release of 3.5 tried to enable best effort compiling on Metals version: 1.3.5+68-5948c4fe-SNAPSHOT by enabling -Dmetals.enable-best-effort=true, but looks like bumped into a similar issue, so disabled it... (to be honest not sure if the issue is actually metals or scala related)

@jchyb
Copy link
Contributor

jchyb commented Aug 26, 2024

Still working on this. It looks like the issue is moreso on the build server side, so the best effort compilation should still work on 3.5 once we fix this issue.

@filipwiech
Copy link
Contributor

filipwiech commented Aug 26, 2024

@jchyb Thank you for looking into this. 👍 Just in case, a related issue #6627 caused by the Best Effort compilation was closed earlier, but that one seems to be somewhat distinct, as it impacts macros specifically (seemingly in the test scope only), even in a completely clean project build with no other errors. I have no idea if the root cause is the same, but wanted to highlight it, as it might have got lost in the meantime. 🙂

@bloznelis
Copy link

Can confirm that this issue persists in metals 1.3.5 with Scala 3.5.0 when enabling best effort compilation with -Dmetals.enable-best-effort=true.

From what I gathered, there's only one type of error—"value foo is not a member of bar"—and is triggered only for the imports. Hope this helps to track it down.

@tgodzik tgodzik moved this from Triage to In progress in Metals Issue Board Sep 11, 2024
@tgodzik
Copy link
Contributor

tgodzik commented Nov 22, 2024

One issue still existing (not sure if caused by best effort) :

  • changing a name of a class isn't picked up in downstream projects

@tgodzik
Copy link
Contributor

tgodzik commented Nov 22, 2024

My guess is that we don't remove early compilation in the client dir, but not sure if that is the only issue.

@bloznelis
Copy link

To be honest, I cannot reproduce this issue anymore with:

  • Metals 1.4.0
  • Scala 3.5.2
  • Bloop 2.0.3

@jchyb
Copy link
Contributor

jchyb commented Nov 25, 2024

The main issue reported here got fixed in bloop v2.0.4. Depending on the dependency graph between files, and which files we recompile, it might not even show in earlier versions, even when -Dmetals.enable-best-effort=true is used (which is how we missed it before). Looks like @tgodzik found an additional problem that we will be fixing shortly

@brndt
Copy link
Author

brndt commented Nov 25, 2024

Copy paste from discord canal:

I've tried to use best effort compilation with the latest version of Bloop and it's unusable :( - having a lot of random and out of sync errors. For example, removing class that's used by the others is no longer detected by the compiler, a vice-versa, like you can see on the screenshot.

Screenshot_2024-11-21_at_15 59 14

and here it can't even detect 'succeed' method of ZIO

Screenshot_2024-11-21_at_16 03 30

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something that is making a piece of functionality unusable
Projects
Status: In progress
Development

No branches or pull requests

6 participants