diff --git a/src/AnnotationReader.php b/src/AnnotationReader.php index 6f126b0012..b8c14c74e7 100644 --- a/src/AnnotationReader.php +++ b/src/AnnotationReader.php @@ -156,35 +156,29 @@ public function getMiddlewareAnnotations(ReflectionMethod $refMethod): Middlewar } /** - * Returns a class annotation. Finds in the parents if not found in the main class. + * Returns a class annotation. Does not look in the parent class. */ private function getClassAnnotation(ReflectionClass $refClass, string $annotationClass): ?object { - do { - $type = null; - try { - $type = $this->reader->getClassAnnotation($refClass, $annotationClass); - } catch (AnnotationException $e) { - switch ($this->mode) { - case self::STRICT_MODE: + $type = null; + try { + $type = $this->reader->getClassAnnotation($refClass, $annotationClass); + } catch (AnnotationException $e) { + switch ($this->mode) { + case self::STRICT_MODE: + throw $e; + case self::LAX_MODE: + if ($this->isErrorImportant($annotationClass, $refClass->getDocComment() ?: '', $refClass->getName())) { throw $e; - case self::LAX_MODE: - if ($this->isErrorImportant($annotationClass, $refClass->getDocComment() ?: '', $refClass->getName())) { - throw $e; - } else { - return null; - } - default: - throw new RuntimeException("Unexpected mode '" . $this->mode . "'."); // @codeCoverageIgnore - } - } - if ($type !== null) { - return $type; + } else { + return null; + } + default: + throw new RuntimeException("Unexpected mode '" . $this->mode . "'."); // @codeCoverageIgnore } - $refClass = $refClass->getParentClass(); - } while ($refClass); + } - return null; + return $type; } /** @var array */ diff --git a/src/Containers/BasicAutoWiringContainer.php b/src/Containers/BasicAutoWiringContainer.php index d6d92fde74..a504e932ab 100644 --- a/src/Containers/BasicAutoWiringContainer.php +++ b/src/Containers/BasicAutoWiringContainer.php @@ -88,7 +88,7 @@ public function has($id): bool if (class_exists($id)) { $refTypeClass = new ReflectionClass($id); - return ! ($refTypeClass->hasMethod('__construct') && $refTypeClass->getMethod('__construct')->getNumberOfRequiredParameters() > 0); + return $refTypeClass->isInstantiable() && ! ($refTypeClass->hasMethod('__construct') && $refTypeClass->getMethod('__construct')->getNumberOfRequiredParameters() > 0); } return false; diff --git a/src/Mappers/GlobTypeMapper.php b/src/Mappers/GlobTypeMapper.php index 836e205ddc..f4f83dbc0f 100644 --- a/src/Mappers/GlobTypeMapper.php +++ b/src/Mappers/GlobTypeMapper.php @@ -67,9 +67,6 @@ protected function getClassList(): array continue; } $refClass = new ReflectionClass($className); - if (! $refClass->isInstantiable() && ! $refClass->isInterface()) { - continue; - } $this->classes[$className] = $refClass; } } diff --git a/src/MissingAnnotationException.php b/src/MissingAnnotationException.php index c9ebc4c8d3..17c7a40a78 100644 --- a/src/MissingAnnotationException.php +++ b/src/MissingAnnotationException.php @@ -13,9 +13,9 @@ public static function missingTypeExceptionToUseSourceField(): self return new self('You cannot use the @SourceField annotation without also adding a @Type annotation or a @ExtendType annotation.'); } - public static function missingTypeException(): self + public static function missingTypeException(string $className): self { - return new self('GraphQL type classes must provide a @Type annotation.'); + return new self('GraphQL type class "' . $className . '" must provide a @Type annotation.'); } public static function missingExtendTypeException(): self diff --git a/src/TypeGenerator.php b/src/TypeGenerator.php index 4e0f52339c..7f53c1364b 100644 --- a/src/TypeGenerator.php +++ b/src/TypeGenerator.php @@ -64,7 +64,7 @@ public function mapAnnotatedObject(string $annotatedObjectClassName): MutableInt $typeField = $this->annotationReader->getTypeAnnotation($refTypeClass); if ($typeField === null) { - throw MissingAnnotationException::missingTypeException(); + throw MissingAnnotationException::missingTypeException($annotatedObjectClassName); } $typeName = $this->namingStrategy->getOutputTypeName($refTypeClass->getName(), $typeField); @@ -74,6 +74,9 @@ public function mapAnnotatedObject(string $annotatedObjectClassName): MutableInt } if (! $typeField->isSelfType()) { + if (! $refTypeClass->isInstantiable()) { + throw new GraphQLException('Class "' . $annotatedObjectClassName . '" annotated with @Type(class="' . $typeField->getClass() . '") must be instantiable.'); + } $annotatedObject = $this->container->get($annotatedObjectClassName); $isInterface = interface_exists($typeField->getClass()); } else { diff --git a/tests/Fixtures/NonInstantiableType/AbstractFooType.php b/tests/Fixtures/NonInstantiableType/AbstractFooType.php new file mode 100644 index 0000000000..f6935f7598 --- /dev/null +++ b/tests/Fixtures/NonInstantiableType/AbstractFooType.php @@ -0,0 +1,18 @@ +assertSame([TestObject::class], $mapper->getSupportedClasses()); $this->assertTrue($mapper->canMapClassToType(TestObject::class)); - $this->assertInstanceOf(ObjectType::class, $mapper->mapClassToType(TestObject::class, null, $this->getTypeMapper())); + $this->assertInstanceOf(ObjectType::class, $mapper->mapClassToType(TestObject::class, null)); $this->assertInstanceOf(ObjectType::class, $mapper->mapNameToType('Foo', $this->getTypeMapper())); $this->assertTrue($mapper->canMapNameToType('Foo')); $this->assertFalse($mapper->canMapNameToType('NotExists')); @@ -57,7 +59,7 @@ public function testGlobTypeMapper(): void $this->assertTrue($anotherMapperSameCache->canMapNameToType('Foo')); $this->expectException(CannotMapTypeException::class); - $mapper->mapClassToType(\stdClass::class, null, $this->getTypeMapper()); + $mapper->mapClassToType(\stdClass::class, null); } public function testGlobTypeMapperDuplicateTypesException(): void @@ -88,9 +90,19 @@ public function testGlobTypeMapperDuplicateInputTypesException(): void $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getTypeMapper(), new NullCache()); - $this->expectException(DuplicateMappingException::class); - $this->expectExceptionMessage('The class \'TheCodingMachine\GraphQLite\Fixtures\TestObject\' should be mapped to only one GraphQL Input type. Two methods are pointing via the @Factory annotation to this class: \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory::myFactory\' and \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory2::myFactory\''); - $mapper->canMapClassToInputType(TestObject::class); + $caught = false; + try { + $mapper->canMapClassToInputType(TestObject::class); + } catch (DuplicateMappingException $e) { + // Depending on the environment, one of the messages can be returned. + $this->assertContains($e->getMessage(), + [ + 'The class \'TheCodingMachine\GraphQLite\Fixtures\TestObject\' should be mapped to only one GraphQL Input type. Two methods are pointing via the @Factory annotation to this class: \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory::myFactory\' and \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory2::myFactory\'', + 'The class \'TheCodingMachine\GraphQLite\Fixtures\TestObject\' should be mapped to only one GraphQL Input type. Two methods are pointing via the @Factory annotation to this class: \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory2::myFactory\' and \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory::myFactory\'' + ]); + $caught = true; + } + $this->assertTrue($caught, 'DuplicateMappingException is thrown'); } public function testGlobTypeMapperInheritedInputTypesException(): void @@ -335,4 +347,24 @@ public function testGlobTypeMapperExtendBadClass(): void $this->expectExceptionMessage('For @ExtendType(class="Exception") annotation declared in class "TheCodingMachine\GraphQLite\Fixtures\BadExtendType2\BadExtendType2", cannot map class "Exception" to a known GraphQL type. Check your TypeMapper configuration.'); $mapper->extendTypeForName('TestObject', $testObjectType); } + + public function testNonInstantiableType(): void + { + $container = new Picotainer([ + FooType::class => function () { + return new FooType(); + } + ]); + + $typeGenerator = $this->getTypeGenerator(); + $inputTypeGenerator = $this->getInputTypeGenerator(); + + $cache = new ArrayCache(); + + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\NonInstantiableType', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getTypeMapper(), $cache); + + $this->expectException(GraphQLException::class); + $this->expectExceptionMessage('Class "TheCodingMachine\GraphQLite\Fixtures\NonInstantiableType\AbstractFooType" annotated with @Type(class="TheCodingMachine\GraphQLite\Fixtures\TestObject") must be instantiable.'); + $mapper->mapClassToType(TestObject::class, null); + } }