Nicolas' Review

yceruto · yceruto · commit 0ed04d737cfb · 2025-08-22T23:53:14.000+02:00
diff --git a/src/Symfony/Component/Console/Attribute/Input.php b/src/Symfony/Component/Console/Attribute/Input.php
@@ -8,19 +8,21 @@
 #[\Attribute(\Attribute::TARGET_PARAMETER)]
 class Input
 {
-    private \ReflectionClass $class;
     /**
      * @var array<string, Argument>
      */
-    private array $arguments = [];
+    public array $arguments = [];
+
     /**
      * @var array<string, Option>
      */
-    private array $options = [];
+    public array $options = [];
+
+    private \ReflectionClass $class;
 
     public static function tryFrom(\ReflectionParameter $parameter): ?self
     {
-        /** @var self $self */
+        /** @var static $self */
         if (!$self = ($parameter->getAttributes(self::class, \ReflectionAttribute::IS_INSTANCEOF)[0] ?? null)?->newInstance()) {
             return null;
         }
@@ -38,14 +40,18 @@ public static function tryFrom(\ReflectionParameter $parameter): ?self
         $self->class = new \ReflectionClass($class);
 
         foreach ($self->class->getProperties() as $property) {
+            if (!$property->isPublic() || $property->isStatic()) {
+                continue;
+            }
+
             if ($argument = Argument::tryFrom($property)) {
                 $self->arguments[$property->name] = $argument;
             } elseif ($option = Option::tryFrom($property)) {
                 $self->options[$property->name] = $option;
             }
         }
 
-        if ([] === $self->arguments && [] === $self->options) {
+        if (!$self->arguments && !$self->options) {
             throw new LogicException(\sprintf('The input class "%s" must have at least one argument or option.', $self->class->name));
         }
 
@@ -60,29 +66,13 @@ public function resolveValue(InputInterface $input): mixed
         $instance = $this->class->newInstanceWithoutConstructor();
 
         foreach ($this->arguments as $name => $argument) {
-            $this->class->getProperty($name)->setValue($instance, $argument->resolveValue($input));
+            $instance->$name = $argument->resolveValue($input);
         }
 
         foreach ($this->options as $name => $option) {
-            $this->class->getProperty($name)->setValue($instance, $option->resolveValue($input));
+            $instance->$name = $option->resolveValue($input);
         }
 
         return $instance;
     }
-
-    /**
-     * @return Argument[]
-     */
-    public function getArguments(): array
-    {
-        return $this->arguments;
-    }
-
-    /**
-     * @return Option[]
-     */
-    public function getOptions(): array
-    {
-        return $this->options;
-    }
 }
diff --git a/src/Symfony/Component/Console/Command/InvokableCommand.php b/src/Symfony/Component/Console/Command/InvokableCommand.php
@@ -77,18 +77,16 @@ public function configure(InputDefinition $definition): void
         foreach ($this->reflection->getParameters() as $parameter) {
             if ($argument = Argument::tryFrom($parameter)) {
                 $definition->addArgument($argument->toInputArgument());
-
                 continue;
             }
 
             if ($option = Option::tryFrom($parameter)) {
                 $definition->addOption($option->toInputOption());
-
                 continue;
             }
 
             if ($input = Input::tryFrom($parameter)) {
-                $inputArguments = array_map(fn (Argument $a) => $a->toInputArgument(), $input->getArguments());
+                $inputArguments = array_map(fn (Argument $a) => $a->toInputArgument(), $input->arguments);
 
                 // make sure optional arguments are defined after required ones
                 usort($inputArguments, fn (InputArgument $a, InputArgument $b) => (int) $b->isRequired() - (int) $a->isRequired());
@@ -97,7 +95,7 @@ public function configure(InputDefinition $definition): void
                     $definition->addArgument($inputArgument);
                 }
 
-                foreach ($input->getOptions() as $option) {
+                foreach ($input->options as $option) {
                     $definition->addOption($option->toInputOption());
                 }
             }

Original file line number	Diff line number	Diff line change
`@@ -77,18 +77,16 @@ public function configure(InputDefinition $definition): void`
`77`	`77`	`foreach ($this->reflection->getParameters() as $parameter) {`
`78`	`78`	`if ($argument = Argument::tryFrom($parameter)) {`
`79`	`79`	`$definition->addArgument($argument->toInputArgument());`
`80`		`-`
`81`	`80`	`continue;`
`82`	`81`	`}`
`83`	`82`
`84`	`83`	`if ($option = Option::tryFrom($parameter)) {`
`85`	`84`	`$definition->addOption($option->toInputOption());`
`86`		`-`
`87`	`85`	`continue;`
`88`	`86`	`}`
`89`	`87`
`90`	`88`	`if ($input = Input::tryFrom($parameter)) {`
`91`		`- $inputArguments = array_map(fn (Argument $a) => $a->toInputArgument(), $input->getArguments());`
	`89`	`+ $inputArguments = array_map(fn (Argument $a) => $a->toInputArgument(), $input->arguments);`
`92`	`90`
`93`	`91`	`// make sure optional arguments are defined after required ones`
`94`	`92`	`usort($inputArguments, fn (InputArgument $a, InputArgument $b) => (int) $b->isRequired() - (int) $a->isRequired());`
`@@ -97,7 +95,7 @@ public function configure(InputDefinition $definition): void`
`97`	`95`	`$definition->addArgument($inputArgument);`
`98`	`96`	`}`
`99`	`97`
`100`		`- foreach ($input->getOptions() as $option) {`
	`98`	`+ foreach ($input->options as $option) {`
`101`	`99`	`$definition->addOption($option->toInputOption());`
`102`	`100`	`}`
`103`	`101`	`}`