-
Notifications
You must be signed in to change notification settings - Fork 659
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
Incorrect issues classification when using baseline #9180
Comments
Hey @mindriven, can you reproduce the issue on https://psalm.dev ? |
not possible due to necessity of baseline |
So it appears baseline is not used by language server at all. Baseline processing is done in |
Ok so looking this over I can't use It looks like I can parse the baseline from https://github.com/tm1000/psalm/blob/b14ed73c0b3717513ac5ec421672a5c03699b028/src/Psalm/Internal/Cli/Psalm.php#L650-653 and perhaps ignore what I need from there. |
The addition for this is in my WIP PR () but you can see the changes here: tm1000@27c0caf This adds one new cli arg of
I had to do a lot of funky psalm suppressions because it didn't like when I had to pass-by reference but I have to do that to keep the tracking the same but other than that it's fine |
thanks for taking care of it so fast @tm1000. |
@mindriven the release is pending because I want to write tests and writing tests for amp (which is what the language server used for non-blocking php) is not for the faint of heart. That said you can use/test the wip branch. It's up to date with 5.6.0. The power of GIT allows me to track what the community does while I slowly work on this and because @weirdan and @orklah are kind enough to tag me in most LSP changes it makes it easy to follow composer.json:
vscode settings:
Note that |
Hey, I gave it a spin. I do not see the "Problems" in vscode - that's the good news. Bad news is that I have this stack-trace in my output window. Not sure what to make of that.
|
Is the server crashing or is that just a message in the output window. A lot of those messages are caught and the server still functions as intended |
Seems like it's still working despite the error message in logs (so far tested only in sample repository here https://github.com/mindriven/vscode-psalm-testing\). Can I please ask what's your best estimation, when will it be officially released? BTW, while testing it I have found another bug (#9187) |
I have resolved the bug mentioned above, "Undefined array key 1". composer update will pull it in.
I just have to get time to work on tests. |
Ok, found some more. Now it works with baseline and I can confirm stacktrace is gone, but for this behavior to be consistent inline suppression should have the same effect as using a baseline. |
I found these snippets: https://psalm.dev/r/4973cb067b<?php
/**
* @psalm-suppress MixedAssignment
*/
$a = $something_that_does_not_exist;
/**
* @psalm-suppress all
*/
$b = "asas";
|
You need to separate the suppression with a comma. It works fine here Also level 1 will always emit even if suppressed. If your intention is to use suppression don't use level 1 I think we are getting into the nitty gritty of how psalm works and we've deviated from this post. I merely work on the LSP. The LSP shares all code bases and paths with the standard client. I use suppression daily (a lot in fact) and I know it works. It is ignored if you set Psalms error level to 1 |
I wanted to test it in a bigger repository, but I must still be doing something wrong... just there might be another issue at play. I generated a baseline with psalm --set-baseline=psalm-baseline.xml file was created, config updated. But not all classes that are in the folders configured for analysis were included in the baseline file. For example, I have 2 classes in same folder Good news is that VSCode behaves as I would expect in the But when I open I have to get this working for all the repository. Any ideas how can I debug this? |
I managed to nail it down. It possibly is another issue at play here. #9197 |
Dead code detection is an optional feature. If it's not enabled when you generate the baseline, the errors it produces won't get included in the baseline. Make sure you generate your baseline with |
I would also like to see this functionality released officially |
Same Problem. Waiting for official release. |
@Itatsi @achanizbekov there were several related but distinct things discussed here. Please clarify what you're referring to. |
@weirdan I'm referring to the initially reported issue: Violations recorded in the baseline should not be reported by language server, to enable smooth IDE integration(reported by @mindriven). |
Hey @weirdan I see you've been releasing new versions is a lightning speed, that's really impressive! |
Propose a pull request fixing this issue 😃
That many patch releases in this small amount of time are mostly an unfortunate consequence of us taking a risky decision to enable JIT by default. It's not something we usually do. Most of them are fixing crashes that affected many people, thus the urgency. |
... or just wait 2 weeks :-) Cool, looking forward to giving next release a spin! |
Hi!
It seems that this switch statement
psalm/src/Psalm/Internal/LanguageServer/LanguageServer.php
Lines 376 to 384 in bfce3a4
is causing a confusion when using baseline. IDE will show "infos" as errors.
Here the originally created issue in repo of vscode plugin psalm/psalm-vscode-plugin#221.
In comments of this issue there are screenshots and link to minimal sample repo that I used to visualize the problem.
FYI @tm1000, @weirdan
The text was updated successfully, but these errors were encountered: