Skip to content

Commit 5d93290

Browse files
author
epriestley
committed
Update Celerity map parser for new docblock code
Summary: After D16431, listing the same `@annotation` multiple times makes the docblock parser return a list. We have some resources which list `@requires` or `@provides` several times, but don't handle the new parser properly. Make the code more flexible, since this is a reasonable way to specify the annotations. See also D16432. This produces a failure in this form: ``` [2016-08-23 21:10:15] ERROR 2: trim() expects parameter 1 to be string, array given at [/core/data/drydock/workingcopy-74/repo/phabricator/src/applications/celerity/CelerityResourceMapGenerator.php:236] 2 arcanist(head=master, ref.master=89e8b4852384), phabricator(head=6c940fb71b0a8850c6a1b7f5fc642a8f8135a76a, ref.master=b521f2349e46), phutil(head=master, ref.master=237549280f08) 3 #0 trim(array) called at [<phabricator>/src/applications/celerity/CelerityResourceMapGenerator.php:236] 4 #1 CelerityResourceMapGenerator::getProvidesAndRequires(string, string) called at [<phabricator>/src/applications/celerity/CelerityResourceMapGenerator.php:193] 5 #2 CelerityResourceMapGenerator::rebuildTextResources(CelerityPhabricatorResources, CelerityResourceTransformer) called at [<phabricator>/src/applications/celerity/CelerityResourceMapGenerator.php:54] 6 #3 CelerityResourceMapGenerator::generate() called at [<phabricator>/src/__tests__/PhabricatorCelerityTestCase.php:16] 7 phacility#4 PhabricatorCelerityTestCase::testCelerityMaps() 8 phacility#5 call_user_func_array(array, array) called at [<arcanist>/src/unit/engine/phutil/PhutilTestCase.php:492] 9 phacility#6 PhutilTestCase::run() called at [<arcanist>/src/unit/engine/PhutilUnitTestEngine.php:69] 10 phacility#7 PhutilUnitTestEngine::run() called at [<arcanist>/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php:147] 11 phacility#8 ArcanistConfigurationDrivenUnitTestEngine::run() called at [<arcanist>/src/workflow/ArcanistUnitWorkflow.php:167] 12 #9 ArcanistUnitWorkflow::run() called at [<arcanist>/scripts/arcanist.php:394] ``` Test Plan: Ran `bin/celerity map`, no more warnings and no change to the actual map. Reviewers: joshuaspence, chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16433
1 parent b521f23 commit 5d93290

File tree

1 file changed

+35
-5
lines changed

1 file changed

+35
-5
lines changed

src/applications/celerity/CelerityResourceMapGenerator.php

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -232,11 +232,8 @@ private function getProvidesAndRequires($name, $data) {
232232

233233
list($description, $metadata) = $parser->parse($matches[0]);
234234

235-
$provides = preg_split('/\s+/', trim(idx($metadata, 'provides')));
236-
$requires = preg_split('/\s+/', trim(idx($metadata, 'requires')));
237-
$provides = array_filter($provides);
238-
$requires = array_filter($requires);
239-
235+
$provides = $this->parseResourceSymbolList(idx($metadata, 'provides'));
236+
$requires = $this->parseResourceSymbolList(idx($metadata, 'requires'));
240237
if (!$provides) {
241238
// Tests and documentation-only JS is permitted to @provide no targets.
242239
return array(null, null);
@@ -364,4 +361,37 @@ private function mergeNameMaps(array $maps) {
364361
return $result;
365362
}
366363

364+
private function parseResourceSymbolList($list) {
365+
if (!$list) {
366+
return array();
367+
}
368+
369+
// This is valid:
370+
//
371+
// @requires x y
372+
//
373+
// But so is this:
374+
//
375+
// @requires x
376+
// @requires y
377+
//
378+
// Accept either form and produce a list of symbols.
379+
380+
$list = (array)$list;
381+
382+
// We can get `true` values if there was a bare `@requires` in the input.
383+
foreach ($list as $key => $item) {
384+
if ($item === true) {
385+
unset($list[$key]);
386+
}
387+
}
388+
389+
$list = implode(' ', $list);
390+
$list = trim($list);
391+
$list = preg_split('/\s+/', $list);
392+
$list = array_filter($list);
393+
394+
return $list;
395+
}
396+
367397
}

0 commit comments

Comments
 (0)