ExportedNodeResolver - Add support for attributes #1427
Conversation
olsavmic
force-pushed
the
olsavmic-exported-node-resolver-support-attributes
branch
from
865dc9f
to
10637e0
Compare
olsavmic
force-pushed
the
olsavmic-exported-node-resolver-support-attributes
branch
from
33aa3f0
to
e9da297
Compare
|
@ondrejmirtes Could you give me a hint on how to test this properly? I don't see any test cases related to Thank you! |
| public function __construct( | ||
| private ?string $name, | ||
| private string $value, | ||
| private bool $byRef, |
There was a problem hiding this comment.
TODO: Most likely not even possible, test it
There was a problem hiding this comment.
I don't think we need the name here, being indexed correctly by int|string in ExportedAttributeNode should be fine.
There was a problem hiding this comment.
I don't think so, you can switch named argument and keep the value & index.
Ok :)
There was a problem hiding this comment.
I removed the src/Dependency/ExportedNode/ExportedAttributeArgumentNode.php entirely as it was storing only the expression value. Let me know if you prefer to keep it
| public function __construct( | ||
| private ?string $name, | ||
| private string $value, | ||
| private bool $byRef, |
There was a problem hiding this comment.
I don't think we need the name here, being indexed correctly by int|string in ExportedAttributeNode should be fine.
| { | ||
|
|
||
| /** | ||
| * @param ExportedAttributeArgumentNode[] $args |
There was a problem hiding this comment.
array<int|string, ExportedAttributeArgumentNode> please
| foreach ($attributeGroup->attrs as $attribute) { | ||
| $args = []; | ||
| foreach ($attribute->args as $arg) { | ||
| $args[] = new ExportedAttributeArgumentNode( |
There was a problem hiding this comment.
This is where the argument should be indexed by $i or $arg–>name.
|
This is mostly fine :) As for testing - I'd say that static analysis mostly covers that because it's just about passing the correct values into the correct parameters. There's a So please just do the small modifications I asked for in the review, and it's ready to merge :) Thanks. |
olsavmic
force-pushed
the
olsavmic-exported-node-resolver-support-attributes
branch
from
8ed8cbc
to
7ffe60e
Compare
|
Thank you! |
Resolves phpstan/phpstan#6797, phpstan/phpstan#7413