@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator

Rewrite i18n extraction to use PHP-Parser rather than Xhpast

Summary:
Also:
- Teach it that `count` returns a number. Maybe things should use `phutil_count` instead, but in practice they sometimes don't.
- Let it trace through certain variable assignments.
- Tell it that certain methods of certain classes take arguments that are pre-typed, and use this for edge types.

Part of T16378, T16289, and T15862 (but doesn't close any of them)

Test Plan:
- Run `./bin/i18n extract --clean` on the old version before the patch.
- Save the `src/.cache/i18n_strings.json` file somewhere.
- Run `./bin/i18n extract --clean` on the new version with this patch.
- Save the `src/.cache/i18n_strings.json` file somewhere.
- Compare the two files. The expected differences are a few more params show up as numbers, but nothing else.

Reviewers: O1 Blessed Committers, mainframe98, avivey

Reviewed By: O1 Blessed Committers, avivey

Subscribers: avivey, aklapper, tobiaswiese, valerio.bozzolan, Matthew, Cigaryno

Differential Revision: https://we.phorge.it/D26554

Pppery 8fbaed60 082c274a

+261 -90
+60 -1
src/infrastructure/edges/type/PhabricatorEdgeType.php
··· 57 57 $actor); 58 58 } 59 59 60 + /** 61 + * Returns a string representing how adding this instances of this 62 + * edge should be rendered in history of an object. 63 + * 64 + * Note that the parameters to this function (and overriding functions in 65 + * subclasses) are special-cased by the i18n string extractor; 66 + * the `add_count` parameter is treated as a number if passed to `pht` 67 + * (even though the extractor can't generally parse numbers 68 + * passed as method arguments) 69 + */ 60 70 public function getTransactionAddString( 61 71 $actor, 62 72 $add_count, ··· 68 78 $add_count, 69 79 $add_edges); 70 80 } 71 - 81 + /** 82 + * Returns a string representing how adding removing instances of this 83 + * edge should be rendered in history of an object. 84 + * 85 + * Note that the parameters to this function (and overriding functions in 86 + * subclasses) are special-cased by the i18n string extractor; 87 + * the `rem_count` parameter is treated as a number if passed to `pht` 88 + * (even though the extractor can't generally parse numbers 89 + * passed as method arguments) 90 + */ 72 91 public function getTransactionRemoveString( 73 92 $actor, 74 93 $rem_count, ··· 81 100 $rem_edges); 82 101 } 83 102 103 + /** 104 + * Returns a string representing how adding and removing instances of this 105 + * edge should be rendered in history of an object. 106 + * 107 + * Note that the parameters to this function (and overriding functions in 108 + * subclasses) are special-cased by the i18n string extractor; 109 + * the `*_count` parameters are treated as a numbers if passed to `pht` 110 + * (even though the extractor can't generally parse numbers 111 + * passed as method arguments) 112 + */ 84 113 public function getTransactionEditString( 85 114 $actor, 86 115 $total_count, ··· 99 128 $rem_edges); 100 129 } 101 130 131 + /** 132 + * Returns a string representing how adding instances of this 133 + * edge should be rendered in the feed. 134 + * 135 + * Note that the parameters to this function (and overriding functions in 136 + * subclasses) are special-cased by the i18n string extractor; 137 + * the `add_count` parameter is treated as a number if passed to `pht` 138 + * (even though the extractor can't generally parse numbers 139 + * passed as method arguments) 140 + */ 102 141 public function getFeedAddString( 103 142 $actor, 104 143 $object, ··· 113 152 $add_edges); 114 153 } 115 154 155 + /** 156 + * Returns a string representing how removing instances of this 157 + * edge should be rendered in the feed. 158 + * 159 + * Note that the parameters to this function (and overriding functions in 160 + * subclasses) are special-cased by the i18n string extractor; 161 + * the `rem_count` parameter is treated as a number if passed to `pht` 162 + * (even though the extractor can't generally parse numbers 163 + * passed as method arguments) 164 + */ 116 165 public function getFeedRemoveString( 117 166 $actor, 118 167 $object, ··· 127 176 $rem_edges); 128 177 } 129 178 179 + /** 180 + * Returns a string representing how adding and removing instances of this 181 + * edge should be rendered in the feed. 182 + * 183 + * Note that the parameters to this function (and overriding functions in 184 + * subclasses) are special-cased by the i18n string extractor; 185 + * the `*_count` parameters are treated as numbers if passed to `pht` 186 + * (even though the extractor can't generally parse numbers 187 + * passed as method arguments) 188 + */ 130 189 public function getFeedEditString( 131 190 $actor, 132 191 $object,
+23 -89
src/infrastructure/internationalization/management/PhabricatorInternationalizationManagementExtractWorkflow.php
··· 1 1 <?php 2 2 3 + /** 4 + * @phutil-external-symbol class PhpParser\NodeTraverser 5 + * @phutil-external-symbol class PhorgePHPParserExtractor 6 + */ 3 7 final class PhabricatorInternationalizationManagementExtractWorkflow 4 8 extends PhabricatorInternationalizationManagementWorkflow { 5 9 ··· 127 131 private function extractFiles($root_path, array $files) { 128 132 $hashes = array(); 129 133 130 - $futures = array(); 131 - foreach ($files as $file => $hash) { 132 - $full_path = $root_path.DIRECTORY_SEPARATOR.$file; 133 - $data = Filesystem::readFile($full_path); 134 - $futures[$full_path] = PhutilXHPASTBinary::getParserFuture($data); 135 - 136 - $hashes[$full_path] = $hash; 137 - } 138 - 139 134 $bar = id(new PhutilConsoleProgressBar()) 140 - ->setTotal(count($futures)); 135 + ->setTotal(count($files)); 141 136 142 137 $messages = array(); 143 138 $results = array(); 144 139 145 - $futures = id(new FutureIterator($futures)) 146 - ->limit(8); 147 - foreach ($futures as $full_path => $future) { 140 + $parser = PhutilPHPParserLibrary::getParser(); 141 + // Load this class now (once PHP-Parser is built so its base class exists) 142 + // this is not in the autoloader to avoid errors from tests that try 143 + // to load every class without installing PHP-Parser 144 + $root = dirname(phutil_get_library_root('phorge')); 145 + require_once $root.'/support/php-parser/PhorgePHPParserExtractor.php'; 146 + foreach ($files as $file => $hash) { 148 147 $bar->update(1); 149 - 150 - $hash = $hashes[$full_path]; 151 - 148 + $full_path = $root_path.DIRECTORY_SEPARATOR.$file; 149 + $hashes[$full_path] = $hash; 150 + $file = Filesystem::readablePath($full_path, $root_path); 152 151 try { 153 - $tree = XHPASTTree::newFromDataAndResolvedExecFuture( 154 - Filesystem::readFile($full_path), 155 - $future->resolve()); 152 + $data = Filesystem::readFile($full_path); 153 + $tree = $parser->parse($data); 154 + $visitor = new PhorgePHPParserExtractor($file); 155 + id(new PhpParser\NodeTraverser($visitor))->traverse($tree); 156 156 } catch (Exception $ex) { 157 157 $messages[] = pht( 158 - 'WARNING: Failed to extract strings from file "%s": %s', 158 + 'Failed to parse file "%s": %s', 159 159 $full_path, 160 160 $ex->getMessage()); 161 161 continue; 162 162 } 163 - 164 - $root = $tree->getRootNode(); 165 - $calls = $root->selectDescendantsOfType('n_FUNCTION_CALL'); 166 - foreach ($calls as $call) { 167 - $name = $call->getChildByIndex(0)->getConcreteString(); 168 - if ($name != 'pht') { 169 - continue; 170 - } 171 - 172 - $params = $call->getChildByIndex(1, 'n_CALL_PARAMETER_LIST'); 173 - $string_node = $params->getChildByIndex(0); 174 - $string_line = $string_node->getLineNumber(); 175 - try { 176 - $string_value = $string_node->evalStatic(); 177 - 178 - $args = $params->getChildren(); 179 - $args = array_slice($args, 1); 180 - 181 - $types = array(); 182 - foreach ($args as $child) { 183 - $type = null; 184 - 185 - switch ($child->getTypeName()) { 186 - case 'n_FUNCTION_CALL': 187 - $call = $child->getChildByIndex(0); 188 - if ($call->getTypeName() == 'n_SYMBOL_NAME') { 189 - switch ($call->getConcreteString()) { 190 - case 'phutil_count': 191 - $type = 'number'; 192 - break; 193 - case 'phutil_person': 194 - $type = 'person'; 195 - break; 196 - } 197 - } 198 - break; 199 - case 'n_NEW': 200 - $class = $child->getChildByIndex(0); 201 - if ($class->getTypeName() == 'n_CLASS_NAME') { 202 - switch ($class->getConcreteString()) { 203 - case 'PhutilNumber': 204 - $type = 'number'; 205 - break; 206 - } 207 - } 208 - break; 209 - default: 210 - break; 211 - } 212 - 213 - $types[] = $type; 214 - } 163 + $results[$hash] = $visitor->getResults(); 164 + $messages += $visitor->getWarnings(); 165 + } 215 166 216 - $results[$hash][] = array( 217 - 'string' => $string_value, 218 - 'file' => Filesystem::readablePath($full_path, $root_path), 219 - 'line' => $string_line, 220 - 'types' => $types, 221 - ); 222 - } catch (Exception $ex) { 223 - $messages[] = pht( 224 - 'WARNING: Failed to evaluate pht() call on line %d in "%s": %s', 225 - $call->getLineNumber(), 226 - $full_path, 227 - $ex->getMessage()); 228 - } 229 - } 230 - 231 - $tree->dispose(); 232 - } 233 167 $bar->done(); 234 168 235 169 foreach ($messages as $message) {
+178
support/php-parser/PhorgePHPParserExtractor.php
··· 1 + <?php 2 + 3 + final class PhorgePHPParserExtractor extends PhpParser\NodeVisitorAbstract { 4 + private static $knownTypes = array( 5 + 'PhabricatorEdgeType' => array( 6 + 'getTransactionAddString' => array(null, 'number', null), 7 + 'getTransactionRemoveString' => array(null, 'number', null), 8 + 'getTransactionEditString' => array( 9 + null, 10 + 'number', 11 + 'number', 12 + null, 13 + 'number', 14 + null, 15 + ), 16 + 'getFeedAddString' => array(null, null, 'number', null), 17 + 'getFeedRemoveString' => array(null, null, 'number', null), 18 + 'getFeedEditString' => array( 19 + null, 20 + null, 21 + 'number', 22 + 'number', 23 + null, 24 + 'number', 25 + null, 26 + ), 27 + ), 28 + ); 29 + private $classDecl = null; 30 + private $classExtends = null; 31 + private $methodDecl = null; 32 + private $paramVars = []; 33 + private $filename = null; 34 + private $results = []; 35 + private $warnings = []; 36 + private $variableAssignments = []; 37 + 38 + public function getWarnings() { 39 + return $this->warnings; 40 + } 41 + 42 + public function getResults() { 43 + return $this->results; 44 + } 45 + 46 + public function __construct($filename) { 47 + $this->filename = $filename; 48 + } 49 + 50 + private function getName($node) { 51 + $name = $node->name; 52 + if (is_string($name)) { 53 + return $name; 54 + } 55 + if ($name instanceof PhpParser\Node\Name || 56 + $name instanceof PhpParser\Node\Identifier 57 + ) { 58 + return $name->name; 59 + } 60 + return null; 61 + } 62 + 63 + private function typeNode(PhpParser\Node $node, array $seen = array()) { 64 + if ($node instanceof PhpParser\Node\Scalar\Int_) { 65 + return 'number'; 66 + } else if ($node instanceof PhpParser\Node\Expr\BinaryOp) { 67 + // i.e count($foos) + count($bars) 68 + $type1 = $this->typeNode($node->left, $seen); 69 + $type2 = $this->typeNode($node->right, $seen); 70 + if ($type1 === $type2) { 71 + return $type1; 72 + } 73 + } else if ($node instanceof PhpParser\Node\Expr\FuncCall) { 74 + switch ($this->getName($node)) { 75 + case 'phutil_count': 76 + case 'count': 77 + return 'number'; 78 + case 'phutil_person': 79 + return 'person'; 80 + } 81 + } else if ($node instanceof PhpParser\Node\Expr\New_ ) { 82 + if ($this->getName($node->class) == 'PhutilNumber') { 83 + return 'number'; 84 + } 85 + } else if ($node instanceof PhpParser\Node\Expr\Variable) { 86 + $name = $this->getName($node); 87 + if (isset($seen[$name])) { 88 + return null; 89 + } 90 + if (isset($this->variableAssignments[$name])) { 91 + return $this->typeNode( 92 + $this->variableAssignments[$name], 93 + $seen + [$name => true]); 94 + } 95 + $known = static::$knownTypes; 96 + if (isset($known[$this->classExtends])) { 97 + $class_data = $known[$this->classExtends]; 98 + } else if (isset($known[$this->classDecl])) { 99 + $class_data = $known[$this->classDecl]; 100 + } 101 + if (isset($class_data[$this->methodDecl])) { 102 + $types = $class_data[$this->methodDecl]; 103 + if ($name && isset($this->paramVars[$name])) { 104 + if (isset($types[$this->paramVars[$name]])) { 105 + return $types[$this->paramVars[$name]]; 106 + } 107 + } 108 + } 109 + } 110 + return null; 111 + } 112 + 113 + public function enterNode(PhpParser\Node $node) { 114 + if ($node instanceof PhpParser\Node\Stmt\Class_) { 115 + $this->classDecl = $this->getName($node); 116 + if ($node->extends) { 117 + $this->classExtends = $this->getName($node->extends); 118 + } 119 + } else if ($node instanceof PhpParser\Node\Stmt\ClassMethod) { 120 + $this->methodDecl = $this->getName($node); 121 + $this->paramVars = []; 122 + $i = 0; 123 + foreach ($node->params as $param) { 124 + $name = $this->getName($param->var); 125 + if ($name) { 126 + $this->paramVars[$name] = $i; 127 + } 128 + $i++; 129 + } 130 + } else if ($node instanceof PhpParser\Node\Expr\Assign) { 131 + if ($node->var instanceof PhpParser\Node\Expr\Variable) { 132 + $name = $this->getName($node->var); 133 + // $name could be null if this is a "variable variable" 134 + // (like $$foo = bar) 135 + if ($name) { 136 + $this->variableAssignments[$name] = $node->expr; 137 + } 138 + } 139 + } else if ($node instanceof PhpParser\Node\Expr\FuncCall) { 140 + if ($this->getName($node) == 'pht') { 141 + $types = []; 142 + $args = $node->args; 143 + $first = array_shift($args); 144 + try { 145 + $key = id(new PhpParser\ConstExprEvaluator()) 146 + ->evaluateSilently($first->value); 147 + } catch (PhpParser\ConstExprEvaluationException $ex) { 148 + $this->warnings[] = pht( 149 + 'Failed to evaluate pht() call on line %d in "%s": %s', 150 + $first->getStartLine(), 151 + $this->filename, 152 + $ex->getMessage()); 153 + return; 154 + } 155 + foreach ($args as $child) { 156 + $types[] = $this->typeNode($child->value); 157 + } 158 + $this->results[] = array( 159 + 'string' => $key, 160 + 'line' => $first->getStartLine(), 161 + 'types' => $types, 162 + 'file' => $this->filename, 163 + ); 164 + } 165 + } 166 + } 167 + 168 + public function leaveNode(PhpParser\Node $node) { 169 + if ($node instanceof PhpParser\Node\Stmt\Class_) { 170 + $this->classDecl = null; 171 + $this->classExtends = null; 172 + } else if ($node instanceof PhpParser\Node\Stmt\ClassMethod) { 173 + $this->methodDecl = null; 174 + $this->paramVars = []; 175 + $this->variableAssignments = []; 176 + } 177 + } 178 + }