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

False positive wrong type and inherit doc #5126

Closed
VincentLanglet opened this issue Jan 29, 2021 · 13 comments · Fixed by #6213
Closed

False positive wrong type and inherit doc #5126

VincentLanglet opened this issue Jan 29, 2021 · 13 comments · Fixed by #6213
Labels

Comments

@VincentLanglet
Copy link
Contributor

Hi @muglug @weirdan

Since fews days I got a weird issue with psalm

MethodSignatureMismatch - src/Admin/AbstractBaseAdmin.php:106:16 - Argument 1 of Sonata\AdminBundle\Admin\AbstractAdmin::setFilterTheme has wrong type 'array<array-key, mixed>', expecting 'array<array-key, string>' as defined by Sonata\AdminBundle\Admin\AdminInterface::setFilterTheme (see https://psalm.dev/042)
abstract class AbstractBaseAdmin extends AbstractAdmin

I'm using the last version of sonata, so you can see
https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Admin/AbstractAdmin.php#L2489
https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Admin/AdminInterface.php#L252

And I don't override the setFilterTheme method on my AbstractBaseAdmin.

  1. It seems like the typehint array is overriding the phpdoc of the Interface
  2. This method is not overriden in my class so I don't expect getting psalm-error from the vendor
  3. What is weirder is the fact I don't always get this error.
@psalm-github-bot
Copy link

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

@weirdan
Copy link
Collaborator

weirdan commented Jan 29, 2021

Is this method defined in any ancestors or interfaces implemented by AbstractAdmin (besides AdminInterface)?

@VincentLanglet
Copy link
Contributor Author

Is this method defined in any ancestors or interfaces implemented by AbstractAdmin (besides AdminInterface)?

  • In my codebase I never implement this method
  • In Sonata codebase, this method is only implemented in AbstractAdmin
  • In Sonata codebase, this method is only interface implemented in AdminInterface

I got sometimes the similar error with generateUrl
https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Admin/AbstractAdmin.php#L1184
https://github.com/sonata-project/SonataAdminBundle/blob/3.x/src/Admin/UrlGeneratorInterface.php#L57

@orklah
Copy link
Collaborator

orklah commented Jun 12, 2021

@VincentLanglet can you check if you use preloadClasses="true" in your psalm config somewhere? If so, this issue is a duplicate of #5626

@VincentLanglet
Copy link
Contributor Author

My config is

<?xml version="1.0"?>
<psalm
    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
    xmlns="https://getpsalm.org/schema/config"
    xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd"
    autoloader="bin/.phpunit/phpunit/vendor/autoload.php"
    cacheDirectory="/tmp/psalm"
    errorLevel="8"
    findUnusedPsalmSuppress="true"
    memoizeMethodCallResults="true"
    reportInfo="false"
    usePhpDocMethodsWithoutMagicCall="true"
>
    <projectFiles>
        <directory name="src"/>
        <directory name="tests"/>
        <ignoreFiles>
            <file name="src/Repository/.phpstorm.meta.php"/>
            <directory name="vendor"/>
        </ignoreFiles>
    </projectFiles>
    <plugins>
        <pluginClass class="Psalm\PhpUnitPlugin\Plugin"/>
        <pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin">
            <containerXml>var/cache/dev/srcAssuranceVie_KernelDevDebugContainer.xml</containerXml>
        </pluginClass>
        <pluginClass class="Weirdan\DoctrinePsalmPlugin\Plugin"/>
    </plugins>
    <fileExtensions>
        <extension name=".php"/>
    </fileExtensions>
    <stubs>
        <file name=".stubs/Mandrill.stub"/>
        <file name=".stubs/Mandrill_Messages.stub"/>
    </stubs>
</psalm>

@orklah
Copy link
Collaborator

orklah commented Jun 12, 2021

Doest it reproduce without stubs and without plugins?

@VincentLanglet
Copy link
Contributor Author

Doest it reproduce without stubs and without plugins?

Really hard to say.

I ran psalm with this config, got the error.
I deleted the cache and run without stub and plugin, no error.
I deleted the cache and run again with the whole config, no error.
And then played with the config, and ran again, still no error.

It occurs sometimes only and I cannot reproduce it.
But the error come back usually

@VincentLanglet
Copy link
Contributor Author

VincentLanglet commented Jul 31, 2021

@weirdan @orklah I have a reproducer (I hope you'll get the error too).

With the repository https://github.com/VincentLanglet/Twig-CS-Fixer,
Commit: b9c4203 (in case I update my code)

Running make psalm is giving me this output locally (but not in the CI):

vendor/bin/psalm
Scanning files...
Analyzing files...

E░░░░E░░░░░E░░░░░E░E░E░░░░E░░

ERROR: MethodSignatureMismatch - src/Ruleset/Generic/BlankEOFSniff.php:24:55 - Argument 2 of TwigCsFixer\Ruleset\Generic\BlankEOFSniff::process has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSniff::process (see https://psalm.dev/042)
    public function process(int $tokenPosition, array $tokens): void


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/DelimiterSpacingSniff.php:21:72 - Argument 2 of TwigCsFixer\Ruleset\Generic\DelimiterSpacingSniff::shouldHaveSpaceBefore has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceBefore (see https://psalm.dev/042)
    protected function shouldHaveSpaceBefore(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/DelimiterSpacingSniff.php:42:71 - Argument 2 of TwigCsFixer\Ruleset\Generic\DelimiterSpacingSniff::shouldHaveSpaceAfter has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceAfter (see https://psalm.dev/042)
    protected function shouldHaveSpaceAfter(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/EmptyLinesSniff.php:24:55 - Argument 2 of TwigCsFixer\Ruleset\Generic\EmptyLinesSniff::process has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSniff::process (see https://psalm.dev/042)
    public function process(int $tokenPosition, array $tokens): void


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/OperatorSpacingSniff.php:21:72 - Argument 2 of TwigCsFixer\Ruleset\Generic\OperatorSpacingSniff::shouldHaveSpaceBefore has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceBefore (see https://psalm.dev/042)
    protected function shouldHaveSpaceBefore(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/OperatorSpacingSniff.php:45:71 - Argument 2 of TwigCsFixer\Ruleset\Generic\OperatorSpacingSniff::shouldHaveSpaceAfter has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceAfter (see https://psalm.dev/042)
    protected function shouldHaveSpaceAfter(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/PunctuationSpacingSniff.php:21:72 - Argument 2 of TwigCsFixer\Ruleset\Generic\PunctuationSpacingSniff::shouldHaveSpaceBefore has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceBefore (see https://psalm.dev/042)
    protected function shouldHaveSpaceBefore(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Ruleset/Generic/PunctuationSpacingSniff.php:37:71 - Argument 2 of TwigCsFixer\Ruleset\Generic\PunctuationSpacingSniff::shouldHaveSpaceAfter has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSpacingSniff::shouldHaveSpaceAfter (see https://psalm.dev/042)
    protected function shouldHaveSpaceAfter(int $tokenPosition, array $tokens): ?int


ERROR: MethodSignatureMismatch - src/Sniff/AbstractSniff.php:62:39 - Argument 1 of TwigCsFixer\Sniff\AbstractSniff::processFile has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\SniffInterface::processFile (see https://psalm.dev/042)
    public function processFile(array $stream): void


ERROR: MethodSignatureMismatch - src/Sniff/AbstractSpacingSniff.php:23:55 - Argument 2 of TwigCsFixer\Sniff\AbstractSpacingSniff::process has wrong type 'array<array-key, mixed>', expecting 'array<int, TwigCsFixer\Token\Token>' as defined by TwigCsFixer\Sniff\AbstractSniff::process (see https://psalm.dev/042)
    public function process(int $tokenPosition, array $tokens): void


------------------------------
10 errors found
------------------------------

@weirdan
Copy link
Collaborator

weirdan commented Jul 31, 2021

@VincentLanglet thanks, reproduced this locally.

@weirdan
Copy link
Collaborator

weirdan commented Jul 31, 2021

Reduced to https://gist.github.com/weirdan/69b9f4fe3d0d384295074a42427f7f7a:

$ git clone [email protected]:69b9f4fe3d0d384295074a42427f7f7a.git gg
Cloning into 'gg'...
remote: Enumerating objects: 7, done.
remote: Counting objects: 100% (7/7), done.
remote: Compressing objects: 100% (6/6), done.
Receiving objects: 100% (7/7), done.
remote: Total 7 (delta 0), reused 0 (delta 0), pack-reused 0

$ cd gg
$ composer install
No composer.lock file present. Updating dependencies to latest instead of installing from lock file. See https://getcomposer.org/install for more information.
Loading composer repositories with package information
Updating dependencies
Lock file operations: 30 installs, 0 updates, 0 removals
  - Locking amphp/amp (v2.6.0)
[...snip...]

$ vendor/bin/psalm --no-cache
Scanning files...
Analyzing files...

E

ERROR: MethodSignatureMismatch - B.php:15:29 - Argument 1 of B::f has wrong type 'array<array-key, mixed>', expecting 'array<int, T>' as defined by A::f (see https://psalm.dev/042)
    public function f(array $t): void


------------------------------
1 errors found
------------------------------

Checks took 0.48 seconds and used 39.760MB of memory
Psalm was unable to infer types in the codebase

@weirdan
Copy link
Collaborator

weirdan commented Jul 31, 2021

Changing phpVersion to 7.4 or removing this attribute from psalm.xml gets rid of the issue.

@weirdan
Copy link
Collaborator

weirdan commented Jul 31, 2021

diff --git a/src/Psalm/Internal/Analyzer/MethodComparator.php b/src/Psalm/Internal/Analyzer/MethodComparator.php
index 977f23448..f1ab10fe6 100644
--- a/src/Psalm/Internal/Analyzer/MethodComparator.php
+++ b/src/Psalm/Internal/Analyzer/MethodComparator.php
@@ -329,7 +329,7 @@ class MethodComparator
             ) {
                 $implementer_param_type = $implementer_param->signature_type;
 
-                $guide_param_signature_type = $guide_param->type;
+                $guide_param_signature_type = $guide_param->signature_type;
 
                 $or_null_guide_param_signature_type = $guide_param->signature_type
                     ? clone $guide_param->signature_type

Haven't properly tested yet, but the above appears to fix the issue at hand.

@weirdan
Copy link
Collaborator

weirdan commented Jul 31, 2021

Ok, I think I've got what's going on here.

There's a visitPreloadedStubFile() method that adds stubs for internal symbols that are missing from the current runtime (e.g. Attribute when you run Psalm on 8.0 codebase using PHP 7.4, that's what causing it to be version-dependent). It adds stub files to the scanner queue, enables register_stub_files flag and performs the scan. However it's called too late, when project files (potentially only some of them, due to caching, which may make it cache-dependent as well) have already been added to the queue. This causes scanned classlikes to be interpreted as built-in, non-user-defined symbols, and Psalm treats them differently in some places.

weirdan added a commit to weirdan/psalm that referenced this issue Jul 31, 2021
This should prevent Psalm from sometimes marking user-defined classes as
built-in.

Fixes vimeo#5126
weirdan added a commit to weirdan/psalm that referenced this issue Jul 31, 2021
This should prevent Psalm from sometimes marking user-defined classes as
built-in.

Fixes vimeo#5126
Fixes vimeo#5626
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants