8000 [DX][Console] Improve error message when the parent constructor is not called · Issue #25423 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[DX][Console] Improve error message when the parent constructor is not called #25423

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

Closed
javiereguiluz opened this issue Dec 10, 2017 · 5 comments
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony)

Comments

@javiereguiluz
Copy link
Member
Q A
Bug report? no
Feature request? yes
BC Break report? no
RFC? no
Symfony version 4.1

If you add a constructor in your command and forget to add the parent::__construct() call, you'll end up with this misleading error:

There are no commands defined in the "app" namespace.

This was not a very important issue in the past, but now that adding a constructor is the recommended way to inject services, we're starting to get issue reports about this (e.g. symfony/symfony-docs#8843 (comment)).

Could we detect this issue and change the error message to tell the user that they must add the parent::__construct() call?

@javiereguiluz javiereguiluz added Console DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Dec 10, 2017
@linaori
Copy link
Contributor
linaori commented Dec 11, 2017

That's probably because the configure() must be called and this is done via the constructor. Instead of telling the developer to add an extra call to the parent constructor, would it be possible to somehow call this configure in a static context? This would allow even lazier commands in certain conditions. I'm not sure if the current proposal is easy to detect.

Ideally I'd not see the parent class at all, just like I can do with controllers. I would like to see a form of composition over inheritance here to solve this issue.

final class MyCommand implements CommandInterface
{
    public static function configure(CommandConfiguratorInterface $config)
    {
        $config->setName('foo:bar');
        $config->setDescription('...');
        $config->addArgument('some-argument');
    }
    
    public function execute(InputInterface $input, OutputInterface $output)
    {
        // do something awesome
    }
}

Could be solved by simply wrapping this inside an instance of the current Command class.

@nicolas-grekas
Copy link
Member

@iltar the issue is that any static is out of any service layer. I know that some projects do fetch configuration of their commands via a DB call. And that's fine. So, "object oriented" is a requirement here.

@linaori
Copy link
Contributor
linaori commented Dec 11, 2017

Both methods could be supported, my proposal would be merely feeding the actual command class with information, which can be cached during container building and would thus make them very lazy. As the current situation is still possible, it wouldn't make an existing solutions impossible to build.

final class CommandWrapper extends Command
{
    public function __construct(CommandInterface $command, CommandMetadata $metadata)
    {
        // ...
    }

    protected function configure()
    {
        // use the CommandMetadata to configure $this
    }

    protected function execute(InputInterface $input, OutputInterface $output)
    {
        $this->command->execute($input, $output);
    }
}

@nicolas-grekas
Copy link
Member
nicolas-grekas commented Dec 11, 2017

I just created a command with the maker bundle, added a constructor that didn't call the parent, and this is the message I'm seeing:

Command class "App 8FF8 \Command\AppBraveKangarooCommand" is not correctly initialized. You probably forgot to call the parent constructor.

Looks perfect to me, nothing to do here. What did I miss?

@chalasr
Copy link
Member
chalasr commented Dec 14, 2017

Can we close? I can't reproduce either

@fabpot fabpot closed this as completed Dec 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony)
Projects
None yet
Development

No branches or pull requests

5 participants
0