From badfe3c3435d28f9ac20687498c8e412ec2b608d Mon Sep 17 00:00:00 2001 From: Ievgen Sentiabov Date: Wed, 20 Jun 2018 20:11:58 +0300 Subject: [PATCH] Simplified TypeProcessor - Removed `preg_match` usage for param description retrieving - Removed `preg_match` usage for param type retrieving --- .../Reflection/Test/Unit/DataObject.php | 39 ++- .../Test/Unit/TypeProcessorTest.php | 229 +++++++++++------- .../Framework/Reflection/TypeProcessor.php | 44 ++-- 3 files changed, 205 insertions(+), 107 deletions(-) diff --git a/lib/internal/Magento/Framework/Reflection/Test/Unit/DataObject.php b/lib/internal/Magento/Framework/Reflection/Test/Unit/DataObject.php index 6816ecdbdf71c..9aada9d5dbe41 100644 --- a/lib/internal/Magento/Framework/Reflection/Test/Unit/DataObject.php +++ b/lib/internal/Magento/Framework/Reflection/Test/Unit/DataObject.php @@ -5,6 +5,8 @@ */ namespace Magento\Framework\Reflection\Test\Unit; +use Magento\Framework\Reflection\Test\Unit\Fixture\TSampleInterface; + /** * Dummy data object to be used by TypeProcessorTest */ @@ -13,18 +15,23 @@ class DataObject /** * @var string */ - protected $attrName; + private $attrName; /** * @var bool */ - protected $isActive; + private $isActive; /** * @var string */ private $name; + /** + * @var array + */ + private $data = []; + /** * @return string */ @@ -70,4 +77,32 @@ public function setName($name = null) $this->name = $name; return $this; } + + /** + * @param string $key Key is used as index + * @param string $value + * @return void + */ + public function setData(string $key, string $value) + { + $this->data[$key] = $value; + } + + /** + * @param array $data + * @return void + */ + public function addData(array $data) + { + $this->data = $data; + } + + /** + * @param TSampleInterface[] $list + * @return void + */ + public function addObjectList(array $list) + { + $this->data['objects'] = $list; + } } diff --git a/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php b/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php index 4543b3b6eec12..4a61d003e32a8 100644 --- a/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php +++ b/lib/internal/Magento/Framework/Reflection/Test/Unit/TypeProcessorTest.php @@ -8,24 +8,22 @@ use Magento\Framework\Exception\SerializationException; use Magento\Framework\Reflection\Test\Unit\Fixture\TSample; +use Magento\Framework\Reflection\TypeProcessor; use Zend\Code\Reflection\ClassReflection; -/** - * Type processor Test - */ class TypeProcessorTest extends \PHPUnit\Framework\TestCase { /** - * @var \Magento\Framework\Reflection\TypeProcessor + * @var TypeProcessor */ - protected $_typeProcessor; + private $typeProcessor; /** * Set up helper. */ protected function setUp() { - $this->_typeProcessor = new \Magento\Framework\Reflection\TypeProcessor(); + $this->typeProcessor = new TypeProcessor(); } /** @@ -33,11 +31,11 @@ protected function setUp() */ public function testGetTypesData() { - $this->_typeProcessor->setTypeData('typeA', ['dataA']); - $this->_typeProcessor->setTypeData('typeB', ['dataB']); + $this->typeProcessor->setTypeData('typeA', ['dataA']); + $this->typeProcessor->setTypeData('typeB', ['dataB']); $this->assertEquals( ['typeA' => ['dataA'], 'typeB' => ['dataB']], - $this->_typeProcessor->getTypesData() + $this->typeProcessor->getTypesData() ); } @@ -46,11 +44,11 @@ public function testGetTypesData() */ public function testSetTypesData() { - $this->_typeProcessor->setTypeData('typeC', ['dataC']); - $this->assertEquals(['typeC' => ['dataC']], $this->_typeProcessor->getTypesData()); + $this->typeProcessor->setTypeData('typeC', ['dataC']); + $this->assertEquals(['typeC' => ['dataC']], $this->typeProcessor->getTypesData()); $typeData = ['typeA' => ['dataA'], 'typeB' => ['dataB']]; - $this->_typeProcessor->setTypesData($typeData); - $this->assertEquals($typeData, $this->_typeProcessor->getTypesData()); + $this->typeProcessor->setTypesData($typeData); + $this->assertEquals($typeData, $this->typeProcessor->getTypesData()); } /** @@ -59,7 +57,7 @@ public function testSetTypesData() */ public function testGetTypeDataInvalidArgumentException() { - $this->_typeProcessor->getTypeData('NonExistentType'); + $this->typeProcessor->getTypeData('NonExistentType'); } /** @@ -67,8 +65,8 @@ public function testGetTypeDataInvalidArgumentException() */ public function testGetTypeData() { - $this->_typeProcessor->setTypeData('typeA', ['dataA']); - $this->assertEquals(['dataA'], $this->_typeProcessor->getTypeData('typeA')); + $this->typeProcessor->setTypeData('typeA', ['dataA']); + $this->assertEquals(['dataA'], $this->typeProcessor->getTypeData('typeA')); } /** @@ -76,85 +74,88 @@ public function testGetTypeData() */ public function testSetTypeDataArrayMerge() { - $this->_typeProcessor->setTypeData('typeA', ['dataA1']); - $this->_typeProcessor->setTypeData('typeA', ['dataA2']); - $this->_typeProcessor->setTypeData('typeA', ['dataA3']); - $this->_typeProcessor->setTypeData('typeA', [null]); - $this->assertEquals(['dataA1', 'dataA2', 'dataA3', null], $this->_typeProcessor->getTypeData('typeA')); + $this->typeProcessor->setTypeData('typeA', ['dataA1']); + $this->typeProcessor->setTypeData('typeA', ['dataA2']); + $this->typeProcessor->setTypeData('typeA', ['dataA3']); + $this->typeProcessor->setTypeData('typeA', [null]); + $this->assertEquals( + ['dataA1', 'dataA2', 'dataA3', null], + $this->typeProcessor->getTypeData('typeA') + ); } public function testNormalizeType() { - $this->assertEquals('blah', $this->_typeProcessor->normalizeType('blah')); - $this->assertEquals('string', $this->_typeProcessor->normalizeType('str')); - $this->assertEquals('int', $this->_typeProcessor->normalizeType('integer')); - $this->assertEquals('boolean', $this->_typeProcessor->normalizeType('bool')); - $this->assertEquals('anyType', $this->_typeProcessor->normalizeType('mixed')); + $this->assertEquals('blah', $this->typeProcessor->normalizeType('blah')); + $this->assertEquals('string', $this->typeProcessor->normalizeType('str')); + $this->assertEquals('int', $this->typeProcessor->normalizeType('integer')); + $this->assertEquals('boolean', $this->typeProcessor->normalizeType('bool')); + $this->assertEquals('anyType', $this->typeProcessor->normalizeType('mixed')); } public function testIsTypeSimple() { - $this->assertTrue($this->_typeProcessor->isTypeSimple('string')); - $this->assertTrue($this->_typeProcessor->isTypeSimple('string[]')); - $this->assertTrue($this->_typeProcessor->isTypeSimple('int')); - $this->assertTrue($this->_typeProcessor->isTypeSimple('float')); - $this->assertTrue($this->_typeProcessor->isTypeSimple('double')); - $this->assertTrue($this->_typeProcessor->isTypeSimple('boolean')); - $this->assertFalse($this->_typeProcessor->isTypeSimple('blah')); + $this->assertTrue($this->typeProcessor->isTypeSimple('string')); + $this->assertTrue($this->typeProcessor->isTypeSimple('string[]')); + $this->assertTrue($this->typeProcessor->isTypeSimple('int')); + $this->assertTrue($this->typeProcessor->isTypeSimple('float')); + $this->assertTrue($this->typeProcessor->isTypeSimple('double')); + $this->assertTrue($this->typeProcessor->isTypeSimple('boolean')); + $this->assertFalse($this->typeProcessor->isTypeSimple('blah')); } public function testIsTypeAny() { - $this->assertTrue($this->_typeProcessor->isTypeAny('mixed')); - $this->assertTrue($this->_typeProcessor->isTypeAny('mixed[]')); - $this->assertFalse($this->_typeProcessor->isTypeAny('int')); - $this->assertFalse($this->_typeProcessor->isTypeAny('int[]')); + $this->assertTrue($this->typeProcessor->isTypeAny('mixed')); + $this->assertTrue($this->typeProcessor->isTypeAny('mixed[]')); + $this->assertFalse($this->typeProcessor->isTypeAny('int')); + $this->assertFalse($this->typeProcessor->isTypeAny('int[]')); } public function testIsArrayType() { - $this->assertFalse($this->_typeProcessor->isArrayType('string')); - $this->assertTrue($this->_typeProcessor->isArrayType('string[]')); + $this->assertFalse($this->typeProcessor->isArrayType('string')); + $this->assertTrue($this->typeProcessor->isArrayType('string[]')); } public function testIsValidTypeDeclaration() { - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('Traversable')); // Interface - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('stdObj')); // Class - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('array')); - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('callable')); - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('self')); - $this->assertTrue($this->_typeProcessor->isValidTypeDeclaration('self')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('string')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('string[]')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('int')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('float')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('double')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('boolean')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('[]')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('mixed[]')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('stdObj[]')); - $this->assertFalse($this->_typeProcessor->isValidTypeDeclaration('Traversable[]')); + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('Traversable')); // Interface + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('stdObj')); // Class + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('array')); + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('callable')); + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('self')); + $this->assertTrue($this->typeProcessor->isValidTypeDeclaration('self')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('string')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('string[]')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('int')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('float')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('double')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('boolean')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('[]')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('mixed[]')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('stdObj[]')); + $this->assertFalse($this->typeProcessor->isValidTypeDeclaration('Traversable[]')); } public function getArrayItemType() { - $this->assertEquals('string', $this->_typeProcessor->getArrayItemType('str[]')); - $this->assertEquals('string', $this->_typeProcessor->getArrayItemType('string[]')); - $this->assertEquals('integer', $this->_typeProcessor->getArrayItemType('int[]')); - $this->assertEquals('boolean', $this->_typeProcessor->getArrayItemType('bool[]')); - $this->assertEquals('any', $this->_typeProcessor->getArrayItemType('mixed[]')); + $this->assertEquals('string', $this->typeProcessor->getArrayItemType('str[]')); + $this->assertEquals('string', $this->typeProcessor->getArrayItemType('string[]')); + $this->assertEquals('integer', $this->typeProcessor->getArrayItemType('int[]')); + $this->assertEquals('boolean', $this->typeProcessor->getArrayItemType('bool[]')); + $this->assertEquals('any', $this->typeProcessor->getArrayItemType('mixed[]')); } public function testTranslateTypeName() { $this->assertEquals( 'TestModule1V1EntityItem', - $this->_typeProcessor->translateTypeName(\Magento\TestModule1\Service\V1\Entity\Item::class) + $this->typeProcessor->translateTypeName(\Magento\TestModule1\Service\V1\Entity\Item::class) ); $this->assertEquals( 'TestModule3V1EntityParameter[]', - $this->_typeProcessor->translateTypeName('\Magento\TestModule3\Service\V1\Entity\Parameter[]') + $this->typeProcessor->translateTypeName('\Magento\TestModule3\Service\V1\Entity\Parameter[]') ); } @@ -164,47 +165,47 @@ public function testTranslateTypeName() */ public function testTranslateTypeNameInvalidArgumentException() { - $this->_typeProcessor->translateTypeName('\Magento\TestModule3\V1\Parameter[]'); + $this->typeProcessor->translateTypeName('\Magento\TestModule3\V1\Parameter[]'); } public function testTranslateArrayTypeName() { - $this->assertEquals('ArrayOfComplexType', $this->_typeProcessor->translateArrayTypeName('complexType')); + $this->assertEquals('ArrayOfComplexType', $this->typeProcessor->translateArrayTypeName('complexType')); } public function testProcessSimpleTypeIntToString() { $value = 1; $type = 'string'; - $this->assertSame('1', $this->_typeProcessor->processSimpleAndAnyType($value, $type)); + $this->assertSame('1', $this->typeProcessor->processSimpleAndAnyType($value, $type)); } public function testProcessSimpleTypeStringToInt() { $value = '1'; $type = 'int'; - $this->assertSame(1, $this->_typeProcessor->processSimpleAndAnyType($value, $type)); + $this->assertSame(1, $this->typeProcessor->processSimpleAndAnyType($value, $type)); } public function testProcessSimpleTypeMixed() { $value = 1; $type = 'mixed'; - $this->assertSame(1, $this->_typeProcessor->processSimpleAndAnyType($value, $type)); + $this->assertSame(1, $this->typeProcessor->processSimpleAndAnyType($value, $type)); } public function testProcessSimpleTypeIntArrayToStringArray() { $value = [1, 2, 3, 4, 5]; $type = 'string[]'; - $this->assertSame(['1', '2', '3', '4', '5'], $this->_typeProcessor->processSimpleAndAnyType($value, $type)); + $this->assertSame(['1', '2', '3', '4', '5'], $this->typeProcessor->processSimpleAndAnyType($value, $type)); } public function testProcessSimpleTypeStringArrayToIntArray() { $value = ['1', '2', '3', '4', '5']; $type = 'int[]'; - $this->assertSame([1, 2, 3, 4, 5], $this->_typeProcessor->processSimpleAndAnyType($value, $type)); + $this->assertSame([1, 2, 3, 4, 5], $this->typeProcessor->processSimpleAndAnyType($value, $type)); } /** @@ -212,10 +213,11 @@ public function testProcessSimpleTypeStringArrayToIntArray() */ public function testProcessSimpleTypeException($value, $type) { - $this->expectException(SerializationException::class); - $this->expectExceptionMessage('The "' - . $value . '" value\'s type is invalid. The "' . $type . '" type was expected. Verify and try again.'); - $this->_typeProcessor->processSimpleAndAnyType($value, $type); + $this->expectException( + SerializationException::class, + 'Invalid type for value: "' . $value . '". Expected Type: "' . $type . '"' + ); + $this->typeProcessor->processSimpleAndAnyType($value, $type); } public static function processSimpleTypeExceptionProvider() @@ -234,32 +236,89 @@ public function testProcessSimpleTypeInvalidType() { $value = 1; $type = 'int[]'; - $this->_typeProcessor->processSimpleAndAnyType($value, $type); + $this->typeProcessor->processSimpleAndAnyType($value, $type); } /** * @expectedException \LogicException * @expectedExceptionMessageRegExp /@param annotation is incorrect for the parameter "name" \w+/ */ - public function testGetParamType() + public function testGetParamTypeWithIncorrectAnnotation() { - $class = new ClassReflection(\Magento\Framework\Reflection\Test\Unit\DataObject::class); + $class = new ClassReflection(DataObject::class); $methodReflection = $class->getMethod('setName'); $paramsReflection = $methodReflection->getParameters(); - $this->_typeProcessor->getParamType($paramsReflection[0]); + $this->typeProcessor->getParamType($paramsReflection[0]); } - public function testGetParameterDescription() + /** + * Checks a case for different array param types. + * + * @param string $methodName + * @param string $type + * @dataProvider arrayParamTypeDataProvider + */ + public function testGetArrayParamType(string $methodName, string $type) { - $class = new ClassReflection(\Magento\Framework\Reflection\Test\Unit\DataObject::class); - $methodReflection = $class->getMethod('setName'); + $class = new ClassReflection(DataObject::class); + $methodReflection = $class->getMethod($methodName); + $params = $methodReflection->getParameters(); + $this->assertEquals($type, $this->typeProcessor->getParamType(array_pop($params))); + } + + /** + * Get list of methods with expected param types. + * + * @return array + */ + public function arrayParamTypeDataProvider() + { + return [ + ['method name' => 'addData', 'type' => 'array[]'], + ['method name' => 'addObjectList', 'type' => 'TSampleInterface[]'] + ]; + } + + /** + * Checks a case when method param has additional description. + * + * @param string $methodName + * @param array $descriptions + * @dataProvider methodParamsDataProvider + */ + public function testGetParameterDescription(string $methodName, array $descriptions) + { + $class = new ClassReflection(DataObject::class); + $methodReflection = $class->getMethod($methodName); $paramsReflection = $methodReflection->getParameters(); - $this->assertEquals('Name of the attribute', $this->_typeProcessor->getParamDescription($paramsReflection[0])); + foreach ($paramsReflection as $paramReflection) { + $description = array_shift($descriptions); + $this->assertEquals( + $description, + $this->typeProcessor->getParamDescription($paramReflection) + ); + } + } + + /** + * Gets list of method names with params and their descriptions. + * + * @return array + */ + public function methodParamsDataProvider() + { + return [ + ['method name' => 'setName', 'descriptions' => ['Name of the attribute']], + ['method name' => 'setData', 'descriptions' => ['Key is used as index', null]], + ]; } public function testGetOperationName() { - $this->assertEquals("resNameMethodName", $this->_typeProcessor->getOperationName("resName", "methodName")); + $this->assertEquals( + "resNameMethodName", + $this->typeProcessor->getOperationName("resName", "methodName") + ); } /** @@ -277,19 +336,19 @@ public function testGetReturnTypeWithInheritDocBlock() $classReflection = new ClassReflection(TSample::class); $methodReflection = $classReflection->getMethod('getPropertyName'); - self::assertEquals($expected, $this->_typeProcessor->getGetterReturnType($methodReflection)); + self::assertEquals($expected, $this->typeProcessor->getGetterReturnType($methodReflection)); } /** * Checks a case when method and parent interface don't have `@return` annotation. * * @expectedException \InvalidArgumentException - * @expectedExceptionMessage Getter return type must be specified using @return annotation. See Magento\Framework\Reflection\Test\Unit\Fixture\TSample::getName() + * @expectedExceptionMessage Method's return type must be specified using @return annotation. See Magento\Framework\Reflection\Test\Unit\Fixture\TSample::getName() */ public function testGetReturnTypeWithoutReturnTag() { $classReflection = new ClassReflection(TSample::class); $methodReflection = $classReflection->getMethod('getName'); - $this->_typeProcessor->getGetterReturnType($methodReflection); + $this->typeProcessor->getGetterReturnType($methodReflection); } } diff --git a/lib/internal/Magento/Framework/Reflection/TypeProcessor.php b/lib/internal/Magento/Framework/Reflection/TypeProcessor.php index cf2f8bf3369ec..d7206032c68c7 100644 --- a/lib/internal/Magento/Framework/Reflection/TypeProcessor.php +++ b/lib/internal/Magento/Framework/Reflection/TypeProcessor.php @@ -9,6 +9,7 @@ use Magento\Framework\Exception\SerializationException; use Magento\Framework\Phrase; use Zend\Code\Reflection\ClassReflection; +use Zend\Code\Reflection\DocBlock\Tag\ParamTag; use Zend\Code\Reflection\DocBlock\Tag\ReturnTag; use Zend\Code\Reflection\DocBlockReflection; use Zend\Code\Reflection\MethodReflection; @@ -522,35 +523,24 @@ public function getParamType(ParameterReflection $param) } if ($type == 'array') { // try to determine class, if it's array of objects - $docBlock = $param->getDeclaringFunction()->getDocBlock(); - $pattern = "/\@param\s+([\w\\\_]+\[\])\s+\\\${$param->getName()}[\n\r]/"; - $matches = []; - if (preg_match($pattern, $docBlock->getContents(), $matches)) { - return $matches[1]; - } - return "{$type}[]"; + $paramDocBlock = $this->getParamDocBlockTag($param); + $paramTypes = $paramDocBlock->getTypes(); + $paramType = array_shift($paramTypes); + return strpos($paramType, '[]') !== false ? $paramType : "{$paramType}[]"; } return $type; } /** - * Get parameter description + * Gets method parameter description. * * @param ParameterReflection $param * @return string|null */ public function getParamDescription(ParameterReflection $param) { - $docBlock = $param->getDeclaringFunction()->getDocBlock(); - $docBlockLines = explode("\n", $docBlock->getContents()); - $pattern = "/\@param\s+([\w\\\_\[\]\|]+)\s+(\\\${$param->getName()})\s(.*)/"; - $matches = []; - - foreach ($docBlockLines as $line) { - if (preg_match($pattern, $line, $matches)) { - return $matches[3]; - } - } + $paramDocBlock = $this->getParamDocBlockTag($param); + return $paramDocBlock->getDescription(); } /** @@ -731,7 +721,7 @@ private function getMethodReturnAnnotation(MethodReflection $methodReflection) // throw an exception if even implemented interface doesn't have return annotations if (empty($returnAnnotations)) { throw new \InvalidArgumentException( - "Getter return type must be specified using @return annotation. " + "Method's return type must be specified using @return annotation. " . "See {$methodReflection->getDeclaringClass()->getName()}::{$methodName}()" ); } @@ -750,10 +740,24 @@ private function getReturnFromDocBlock(MethodReflection $methodReflection) $methodDocBlock = $methodReflection->getDocBlock(); if (!$methodDocBlock) { throw new \InvalidArgumentException( - "Each getter must have a doc block. " + "Each method must have a doc block. " . "See {$methodReflection->getDeclaringClass()->getName()}::{$methodReflection->getName()}()" ); } return current($methodDocBlock->getTags('return')); } + + /** + * Gets method's param doc block. + * + * @param ParameterReflection $param + * @return ParamTag + */ + private function getParamDocBlockTag(ParameterReflection $param): ParamTag + { + $docBlock = $param->getDeclaringFunction() + ->getDocBlock(); + $paramsTag = $docBlock->getTags('param'); + return $paramsTag[$param->getPosition()]; + } }