minor #40322 [Console] improve exception message when required argument is added after an optional one (marbul)

This PR was merged into the 5.3-dev branch.

Discussion
----------

[Console] improve exception message when required argument is added after an optional one

| Q             | A
| ------------- | ---
| Branch?       | 5.x <!-- see below -->
| Bug fix?      | no
| New feature?  | wouldn't say so, rather DX improvement <!-- please update src/**/CHANGELOG.md files -->
| Deprecations? | no <!-- please update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tickets       | Fix #40302 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT
| Doc PR        | n/a

Hello, this is my first contribution to Symfony Framework. It's time to pull my weight.
About the issue:
I did improve an error message to include passed argument's name and the latest optional argument's name.
![Screenshot at 2021-02-26 23-08-10](https://user-images.githubusercontent.com/79662742/109361609-8f011e80-7889-11eb-8700-cbbd388c0109.png)
An author also mentioned "But which command?", however this is shown twice as we can see on the screenshot above so I skipped that.

Commits
-------

9bddbbd7ed #40302 improve exception message when required argument is added after an optional one
This commit is contained in:
Fabien Potencier 2021-03-12 09:40:58 +01:00
commit c487a56bf4
2 changed files with 14 additions and 14 deletions

View File

@ -30,8 +30,8 @@ class InputDefinition
{ {
private $arguments; private $arguments;
private $requiredCount; private $requiredCount;
private $hasAnArrayArgument = false; private $lastArrayArgument;
private $hasOptional; private $lastOptionalArgument;
private $options; private $options;
private $negations; private $negations;
private $shortcuts; private $shortcuts;
@ -72,8 +72,8 @@ class InputDefinition
{ {
$this->arguments = []; $this->arguments = [];
$this->requiredCount = 0; $this->requiredCount = 0;
$this->hasOptional = false; $this->lastOptionalArgument = null;
$this->hasAnArrayArgument = false; $this->lastArrayArgument = null;
$this->addArguments($arguments); $this->addArguments($arguments);
} }
@ -100,22 +100,22 @@ class InputDefinition
throw new LogicException(sprintf('An argument with name "%s" already exists.', $argument->getName())); throw new LogicException(sprintf('An argument with name "%s" already exists.', $argument->getName()));
} }
if ($this->hasAnArrayArgument) { if (null !== $this->lastArrayArgument) {
throw new LogicException('Cannot add an argument after an array argument.'); throw new LogicException(sprintf('Cannot add a required argument "%s" after an array argument "%s".', $argument->getName(), $this->lastArrayArgument->getName()));
} }
if ($argument->isRequired() && $this->hasOptional) { if ($argument->isRequired() && null !== $this->lastOptionalArgument) {
throw new LogicException('Cannot add a required argument after an optional one.'); throw new LogicException(sprintf('Cannot add a required argument "%s" after an optional one "%s".', $argument->getName(), $this->lastOptionalArgument->getName()));
} }
if ($argument->isArray()) { if ($argument->isArray()) {
$this->hasAnArrayArgument = true; $this->lastArrayArgument = $argument;
} }
if ($argument->isRequired()) { if ($argument->isRequired()) {
++$this->requiredCount; ++$this->requiredCount;
} else { } else {
$this->hasOptional = true; $this->lastOptionalArgument = $argument;
} }
$this->arguments[$argument->getName()] = $argument; $this->arguments[$argument->getName()] = $argument;
@ -172,7 +172,7 @@ class InputDefinition
*/ */
public function getArgumentCount() public function getArgumentCount()
{ {
return $this->hasAnArrayArgument ? \PHP_INT_MAX : \count($this->arguments); return null !== $this->lastArrayArgument ? \PHP_INT_MAX : \count($this->arguments);
} }
/** /**

View File

@ -101,7 +101,7 @@ class InputDefinitionTest extends TestCase
public function testArrayArgumentHasToBeLast() public function testArrayArgumentHasToBeLast()
{ {
$this->expectException(\LogicException::class); $this->expectException(\LogicException::class);
$this->expectExceptionMessage('Cannot add an argument after an array argument.'); $this->expectExceptionMessage('Cannot add a required argument "anotherbar" after an array argument "fooarray".');
$this->initializeArguments(); $this->initializeArguments();
$definition = new InputDefinition(); $definition = new InputDefinition();
@ -111,9 +111,9 @@ class InputDefinitionTest extends TestCase
public function testRequiredArgumentCannotFollowAnOptionalOne() public function testRequiredArgumentCannotFollowAnOptionalOne()
{ {
$this->expectException(\LogicException::class);
$this->expectExceptionMessage('Cannot add a required argument after an optional one.');
$this->initializeArguments(); $this->initializeArguments();
$this->expectException(\LogicException::class);
$this->expectExceptionMessage(sprintf('Cannot add a required argument "%s" after an optional one "%s".', $this->foo2->getName(), $this->foo->getName()));
$definition = new InputDefinition(); $definition = new InputDefinition();
$definition->addArgument($this->foo); $definition->addArgument($this->foo);