PHP-Parser icon indicating copy to clipboard operation
PHP-Parser copied to clipboard

Add `Node->getSubNodes()`

Open staabm opened this issue 1 year ago • 1 comments

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: https://github.com/nikic/PHP-Parser/blob/3abf7425cd284141dc5d8d14a9ee444de3345d1a/lib/PhpParser/PrettyPrinterAbstract.php#L643-L651 https://github.com/nikic/PHP-Parser/blob/3abf7425cd284141dc5d8d14a9ee444de3345d1a/lib/PhpParser/NodeTraverser.php#L93-L98

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?

staabm avatar Oct 06 '24 11:10 staabm

I have something prepared which can make the code static analysis friendly and does not require a new method. stay tuned.

staabm avatar Oct 07 '24 07:10 staabm