Skip to content

Add Node->getSubNodes() #1033

Open
@staabm

Description

@staabm

the current Node->getSubNodeNames() api leads to consuming code with dynamic property fetches like:

			foreach ($node->getSubNodeNames() as $subNodeName) {
				if ($node instanceof Node\Expr\Closure && $subNodeName !== 'uses') {
					continue;
				}
				$subNode = $node->{$subNodeName};
				$variableNames = array_merge($variableNames, $this->getUsedVariables($scope, $subNode));
			}

In PHPStan-src alone this pattern repeats 8 times.

Its also visibile in the php-parser codebase 2 times:

$result = '';
$pos = $startPos;
foreach ($node->getSubNodeNames() as $subNodeName) {
$subNode = $node->$subNodeName;
$origSubNode = $origNode->$subNodeName;
if ((!$subNode instanceof Node && $subNode !== null)
|| (!$origSubNode instanceof Node && $origSubNode !== null)
) {

protected function traverseNode(Node $node): void {
foreach ($node->getSubNodeNames() as $name) {
$subNode = $node->$name;
if (\is_array($subNode)) {
$node->$name = $this->traverseArray($subNode);

this dynamic property fetches lead to badly static analysable code

as soon as a class contains a single dynamic property lookup, phpstan is no longer able to tell you whether one of the declared properties is unused (analog for methods).

what do you think about adding a new Node->getSubNodes() method, so the dynamic property lookup is only available in one place insider php-parser and caller code would be more static analysis friendly?

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions