-   Notifications  
You must be signed in to change notification settings  - Fork 1.5k
 
Description
Describe the bug
 Parameters in a function declaration can have attributes attached to them. This is currently not handled correctly in the File::getMethodParameters() method.
Code sample
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, ) {} }For the above code, the T_STRING tokens from within the attribute would be added to the type_hint array key in the return value, so for example, for the first parameter, the type_hint would (incorrectly) come back as 'type_hint' => '\MyExample\MyAttributestring'
Proposed behavior
 I'm currently looking into fixing this.
My current thinking hinges on four possible "solutions".
- Ignore attributes in function declarations altogether. Just skip over them.
 - Add an 
attributesindex key to the array for each parameter with a boolean flag to indicate whether there are attribute(s) attached to the parameter. This index key would always be set and defaults tofalse.
Thecontentkey would remain the same as it is now and not contain the attribute(s).
This option will give sniff writers an indication whether attributes are attached to the parameter. If the sniff writer would need the attribute details, they can do afindPrevious()for anT_ATTRIBUTE_ENDtoken before'token'and walk the attribute tokens from there. - Add three new index keys to the array for each parameter which would only be set when there are attribute(s) attached to the parameter.
Thecontentkey would now also contain the complete attribute(s).attributescontaining a string representation of the attribute(s).attributes_startcontaining the stack pointer to the first attribute opener for this parameterattributes_endcontaining the stack pointer to the last attribute closer for this parameter.
 - Add a new multi-level array index key 
attributes, which would only be set when there are attribute(s) attached to the parameter. The array format would be along the lines of the below.
Thecontentkey would now also contain the complete attribute(s).'attributes' => [ [0] => [ - `attribute` containing a string representation of the first attribute. - `attribute_start` containing the stack pointer to the attribute opener for the first attribute. - `attribute_end` containing the stack pointer to the attribute closer for the first attribute. ], [1] => ... ] 
I'm personally leaning towards either option 1 or option 2.
I don't think option 3 is a good solution as there can be multiple attributes attached to the parameter and that solution does not do that justice.
As for option 4, I'm not so sure that this is really needed as sniffs examining attributes can just listen to T_ATTRIBUTE instead and for sniffs specifically only examining attributes in function declarations, having the attributes indicator (option 2) or walking the tokens between the open/close parentheses looking for T_ATTRIBUTE tokens, is probably enough and would prevent the performance hit of processing potential attributes on each call to File::getMethodParameters().
@gsherwood Your input/opinion on which solution is preferred would be much appreciated.
Versions (please complete the following information):
- PHPCS: 
master