psalm-plugin-symfony icon indicating copy to clipboard operation
psalm-plugin-symfony copied to clipboard

Psalm false positive with ParamConverter getClass method

Open tarlepp opened this issue 2 years ago • 4 comments

https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/src/Configuration/ParamConverter.php vs. https://github.com/psalm/psalm-plugin-symfony/blob/master/src/Stubs/common/Bundle/FrameworkExtraBundle/Configuration/ParamConverter.stubphp

Latest version doesn't return null anymore.

tarlepp avatar Aug 31 '21 19:08 tarlepp

Any updates on this?

tarlepp avatar Sep 11 '21 11:09 tarlepp

@tarlepp I just checked and it seems the phpdoc on class property is incorrect. As you can see from the class constructor class can actually be null. There is even check for it in DoctrineParamConverter.

See the PR about it https://github.com/psalm/psalm-plugin-symfony/pull/208

seferov avatar Sep 11 '21 11:09 seferov

hmm, in - https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/src/Configuration/ParamConverter.php#L114 - it's returning a string and also that property is string - https://github.com/sensiolabs/SensioFrameworkExtraBundle/blob/master/src/Configuration/ParamConverter.php#L33

Need to raise an issue about this to SensioFrameworkExtraBundle

tarlepp avatar Sep 11 '21 11:09 tarlepp

Just added ?? to my code;

<?php
declare(strict_types = 1);

namespace App\Request\ParamConverter;

use App\Resource\ResourceCollection;
use Sensio\Bundle\FrameworkExtraBundle\Configuration\ParamConverter;
use Sensio\Bundle\FrameworkExtraBundle\Request\ParamConverter\ParamConverterInterface;
use Symfony\Component\HttpFoundation\Request;
use Throwable;

class RestResourceConverter implements ParamConverterInterface
{
    public function __construct(
        private ResourceCollection $collection,
    ) {
    }

    /**
     * {@inheritdoc}
     *
     * @throws Throwable
     */
    public function apply(Request $request, ParamConverter $configuration): bool
    {
        $name = $configuration->getName();
        $identifier = (string)$request->attributes->get($name, '');
        $resource = $this->collection->get($configuration->getClass() ?? '');

        if ($identifier !== '') {
            // Reminder make throw to exists on options
            $request->attributes->set($name, $resource->findOne($identifier, true));
        }

        return true;
    }

    public function supports(ParamConverter $configuration): bool
    {
        return $this->collection->has($configuration->getClass());
    }
}

and with that Psalm is ok but Phpstan gives me;

PHPStan - PHP Static Analysis Tool 0.12.98
Note: Using configuration file /app/phpstan.neon.dist.
 510/510 [▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓▓] 100%

 ------ ------------------------------------------------------------------ 
  Line   src/Request/ParamConverter/RestResourceConverter.php              
 ------ ------------------------------------------------------------------ 
  28     Expression on left side of ?? is not nullable.                    
         ✏️  /app/src/Request/ParamConverter/RestResourceConverter.php:28  
 ------ ------------------------------------------------------------------ 
                                                                                                                        
 [ERROR] Found 1 error                                                                                                  

tarlepp avatar Sep 11 '21 22:09 tarlepp