Skip to content

PHP 8.0 | File::getMethodParameters() needs to support attributes #3298

@jrfnl

Description

@jrfnl

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".

  1. Ignore attributes in function declarations altogether. Just skip over them.
  2. Add an attributes index 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 to false.
    The content key 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 a findPrevious() for an T_ATTRIBUTE_END token before 'token' and walk the attribute tokens from there.
  3. 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.
    The content key would now also contain the complete attribute(s).
    • attributes containing a string representation of the attribute(s).
    • attributes_start containing the stack pointer to the first attribute opener for this parameter
    • attributes_end containing the stack pointer to the last attribute closer for this parameter.
  4. 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.
    The content key 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions