grom358 / pharborist

A PHP library to query and transform source code via tree operations.
GNU General Public License v3.0
44 stars 10 forks source link

Traversing child nodes, except of nodes being descendants of specific node types #186

Closed pako-pl closed 9 years ago

pako-pl commented 10 years ago

When doing static code analisys, context of a currenly processed call is very important. Right now there is no easy way to "skip" some nodes that shouldn't be matched by find() based on the type of their parent.

public function example() {
  $x = 1;
  $fun = function() use($x) {
    if($x == 1) {
       $z = 2;
    }
  }
}

Let's say I would like to find all assignments inside the example() method, except for assignments contained inside the anonymous function, which belong to a different scope. I can use find() with an isInstanceOf() filter on the example() method node, but it will also match the assignments from the anonynous function. Any suggestions as to filter those out in an efficient way? Or perhaps there should be some more specialized filter for this use case?

grom358 commented 10 years ago

Need some more info here. If you have the function declaration node it would be:

$functionDeclarationNode->getBody()
  ->children(Filter::isInstanceOf('\Pharborist\Operators\AssignNode'));

Or if you want to find all assignments for all functions:

$assignments = $tree->find(Filter::isInstanceOf('\Pharborist\Functions\FunctionDeclarationNode'))
  ->map(function(FunctionDeclarationNode $node) {
    return $node->getBody();
  })
  ->children(Filter::isInstanceOf('\Pharborist\Operators\AssignNode'));
pako-pl commented 10 years ago

For now I added a specialized version of the walk() method to ParentNode:

public function walkSkippingAnonymousFunctions(callable $callback) {
    $child = $this->head;
    while($child) {
      $callback($child);
      if($child instanceof ParentNode && !($child instanceof Functions\AnonymousFunctionNode)) {
        $child->walk($callback);
      }
      $child = $child->next;
    }
  }

This way when processing a method or a function I can skip bodies of anonymous functions it contains. Anything that happens inside the body of an anonymous function (for example variable assignments) isn't visible in the parent scope, so it should be processed separately.

grom358 commented 10 years ago

A more generic solution would be to either have the callback return a boolean on wether it should continue processing that node. Or take an additional callback that decides which nodes to walk down.

public function walk(callable $callback) {
  $child = $this->head;
  while ($child) {
    if ($callback($child) && $child instanceof ParentNode) {
      $child->walk($callback);
    }
    $child = $child->next;
  }
}

Or

public function walk(callable $callback, callaback $walkTest) {
  $child = $this->head;
  while ($child) {
    $callback($child);
    if ($child instanceof ParentNode && $walkTest($child)) {
      $child->walk($callback, $walkTest);
    }
    $child = $child->next;
  }
}
pako-pl commented 10 years ago

A second callback would make the code a little cleaner, but it's an extra function call for every node, so there would be a small performance penalty.

I think the first option - returning a boolean from the callback - is the way to go.

grom358 commented 9 years ago

Added the callback returning FALSE to not walk down that parent node.