diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 8d1a03368c..cc1b45e2ea 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1147,6 +1147,15 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/css/application/diffusion/diffusion-source.css', ), + 'diviner-shared-css' => + array( + 'uri' => '/res/f462d51d/rsrc/css/diviner/diviner-shared.css', + 'type' => 'css', + 'requires' => + array( + ), + 'disk' => '/rsrc/css/diviner/diviner-shared.css', + ), 'global-drag-and-drop-css' => array( 'uri' => '/res/4e24cb65/rsrc/css/application/files/global-drag-and-drop.css', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d7d80a080c..48511a516a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -534,10 +534,13 @@ phutil_register_library_map(array( 'DivinerLiveSymbol' => 'applications/diviner/storage/DivinerLiveSymbol.php', 'DivinerPHIDTypeAtom' => 'applications/diviner/phid/DivinerPHIDTypeAtom.php', 'DivinerPHIDTypeBook' => 'applications/diviner/phid/DivinerPHIDTypeBook.php', + 'DivinerPHPAtomizer' => 'applications/diviner/atomizer/DivinerPHPAtomizer.php', + 'DivinerParameterTableView' => 'applications/diviner/view/DivinerParameterTableView.php', 'DivinerPublishCache' => 'applications/diviner/cache/DivinerPublishCache.php', 'DivinerPublisher' => 'applications/diviner/publisher/DivinerPublisher.php', 'DivinerRemarkupRuleSymbol' => 'applications/diviner/markup/DivinerRemarkupRuleSymbol.php', 'DivinerRenderer' => 'applications/diviner/renderer/DivinerRenderer.php', + 'DivinerReturnTableView' => 'applications/diviner/view/DivinerReturnTableView.php', 'DivinerStaticPublisher' => 'applications/diviner/publisher/DivinerStaticPublisher.php', 'DivinerWorkflow' => 'applications/diviner/workflow/DivinerWorkflow.php', 'DoorkeeperBridge' => 'applications/doorkeeper/bridge/DoorkeeperBridge.php', @@ -2559,8 +2562,11 @@ phutil_register_library_map(array( ), 'DivinerPHIDTypeAtom' => 'PhabricatorPHIDType', 'DivinerPHIDTypeBook' => 'PhabricatorPHIDType', + 'DivinerPHPAtomizer' => 'DivinerAtomizer', + 'DivinerParameterTableView' => 'AphrontTagView', 'DivinerPublishCache' => 'DivinerDiskCache', 'DivinerRemarkupRuleSymbol' => 'PhutilRemarkupRule', + 'DivinerReturnTableView' => 'AphrontTagView', 'DivinerStaticPublisher' => 'DivinerPublisher', 'DivinerWorkflow' => 'PhutilArgumentWorkflow', 'DoorkeeperBridge' => 'Phobject', diff --git a/src/applications/diviner/atom/DivinerAtom.php b/src/applications/diviner/atom/DivinerAtom.php index 31a067fed5..b4190ca7f9 100644 --- a/src/applications/diviner/atom/DivinerAtom.php +++ b/src/applications/diviner/atom/DivinerAtom.php @@ -23,6 +23,7 @@ final class DivinerAtom { private $extends = array(); private $links = array(); private $book; + private $properties = array(); /** * Returns a sorting key which imposes an unambiguous, stable order on atoms. @@ -59,7 +60,7 @@ final class DivinerAtom { } public static function getAtomSerializationVersion() { - return 1; + return 2; } public function addWarning($warning) { @@ -194,6 +195,7 @@ final class DivinerAtom { $this->getLanguage(), $this->getContentRaw(), $this->getDocblockRaw(), + $this->getProperties(), mpull($this->extends, 'toHash'), mpull($this->links, 'toHash'), ); @@ -280,6 +282,7 @@ final class DivinerAtom { 'extends' => $this->getExtendsDictionaries(), 'links' => $this->getLinkDictionaries(), 'ref' => $this->getRef()->toDictionary(), + 'properties' => $this->getProperties(), ); } @@ -312,7 +315,8 @@ final class DivinerAtom { ->setContext(idx($dictionary, 'context')) ->setLanguage(idx($dictionary, 'language')) ->setParentHash(idx($dictionary, 'parentHash')) - ->setDocblockRaw(idx($dictionary, 'docblockRaw')); + ->setDocblockRaw(idx($dictionary, 'docblockRaw')) + ->setProperties(idx($dictionary, 'properties')); foreach (idx($dictionary, 'warnings', array()) as $warning) { $atom->addWarning($warning); @@ -325,4 +329,37 @@ final class DivinerAtom { return $atom; } + public function getProperty($key, $default = null) { + return idx($this->properties, $key, $default); + } + + public function setProperty($key, $value) { + $this->properties[$key] = $value; + } + + public function getProperties() { + return $this->properties; + } + + public function setProperties(array $properties) { + $this->properties = $properties; + return $this; + } + + public static function getThisAtomIsNotDocumentedString($type) { + switch ($type) { + case 'function': + return pht('This function is not documented.'); + case 'class': + return pht('This class is not documented.'); + case 'article': + return pht('This article is not documented.'); + case 'method': + return pht('This method is not documented.'); + default: + phlog("Need translation for '{$type}'."); + return pht('This %s is not documented.', $type); + } + } + } diff --git a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php new file mode 100644 index 0000000000..e9161ac3f2 --- /dev/null +++ b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php @@ -0,0 +1,229 @@ +resolve()); + + $atoms = array(); + + $root = $tree->getRootNode(); + + $func_decl = $root->selectDescendantsOfType('n_FUNCTION_DECLARATION'); + foreach ($func_decl as $func) { + + $name = $func->getChildByIndex(2); + + $atom = id(new DivinerAtom()) + ->setType('function') + ->setName($name->getConcreteString()) + ->setLine($func->getLineNumber()) + ->setFile($file_name); + + $this->findAtomDocblock($atom, $func); + + $this->parseParams($atom, $func); + $this->parseReturnType($atom, $func); + + $atoms[] = $atom; + } + + return $atoms; + } + + private function parseParams(DivinerAtom $atom, AASTNode $func) { + $params = $func + ->getChildByIndex(3, 'n_DECLARATAION_PARAMETER_LIST') + ->selectDescendantsOfType('n_DECLARATION_PARAMETER'); + + $param_spec = array(); + + if ($atom->getDocblockRaw()) { + $metadata = $atom->getDocblockMeta(); + } else { + $metadata = array(); + } + + $docs = idx($metadata, 'param'); + if ($docs) { + $docs = explode("\n", $docs); + $docs = array_filter($docs); + } else { + $docs = array(); + } + + if (count($docs)) { + if (count($docs) < count($params)) { + $atom->addWarning( + pht( + 'This call takes %d parameters, but only %d are documented.', + count($params), + count($docs))); + } + } + + foreach ($params as $param) { + $name = $param->getChildByIndex(1)->getConcreteString(); + $dict = array( + 'type' => $param->getChildByIndex(0)->getConcreteString(), + 'default' => $param->getChildByIndex(2)->getConcreteString(), + ); + + if ($docs) { + $doc = array_shift($docs); + if ($doc) { + $dict += $this->parseParamDoc($atom, $doc, $name); + } + } + + $param_spec[] = array( + 'name' => $name, + ) + $dict; + } + + if ($docs) { + foreach ($docs as $doc) { + if ($doc) { + $param_spec[] = $this->parseParamDoc($atom, $doc, null); + } + } + } + + // TODO: Find `assert_instances_of()` calls in the function body and + // add their type information here. See T1089. + + $atom->setProperty('parameters', $param_spec); + } + + + private function findAtomDocblock(DivinerAtom $atom, XHPASTNode $node) { + $token = $node->getDocblockToken(); + if ($token) { + $atom->setDocblockRaw($token->getValue()); + return true; + } else { + $tokens = $node->getTokens(); + if ($tokens) { + $prev = head($tokens); + while ($prev = $prev->getPrevToken()) { + if ($prev->isAnyWhitespace()) { + continue; + } + break; + } + + if ($prev && $prev->isComment()) { + $value = $prev->getValue(); + $matches = null; + if (preg_match('/@(return|param|task|author)/', $value, $matches)) { + $atom->addWarning( + pht( + 'Atom "%s" is preceded by a comment containing "@%s", but the '. + 'comment is not a documentation comment. Documentation '. + 'comments must begin with "/**", followed by a newline. Did '. + 'you mean to use a documentation comment? (As the comment is '. + 'not a documentation comment, it will be ignored.)', + $atom->getName(), + $matches[1])); + } + } + } + + $atom->setDocblockRaw(''); + return false; + } + } + + protected function parseParamDoc(DivinerAtom $atom, $doc, $name) { + $dict = array(); + $split = preg_split('/\s+/', trim($doc), $limit = 2); + if (!empty($split[0])) { + $dict['doctype'] = $split[0]; + } + + if (!empty($split[1])) { + $docs = $split[1]; + + // If the parameter is documented like "@param int $num Blah blah ..", + // get rid of the `$num` part (which Diviner considers optional). If it + // is present and different from the declared name, raise a warning. + $matches = null; + if (preg_match('/^(\\$\S+)\s+/', $docs, $matches)) { + if ($name !== null) { + if ($matches[1] !== $name) { + $atom->addWarning( + pht( + 'Parameter "%s" is named "%s" in the documentation. The '. + 'documentation may be out of date.', + $name, + $matches[1])); + } + } + $docs = substr($docs, strlen($matches[0])); + } + + $dict['docs'] = $docs; + } + + return $dict; + } + + private function parseReturnType(DivinerAtom $atom, XHPASTNode $decl) { + $return_spec = array(); + + $metadata = $atom->getDocblockMeta(); + $return = idx($metadata, 'return'); + + if (!$return) { + $return = idx($metadata, 'returns'); + if ($return) { + $atom->addWarning( + pht('Documentation uses `@returns`, but should use `@return`.')); + } + } + + if ($atom->getName() == '__construct' && $atom->getType() == 'method') { + $return_spec = array( + 'doctype' => 'this', + 'docs' => '//Implicit.//', + ); + + if ($return) { + $atom->addWarning( + 'Method __construct() has explicitly documented @return. The '. + '__construct() method always returns $this. Diviner documents '. + 'this implicitly.'); + } + } else if ($return) { + $split = preg_split('/\s+/', trim($return), $limit = 2); + if (!empty($split[0])) { + $type = $split[0]; + } + + if ($decl->getChildByIndex(1)->getTypeName() == 'n_REFERENCE') { + $type = $type.' &'; + } + + $docs = null; + if (!empty($split[1])) { + $docs = $split[1]; + } + + $return_spec = array( + 'doctype' => $type, + 'docs' => $docs, + ); + } else { + $return_spec = array( + 'type' => 'wild', + ); + } + + $atom->setProperty('return', $return_spec); + } + +} + diff --git a/src/applications/diviner/controller/DivinerAtomController.php b/src/applications/diviner/controller/DivinerAtomController.php index 0ab2caa099..d5be8213c2 100644 --- a/src/applications/diviner/controller/DivinerAtomController.php +++ b/src/applications/diviner/controller/DivinerAtomController.php @@ -33,6 +33,11 @@ final class DivinerAtomController extends DivinerController { return new Aphront404Response(); } + // TODO: This query won't load ghosts, because they'll fail `needAtoms()`. + // Instead, we might want to load ghosts and render a message like + // "this thing existed in an older version, but no longer does", especially + // if we add content like comments. + $symbol = id(new DivinerAtomQuery()) ->setViewer($viewer) ->withBookPHIDs(array($book->getPHID())) @@ -85,6 +90,14 @@ final class DivinerAtomController extends DivinerController { pht('Defined'), $atom->getFile().':'.$atom->getLine()); + $warnings = $atom->getWarnings(); + if ($warnings) { + $warnings = id(new AphrontErrorView()) + ->setErrors($warnings) + ->setTitle(pht('Documentation Warnings')) + ->setSeverity(AphrontErrorView::SEVERITY_WARNING); + } + $field = 'default'; $engine = id(new PhabricatorMarkupEngine()) ->setViewer($viewer) @@ -93,6 +106,23 @@ final class DivinerAtomController extends DivinerController { $content = $engine->getOutput($symbol, $field); + if (strlen(trim($symbol->getMarkupText($field)))) { + $content = phutil_tag( + 'div', + array( + 'class' => 'phabricator-remarkup', + ), + array( + $content, + )); + } else { + $undoc = DivinerAtom::getThisAtomIsNotDocumentedString($atom->getType()); + $content = id(new AphrontErrorView()) + ->appendChild($undoc) + ->setSeverity(AphrontErrorView::SEVERITY_NODATA); + } + + $toc = $engine->getEngineMetadata( $symbol, $field, @@ -103,15 +133,29 @@ final class DivinerAtomController extends DivinerController { ->setBook($book->getTitle(), $group_name) ->setHeader($header) ->appendChild($properties) - ->appendChild( - phutil_tag( - 'div', - array( - 'class' => 'phabricator-remarkup', - ), - array( - $content, - ))); + ->appendChild($warnings) + ->appendChild($content); + + $parameters = $atom->getProperty('parameters'); + if ($parameters !== null) { + $document->appendChild( + id(new PhabricatorHeaderView()) + ->setHeader(pht('Parameters'))); + + $document->appendChild( + id(new DivinerParameterTableView()) + ->setParameters($parameters)); + } + + $return = $atom->getProperty('return'); + if ($return !== null) { + $document->appendChild( + id(new PhabricatorHeaderView()) + ->setHeader(pht('Return'))); + $document->appendChild( + id(new DivinerReturnTableView()) + ->setReturn($return)); + } if ($toc) { $side = new PHUIListView(); diff --git a/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php b/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php index ca345d81d4..0e9aab53eb 100644 --- a/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php +++ b/src/applications/diviner/markup/DivinerRemarkupRuleSymbol.php @@ -114,6 +114,17 @@ final class DivinerRemarkupRuleSymbol extends PhutilRemarkupRule { )); } + // TODO: This probably is not the best place to do this. Move it somewhere + // better when it becomes more clear where it should actually go. + if ($ref) { + switch ($ref->getType()) { + case 'function': + case 'method': + $title = $title.'()'; + break; + } + } + if ($this->getEngine()->isTextMode()) { if ($href) { $link = $title.' <'.PhabricatorEnv::getProductionURI($href).'>'; diff --git a/src/applications/diviner/publisher/DivinerLivePublisher.php b/src/applications/diviner/publisher/DivinerLivePublisher.php index 4ade54dab2..6e2fc92170 100644 --- a/src/applications/diviner/publisher/DivinerLivePublisher.php +++ b/src/applications/diviner/publisher/DivinerLivePublisher.php @@ -34,6 +34,7 @@ final class DivinerLivePublisher extends DivinerPublisher { ->withContexts(array($atom->getContext())) ->withIndexes(array($this->getAtomSimilarIndex($atom))) ->withIncludeUndocumentable(true) + ->withIncludeGhosts(true) ->executeOne(); if ($symbol) { diff --git a/src/applications/diviner/query/DivinerAtomQuery.php b/src/applications/diviner/query/DivinerAtomQuery.php index 6de47c4614..a461c40c51 100644 --- a/src/applications/diviner/query/DivinerAtomQuery.php +++ b/src/applications/diviner/query/DivinerAtomQuery.php @@ -11,6 +11,7 @@ final class DivinerAtomQuery private $contexts; private $indexes; private $includeUndocumentable; + private $includeGhosts; private $needAtoms; @@ -54,6 +55,29 @@ final class DivinerAtomQuery return $this; } + /** + * Include "ghosts", which are symbols which used to exist but do not exist + * currently (for example, a function which existed in an older version of + * the codebase but was deleted). + * + * These symbols had PHIDs assigned to them, and may have other sorts of + * metadata that we don't want to lose (like comments or flags), so we don't + * delete them outright. They might also come back in the future: the change + * which deleted the symbol might be reverted, or the documentation might + * have been generated incorrectly by accident. In these cases, we can + * restore the original data. + * + * However, most callers are not interested in these symbols, so they are + * excluded by default. You can use this method to include them in results. + * + * @param bool True to include ghosts. + * @return this + */ + public function withIncludeGhosts($include) { + $this->includeGhosts = $include; + return $this; + } + public function withIncludeUndocumentable($include) { $this->includeUndocumentable = $include; return $this; @@ -190,6 +214,12 @@ final class DivinerAtomQuery 'isDocumentable = 1'); } + if (!$this->includeGhosts) { + $where[] = qsprintf( + $conn_r, + 'graphHash IS NOT NULL'); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/diviner/view/DivinerParameterTableView.php b/src/applications/diviner/view/DivinerParameterTableView.php new file mode 100644 index 0000000000..908a4006fb --- /dev/null +++ b/src/applications/diviner/view/DivinerParameterTableView.php @@ -0,0 +1,64 @@ +parameters = $parameters; + return $this; + } + + public function getTagName() { + return 'table'; + } + + public function getTagAttributes() { + return array( + 'class' => 'diviner-parameter-table-view', + ); + } + + public function getTagContent() { + require_celerity_resource('diviner-shared-css'); + + $rows = array(); + foreach ($this->parameters as $param) { + $cells = array(); + + $type = idx($param, 'doctype'); + if (!$type) { + $type = idx($param, 'type'); + } + + $name = idx($param, 'name'); + $docs = idx($param, 'docs'); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'diviner-parameter-table-type diviner-monospace', + ), + $type); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'diviner-parameter-table-name diviner-monospace', + ), + $name); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'diviner-parameter-table-docs', + ), + $docs); + + $rows[] = phutil_tag('tr', array(), $cells); + } + + return $rows; + } + +} diff --git a/src/applications/diviner/view/DivinerReturnTableView.php b/src/applications/diviner/view/DivinerReturnTableView.php new file mode 100644 index 0000000000..dd0868e3ed --- /dev/null +++ b/src/applications/diviner/view/DivinerReturnTableView.php @@ -0,0 +1,53 @@ +return = $return; + return $this; + } + + public function getTagName() { + return 'table'; + } + + public function getTagAttributes() { + return array( + 'class' => 'diviner-return-table-view', + ); + } + + public function getTagContent() { + require_celerity_resource('diviner-shared-css'); + + $return = $this->return; + + $type = idx($return, 'doctype'); + if (!$type) { + $type = idx($return, 'type'); + } + + $docs = idx($return, 'docs'); + + $cells = array(); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'diviner-return-table-type diviner-monospace', + ), + $type); + + $cells[] = phutil_tag( + 'td', + array( + 'class' => 'diviner-return-table-docs', + ), + $docs); + + return phutil_tag('tr', array(), $cells); + } + +} diff --git a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php index 5599d862e2..afaaab8615 100644 --- a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php +++ b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php @@ -170,10 +170,17 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { private function getAtomizersForFiles(array $files) { $rules = $this->getRules(); + $exclude = $this->getExclude(); $atomizers = array(); foreach ($files as $file) { + foreach ($exclude as $pattern) { + if (preg_match($pattern, $file)) { + continue 2; + } + } + foreach ($rules as $rule => $atomizer) { $ok = preg_match($rule, $file); if ($ok === false) { @@ -191,9 +198,32 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } private function getRules() { - return $this->getConfig('rules', array()) + array( + $rules = $this->getConfig('rules', array( '/\\.diviner$/' => 'DivinerArticleAtomizer', - ); + '/\\.php$/' => 'DivinerPHPAtomizer', + )); + + foreach ($rules as $rule => $atomizer) { + if (@preg_match($rule, '') === false) { + throw new Exception( + "Rule '{$rule}' is not a valid regular expression!"); + } + } + + return $rules; + } + + private function getExclude() { + $exclude = $this->getConfig('exclude', array()); + + foreach ($exclude as $rule) { + if (@preg_match($rule, '') === false) { + throw new Exception( + "Exclude rule '{$rule}' is not a valid regular expression!"); + } + } + + return $exclude; } diff --git a/src/docs/book/phabricator.book b/src/docs/book/phabricator.book new file mode 100644 index 0000000000..6974b01453 --- /dev/null +++ b/src/docs/book/phabricator.book @@ -0,0 +1,17 @@ +{ + "name" : "phabdev", + "title" : "Phabricator Technical Documentation", + "short" : "Phabricator Tech Docs", + "root" : "../../../", + "rules" : { + "(\\.php$)" : "DivinerPHPAtomizer" + }, + "exclude" : [ + "(^externals/)", + "(^scripts/)", + "(^support/)", + "(^resources/)" + ], + "groups" : { + } +} diff --git a/src/docs/book/user.book b/src/docs/book/user.book index e0c062fae5..f8f69df87c 100644 --- a/src/docs/book/user.book +++ b/src/docs/book/user.book @@ -3,6 +3,14 @@ "title" : "Phabricator User Documentation", "short" : "Phabricator User Docs", "root" : "../../../", + "rules" : { + "(\\.diviner$)" : "DivinerArticleAtomizer" + }, + "exclude" : [ + "(^externals/)", + "(^scripts/)", + "(^support/)" + ], "groups" : { "intro" : { "name" : "Introduction" diff --git a/webroot/rsrc/css/diviner/diviner-shared.css b/webroot/rsrc/css/diviner/diviner-shared.css new file mode 100644 index 0000000000..491a9454d8 --- /dev/null +++ b/webroot/rsrc/css/diviner/diviner-shared.css @@ -0,0 +1,38 @@ +/** + * @provides diviner-shared-css + */ + +.diviner-monospace { + font-family: monospace; + font-size: 13px; +} + +.diviner-return-table-view, +.diviner-parameter-table-view { + width: 100%; + margin: 0 0 16px; + background: #f6f6f6; + border-bottom: 1px solid #c0c5d1; + width: 100%; +} + +.diviner-return-table-type, +.diviner-parameter-table-type { + padding: 4px 8px 4px 12px; + white-space: nowrap; + text-align: right; + color: #666666; + width: 20%; +} + +.diviner-parameter-table-name { + padding: 4px 8px; + white-space: nowrap; + font-weight: bold; +} + +.diviner-return-table-docs, +.diviner-parameter-table-docs { + padding: 4px 12px 4px 8px; + width: 80%; +}