#25656: Fixes a bug in reflection, where null couldn't be the first t…

…ype in return type
magento · Nov 28, 2019 · 0cea191 · 0cea191
1 parent c7ca62e
commit 0cea191
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 1 deletion.
diff --git a/lib/internal/Magento/Framework/Reflection/Test/Unit/Fixture/TSample.php b/lib/internal/Magento/Framework/Reflection/Test/Unit/Fixture/TSample.php
@@ -22,4 +22,20 @@ public function getName()
     {
         return '';
     }
+
+    /**
+     * @inheritdoc
+     */
+    public function getWithNull()
+    {
+        return null;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function getOnlyNull()
+    {
+        return null;
+    }
 }
diff --git a/lib/internal/Magento/Framework/Reflection/Test/Unit/Fixture/TSampleInterface.php b/lib/internal/Magento/Framework/Reflection/Test/Unit/Fixture/TSampleInterface.php
@@ -18,4 +18,16 @@ public function getPropertyName();
      * Doc block without return tag.
      */
     public function getName();
+
+    /**
+     * return annotation with a null type at first position
+     * @return null|string
+     */
+    public function getWithNull();
+
+    /**
+     * return annotation with only null type
+     * @return null
+     */
+    public function getOnlyNull();
 }
diff --git a/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php b/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php
@@ -364,6 +364,39 @@ public function testGetReturnTypeWithoutReturnTag()
         $this->typeProcessor->getGetterReturnType($methodReflection);
     }
 
+    /**
+     * Checks a case when method return annotation has a null-type at first position,
+     * and a valid type at second.
+     */
+    public function testGetReturnTypeNullAtFirstPos()
+    {
+        $expected = [
+            'type' => 'string',
+            'isRequired' => false,
+            'description' => null,
+            'parameterCount' => 0
+        ];
+
+        $classReflection = new ClassReflection(TSample::class);
+        $methodReflection = $classReflection->getMethod('getWithNull');
+
+        self::assertEquals($expected, $this->typeProcessor->getGetterReturnType($methodReflection));
+    }
+
+    /**
+     * Checks a case when method return annotation has a null-type at first position,
+     * and no other valid return type.
+     *
+     * @expectedException \InvalidArgumentException
+     * @expectedExceptionMessage No valid return type for Magento\Framework\Reflection\Test\Unit\Fixture\TSample::getOnlyNull() specified. Verify the return type and try again.
+     */
+    public function testGetReturnTypeNullAtFirstPosNoValidType()
+    {
+        $classReflection = new ClassReflection(TSample::class);
+        $methodReflection = $classReflection->getMethod('getOnlyNull');
+        $this->typeProcessor->getGetterReturnType($methodReflection);
+    }
+
     /**
      * Simple and complex data provider
      *

diff --git a/lib/internal/Magento/Framework/Reflection/TypeProcessor.php b/lib/internal/Magento/Framework/Reflection/TypeProcessor.php
@@ -286,7 +286,22 @@ public function getGetterReturnType($methodReflection)
     {
         $returnAnnotation = $this->getMethodReturnAnnotation($methodReflection);
         $types = $returnAnnotation->getTypes();
-        $returnType = current($types);
+        $returnType = null;
+        foreach ($types as $type) {
+            if ($type != 'null') {
+                $returnType = $type;
+                break;
+            }
+        }
+        if (!$returnType) {
+            throw new \InvalidArgumentException(
+                sprintf(
+                    'No valid return type for %s::%s() specified. Verify the return type and try again.',
+                    $methodReflection->getDeclaringClass()->getName(),
+                    $methodReflection->getName()
+                )
+            );
+        }
         $nullable = in_array('null', $types);
 
         return [