From 5d93290e42d74f05a86d985a89f58e1a8fe87f88 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Aug 2016 14:32:36 -0700 Subject: [PATCH] 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 [/src/applications/celerity/CelerityResourceMapGenerator.php:236] 4 #1 CelerityResourceMapGenerator::getProvidesAndRequires(string, string) called at [/src/applications/celerity/CelerityResourceMapGenerator.php:193] 5 #2 CelerityResourceMapGenerator::rebuildTextResources(CelerityPhabricatorResources, CelerityResourceTransformer) called at [/src/applications/celerity/CelerityResourceMapGenerator.php:54] 6 #3 CelerityResourceMapGenerator::generate() called at [/src/__tests__/PhabricatorCelerityTestCase.php:16] 7 #4 PhabricatorCelerityTestCase::testCelerityMaps() 8 #5 call_user_func_array(array, array) called at [/src/unit/engine/phutil/PhutilTestCase.php:492] 9 #6 PhutilTestCase::run() called at [/src/unit/engine/PhutilUnitTestEngine.php:69] 10 #7 PhutilUnitTestEngine::run() called at [/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php:147] 11 #8 ArcanistConfigurationDrivenUnitTestEngine::run() called at [/src/workflow/ArcanistUnitWorkflow.php:167] 12 #9 ArcanistUnitWorkflow::run() called at [/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 --- .../celerity/CelerityResourceMapGenerator.php | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/applications/celerity/CelerityResourceMapGenerator.php b/src/applications/celerity/CelerityResourceMapGenerator.php index a5bb657bd9..91d0193ade 100644 --- a/src/applications/celerity/CelerityResourceMapGenerator.php +++ b/src/applications/celerity/CelerityResourceMapGenerator.php @@ -232,11 +232,8 @@ EOFILE; list($description, $metadata) = $parser->parse($matches[0]); - $provides = preg_split('/\s+/', trim(idx($metadata, 'provides'))); - $requires = preg_split('/\s+/', trim(idx($metadata, 'requires'))); - $provides = array_filter($provides); - $requires = array_filter($requires); - + $provides = $this->parseResourceSymbolList(idx($metadata, 'provides')); + $requires = $this->parseResourceSymbolList(idx($metadata, 'requires')); if (!$provides) { // Tests and documentation-only JS is permitted to @provide no targets. return array(null, null); @@ -364,4 +361,37 @@ EOFILE; return $result; } + private function parseResourceSymbolList($list) { + if (!$list) { + return array(); + } + + // This is valid: + // + // @requires x y + // + // But so is this: + // + // @requires x + // @requires y + // + // Accept either form and produce a list of symbols. + + $list = (array)$list; + + // We can get `true` values if there was a bare `@requires` in the input. + foreach ($list as $key => $item) { + if ($item === true) { + unset($list[$key]); + } + } + + $list = implode(' ', $list); + $list = trim($list); + $list = preg_split('/\s+/', $list); + $list = array_filter($list); + + return $list; + } + }