False positive MethodSignatureMustProvideReturnType with stubs.

[VincentLanglet]

In 4.19, the MethodSignatureMustProvideReturnType error was introduced.

In the following project sonata-project/EntityAuditBundle#474, I now have errors like:

MethodSignatureMustProvideReturnType - src/Collection/AuditedCollection.php:127:21 - Method SimpleThings\EntityAudit\Collection\AuditedCollection::add must have a return type signature! (see https://psalm.dev/282)
    public function add($element)

Removing the doctrine plugin

<pluginClass class=“Weirdan\DoctrinePsalmPlugin\Plugin”/>

solve my issue.

So it would means that the Collection stub provided by the plugin is considered as a native php interface.

Related to #7363
cc @weirdan @orklah @danog

[psalm-github-bot]

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

[orklah]

@bendavies made a similar claim a while ago psalm/psalm-plugin-symfony#146

This is causing all service classes to be marked as not user_defined

It could be the same bug that is now revealed

Should we disable this feature/Issue and release a new bugfix version until we can find and fix the issue?

[bendavies]

I understand the issue here now and have a way forward. I'll try this weekend

[orklah]

Cool! Glad to hear that!

[VincentLanglet]

As a quick fix, shouldn't the MethodSignatureMustProvideReturnType rule be temporary disabled in a patch release ?

[bendavies]

well, i spent a bit of time on this over the weekend and the bug i was looking at as part of psalm/psalm-plugin-symfony#146 is no longer reproducable.

there still is a bug here which i discussed with @orklah . i think the way forward is understood. @orklah could probably give a better summary as he understands the psalm internals better than me

[caugner]

After upgrading from 4.17 to 4.19, we're seeing unexpected MethodSignatureMustProvideReturnType issues as well:

# app/Http/Requests/Customer/IndexCustomerRequest.php
<?php

namespace App\Http\Requests\Customer;

use App\Models\Customer;
use App\Models\Team;
use App\Models\User;
use Illuminate\Foundation\Http\FormRequest;

/**
 * @property-read Team $team
 * @method User user($guard = null)
 */
class IndexCustomerRequest extends FormRequest
{
  // ...
}

Weirdly, the declared user() method does have a return type, and Psalm is reporting it anyhow (and at the wrong location).

[danog]

Weirdly, the declared user() method does have a return type, and Psalm is reporting it anyhow (and at the wrong location).

@caugner This might be a cache issue, try removing the psalm cache and running psalm with --threads=1 --no-cache.

[caugner]

This might be a cache issue

@danog This was also happening on fresh CI runs.

[bendavies]

well, i spent a bit of time on this over the weekend and the bug i was looking at as part of psalm/psalm-plugin-symfony#146 is no longer reproducable.

there still is a bug here which i discussed with @orklah . i think the way forward is understood. @orklah could probably give a better summary as he understands the psalm internals better than me

scrap that, reproduced it again

[danog]

I tried playing around a bit, replacing the user_defined check with

        $storage->user_defined = true;
        if (class_exists($fq_classlike_name)) {
            $reflect = new ReflectionClass($fq_classlike_name);
            $storage->user_defined = !$reflect->isInternal();
        }

However I haven't yet managed to reproduce the original issue in the stub tests, but maybe this can help you @bendavies

[orklah]

I released a new version. Sorry, I didn't make it a bugfix (mainly because I have no idea how >< )

This should be solved for now, we'll try to find a solution to enable it again at a later date (probably be on Psalm 5 at this point)

[weirdan]

(mainly because I have no idea how >< )

You check out a tag (say, git checkout 4.19.0), then create a branch off it (git checkout -b 4.19.x), rebase a bugfix branch on top of that and merge it into your new branch (or just cherry-pick some commits). Then you push your new branch and tag it (by creating a release targeting that branch).