From dd4cca5663f4a2d1497c240409d590b83a3a402a Mon Sep 17 00:00:00 2001 From: Vincent Bouzeran Date: Tue, 2 Apr 2024 15:21:06 +0200 Subject: [PATCH 1/2] Ensure that a #[GQL\Arg] on method has a matching parameter --- .../Parser/MetadataParser/MetadataParser.php | 13 +++++++-- tests/Config/Parser/MetadataParserTest.php | 12 ++++++++ .../Invalid/InvalidArgumentNaming.php | 28 +++++++++++++++++++ 3 files changed, 51 insertions(+), 2 deletions(-) create mode 100644 tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php diff --git a/src/Config/Parser/MetadataParser/MetadataParser.php b/src/Config/Parser/MetadataParser/MetadataParser.php index 2df977942..ba1b709ab 100644 --- a/src/Config/Parser/MetadataParser/MetadataParser.php +++ b/src/Config/Parser/MetadataParser/MetadataParser.php @@ -19,6 +19,7 @@ use ReflectionClassConstant; use ReflectionException; use ReflectionMethod; +use ReflectionParameter; use ReflectionProperty; use Reflector; use RuntimeException; @@ -565,6 +566,8 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ $accessMetadata = self::getFirstMetadataMatching($metadatas, Metadata\Access::class); $publicMetadata = self::getFirstMetadataMatching($metadatas, Metadata\IsPublic::class); + $isMethod = $reflector instanceof ReflectionMethod; + if (null === $fieldMetadata) { if (null !== $accessMetadata || null !== $publicMetadata) { throw new InvalidArgumentException(sprintf('The metadatas %s and/or %s defined on "%s" are only usable in addition of metadata %s', self::formatMetadata('Access'), self::formatMetadata('Visible'), $reflector->getName(), self::formatMetadata('Field'))); @@ -573,7 +576,7 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ return []; } - if ($reflector instanceof ReflectionMethod && !$reflector->isPublic()) { + if ($isMethod && !$reflector->isPublic()) { throw new InvalidArgumentException(sprintf('The metadata %s can only be applied to public method. The method "%s" is not public.', self::formatMetadata('Field'), $reflector->getName())); } @@ -591,7 +594,13 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ /** @var Metadata\Arg[] $argAnnotations */ $argAnnotations = self::getMetadataMatching($metadatas, Metadata\Arg::class); + $validArgNames = array_map(fn (ReflectionParameter $parameter) => $parameter->getName(), $isMethod ? $reflector->getParameters() : []); + foreach ($argAnnotations as $arg) { + if ($isMethod && !in_array($arg->name, $validArgNames, true)) { + throw new InvalidArgumentException(sprintf('The argument "%s" defined with #[GQL\Arg] attribute/annotation on method "%s" does not match any parameter name in the method.', $arg->name, $reflector->getName())); + } + $args[$arg->name] = ['type' => $arg->type]; if (isset($arg->description)) { @@ -606,7 +615,7 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ } } - if ($reflector instanceof ReflectionMethod) { + if ($isMethod) { $args = self::guessArgs($reflectionClass, $reflector, $args); } diff --git a/tests/Config/Parser/MetadataParserTest.php b/tests/Config/Parser/MetadataParserTest.php index 00290fd81..c24695f21 100644 --- a/tests/Config/Parser/MetadataParserTest.php +++ b/tests/Config/Parser/MetadataParserTest.php @@ -524,6 +524,18 @@ public function testInvalidParamGuessing(): void } } + public function testInvalidArgumentMatching(): void + { + try { + $file = __DIR__.'/fixtures/annotations/Invalid/InvalidArgumentNaming.php'; + $this->parser('parse', new SplFileInfo($file), $this->containerBuilder, $this->parserConfig); + $this->fail('Missing matching argument should have raise an exception'); + } catch (Exception $e) { + $this->assertInstanceOf(InvalidArgumentException::class, $e); + $this->assertMatchesRegularExpression('/The argument "missingParameter" defined/', $e->getPrevious()->getMessage()); + } + } + public function testInvalidReturnGuessing(): void { try { diff --git a/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php b/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php new file mode 100644 index 000000000..b1841b6da --- /dev/null +++ b/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php @@ -0,0 +1,28 @@ + Date: Tue, 2 Apr 2024 15:35:31 +0200 Subject: [PATCH 2/2] Fix static analysis --- src/Config/Parser/MetadataParser/MetadataParser.php | 10 ++++------ .../annotations/Invalid/InvalidArgumentNaming.php | 2 -- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/Config/Parser/MetadataParser/MetadataParser.php b/src/Config/Parser/MetadataParser/MetadataParser.php index ba1b709ab..fe906a474 100644 --- a/src/Config/Parser/MetadataParser/MetadataParser.php +++ b/src/Config/Parser/MetadataParser/MetadataParser.php @@ -566,8 +566,6 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ $accessMetadata = self::getFirstMetadataMatching($metadatas, Metadata\Access::class); $publicMetadata = self::getFirstMetadataMatching($metadatas, Metadata\IsPublic::class); - $isMethod = $reflector instanceof ReflectionMethod; - if (null === $fieldMetadata) { if (null !== $accessMetadata || null !== $publicMetadata) { throw new InvalidArgumentException(sprintf('The metadatas %s and/or %s defined on "%s" are only usable in addition of metadata %s', self::formatMetadata('Access'), self::formatMetadata('Visible'), $reflector->getName(), self::formatMetadata('Field'))); @@ -576,7 +574,7 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ return []; } - if ($isMethod && !$reflector->isPublic()) { + if ($reflector instanceof ReflectionMethod && !$reflector->isPublic()) { throw new InvalidArgumentException(sprintf('The metadata %s can only be applied to public method. The method "%s" is not public.', self::formatMetadata('Field'), $reflector->getName())); } @@ -594,10 +592,10 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ /** @var Metadata\Arg[] $argAnnotations */ $argAnnotations = self::getMetadataMatching($metadatas, Metadata\Arg::class); - $validArgNames = array_map(fn (ReflectionParameter $parameter) => $parameter->getName(), $isMethod ? $reflector->getParameters() : []); + $validArgNames = array_map(fn (ReflectionParameter $parameter) => $parameter->getName(), $reflector instanceof ReflectionMethod ? $reflector->getParameters() : []); foreach ($argAnnotations as $arg) { - if ($isMethod && !in_array($arg->name, $validArgNames, true)) { + if ($reflector instanceof ReflectionMethod && !in_array($arg->name, $validArgNames, true)) { throw new InvalidArgumentException(sprintf('The argument "%s" defined with #[GQL\Arg] attribute/annotation on method "%s" does not match any parameter name in the method.', $arg->name, $reflector->getName())); } @@ -615,7 +613,7 @@ private static function getTypeFieldConfigurationFromReflector(ReflectionClass $ } } - if ($isMethod) { + if ($reflector instanceof ReflectionMethod) { $args = self::guessArgs($reflectionClass, $reflector, $args); } diff --git a/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php b/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php index b1841b6da..fe45a3723 100644 --- a/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php +++ b/tests/Config/Parser/fixtures/annotations/Invalid/InvalidArgumentNaming.php @@ -16,8 +16,6 @@ final class InvalidArgumentNaming * @GQL\Field(name="guessFailed") * * @GQL\Arg(name="missingParameter", type="String") - * - * @param mixed $test */ #[GQL\Field(name: 'guessFailed')] #[GQL\Arg(name: 'missingParameter', type: 'String')]