phpstan / phpstan-symfony

Symfony extension for PHPStan
MIT License
714 stars 89 forks source link

console_application_loader for Maker #167

Closed VincentLanglet closed 3 years ago

VincentLanglet commented 3 years ago

I'm getting a lot of error from this file https://github.com/sonata-project/SonataAdminBundle/blob/master/src/Maker/AdminMaker.php

Most of them are because $input->getArgument('...') are considered as string|string[]|null.

I solve some of these issues for Command, thanks to the console_application_loader config. Is it possible to have a similar option or using the same for maker ?

ondrejmirtes commented 3 years ago

I don't know what's a maker?

VincentLanglet commented 3 years ago

I don't know what's a maker?

https://symfony.com/doc/current/bundles/SymfonyMakerBundle/index.html

A maker is a console command which generate some code. It implements this interface: https://github.com/symfony/maker-bundle/blob/main/src/MakerInterface.php

I assume symfony is transforming the Maker to a real Command with this MakerCommand: https://github.com/symfony/maker-bundle/blob/main/src/Command/MakerCommand.php

VincentLanglet commented 3 years ago

@javiereguiluz and @weaverryan, I saw you work a lot on the MakerBundle, maybe you could help me to understand how to solve.

Phpstan-symfony allow to register a console-loader

$kernel = new AppKernel();
return new Application($kernel);

In order for phpstan to access to the different command of the application.

Then, when we're doing $input->getOption('foo'), phpstan is using

foreach ($app->all() as $name => $command) {
    $command->getDefinition()
}

to get all the command, and input definitions, in order to find the type of the option foo.

Would it be possible to do something similar for makers ? I look at the code, and seems like a CompilerPass is transforming the Maker into a Command, thanks to the MakerCommand. But when I do

$kernel = new AppKernel();
new Application($kernel);

and look at all the command of the application I don't find the one from the Maker.

jordisala1991 commented 3 years ago

Could be related to the lazy commands thing? can you check what thing returns for the $command? Or it is not even on the container?

jordisala1991 commented 3 years ago

Yes, it is related too. In SonataAdmin there are one problems @VincentLanglet :

MakerBundle is not registered on AppKernel.php

And on PHPStan-symfony there are kind of 2:

  1. make commands appear, BUT they are registered as LazyCommands, so we need to solve first: #186
  2. make commands are not even picked because of this code:
        $classType = new ObjectType($classReflection->getName());
        if (!(new ObjectType('Symfony\Component\Console\Command\Command'))->isSuperTypeOf($classType)->yes()) {
            return [];
        }

The maker class is not the same as a command (even though it is transformed by a compiler pass). I did a simplification to this and it started detecting the problems on maker-commands (Actually making me delete the ignored issues because they were fixed)

    public function findCommands(ClassReflection $classReflection): array
    {
        if ($this->consoleApplication === null) {
            return [];
        }

        $classType = new ObjectType($classReflection->getName());
        $isCommand = (new ObjectType('Symfony\Component\Console\Command\Command'))->isSuperTypeOf($classType)->yes();
        $isMakerCommand = (new ObjectType('Symfony\Bundle\MakerBundle\MakerInterface'))->isSuperTypeOf($classType)->yes();

        if (!$isCommand && !$isMakerCommand) {
            return [];
        }

        $commandName = $isCommand ? ($classReflection->getName())::getDefaultName() : ($classReflection->getName())::getCommandName();
        return [$this->consoleApplication->get($commandName)];
    }

Not sure if this plugin should solve the maker thing, or why this function is suposed to return an array. @ondrejmirtes do you know why given a classReflection we should return an array here?

Edit: Not sure if this is the correct fix for that, because here I am executing user code to know the command name (static methods).

VincentLanglet commented 3 years ago

Now the LazyCommand PR is merged, would you be able to provide a PR for Maker ?

Not sure if this plugin should solve the maker thing

Why it shouldn't ?

VincentLanglet commented 3 years ago

Seems like it now works for Maker too: https://github.com/sonata-project/SonataAdminBundle/pull/7352

github-actions[bot] commented 3 years ago

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.