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

Psalm false positive with ParamConverter getClass method #213

Open
tarlepp opened this issue Aug 31, 2021 · 4 comments
Open

Psalm false positive with ParamConverter getClass method #213

tarlepp opened this issue Aug 31, 2021 · 4 comments
Labels
question Further information is requested

Comments

@seferov seferov added the bug Something isn't working label Sep 1, 2021
@tarlepp
Copy link
Author

tarlepp commented Sep 11, 2021

Any updates on this?

@seferov seferov added question Further information is requested and removed bug Something isn't working labels Sep 11, 2021
@seferov
Copy link
Member

seferov commented Sep 11, 2021

@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 #208

@tarlepp
Copy link
Author

tarlepp commented Sep 11, 2021

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
Copy link
Author

tarlepp commented Sep 11, 2021

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                                                                                                  

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants