Merge pull request #118 from moufmouf/allow_abstract_types

Allow abstract types
thecodingmachine · Jul 31, 2019 · 182b577 · 182b577
2 parents 618c3dd + 1df8699
commit 182b577
Show file tree

Hide file tree

Showing 9 changed files with 82 additions and 36 deletions.
diff --git 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<string, (object|null)> */

diff --git 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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,18 @@
+<?php
+
+
+namespace TheCodingMachine\GraphQLite\Fixtures\NonInstantiableType;
+
+use TheCodingMachine\GraphQLite\Annotations\Field;
+use TheCodingMachine\GraphQLite\Annotations\Right;
+use TheCodingMachine\GraphQLite\Annotations\SourceField;
+use TheCodingMachine\GraphQLite\Annotations\Type;
+use TheCodingMachine\GraphQLite\Fixtures\TestObject;
+
+/**
+ * @Type(class=TheCodingMachine\GraphQLite\Fixtures\TestObject::class)
+ * @SourceField(name="test")
+ */
+abstract class AbstractFooType
+{
+}
diff --git a/tests/Fixtures/Types/AbstractFooType.php b/tests/Fixtures/Types/AbstractFooType.php
@@ -10,7 +10,6 @@
 use TheCodingMachine\GraphQLite\Fixtures\TestObject;
 
 /**
- * @Type(class=TheCodingMachine\GraphQLite\Fixtures\TestObject::class)
  * @SourceField(name="test")
  * @SourceField(name="testBool", logged=true)
  * @SourceField(name="testRight", right=@Right(name="FOOBAR"))

diff --git a/tests/Fixtures/Types/FooType.php b/tests/Fixtures/Types/FooType.php
@@ -7,6 +7,9 @@
 use TheCodingMachine\GraphQLite\Annotations\SourceField;
 use TheCodingMachine\GraphQLite\Annotations\Type;
 
+/**
+ * @Type(class=TheCodingMachine\GraphQLite\Fixtures\TestObject::class)
+ */
 class FooType extends AbstractFooType
 {
 }
diff --git a/tests/Mappers/GlobTypeMapperTest.php b/tests/Mappers/GlobTypeMapperTest.php
@@ -21,11 +21,13 @@
 use TheCodingMachine\GraphQLite\Fixtures\Types\FooType;
 use TheCodingMachine\GraphQLite\Fixtures\Types\TestFactory;
 use TheCodingMachine\GraphQLite\Fixtures\Types\TestFactoryNoType;
+use TheCodingMachine\GraphQLite\GraphQLException;
 use TheCodingMachine\GraphQLite\NamingStrategy;
 use TheCodingMachine\GraphQLite\TypeGenerator;
 use GraphQL\Type\Definition\ObjectType;
 use TheCodingMachine\GraphQLite\Types\MutableObjectType;
 use TheCodingMachine\GraphQLite\Types\ResolvableMutableInputObjectType;
+use function array_keys;
 
 class GlobTypeMapperTest extends AbstractQueryProviderTest
 {
@@ -46,7 +48,7 @@ public function testGlobTypeMapper(): void
 
         $this->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);
+    }
 }