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

different results depending on --threads option #6939

Closed
dzentota opened this issue Nov 17, 2021 · 10 comments
Closed

different results depending on --threads option #6939

dzentota opened this issue Nov 17, 2021 · 10 comments
Labels

Comments

@dzentota
Copy link

We have a big project with a legacy code. Psalm produces different results depending on the --threads option we provide.
Initially, we've added all the found issues to the baseline (using the default number of threads, 16 in my case). Then I try to run:

first run

./vendor/bin/psalm.phar --no-cache --threads 10
------------------------------
47 errors found
------------------------------
68200 other issues found.
You can display them with --show-info=true

second run

./vendor/bin/psalm.phar --no-cache --threads 12
------------------------------
186 errors found
------------------------------
68189 other issues found.
You can display them with --show-info=true

third run

./vendor/bin/psalm.phar --no-cache --threads 10
------------------------------
36 errors found
------------------------------
68200 other issues found.
You can display them with --show-info=true
------------------------------

fourth run

./vendor/bin/psalm.phar --no-cache --threads 10
------------------------------
36 errors found
------------------------------
68200 other issues found.
You can display them with --show-info=true
------------------------------

Note:
As you can see only 3) and 4) have the same results, while the same number of threads was in 1) 3) 4)

@psalm-github-bot
Copy link

Hey @dzentota, can you reproduce the issue on https://psalm.dev ?

@dzentota
Copy link
Author

dzentota commented Nov 17, 2021

Possibly related to #5200 and #4662

@orklah orklah added the bug label Nov 18, 2021
@danog
Copy link
Collaborator

danog commented Nov 19, 2021

This has forced our project to switch to the much slower --threads=1 in our CI pipelines, on multithreaded scans psalm would either miss errors or find weird false positives.

@orklah
Copy link
Collaborator

orklah commented Nov 19, 2021

Ideally, we would need a reproducible case with a very small project, but this will be tough due to the nature of the issue.

@MelechMizrachi
Copy link
Contributor

We had something very similar to this issue recently when upgrading Psalm. Turned out for us to be caused by a return typehint stating nullable, whereas the docblock return was a constant glob with no null value, and the method in question also did not return null at any point. It failed silently for us when --threads=1, but showed up with different error counts with >=2, but usually hovering around ~300. For us the errors revolved around class not found for the class that had those constants (in the below example the equivalent of Test). I made a small snippet that can repro the fact the warning doesn't show up, but not that it causes the class to be false positived as not found.

https://psalm.dev/r/44db48d2c8

I only bring this up because those errors you're seeing might lead you to something silly like this in your codebase where this wasn't an issue in a previous Psalm version but may have started after an upgrade and is only visible when using multi-threading. Hopefully it points you to the right solution.

That said it is curious that the function gets the warning in my example, but the class method does not.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/44db48d2c8
<?php

class Test 
{
	public const TEST_1 = '1';
	public const TEST_2 = '2';
	public const TEST_3 = '3';
	public const TEST_4 = '4';
	public const TEST_5 = '5';
}

class Test2
{
    /**
     * @return Test::TEST_*
     */
    public function test(): ?string {
        return Test::TEST_2;
    }
}

/**
 * @return Test::TEST_*
 */
function test(): ?string {
    return Test::TEST_1;
}

$test2 = new Test2();

$test2->test();
test();
Psalm output (using commit e83ac65):

INFO: LessSpecificReturnType - 23:12 - The inferred return type ''1'' for test is more specific than the declared return type ''1'|'2'|'3'|'4'|'5'|null'

@danog
Copy link
Collaborator

danog commented Nov 23, 2022

This class of issues should be fixed with Psalm v5.

@SCIF
Copy link
Contributor

SCIF commented Jan 8, 2023

@danog , which one are you referring to? All issues related to --threads? Just got different results using v5.4.0:

php ./vendor/bin/psalm --no-cache --threads=1:

------------------------------
                              
       No errors found!       
                              
------------------------------
3236 other issues found.

php ./vendor/bin/psalm --no-cache --threads=2:

------------------------------
1 errors found
------------------------------
3241 other issues found.

php ./vendor/bin/psalm --no-cache --threads=4:

------------------------------
1 errors found
------------------------------
3243 other issues found.

A few observations:

  1. Total issue counter differs for 2 and 4 threads. But it differs exactly for the number of threads (3243-3241=2, 4-2=2)
  2. The error found by psalm was a deprecation notice present in the file 8 times, but in baseline file it was just 7 occurances of them. Looks like multithreaded run shows more correct results (which is weird and not expected tbh)

@danog
Copy link
Collaborator

danog commented Jan 8, 2023

This is probably just a missing issue key, not a big deal/bug thankfully.
Mind sending over the exact issue text?

@danog
Copy link
Collaborator

danog commented Jan 8, 2023

Actually let's open a new issue for this one, because this issue was probably originally caused by (now-fixed) mutability bugs, not by missing issue keys.

@danog danog closed this as completed Jan 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants