Page MenuHomePhabricator
Create Task
Maniphest T6885

XHPAST does not include modifiers in n_METHOD_DECLARATION's concrete string
Closed, ResolvedPublic
Actions

Description

Script to reproduce:

<?php
require __DIR__.'/scripts/__init_script__.php';

$input = <<<'PHP'
<?php
class Test {
  public function one() {
  }

  abstract function two() {
  }

  static function three() {
  }
}
PHP;

$tree = XHPASTTree::newFromData($input);
$root = $tree->getRootNode();
$methods = $root->selectDescendantsOfType('n_METHOD_DECLARATION');
foreach ($methods as $method) {
  echo $method->getConcreteString()."\n";
}

Expected output:

public function one() {
  }
abstract function two() {
  }
static function three() {
  }

Actual output:

function one() {
  }
function two() {
  }
function three() {
  }

Revisions and Commits

rPHU libphutil
ClosedD11268 Extend nodes in both directions in XHPAST

Related Objects

Event Timeline

richardvanvelzen created this task.Jan 7 2015, 1:03 PM
richardvanvelzen raised the priority of this task from to Normal.
richardvanvelzen updated the task description. (Show Details)
richardvanvelzen added a project: XHPAST.
richardvanvelzen added a subscriber: richardvanvelzen.
richardvanvelzen added a comment.Jan 7 2015, 1:06 PM

FWIW, this patch fixes it for me

diff --git a/support/xhpast/parser.y b/support/xhpast/parser.y
index 1697c26..e07704b 100644
--- a/support/xhpast/parser.y
+++ b/support/xhpast/parser.y
@@ -1176,7 +1176,7 @@ class_statement:
     yyextra->expecting_xhp_class_statements = yyextra->old_expecting_xhp_class_statements;
 
     $$ = NNEW(n_METHOD_DECLARATION);
-    NLMORE($$, $2);
+    NLMORE($$, $1);
     $$->appendChild($1);
     $$->appendChild($4);
     $$->appendChild(NTYPE($5, n_STRING));
epriestley added a revision: D11268: Extend nodes in both directions in XHPAST.Jan 7 2015, 1:38 PM
epriestley added a subscriber: epriestley.

I expect that patch breaks this:

class C {
  function m() {}
}

The function is not included in the n_METHOD_DECLARATION, which was the original issue in D11215.

richardvanvelzen added a comment.Jan 7 2015, 1:42 PM

Yeah, it did indeed. D11268 does solve the issue correctly for me in all cases I tested.

epriestley mentioned this in rPHU31110f222572: Extend nodes in both directions in XHPAST.Jan 7 2015, 1:49 PM
epriestley closed this task as Resolved.Jan 7 2015, 1:59 PM
epriestley claimed this task.
Phabricator · Built by Phacility