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

Inconsistent MethodSignatureMismatch reporting: it yields randomly and poiting to the wrong class #6773

Open
fluffycondor opened this issue Oct 28, 2021 · 6 comments · May be fixed by psalm/psalm-plugin-doctrine#102
Labels

Comments

@fluffycondor
Copy link
Contributor

psalm 4.11.2

Sometimes Psalm yields MethodSignatureMismatch errors on some of my project's Repositories classes extended from the Doctrine one.

ERROR: MethodSignatureMismatch - src/Repository/FooRepository.php:15:7 - Argument 1 of Doctrine\ORM\EntityRepository::findBy has wrong type 'array<array-key, mixed>', expecting 'array<string, mixed>' as defined by Doctrine\Persistence\ObjectRepository::findBy (see https://psalm.dev/042)
class FooRepository extends ServiceEntityRepository

If I do psalm --clear-cache then this kind of errors disappears for some time. (?!)

One more interesting thing is that Doctrine's code is perfectly correct, the first argument is typed array<string, mixed> in both cases (it's a recent change, it was just an array). But! There are @psalm-method stubs in the FooRepository itself that actually causes the issue:

/*
 * @psalm-method list<Foo> findBy(array $criteria, array $orderBy = null, $limit = null, $offset = null)
 */
class FooRepository extends ServiceEntityRepository
{

So there are two bound issues:

  1. @method annotation causes Psalm to blame wrong class.
  2. MethodSignatureMismatch error is reported in a really inconsistent way. (It reports the issue something like two times per ten tries on my code with empty psalm cache, and doesn't report it at all in the sandbox: https://psalm.dev/r/dd7372a1bf No issues? Why?)

Also I don't know how to fix my @psalm-method annotation, because I can't specify array<string, mixed> inside a @method, it's a phpDoc syntax error, as I understand. At least looks like psalm doesn't understand this syntax, but doesn't yield a syntax error either. So any advice will be appreciated.

So, as I can't reproduce the issue(s) on psalm.dev properly as it tightly bound to the Doctrine library code, and I failed to find a minimal reproducible case, feel free to ask any debug info or point the place in the psalm's source code where I can investigate the issue(s).

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/dd7372a1bf
<?php

class Foo
{
    /** @param array<string, mixed> $array */
    public function foo(array $array): void {}
}

/** @psalm-method void foo(array $array) */
class Bar extends Foo
{
}
Psalm output (using commit 526debb):

No issues!

@weirdan
Copy link
Collaborator

weirdan commented Oct 28, 2021

Can you perhaps create a reproducer in a separate repo then?

@orklah
Copy link
Collaborator

orklah commented Oct 28, 2021

Can you make sure you don't have your cache folder in your config. Did you install the plugin for doctrine? Do you sometimes use psalm with only one file in in command line?

@orklah orklah added the bug label Oct 28, 2021
@fluffycondor
Copy link
Contributor Author

fluffycondor commented Oct 29, 2021

Can you perhaps create a reproducer in a separate repo then?

I tried to make a new repo with Doctrine and Symfony in order to reproduce this inconsistent error, but Psalm doesn't yield MethodSignatureMismatch in a brand new repo.
As I understand, Psalm should yield MethodSignatureMismatch in this minimal case, right? But it doesn't. https://psalm.dev/r/dd7372a1bf

Can you make sure you don't have your cache folder in your config.

My config is

<?xml version="1.0"?>
<psalm
    errorLevel="1"
    resolveFromConfigFile="true"
    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"
>
    <projectFiles>
        <directory name="src"/>
        <ignoreFiles>
            <directory name="vendor"/>
            <file name="src/Kernel.php"/>
        </ignoreFiles>
    </projectFiles>
    <forbiddenFunctions>
        <function name="dd"/>
        <function name="dump"/>
    </forbiddenFunctions>
    <plugins>
        <pluginClass class="Psalm\SymfonyPsalmPlugin\Plugin"/>
        <pluginClass class="Weirdan\DoctrinePsalmPlugin\Plugin"/>
    </plugins>
</psalm>

Did you install the plugin for doctrine?

Yes. Sorry, forgot to mention it.

Do you sometimes use psalm with only one file in in command line?

In CLI I use Psalm only as vendor/bin/psalm command, but I use PHPStorm built-in Psalm inspection, that relies on the Psalm instance installed in the project. I think the way how PHPStorm calls Psalm can be related to the issue indeed. It looks like Psalm doesn't yield MethodSignatureMismatch anymore when I close PHPStorm and clear Psalm's cache. It's interesting that right after the PHPStorm built-in inspection find MethodSignatureMismatch issue, the CLI Psalm instance still reports "No issues found!", it takes some time to yield MethodSignatureMismatch in the CLI too.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/dd7372a1bf
<?php

class Foo
{
    /** @param array<string, mixed> $array */
    public function foo(array $array): void {}
}

/** @psalm-method void foo(array $array) */
class Bar extends Foo
{
}
Psalm output (using commit 8c33b21):

No issues!

@orklah
Copy link
Collaborator

orklah commented Oct 29, 2021

I think I already observed this bug but not through PHPStorm. It looked like this:

  • Have a basic project with doctrine + doctrine plugin
  • Run Psalm on a single file
  • Doctrine plugin can't link entity with repository with such a limited scope
  • Report on a single file emits an error
  • Running psalm on full project use cache from single file analysis and re-emit the error that is no longer legit because repository is in scope

It may be more subtle than that but I was at work and couldn't spend a long time on this

@fluffycondor fluffycondor linked a pull request Nov 9, 2021 that will close this issue
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