Address PR review feedback

Author: koriym

.github/workflows/static-analysis.yml

@@ -9,4 +9,4 @@ jobs:
   sa:
     uses: ray-di/.github/.github/workflows/static-analysis.yml@v1
     with:
-      php_version: 8.3
+      php_version: 8.4

.travis.yml

@@ -79,7 +79,7 @@ public function createAction($id, $name, $body)
 
 ### Controller
 
-コントローラークラスにフォームをインジェクトします。フォームのバリデーションを行うメソッドを`@FormValidation`で
+コントローラークラスにフォームをインジェクトします。フォームのバリデーションを行うメソッドを`#[FormValidation]`で
 アノテートします。この時フォームのプロパティ名を`form`で、バリデーションが失敗したときのメソッドを`onFailure`で指定します。
 
 ```php
@@ -95,18 +95,13 @@ class MyController
      */
     protected $contactForm;
 
-    /**
-     * @Inject
-     * @Named("contact_form")
-     */
-    public function setForm(FormInterface $form)
+    #[Inject]
+    public function setForm(#[Named("contact_form")] FormInterface $form)
     {
         $this->contactForm = $form;
     }
 
-    /**
-     * @FormValidation(form="contactForm", onFailure="badRequestAction")
-     */
+    #[FormValidation(form: "contactForm", onFailure: "badRequestAction")]
     public function createAction()
     {
         // validation success
@@ -144,16 +139,14 @@ class MyForm extends AbstractAuraForm
 
 ## Validation Exception
 
-`@FormValidation`の代わりに`@InputValidation`とアノテートするとバリデーションが失敗したときに`Ray\WebFormModule\Exception\ValidationException`が投げられるよになります。この場合はHTML表現は使われません。Web APIアプリケーションなどに便利です。
+`#[FormValidation]`の代わりに`#[InputValidation]`とアノテートするとバリデーションが失敗したときに`Ray\WebFormModule\Exception\ValidationException`が投げられるよになります。この場合はHTML表現は使われません。Web APIアプリケーションなどに便利です。
 
 ```php
 use Ray\WebFormModule\Annotation\InputValidation;
 
 class Foo
 {
-    /**
-     * @InputValidation(form="form1")
-     */
+    #[InputValidation(form: "form1")]
     public function createAction($name)
     {
       // ...
@@ -190,17 +183,11 @@ echo $e->error;
 //}
 ```
 
-`@VndError`アノテーションで`vnd.error+json`に必要な情報を加えることができます。
+`#[VndError]`属性で`vnd.error+json`に必要な情報を加えることができます。
 
 ```php
-    /**
-     * @FormValidation(form="contactForm")
-     * @VndError(
-     *   message="foo validation failed",
-     *   logref="a1000", path="/path/to/error",
-     *   href={"_self"="/path/to/error", "help"="/path/to/help"}
-     * )
-     */
+    #[FormValidation(form: "contactForm")]
+    #[VndError(message: "foo validation failed", logref: "a1000", path: "/path/to/error", href: ["_self" => "/path/to/error", "help" => "/path/to/help"])]
 ```
 
 このオプションのモジュールはAPIアプリケーションの時に有用です。

README.JA.md

@@ -88,7 +88,7 @@ class MyForm extends AbstractForm
 ```
 ### Controller
 
-We annotate the methods which web form validation is required with `@FormValidation`. We can specify form object property name with `name` and failiure method name with `@onFailure`.
+We annotate the methods which web form validation is required with `#[FormValidation]`. We can specify form object property name with `form` and failure method name with `onFailure`.
 
 ```php
 use Ray\Di\Di\Inject;
@@ -103,18 +103,13 @@ class MyController
      */
     protected $contactForm;
 
-    /**
-     * @Inject
-     * @Named("contact_form")
-     */
-    public function setForm(FormInterface $form)
+    #[Inject]
+    public function setForm(#[Named("contact_form")] FormInterface $form)
     {
         $this->contactForm = $form;
     }
 
-    /**
-     * @FormValidation(form="contactForm", onFailure="badRequestAction")
-     */
+    #[FormValidation(form: "contactForm", onFailure: "badRequestAction")]
     public function createAction()
     {
         // validation success
@@ -183,17 +178,11 @@ echo $e->error;
 //}
 ```
 
-More detail for `vnd.error+json`can be add with `@VndError` annotation. 
+More detail for `vnd.error+json` can be added with the `#[VndError]` attribute.
 
 ```php
-    /**
-     * @FormValidation(form="contactForm")
-     * @VndError(
-     *   message="foo validation failed",
-     *   logref="a1000", path="/path/to/error",
-     *   href={"_self"="/path/to/error", "help"="/path/to/help"}
-     * )
-     */
+    #[FormValidation(form: "contactForm")]
+    #[VndError(message: "foo validation failed", logref: "a1000", path: "/path/to/error", href: ["_self" => "/path/to/error", "help" => "/path/to/help"])]
 ```
 
 This optional module is handy for API application. 

README.md

@@ -25,18 +25,22 @@
 use const E_USER_ERROR;
 use const PHP_EOL;
 
+/** @psalm-suppress PropertyNotSetInConstructor */
 abstract class AbstractForm extends Fieldset implements FormInterface
 {
-    /** @var SubjectFilter */
-    protected $filter;
+    /**
+     * @var SubjectFilter
+     * @psalm-suppress NonInvariantDocblockPropertyType
+     */
+    protected $filter; // @phpstan-ignore-line
 
     /** @var array<string, array<string>>|null */
     protected $errorMessages;
 
     /** @var HelperLocator */
     protected $helper;
 
-    /** @var AntiCsrfInterface */
+    /** @var AntiCsrfInterface|null */
     protected $antiCsrf;
 
     public function __construct()
@@ -65,7 +69,7 @@ public function __toString()
         } catch (Throwable $e) {
             trigger_error($e->getMessage() . PHP_EOL . $e->getTraceAsString(), E_USER_ERROR);
 
-            return '';
+            return ''; // @codeCoverageIgnore @phpstan-ignore deadCode.unreachable
         }
     }
 
@@ -74,22 +78,23 @@ public function setBaseDependencies(
         BuilderInterface $builder,
         FilterFactory $filterFactory,
         HelperLocatorFactory $helperFactory,
-    ) {
-        $this->builder = $builder;
+    ): void {
+        /** @psalm-suppress PropertyTypeCoercion */
+        $this->builder = $builder; // @phpstan-ignore-line
         $this->filter = $filterFactory->newSubjectFilter();
         $this->helper = $helperFactory->newInstance();
     }
 
-    public function setAntiCsrf(AntiCsrfInterface $antiCsrf)
+    public function setAntiCsrf(AntiCsrfInterface $antiCsrf): void
     {
         $this->antiCsrf = $antiCsrf;
     }
 
     #[PostConstruct]
-    public function postConstruct()
+    public function postConstruct(): void
     {
         $this->init();
-        if (! ($this->antiCsrf instanceof AntiCsrfInterface)) {
+        if ($this->antiCsrf === null) {
             return;
         }
 
@@ -101,18 +106,32 @@ public function postConstruct()
      */
     public function input($input)
     {
-        return $this->helper->input($this->get($input));
+        /**
+         * @var string $result
+         * @psalm-suppress UndefinedMagicMethod
+         */
+        $result = $this->helper->input($this->get($input)); // @phpstan-ignore-line
+
+        return $result;
     }
 
     /**
      * {@inheritDoc}
      */
     public function error($input)
     {
-        if (! $this->errorMessages) {
+        if ($this->errorMessages === null) {
             $failure = $this->filter->getFailures();
-            if ($failure) {
-                $this->errorMessages = $failure->getMessages();
+            /**
+             * @psalm-suppress RedundantConditionGivenDocblockType - getFailures() can return null at runtime
+             * @phpstan-ignore notIdentical.alwaysTrue
+             */
+            if ($failure !== null) {
+                /** @var array<string, array<string>> $messages */
+                $messages = $failure->getMessages();
+                $this->errorMessages = $messages;
+            } else {
+                $this->errorMessages = [];
             }
         }
 
@@ -133,9 +152,14 @@ public function error($input)
      */
     public function form($attr = [])
     {
-        $form = $this->helper->form($attr);
+        /**
+         * @var string $form
+         * @psalm-suppress UndefinedMagicMethod
+         */
+        $form = $this->helper->form($attr); // @phpstan-ignore-line
         if (isset($this->inputs['__csrf_token'])) {
-            $form .= $this->helper->input($this->get('__csrf_token'));
+            /** @psalm-suppress UndefinedMagicMethod */
+            $form .= $this->helper->input($this->get('__csrf_token')); // @phpstan-ignore-line
         }
 
         return $form;
@@ -152,7 +176,7 @@ public function form($attr = [])
      */
     public function apply(array $data)
     {
-        if ($this->antiCsrf && ! $this->antiCsrf->isValid($data)) {
+        if ($this->antiCsrf !== null && ! $this->antiCsrf->isValid($data)) {
             throw new CsrfViolationException();
         }
 
@@ -168,15 +192,18 @@ public function apply(array $data)
      */
     public function getFailureMessages()
     {
-        return $this->filter->getFailures()->getMessages();
+        /** @var array<string, array<string>> $messages */
+        $messages = $this->filter->getFailures()->getMessages();
+
+        return $messages;
     }
 
     /**
      * Returns all the fields collection
      *
-     * @return ArrayIterator
+     * @return ArrayIterator<array-key, mixed>
      */
-    public function getIterator()
+    public function getIterator(): ArrayIterator
     {
         return new ArrayIterator($this->inputs);
     }

src/AbstractForm.php

@@ -4,10 +4,7 @@
 
 namespace Ray\WebFormModule\Annotation;
 
-use Attribute;
-
-#[Attribute(Attribute::TARGET_METHOD)]
-class AbstractValidation
+abstract class AbstractValidation
 {
     public function __construct(public string $form = 'form')
     {

src/Annotation/AbstractValidation.php

@@ -38,7 +38,7 @@ public function setField(Fieldset $fieldset)
     }
 
     /**
-     * @param array<string, mixed> $data
+     * @param array<array-key, mixed> $data
      *
      * @return bool
      */

src/AntiCsrf.php

@@ -15,6 +15,7 @@
 use ReflectionMethod;
 
 use function array_shift;
+use function is_array;
 use function property_exists;
 
 class AuraInputInterceptor implements MethodInterceptor
@@ -35,11 +36,12 @@ public function __construct(FailureHandlerInterface $handler)
     public function invoke(MethodInvocation $invocation)
     {
         $object = $invocation->getThis();
-        /** @var FormValidation $formValidation */
         $method = $invocation->getMethod();
         $formValidation = $this->getValidationAttribute($method);
         $form = $this->getFormProperty($formValidation, $object);
-        $data = $form instanceof SubmitInterface ? $form->submit() : $this->getNamedArguments($invocation);
+        $submit = $form instanceof SubmitInterface ? $form->submit() : $this->getNamedArguments($invocation);
+        /** @var array<string, mixed> $data */
+        $data = is_array($submit) ? $submit : (array) $submit;
         $isValid = $this->isValid($data, $form);
         if ($isValid === true) {
             // validation   success
@@ -64,6 +66,8 @@ public function isValid(array $submit, AbstractForm $form)
     /**
      * Return arguments as named arguments.
      *
+     * @param MethodInvocation<object> $invocation
+     *
      * @return array<string, mixed>
      */
     private function getNamedArguments(MethodInvocation $invocation)
@@ -72,7 +76,9 @@ private function getNamedArguments(MethodInvocation $invocation)
         $params = $invocation->getMethod()->getParameters();
         $args = $invocation->getArguments()->getArrayCopy();
         foreach ($params as $param) {
+            /** @var mixed $arg */
             $arg = array_shift($args);
+            /** @psalm-suppress MixedAssignment */
             $submit[$param->getName()] = $arg;
         }
 

src/AuraInputInterceptor.php

@@ -17,7 +17,7 @@
 use Ray\WebFormModule\Annotation\FormValidation;
 use Ray\WebFormModule\Annotation\InputValidation;
 
-class AuraInputModule extends AbstractModule
+final class AuraInputModule extends AbstractModule
 {
     /**
      * {@inheritDoc}

src/AuraInputModule.php

@@ -6,6 +6,6 @@
 
 use Aura\Input\Exception\CsrfViolation;
 
-class CsrfViolationException extends CsrfViolation
+final class CsrfViolationException extends CsrfViolation
 {
 }