Incorrect errors reported by Metals #4652
Comments
|
Thanks for reporting! Does it help to run clean compile in this case (Metals: Recompile workspace)? There is a possibility that the Zinc integration in Bloop or even Zinc itself has an issue. Is the repo publicly available and could we try to reproduce your steps? |
|
Hi @tgodzik, it's not public yet but will be by the end of the year. I didn't try |
|
Managed it again, this time after a(nother) compiler crash. This time I used commit hash for compiling code (for when the repo is public): libs/zio-lsp/shared/src/main/scala/zio/lsp/LspProtocol.scala:51 has a comment noting how to trigger the compiler crash (though I'm not sure it's related to this issue). |
|
ZIO stuff is usually pretty complicated (I found a number of Metals bugs in the codebases related to the ecosystem), so this might be something to take a closer look into with a proper reproduction. You could also try using sbt BSP with |
|
sorry, I missed this reply. "ZIO stuff is usually pretty complicated" might explain why we're constantly experiencing compiler crashes with scala 3 where others seem to have limited issues... though I'm not sure I entirely agree with this characterisation. Most of the apparent complexity (zio-sql aside) in the zio ecosystem falls into a few basic patterns which pretty much amount to sacrificing ourselves to the boilerplate gods to provide a better user experience. It's confusing at first glance but once you wrap your head around the patterns it all becomes much clearer. There's a video here explaining some of them https://www.youtube.com/watch?v=48fpPffgnMo - some of it's pretty basic (e.g. I'll give the SBT build server a go and see if it happens again - I generally prefer to use bloop because it's heaps faster (particularly when I'm using sbt in the command line at the same time!). |
|
I just meant that I feel like ZIO is using the type system heavily more than some other libraries (just my opinion) and that's why we're seeing more bugs pop up there. That's a good thing though 😅 since we want those bugs to show up sooner or later. Thanks for the video, I will take a look! |
|
notice some more weirdness - I was getting a type error showing up with a very unexpected type in metals. There was no error for this in SBT. Unfortunately I didn't capture this immediately, but here's a screenshot from a little down the debugging path. This
I tried "clean compile workspace" which didn't change anything. Restarting the build server cleared the error but now I'm not getting hover at This was using bloop still - I swapped the build server over to sbt, but this got stuck importing the build. I restarted vscode and got this error: details shows a semanticdb issue used swapped back to bloop; semanticdb issues disappeared. still no hover for perhaps this should be a new issue but it at least started off as incorrect errors reported by metals :) commit a3ebb92c45c9cff45ebd716a4c808c4c692276e8 |
|
semanticdb error might pop up if the sbt-metals plugin has issues enabling it. Might be good to see what the target has defined there (you can click on the name to see summary) and whether it's not flaky As for the hover, that might be an issue in the compiler, since insert type annotation also uses it. There should be some more info in |
|
Still getting the same issues. Now using sbt as the build server. cc @ckipp01 there are a bunch of errors like this, all the same. The type did have 4 params, but I deleted one. |
|
Ah thanks for the extra info @robmwalsh. This one I'm very interested in, because if sbt is reporting an incorrect error over BSP (or stale I guess) but just a normal sbt compile works fine, then something is definitely off. Is there any way at all you can minimize this so that I can reproduce it? I'm assuming this is still in the private repo? |
|
It's very inconsistent. If I restart the build server, all is well again. |
Sure, but even before that it's the actual build server that let's Metals know if we should clear them or not. Metals really doesn't do much here apart from forwarding the information that the build server provides. For example you can see the format (that you're probably familiar with in LSP) in BSP here. If we get a |
|
I did suffer another compiler crash today so I guess that's a possibility, but I deleted the param after the crash, so I don't think its likely. |
|
I've created the BSP and LSP traces, I'll have a look for the reset flag in both next time it happens - thanks for the suggestion |
It seems it is, but not always when it needs to. I had this happen again and did some digging through the bsp logs. Here's the reported diagnostic (when last reported): and then... Clean Compile workspace triggered this: which reset the diagnostics as expected. The interesting thing here is that the build target is home/rob/code/lsp/#zioLspJS - this is a cross-built project. There's another build target |
|
Ah, I think part of this is probably related to scalameta/metals-feature-requests#13 which we've honestly just never tackled yet. sbt projects suffer from this more than Mill projects for example. From looking at the trace it's not fully clear why there wouldn't be more diagnostics being reported from the build server, but as you can imagine, there's not a lot Metals can do if the build server isn't reporting anything. It's also sort of impossible to really debug this on our side without some type of reproduction, especially seeing that the issue seems to stem from the actual build server not reporting what is expected here. |
|
Perhaps the problem is in BSP4J since I'm getting stale diagnostics with both bloop & sbt. I'll swap back to bloop and see if the bsp trace shows similar behaviour next time. |
|
Bloop is now consistently reporting errors that just shouldn't exist.
All of this compiles fine with SBT. All the errors are from another project (in the same repo) that this project depends on. None of these errors appear when using SBT as a build server, but I have had this problem with SBT as a build server. So now we're down to some combination of:
It's all just too complicated 🥲 |
This is the most likely scenario especially with different Scala versions. Native/JS should work mostly ok, though might also suffer from some undercompilation somtimes. I am curious, how is the other project using the same sources? Would be able to reproduce some skeleton of the dependencies between projects look like? |
|
@tgodzik https://github.com/robmwalsh/buildsketch Here's a rather cut down build.sbt file and the plugins etc that I'm using What's not typical is it's all scala 3 at this stage (3.2.1 at the moment, though I have been using nightlies at times), using JS and JVM platforms (no native, though I may add this at some point).There's no platform-specific code in these projects, so all the sources are in shared folders. I'm not sure how much the crossbuild is affecting this particular issue - a plausible scenario is that a build server reports errors against a JS build target, then tells it to clear errors for a JVM build target. I've got no idea how metals translates bsp diagnostics to lsp diagnostics, so I guess anything is possible, but I'd expect this would be more likely to cause issues where there's platform specific sources. |
|
Thanks for the repro! I will take a look at it! |
|
I'm running into similar issues on a large project. I can't share it unfortunately, but I can describe what I observe. Let's say I do an initial compile, no errors reported but 50 "unused import" messages. I then start removing those imports one by one. If I do it slowly, no problem. But if I do it fast (which is normal), I'm modifying code while the project is compiling. When I do that, at some point it becomes out of sync: some imports that I removed are still reported as problems. From that point, I can't find a nice way to "recover" except doing a clean compile which I don't want to do because it takes 10 minutes on that project. Is there a way to make it synced again? Sbt or bloop compile in a separate terminal doesn't have any impact on what I see in VS Code. What's worse is that I can't even clean compile only the current module, VS Code only offers to do it on the whole project. Note that my example is trivial but it gets much worse when it's errors, and I'm constantly triggering it because the project is slow to compile so I'm constantly modifying code while it compiles. Thanks in advance if you have any suggestions 😄 |
This should be relatively easy to implement and expose to users. I was thinking about doing it few times when bloop went crazy, but unfortunately I'm a power user who uses bloop cli so I don't need UI for invalidating one module :| |
Even if you use bloop cli (which I tried too), how does that get synced with the UI? (= how does the UI stop showing your code as red, when you re-trigger a build with bloop? Whatever I did in bloop did not impact VS Code so I'm curious.) |
The trick to using bloop is... Don't! Using sbt as the BSP server has been a far better experience for me (except for bsp compilation blocking sbt cli) |
|
@ghostdogpr
Unfortunately, this is true. Using sbt definitely saves you from pain which bloop is inflicting 😢 But bloop is really fast and I can't resist that speed. |
|
@kpodsiad I did that but the change&save didn't trigger a re-sync of the incorrect "problems", they just stayed there. |
|
This is a curious case, the issue might be connected to the fact that we don't run multiple compiles at the same time. Do you have automatic save enabled maybe? |
I do save instantly after each modification. |
|
Because the project is large and takes time to compile, I'm always modifying files while compilation is going on (there are a lot of messages about deduplicating compilation in the output). It does look like this is what's causing Metals to be out of sync. |
@tgodzik are any diagnostics reported from the presentation compiler? these may clash with those being reported via bsp. @ghostdogpr I've found that restarting the build server is the only way to clear out all the errors and get things to resync. sometimes you need to be more forceful with restarting ( |
We don't report errors from the presentation compiler to limit the amount of errors while writing code, that might not compiler. We rely on errors from the compiler via BSP.
I want to look into this, since this seems a bigger issue than I thought. |
|
This is a repro capture I did... happens mostly using mill-bsp: |
|
Metals needs to get empty diagnostics for a file to clear the errors, so this might be an issue with Mill. Otherwise we would need to keep that information inside Metals, which I would prefer to avoid, since that might cause some unexpected issues. If you do another error inside that file does it remove the old one? Does the situation with stale errors happens often when fixing all of them? |
|
Yes, forcing another error somewhere else does indeed clean the previous one that was stuck. Here's metals.log file from the moment I switched to mill-bsp, forced an error, removed that error by commenting it out and making another error that cleaned the previous one: |
|
I don't see any empty diagnostics being sent, so I think that's the issue with stale errors. When everything is fixed Mill should send empty diagnostics with reset set to true. |
|
Worth opening an issue on Mill about this error staying stuck? |
|
Looks like this is now fixed in Mill! 🎉 |
Describe the bug
Metals sometimes reports "errors" that sbt doesn't. Note that this doesn't only happen with a project that compiles cleanly, but it's most obvious then.
(apologies for the pic, couldn't seem to get this in text form)
from metals.log
happily I had lsp tracing enabled at the time - here's the diagnostics info being sent
lsp trace.zip
Expected behavior
errors reported in metals should be the same as those reported by sbt. If sbt can compile the whole project, there should be no reported errors.
Operating system
Linux
Editor/Extension
VS Code
Version of Metals
0.11.9
Extra context or search terms
I've listed linux as the OS - this is ubuntu running in WSL.
Answers to @ckipp01's questions from discord
Are you using Bloop or are you using sbt as a build server?
Bloop
Does it happen after you change your build definition?
No changes to build definition for a few days
Does the build import work after that?
Didn't change the build so didn't try it
Just understanding your flow and what you're hitting on is helpful
I added some helper methods to some case objects that serve as type tags to safely recover the type information they're holding on to. This took a few attempts to get right.
As part of this I used "rename symbol" (
JsonRpctoJsonRpcMethod). It looks like most of the errors relate to this except for those in "HelloWorldSpec.scala" which is a nothing file from the g8 template which has never changed. I guess it's possible that the text doc sync is not handling this properly, resulting in metals having out of date file contents. I don't recall if I had recently renamed anything last time I encountered this.The text was updated successfully, but these errors were encountered: