From e3b113f208be5f149e785beb6bd3619f7752dcda Mon Sep 17 00:00:00 2001 From: jrfnl Date: Fri, 23 Apr 2021 07:10:19 +0200 Subject: [PATCH] File::getMethodParameters(): bug fix for attributes leaking into type hint This commit adds handling of parameter attributes to the `File::getMethodParameters()` method as per option [2] discussed in issue 3298. In practice this means that: * [New] A new `attributes` index is introduced into the returned array which will hold a boolean value indicating whether attributes are attached to the parameter. * [Unchanged] The `content` index in the returned array includes the textual representation of any attributes attached to a parameter. * [Fixed] The `type_hint` and `type_hint_token` indexes will no longer be polluted (set incorrectly) with information belonging to the attribute(s) instead of to the type declaration. Includes minor efficiency fix for handling of parenthesis and brackets in default values. Includes dedicated unit test. Fixes 3298 --- src/Files/File.php | 17 +++- tests/Core/File/GetMethodParametersTest.inc | 14 +++ tests/Core/File/GetMethodParametersTest.php | 107 ++++++++++++++++++++ 3 files changed, 135 insertions(+), 3 deletions(-) diff --git a/src/Files/File.php b/src/Files/File.php index ca01f37e95..d9e52a7e30 100644 --- a/src/Files/File.php +++ b/src/Files/File.php @@ -1283,6 +1283,7 @@ public function getDeclarationName($stackPtr) * 'name' => '$var', // The variable name. * 'token' => integer, // The stack pointer to the variable name. * 'content' => string, // The full content of the variable definition. + * 'attributes' => boolean, // Does the parameter have one or more attributes attached ? * 'pass_by_reference' => boolean, // Is the variable passed by reference? * 'reference_token' => integer, // The stack pointer to the reference operator * // or FALSE if the param is not passed by reference. @@ -1355,6 +1356,7 @@ public function getMethodParameters($stackPtr) $defaultStart = null; $equalToken = null; $paramCount = 0; + $attributes = false; $passByReference = false; $referenceToken = false; $variableLength = false; @@ -1373,18 +1375,25 @@ public function getMethodParameters($stackPtr) if (isset($this->tokens[$i]['parenthesis_opener']) === true) { // Don't do this if it's the close parenthesis for the method. if ($i !== $this->tokens[$i]['parenthesis_closer']) { - $i = ($this->tokens[$i]['parenthesis_closer'] + 1); + $i = $this->tokens[$i]['parenthesis_closer']; + continue; } } if (isset($this->tokens[$i]['bracket_opener']) === true) { - // Don't do this if it's the close parenthesis for the method. if ($i !== $this->tokens[$i]['bracket_closer']) { - $i = ($this->tokens[$i]['bracket_closer'] + 1); + $i = $this->tokens[$i]['bracket_closer']; + continue; } } switch ($this->tokens[$i]['code']) { + case T_ATTRIBUTE: + $attributes = true; + + // Skip to the end of the attribute. + $i = $this->tokens[$i]['attribute_closer']; + break; case T_BITWISE_AND: if ($defaultStart === null) { $passByReference = true; @@ -1501,6 +1510,7 @@ public function getMethodParameters($stackPtr) $vars[$paramCount]['default_equal_token'] = $equalToken; } + $vars[$paramCount]['attributes'] = $attributes; $vars[$paramCount]['pass_by_reference'] = $passByReference; $vars[$paramCount]['reference_token'] = $referenceToken; $vars[$paramCount]['variable_length'] = $variableLength; @@ -1526,6 +1536,7 @@ public function getMethodParameters($stackPtr) $paramStart = ($i + 1); $defaultStart = null; $equalToken = null; + $attributes = false; $passByReference = false; $referenceToken = false; $variableLength = false; diff --git a/tests/Core/File/GetMethodParametersTest.inc b/tests/Core/File/GetMethodParametersTest.inc index 6b66e0543b..ed1762e7d2 100644 --- a/tests/Core/File/GetMethodParametersTest.inc +++ b/tests/Core/File/GetMethodParametersTest.inc @@ -126,3 +126,17 @@ function commentsInParams( // Leading comment. ?MyClass /*-*/ & /*-*/.../*-*/ $param /*-*/ = /*-*/ 'default value' . /*-*/ 'second part' // Trailing comment. ) {} + +/* testParameterAttributesInFunctionDeclaration */ +class ParametersWithAttributes( + public function __construct( + #[\MyExample\MyAttribute] private string $constructorPropPromTypedParamSingleAttribute, + #[MyAttr([1, 2])] + Type|false + $typedParamSingleAttribute, + #[MyAttribute(1234), MyAttribute(5678)] ?int $nullableTypedParamMultiAttribute, + #[WithoutArgument] #[SingleArgument(0)] $nonTypedParamTwoAttributes, + #[MyAttribute(array("key" => "value"))] + &...$otherParam, + ) {} +} diff --git a/tests/Core/File/GetMethodParametersTest.php b/tests/Core/File/GetMethodParametersTest.php index 2f8f4791e1..6de4622aed 100644 --- a/tests/Core/File/GetMethodParametersTest.php +++ b/tests/Core/File/GetMethodParametersTest.php @@ -26,6 +26,7 @@ public function testPassByReference() $expected[0] = [ 'name' => '$var', 'content' => '&$var', + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => false, 'type_hint' => '', @@ -48,6 +49,7 @@ public function testArrayHint() $expected[0] = [ 'name' => '$var', 'content' => 'array $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'array', @@ -70,6 +72,7 @@ public function testTypeHint() $expected[0] = [ 'name' => '$var1', 'content' => 'foo $var1', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'foo', @@ -79,6 +82,7 @@ public function testTypeHint() $expected[1] = [ 'name' => '$var2', 'content' => 'bar $var2', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'bar', @@ -101,6 +105,7 @@ public function testSelfTypeHint() $expected[0] = [ 'name' => '$var', 'content' => 'self $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'self', @@ -123,6 +128,7 @@ public function testNullableTypeHint() $expected[0] = [ 'name' => '$var1', 'content' => '?int $var1', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?int', @@ -132,6 +138,7 @@ public function testNullableTypeHint() $expected[1] = [ 'name' => '$var2', 'content' => '?\bar $var2', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?\bar', @@ -154,6 +161,7 @@ public function testVariable() $expected[0] = [ 'name' => '$var', 'content' => '$var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -176,6 +184,7 @@ public function testSingleDefaultValue() $expected[0] = [ 'name' => '$var1', 'content' => '$var1=self::CONSTANT', + 'attributes' => false, 'default' => 'self::CONSTANT', 'pass_by_reference' => false, 'variable_length' => false, @@ -199,6 +208,7 @@ public function testDefaultValues() $expected[0] = [ 'name' => '$var1', 'content' => '$var1=1', + 'attributes' => false, 'default' => '1', 'pass_by_reference' => false, 'variable_length' => false, @@ -208,6 +218,7 @@ public function testDefaultValues() $expected[1] = [ 'name' => '$var2', 'content' => "\$var2='value'", + 'attributes' => false, 'default' => "'value'", 'pass_by_reference' => false, 'variable_length' => false, @@ -232,6 +243,7 @@ public function testBitwiseAndConstantExpressionDefaultValue() 'name' => '$a', 'content' => '$a = 10 & 20', 'default' => '10 & 20', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -254,6 +266,7 @@ public function testArrowFunction() $expected[0] = [ 'name' => '$a', 'content' => 'int $a', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'int', @@ -263,6 +276,7 @@ public function testArrowFunction() $expected[1] = [ 'name' => '$b', 'content' => '...$b', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => true, 'type_hint' => '', @@ -285,6 +299,7 @@ public function testPHP8MixedTypeHint() $expected[0] = [ 'name' => '$var1', 'content' => 'mixed &...$var1', + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => true, 'type_hint' => 'mixed', @@ -307,6 +322,7 @@ public function testPHP8MixedTypeHintNullable() $expected[0] = [ 'name' => '$var1', 'content' => '?Mixed $var1', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?Mixed', @@ -329,6 +345,7 @@ public function testNamespaceOperatorTypeHint() $expected[0] = [ 'name' => '$var1', 'content' => '?namespace\Name $var1', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?namespace\Name', @@ -351,6 +368,7 @@ public function testPHP8UnionTypesSimple() $expected[0] = [ 'name' => '$number', 'content' => 'int|float $number', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'int|float', @@ -359,6 +377,7 @@ public function testPHP8UnionTypesSimple() $expected[1] = [ 'name' => '$obj', 'content' => 'self|parent &...$obj', + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => true, 'type_hint' => 'self|parent', @@ -381,6 +400,7 @@ public function testPHP8UnionTypesWithSpreadOperatorAndReference() $expected[0] = [ 'name' => '$paramA', 'content' => 'float|null &$paramA', + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => false, 'type_hint' => 'float|null', @@ -389,6 +409,7 @@ public function testPHP8UnionTypesWithSpreadOperatorAndReference() $expected[1] = [ 'name' => '$paramB', 'content' => 'string|int ...$paramB', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => true, 'type_hint' => 'string|int', @@ -412,6 +433,7 @@ public function testPHP8UnionTypesSimpleWithBitwiseOrInDefault() 'name' => '$var', 'content' => 'int|float $var = CONSTANT_A | CONSTANT_B', 'default' => 'CONSTANT_A | CONSTANT_B', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'int|float', @@ -434,6 +456,7 @@ public function testPHP8UnionTypesTwoClasses() $expected[0] = [ 'name' => '$var', 'content' => 'MyClassA|\Package\MyClassB $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'MyClassA|\Package\MyClassB', @@ -456,6 +479,7 @@ public function testPHP8UnionTypesAllBaseTypes() $expected[0] = [ 'name' => '$var', 'content' => 'array|bool|callable|int|float|null|object|string $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'array|bool|callable|int|float|null|object|string', @@ -478,6 +502,7 @@ public function testPHP8UnionTypesAllPseudoTypes() $expected[0] = [ 'name' => '$var', 'content' => 'false|mixed|self|parent|iterable|Resource $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'false|mixed|self|parent|iterable|Resource', @@ -500,6 +525,7 @@ public function testPHP8UnionTypesNullable() $expected[0] = [ 'name' => '$number', 'content' => '?int|float $number', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?int|float', @@ -523,6 +549,7 @@ public function testPHP8PseudoTypeNull() 'name' => '$var', 'content' => 'null $var = null', 'default' => 'null', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'null', @@ -546,6 +573,7 @@ public function testPHP8PseudoTypeFalse() 'name' => '$var', 'content' => 'false $var = false', 'default' => 'false', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'false', @@ -569,6 +597,7 @@ public function testPHP8PseudoTypeFalseAndBool() 'name' => '$var', 'content' => 'bool|false $var = false', 'default' => 'false', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'bool|false', @@ -591,6 +620,7 @@ public function testPHP8ObjectAndClass() $expected[0] = [ 'name' => '$var', 'content' => 'object|ClassName $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'object|ClassName', @@ -613,6 +643,7 @@ public function testPHP8PseudoTypeIterableAndArray() $expected[0] = [ 'name' => '$var', 'content' => 'iterable|array|Traversable $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'iterable|array|Traversable', @@ -635,6 +666,7 @@ public function testPHP8DuplicateTypeInUnionWhitespaceAndComment() $expected[0] = [ 'name' => '$var', 'content' => 'int | string /*comment*/ | INT $var', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'int|string|INT', @@ -658,6 +690,7 @@ public function testPHP8ConstructorPropertyPromotionNoTypes() 'name' => '$x', 'content' => 'public $x = 0.0', 'default' => '0.0', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -668,6 +701,7 @@ public function testPHP8ConstructorPropertyPromotionNoTypes() 'name' => '$y', 'content' => 'protected $y = \'\'', 'default' => "''", + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -678,6 +712,7 @@ public function testPHP8ConstructorPropertyPromotionNoTypes() 'name' => '$z', 'content' => 'private $z = null', 'default' => 'null', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -701,6 +736,7 @@ public function testPHP8ConstructorPropertyPromotionWithTypes() $expected[0] = [ 'name' => '$x', 'content' => 'protected float|int $x', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'float|int', @@ -711,6 +747,7 @@ public function testPHP8ConstructorPropertyPromotionWithTypes() 'name' => '$y', 'content' => 'public ?string &$y = \'test\'', 'default' => "'test'", + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => false, 'type_hint' => '?string', @@ -720,6 +757,7 @@ public function testPHP8ConstructorPropertyPromotionWithTypes() $expected[2] = [ 'name' => '$z', 'content' => 'private mixed $z', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'mixed', @@ -743,6 +781,7 @@ public function testPHP8ConstructorPropertyPromotionAndNormalParam() $expected[0] = [ 'name' => '$promotedProp', 'content' => 'public int $promotedProp', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'int', @@ -752,6 +791,7 @@ public function testPHP8ConstructorPropertyPromotionAndNormalParam() $expected[1] = [ 'name' => '$normalArg', 'content' => '?int $normalArg', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '?int', @@ -774,6 +814,7 @@ public function testPHP8ConstructorPropertyPromotionGlobalFunction() $expected[0] = [ 'name' => '$x', 'content' => 'private $x', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => '', @@ -797,6 +838,7 @@ public function testPHP8ConstructorPropertyPromotionAbstractMethod() $expected[0] = [ 'name' => '$y', 'content' => 'public callable $y', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => false, 'type_hint' => 'callable', @@ -806,6 +848,7 @@ public function testPHP8ConstructorPropertyPromotionAbstractMethod() $expected[1] = [ 'name' => '$x', 'content' => 'private ...$x', + 'attributes' => false, 'pass_by_reference' => false, 'variable_length' => true, 'type_hint' => '', @@ -830,6 +873,7 @@ public function testCommentsInParameter() 'name' => '$param', 'content' => '// Leading comment. ?MyClass /*-*/ & /*-*/.../*-*/ $param /*-*/ = /*-*/ \'default value\' . /*-*/ \'second part\' // Trailing comment.', + 'attributes' => false, 'pass_by_reference' => true, 'variable_length' => true, 'type_hint' => '?MyClass', @@ -841,6 +885,69 @@ public function testCommentsInParameter() }//end testCommentsInParameter() + /** + * Verify behaviour when parameters have attributes attached. + * + * @return void + */ + public function testParameterAttributesInFunctionDeclaration() + { + $expected = []; + $expected[0] = [ + 'name' => '$constructorPropPromTypedParamSingleAttribute', + 'content' => '#[\MyExample\MyAttribute] private string $constructorPropPromTypedParamSingleAttribute', + 'attributes' => true, + 'pass_by_reference' => false, + 'variable_length' => false, + 'type_hint' => 'string', + 'nullable_type' => false, + 'property_visibility' => 'private', + ]; + $expected[1] = [ + 'name' => '$typedParamSingleAttribute', + 'content' => '#[MyAttr([1, 2])] + Type|false + $typedParamSingleAttribute', + 'attributes' => true, + 'pass_by_reference' => false, + 'variable_length' => false, + 'type_hint' => 'Type|false', + 'nullable_type' => false, + ]; + $expected[2] = [ + 'name' => '$nullableTypedParamMultiAttribute', + 'content' => '#[MyAttribute(1234), MyAttribute(5678)] ?int $nullableTypedParamMultiAttribute', + 'attributes' => true, + 'pass_by_reference' => false, + 'variable_length' => false, + 'type_hint' => '?int', + 'nullable_type' => true, + ]; + $expected[3] = [ + 'name' => '$nonTypedParamTwoAttributes', + 'content' => '#[WithoutArgument] #[SingleArgument(0)] $nonTypedParamTwoAttributes', + 'attributes' => true, + 'pass_by_reference' => false, + 'variable_length' => false, + 'type_hint' => '', + 'nullable_type' => false, + ]; + $expected[4] = [ + 'name' => '$otherParam', + 'content' => '#[MyAttribute(array("key" => "value"))] + &...$otherParam', + 'attributes' => true, + 'pass_by_reference' => true, + 'variable_length' => true, + 'type_hint' => '', + 'nullable_type' => false, + ]; + + $this->getMethodParametersTestHelper('/* '.__FUNCTION__.' */', $expected); + + }//end testParameterAttributesInFunctionDeclaration() + + /** * Test helper. *