bugfix: Don't republish old errors on successful compilation #2427
Conversation
We need to report all problems if a BSP client just connected to the server and never compiled. `reportAllPreviousProblems` will be set to true in that case. However, if previously we had an error, which was reverted and got back to the state before the error, we will get a successfull noop compilation, which will not produce any problems. In this case we don't want to republish everything again, since diagnostics will contain the error which was reverted. Connected to VirtusLab/scala-cli#2226 where CLI always connected anew.
tgodzik
force-pushed
the
fix-republish-diags
branch
from
2fc06f5 to
0727b16
Compare
| * In this case we don't want to republish everything again, since diagnostics will contain the | ||
| * error which was reverted. |
There was a problem hiding this comment.
I am not sure to understand this. If the previous was a noop compilation, why would diagnostics contain any error? In other words, I think a noop compilation should clear the diagnostics that were stored anywhere.
There was a problem hiding this comment.
We can also have a noop compilation that was an actual error. If nothing changed we would still have the same exact errors, so no need to recompile.
There was a problem hiding this comment.
Okay but if it is a success, we should first clear problemsPerFiles so that it does not contain any error anymore.
There was a problem hiding this comment.
That would make sense, but then when we have reportAllPreviousProblems set (this is set when first compilation for a target and client is run) then we would not report old warnings, since we would clean it. We can try to clean just the errors out, but we don't know if some warnings weren't generate along those errors.
So in essence, we don't know what exact problems were generated in the compilation before and those are the ones we should clear.
It's much easier to just special case this situation and just report everything in the previous successful compilation. And while I agree with you that something is wrong here, I can't figure out a more sensible solution, all alternative paths seem more complicated and carry their own drawbacks. This might be due to the fact that noop compilation is a bit of a trick, which is not really something that a lot of other tools using zinc do.
That's the whole issue actually. If a new client connects (which Scala CLI does every time it's run) and noop compilation is run then if we clear the errors nothing will be reported the new client. |
We need to report all problems if a BSP client just connected to the server and never compiled.
reportAllPreviousProblemswill be set to true in that case.However, if previously we had an error, which was reverted and got back to the state before the error, we will get a successfull noop compilation, which will not produce any problems.
In this case we don't want to republish everything again, since diagnostics will contain the error which was reverted.
Connected to VirtusLab/scala-cli#2226 where CLI always connected anew.