[Console] don't discard existing aliases when constructing Command

[henderkes]

Q	A
Branch?	7.4
Bug fix?	yes
New feature? no
Deprecations?	no
Issues	Fix #62557
License	MIT

Contributor guidelines:

• Add tests and ensure they pass
• Bug fixes must target the lowest maintained branch where they apply
  https://symfony.com/releases#maintained-symfony-branches
• New features and deprecations must target the feature branch
  and must add an entry to the changelog file of the patched component:
  https://symfony.com/doc/current/contributing/code/conventions.html#writing-a-changelog-entry
• Do not break backward compatibility:
  https://symfony.com/bc

[carsonbot]

Hey!

I see that this is your first PR. That is great! Welcome!

Symfony has a contribution guide which I suggest you to read.

In short:

• Always add tests
• Keep backward compatibility (see https://symfony.com/bc).
• Bug fixes must be submitted against the lowest maintained branch where they apply (see https://symfony.com/releases)
• Features and deprecations must be submitted against the 8.1 branch.

Review the GitHub status checks of your pull request and try to solve the reported issues. If some tests are failing, try to see if they are failing because of this change.

When two Symfony core team members approve this change, it will be merged and you will become an official Symfony contributor!
If this PR is merged in a lower version branch, it will be merged up to all maintained branches within a few days.

I am going to sit back now and wait for the reviews.

Cheers!

Carsonbot

[xabbuh]

I think I would rather guard the setAliases() call below to not call it with an empty array.

[andesk]

What's the intended behavior most likely?

Adding aliases from $name being split by | seems to be the latest change. Merging those into the array of existing ones feels most BC-safe over overwriting it if aliases here are not an empty array? 🤔

[henderkes]

I think I would rather guard the setAliases() call below to not call it with an empty array.

            if ($aliases !== []) {
                $this->setAliases($aliases);
            }

Like this? That alone wouldn't fix the issue of existing aliases, because if $name was 'name|newalias', it would still overwrite the existing ones.

[xabbuh]

I thought about that too and then I wondered if it is really realistic that someone updates the name to make use of the new opportunity to define name and aliases at once and then additionally also calls setAlias(). 🤷

[xabbuh]

Though if we say that's something we want to keep we can indeed remove the if again.

[henderkes]

Perhaps not realistic, but entirely possible.

[chalasr]

Thank you @henderkes.