Merge pull request #75 from andrew-demb/deprecate-target-method-annotation

✨ Support for using attribute as a target to parameter…

Author: andrew-demb

Diff for: composer.json

@@ -30,7 +30,7 @@
     "doctrine/coding-standard": "^11.1|^12.0"
   },
   "scripts": {
-    "phpstan": "phpstan analyse src/ -c phpstan.neon --level=7 --no-progress",
+    "phpstan": "phpstan analyse --no-progress",
     "cs-check": "phpcs",
     "cs-fix": "phpcbf",
     "test": ["@cs-check", "@phpstan", "phpunit"]

Diff for: phpstan.neon

@@ -1,4 +1,9 @@
 parameters:
+    level: 7
+
+    paths:
+        - src
+
     ignoreErrors:
 
     excludePaths:
@@ -7,4 +12,4 @@ parameters:
             - .phpstan-cache
 
 #includes:
-#    - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon
+#    - vendor/thecodingmachine/phpstan-strict-rules/phpstan-strict-rules.neon

Diff for: src/Annotations/Assertion.php

@@ -11,21 +11,28 @@
 
 use function is_array;
 use function ltrim;
+use function trigger_error;
+
+use const E_USER_DEPRECATED;
 
 /**
  * Use this annotation to validate a parameter for a query or mutation.
  *
+ * Note 1: using this attribute as a target to the method (not parameter) is deprecated and will be removed in 8.0.
+ * Note 2: support for `doctrine/annotations` will be removed in 8.0.
+ * Note 3: this class won't implement `ParameterAnnotationInterface` in 8.0.
+ *
  * @Annotation
  * @Target({"METHOD"})
  * @Attributes({
  *   @Attribute("for", type = "string"),
  *   @Attribute("constraint", type = "Symfony\Component\Validator\Constraint[]|Symfony\Component\Validator\Constraint")
  * })
  */
-#[Attribute(Attribute::TARGET_METHOD)]
+#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_PARAMETER)]
 class Assertion implements ParameterAnnotationInterface
 {
-    private string $for;
+    private string|null $for = null;
     /** @var Constraint[] */
     private array $constraint;
 
@@ -40,20 +47,32 @@ public function __construct(
     ) {
         $for = $for ?? $values['for'] ?? null;
         $constraint = $constraint ?? $values['constraint'] ?? null;
-        if ($for === null) {
-            throw new BadMethodCallException('The Assert attribute must be passed a target. For instance: "#[Assert(for: "$email", constraint: new Email())"');
-        }
 
         if ($constraint === null) {
-            throw new BadMethodCallException('The Assert attribute must be passed one or many constraints. For instance: "#[Assert(for: "$email", constraint: new Email())"');
+            throw new BadMethodCallException('The Assertion attribute must be passed one or many constraints. For instance: "#[Assertion(constraint: new Email())"');
         }
 
-        $this->for = ltrim($for, '$');
         $this->constraint = is_array($constraint) ? $constraint : [$constraint];
+
+        if ($for === null) {
+            return;
+        }
+
+        trigger_error(
+            "Using #[Assertion(for='" . $for . "', constaint='...')] on methods is deprecated in favor " .
+            "of #[Assertion(constraint='...')] the parameter itself.",
+            E_USER_DEPRECATED,
+        );
+
+        $this->for = ltrim($for, '$');
     }
 
     public function getTarget(): string
     {
+        if ($this->for === null) {
+            throw new BadMethodCallException('The Assertion attribute must be passed a target. For instance: "#[Assertion(for: "$email", constraint: new Email())"');
+        }
+
         return $this->for;
     }
 

Diff for: tests/Annotations/AssertionTest.php

@@ -9,17 +9,10 @@
 
 class AssertionTest extends TestCase
 {
-    public function testException1(): void
-    {
-        $this->expectException(BadMethodCallException::class);
-        $this->expectExceptionMessage('The Assert attribute must be passed a target. For instance: "#[Assert(for: "$email", constraint: new Email())"');
-        new Assertion([]);
-    }
-
     public function testException2(): void
     {
         $this->expectException(BadMethodCallException::class);
-        $this->expectExceptionMessage('The Assert attribute must be passed one or many constraints. For instance: "#[Assert(for: "$email", constraint: new Email())"');
+        $this->expectExceptionMessage('The Assertion attribute must be passed one or many constraints. For instance: "#[Assertion(constraint: new Email())"');
         new Assertion(['for' => 'foo']);
     }
 }

Diff for: tests/Fixtures/Controllers/UserController.php

@@ -35,8 +35,17 @@ public function createUser(string $email, string $password): User
 
     #[Query]
     #[Assertion(for: '$email', constraint: new Assert\Email())]
-    public function findByMail(string $email = '[email protected]'): User
+    public function findByMailTargetMethod(string $email = '[email protected]'): User
     {
+        // deprecated way of using the Assertion annotation - as a target of the method
+        return new User($email, 'foo');
+    }
+
+    #[Query]
+    public function findByMail(
+        #[Assertion(constraint: new Assert\Email())]
+        string $email = '[email protected]',
+    ): User {
         return new User($email, 'foo');
     }
 }

Diff for: tests/Fixtures/InvalidControllers/InvalidController.php

@@ -12,9 +12,10 @@
 class InvalidController
 {
     #[Query]
-    #[Assertion(for: '$resolveInfo', constraint: new Assert\Email())]
-    public function invalid(ResolveInfo $resolveInfo): string
-    {
+    public function invalid(
+        #[Assertion(for: '$resolveInfo', constraint: new Assert\Email())]
+        ResolveInfo $resolveInfo,
+    ): string {
         return 'foo';
     }
 }

Diff for: tests/IntegrationTest.php

@@ -108,7 +108,6 @@ public function testEndToEndAssert(): void
 
         $errors = $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors'];
 
-        // TODO: find why message is not in French...
         $this->assertSame('This value is not a valid email address.', $errors[0]['message']);
         $this->assertSame('email', $errors[0]['extensions']['field']);
         $this->assertSame('Validate', $errors[0]['extensions']['category']);
@@ -151,6 +150,70 @@ public function testEndToEndAssert(): void
         $this->assertSame('[email protected]', $data['findByMail']['email']);
     }
 
+    public function testEndToEndAssertForDeprecatedWay(): void
+    {
+        $schema = $this->getSchema();
+        $schema->assertValid();
+
+        $queryString = '
+        {
+          findByMailTargetMethod(email: "notvalid")  {
+            email
+          }
+        }
+        ';
+
+        $result = GraphQL::executeQuery(
+            $schema,
+            $queryString,
+        );
+        $result->setErrorsHandler([WebonyxErrorHandler::class, 'errorHandler']);
+        $result->setErrorFormatter([WebonyxErrorHandler::class, 'errorFormatter']);
+
+        $errors = $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors'];
+
+        $this->assertSame('This value is not a valid email address.', $errors[0]['message']);
+        $this->assertSame('email', $errors[0]['extensions']['field']);
+        $this->assertSame('Validate', $errors[0]['extensions']['category']);
+
+        $queryString = '
+        {
+          findByMailTargetMethod(email: "[email protected]")  {
+            email
+          }
+        }
+        ';
+
+        $result = GraphQL::executeQuery(
+            $schema,
+            $queryString,
+        );
+        $result->setErrorsHandler([WebonyxErrorHandler::class, 'errorHandler']);
+        $result->setErrorFormatter([WebonyxErrorHandler::class, 'errorFormatter']);
+
+        $data = $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['data'];
+        $this->assertSame('[email protected]', $data['findByMailTargetMethod']['email']);
+
+        // Test default parameter
+        $queryString = '
+        {
+          findByMailTargetMethod  {
+            email
+          }
+        }
+        ';
+
+        $result = GraphQL::executeQuery(
+            $schema,
+            $queryString,
+        );
+        $result->setErrorsHandler([WebonyxErrorHandler::class, 'errorHandler']);
+        $result->setErrorFormatter([WebonyxErrorHandler::class, 'errorFormatter']);
+
+        $data = $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['data'];
+        $this->assertSame('[email protected]', $data['findByMailTargetMethod']['email']);
+    }
+
     public function testException(): void
     {
         $schemaFactory = $this->getSchemaFactory();