From 41ac06959e327ca3167264a6994fa4c450662eb5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 28 Aug 2013 09:54:39 -0700 Subject: [PATCH] Generate some amount of PHP class documentation Summary: Ref T988. This brings the class/interface atomizer over. A lot of parts of this are still varying degrees of very-rough, but most of the data ends up in approximatley the right place. ALSO: PROGRESS BARS Test Plan: See screenshots. Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T988 Differential Revision: https://secure.phabricator.com/D6817 --- .../sql/patches/20130826.divinernode.sql | 5 + src/applications/diviner/atom/DivinerAtom.php | 9 ++ .../diviner/atomizer/DivinerPHPAtomizer.php | 84 ++++++++++++++++ .../controller/DivinerAtomController.php | 93 ++++++++++++++++++ .../publisher/DivinerLivePublisher.php | 14 ++- .../diviner/publisher/DivinerPublisher.php | 1 + .../diviner/query/DivinerAtomQuery.php | 97 +++++++++++++++++++ .../diviner/storage/DivinerLiveSymbol.php | 26 +++-- .../workflow/DivinerAtomizeWorkflow.php | 4 +- .../workflow/DivinerGenerateWorkflow.php | 25 ++++- .../patch/PhabricatorBuiltinPatchList.php | 4 + 11 files changed, 342 insertions(+), 20 deletions(-) create mode 100644 resources/sql/patches/20130826.divinernode.sql diff --git a/resources/sql/patches/20130826.divinernode.sql b/resources/sql/patches/20130826.divinernode.sql new file mode 100644 index 0000000000..265a7769b5 --- /dev/null +++ b/resources/sql/patches/20130826.divinernode.sql @@ -0,0 +1,5 @@ +ALTER TABLE {$NAMESPACE}_diviner.diviner_livesymbol + ADD nodeHash VARCHAR(64) COLLATE utf8_bin; + +ALTER TABLE {$NAMESPACE}_diviner.diviner_livesymbol + ADD UNIQUE KEY (nodeHash); diff --git a/src/applications/diviner/atom/DivinerAtom.php b/src/applications/diviner/atom/DivinerAtom.php index b4190ca7f9..22b0618162 100644 --- a/src/applications/diviner/atom/DivinerAtom.php +++ b/src/applications/diviner/atom/DivinerAtom.php @@ -4,6 +4,7 @@ final class DivinerAtom { const TYPE_FILE = 'file'; const TYPE_ARTICLE = 'article'; + const TYPE_METHOD = 'method'; private $type; private $name; @@ -181,6 +182,10 @@ final class DivinerAtom { return mpull($this->extends, 'toDictionary'); } + public function getExtends() { + return $this->extends; + } + public function getHash() { if ($this->hash) { return $this->hash; @@ -326,6 +331,10 @@ final class DivinerAtom { $atom->addChildHash($child); } + foreach (idx($dictionary, 'extends', array()) as $extends) { + $atom->addExtends(DivinerAtomRef::newFromDictionary($extends)); + } + return $atom; } diff --git a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php index e9161ac3f2..30928fe319 100644 --- a/src/applications/diviner/atomizer/DivinerPHPAtomizer.php +++ b/src/applications/diviner/atomizer/DivinerPHPAtomizer.php @@ -31,6 +31,90 @@ final class DivinerPHPAtomizer extends DivinerAtomizer { $atoms[] = $atom; } + $class_types = array( + 'class' => 'n_CLASS_DECLARATION', + 'interface' => 'n_INTERFACE_DECLARATION', + ); + foreach ($class_types as $atom_type => $node_type) { + $class_decls = $root->selectDescendantsOfType($node_type); + foreach ($class_decls as $class) { + $name = $class->getChildByIndex(1, 'n_CLASS_NAME'); + + $atom = id(new DivinerAtom()) + ->setType($atom_type) + ->setName($name->getConcreteString()) + ->setFile($file_name) + ->setLine($class->getLineNumber()); + + // If this exists, it is n_EXTENDS_LIST. + $extends = $class->getChildByIndex(2); + $extends_class = $extends->selectDescendantsOfType('n_CLASS_NAME'); + foreach ($extends_class as $parent_class) { + $atom->addExtends( + DivinerAtomRef::newFromDictionary( + array( + 'type' => 'class', + 'name' => $parent_class->getConcreteString(), + ))); + } + + // If this exists, it is n_IMPLEMENTS_LIST. + $implements = $class->getChildByIndex(3); + $iface_names = $implements->selectDescendantsOfType('n_CLASS_NAME'); + foreach ($iface_names as $iface_name) { + $atom->addExtends( + DivinerAtomRef::newFromDictionary( + array( + 'type' => 'interface', + 'name' => $iface_name->getConcreteString(), + ))); + } + + $this->findAtomDocblock($atom, $class); + + $methods = $class->selectDescendantsOfType('n_METHOD_DECLARATION'); + foreach ($methods as $method) { + $matom = id(new DivinerAtom()) + ->setType('method'); + + $this->findAtomDocblock($matom, $method); + + $attribute_list = $method->getChildByIndex(0); + $attributes = $attribute_list->selectDescendantsOfType('n_STRING'); + if ($attributes) { + foreach ($attributes as $attribute) { + $attr = strtolower($attribute->getConcreteString()); + switch ($attr) { + case 'static': + $matom->setProperty($attr, true); + break; + case 'public': + case 'protected': + case 'private': + $matom->setProperty('access', $attr); + break; + } + } + } else { + $matom->setProperty('access', 'public'); + } + + $this->parseParams($matom, $method); + + $matom->setName($method->getChildByIndex(2)->getConcreteString()); + $matom->setLine($method->getLineNumber()); + $matom->setFile($file_name); + + $this->parseReturnType($matom, $method); + $atom->addChild($matom); + + $atoms[] = $matom; + } + + $atoms[] = $atom; + } + } + return $atoms; } diff --git a/src/applications/diviner/controller/DivinerAtomController.php b/src/applications/diviner/controller/DivinerAtomController.php index d5be8213c2..028800f28d 100644 --- a/src/applications/diviner/controller/DivinerAtomController.php +++ b/src/applications/diviner/controller/DivinerAtomController.php @@ -46,6 +46,7 @@ final class DivinerAtomController extends DivinerController { ->withContexts(array($this->atomContext)) ->withIndexes(array($this->atomIndex)) ->needAtoms(true) + ->needExtends(true) ->executeOne(); if (!$symbol) { @@ -54,6 +55,19 @@ final class DivinerAtomController extends DivinerController { $atom = $symbol->getAtom(); + $extends = $atom->getExtends(); + + $child_hashes = $atom->getChildHashes(); + if ($child_hashes) { + $children = id(new DivinerAtomQuery()) + ->setViewer($viewer) + ->withIncludeUndocumentable(true) + ->withNodeHashes($child_hashes) + ->execute(); + } else { + $children = array(); + } + $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( @@ -90,6 +104,9 @@ final class DivinerAtomController extends DivinerController { pht('Defined'), $atom->getFile().':'.$atom->getLine()); + + $this->buildExtendsAndImplements($properties, $symbol); + $warnings = $atom->getWarnings(); if ($warnings) { $warnings = id(new AphrontErrorView()) @@ -157,6 +174,17 @@ final class DivinerAtomController extends DivinerController { ->setReturn($return)); } + if ($children) { + $document->appendChild( + id(new PhabricatorHeaderView()) + ->setHeader(pht('Methods'))); + foreach ($children as $child) { + $document->appendChild( + id(new PhabricatorHeaderView()) + ->setHeader($child->getName())); + } + } + if ($toc) { $side = new PHUIListView(); $side->addMenuItem( @@ -188,4 +216,69 @@ final class DivinerAtomController extends DivinerController { return phutil_utf8_ucwords($name); } + private function buildExtendsAndImplements( + PhabricatorPropertyListView $view, + DivinerLiveSymbol $symbol) { + + $lineage = $this->getExtendsLineage($symbol); + if ($lineage) { + $lineage = mpull($lineage, 'getName'); + $lineage = implode(' > ', $lineage); + $view->addProperty(pht('Extends'), $lineage); + } + + $implements = $this->getImplementsLineage($symbol); + if ($implements) { + $items = array(); + foreach ($implements as $spec) { + $via = $spec['via']; + $iface = $spec['interface']; + if ($via == $symbol) { + $items[] = $iface->getName(); + } else { + $items[] = $iface->getName().' (via '.$via->getName().')'; + } + } + + $view->addProperty( + pht('Implements'), + phutil_implode_html(phutil_tag('br'), $items)); + } + + } + + private function getExtendsLineage(DivinerLiveSymbol $symbol) { + foreach ($symbol->getExtends() as $extends) { + if ($extends->getType() == 'class') { + $lineage = $this->getExtendsLineage($extends); + $lineage[] = $extends; + return $lineage; + } + } + return array(); + } + + private function getImplementsLineage(DivinerLiveSymbol $symbol) { + $implements = array(); + + // Do these first so we get interfaces ordered from most to least specific. + foreach ($symbol->getExtends() as $extends) { + if ($extends->getType() == 'interface') { + $implements[$extends->getName()] = array( + 'interface' => $extends, + 'via' => $symbol, + ); + } + } + + // Now do parent interfaces. + foreach ($symbol->getExtends() as $extends) { + if ($extends->getType() == 'class') { + $implements += $this->getImplementsLineage($extends); + } + } + + return $implements; + } + } diff --git a/src/applications/diviner/publisher/DivinerLivePublisher.php b/src/applications/diviner/publisher/DivinerLivePublisher.php index 6e2fc92170..164aa6f548 100644 --- a/src/applications/diviner/publisher/DivinerLivePublisher.php +++ b/src/applications/diviner/publisher/DivinerLivePublisher.php @@ -63,9 +63,11 @@ final class DivinerLivePublisher extends DivinerPublisher { } protected function loadAllPublishedHashes() { - $symbols = id(new DivinerLiveSymbol())->loadAllWhere( - 'bookPHID = %s AND graphHash IS NOT NULL', - $this->loadBook()->getPHID()); + $symbols = id(new DivinerAtomQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withBookPHIDs(array($this->loadBook()->getPHID())) + ->withIncludeUndocumentable(true) + ->execute(); return mpull($symbols, 'getGraphHash'); } @@ -84,7 +86,8 @@ final class DivinerLivePublisher extends DivinerPublisher { foreach (PhabricatorLiskDAO::chunkSQL($strings, ', ') as $chunk) { queryfx( $conn_w, - 'UPDATE %T SET graphHash = NULL WHERE graphHash IN (%Q)', + 'UPDATE %T SET graphHash = NULL, nodeHash = NULL + WHERE graphHash IN (%Q)', $symbol_table->getTableName(), $chunk); } @@ -111,7 +114,8 @@ final class DivinerLivePublisher extends DivinerPublisher { ->setGraphHash($hash) ->setIsDocumentable((int)$is_documentable) ->setTitle($ref->getTitle()) - ->setGroupName($ref->getGroup()); + ->setGroupName($ref->getGroup()) + ->setNodeHash($atom->getHash()); if ($is_documentable) { $renderer = $this->getRenderer(); diff --git a/src/applications/diviner/publisher/DivinerPublisher.php b/src/applications/diviner/publisher/DivinerPublisher.php index 150cd42918..33109933a2 100644 --- a/src/applications/diviner/publisher/DivinerPublisher.php +++ b/src/applications/diviner/publisher/DivinerPublisher.php @@ -142,6 +142,7 @@ abstract class DivinerPublisher { protected function shouldGenerateDocumentForAtom(DivinerAtom $atom) { switch ($atom->getType()) { + case DivinerAtom::TYPE_METHOD: case DivinerAtom::TYPE_FILE: return false; case DivinerAtom::TYPE_ARTICLE: diff --git a/src/applications/diviner/query/DivinerAtomQuery.php b/src/applications/diviner/query/DivinerAtomQuery.php index a461c40c51..06e6fda1e3 100644 --- a/src/applications/diviner/query/DivinerAtomQuery.php +++ b/src/applications/diviner/query/DivinerAtomQuery.php @@ -12,8 +12,10 @@ final class DivinerAtomQuery private $indexes; private $includeUndocumentable; private $includeGhosts; + private $nodeHashes; private $needAtoms; + private $needExtends; public function withIDs(array $ids) { $this->ids = $ids; @@ -50,11 +52,17 @@ final class DivinerAtomQuery return $this; } + public function withNodeHashes(array $hashes) { + $this->nodeHashes = $hashes; + return $this; + } + public function needAtoms($need) { $this->needAtoms = $need; 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 @@ -78,6 +86,12 @@ final class DivinerAtomQuery return $this; } + + public function needExtends($need) { + $this->needExtends = $need; + return $this; + } + public function withIncludeUndocumentable($include) { $this->includeUndocumentable = $include; return $this; @@ -116,6 +130,8 @@ final class DivinerAtomQuery $atom->attachBook($book); } + $need_atoms = $this->needAtoms; + if ($this->needAtoms) { $atom_data = id(new DivinerLiveAtom())->loadAllWhere( 'symbolPHID IN (%Ls)', @@ -132,6 +148,80 @@ final class DivinerAtomQuery } } + // Load all of the symbols this symbol extends, recursively. Commonly, + // this means all the ancestor classes and interfaces it extends and + // implements. + + if ($this->needExtends) { + + // First, load all the matching symbols by name. This does 99% of the + // work in most cases, assuming things are named at all reasonably. + + $names = array(); + foreach ($atoms as $atom) { + foreach ($atom->getAtom()->getExtends() as $xref) { + $names[] = $xref->getName(); + } + } + + if ($names) { + $xatoms = id(new DivinerAtomQuery()) + ->setViewer($this->getViewer()) + ->withNames($names) + ->needExtends(true) + ->needAtoms(true) + ->execute(); + $xatoms = mgroup($xatoms, 'getName', 'getType', 'getBookPHID'); + } else { + $xatoms = array(); + } + + foreach ($atoms as $atom) { + $alang = $atom->getAtom()->getLanguage(); + $extends = array(); + foreach ($atom->getAtom()->getExtends() as $xref) { + + // If there are no symbols of the matching name and type, we can't + // resolve this. + if (empty($xatoms[$xref->getName()][$xref->getType()])) { + continue; + } + + // If we found matches in the same documentation book, prefer them + // over other matches. Otherwise, look at all the the matches. + $matches = $xatoms[$xref->getName()][$xref->getType()]; + if (isset($matches[$atom->getBookPHID()])) { + $maybe = $matches[$atom->getBookPHID()]; + } else { + $maybe = array_mergev($matches); + } + + if (!$maybe) { + continue; + } + + // Filter out matches in a different language, since, e.g., PHP + // classes can not implement JS classes. + $same_lang = array(); + foreach ($maybe as $xatom) { + if ($xatom->getAtom()->getLanguage() == $alang) { + $same_lang[] = $xatom; + } + } + + if (!$same_lang) { + continue; + } + + // If we have duplicates remaining, just pick the first one. There's + // nothing more we can do to figure out which is the real one. + $extends[] = head($same_lang); + } + + $atom->attachExtends($extends); + } + } + return $atoms; } @@ -220,6 +310,13 @@ final class DivinerAtomQuery 'graphHash IS NOT NULL'); } + if ($this->nodeHashes) { + $where[] = qsprintf( + $conn_r, + 'nodeHash IN (%Ls)', + $this->nodeHashes); + } + $where[] = $this->buildPagingClause($conn_r); return $this->formatWhereClause($where); diff --git a/src/applications/diviner/storage/DivinerLiveSymbol.php b/src/applications/diviner/storage/DivinerLiveSymbol.php index a4eef16282..8a8a4bb79f 100644 --- a/src/applications/diviner/storage/DivinerLiveSymbol.php +++ b/src/applications/diviner/storage/DivinerLiveSymbol.php @@ -11,14 +11,16 @@ final class DivinerLiveSymbol extends DivinerDAO protected $atomIndex; protected $graphHash; protected $identityHash; + protected $nodeHash; protected $title; protected $groupName; protected $summary; protected $isDocumentable = 0; - private $book; - private $atom; + private $book = self::ATTACHABLE; + private $atom = self::ATTACHABLE; + private $extends = self::ATTACHABLE; public function getConfiguration() { return array( @@ -33,10 +35,7 @@ final class DivinerLiveSymbol extends DivinerDAO } public function getBook() { - if ($this->book === null) { - throw new Exception("Call attachBook() before getBook()!"); - } - return $this->book; + return $this->assertAttached($this->book); } public function attachBook(DivinerLiveBook $book) { @@ -45,10 +44,7 @@ final class DivinerLiveSymbol extends DivinerDAO } public function getAtom() { - if ($this->atom === null) { - throw new Exception("Call attachAtom() before getAtom()!"); - } - return $this->atom; + return $this->assertAttached($this->atom); } public function attachAtom(DivinerLiveAtom $atom) { @@ -109,6 +105,16 @@ final class DivinerLiveSymbol extends DivinerDAO return $title; } + public function attachExtends(array $extends) { + assert_instances_of($extends, 'DivinerLiveSymbol'); + $this->extends = $extends; + return $this; + } + + public function getExtends() { + return $this->assertAttached($this->extends); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php b/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php index ace4b1dd15..1b1c272944 100644 --- a/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php +++ b/src/applications/diviner/workflow/DivinerAtomizeWorkflow.php @@ -86,7 +86,9 @@ final class DivinerAtomizeWorkflow extends DivinerWorkflow { $atoms = $atomizer->atomize($file, $data); foreach ($atoms as $atom) { - $file_atom->addChild($atom); + if (!$atom->getParentHash()) { + $file_atom->addChild($atom); + } } $all_atoms[] = $atoms; diff --git a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php index afaaab8615..8f23257069 100644 --- a/src/applications/diviner/workflow/DivinerGenerateWorkflow.php +++ b/src/applications/diviner/workflow/DivinerGenerateWorkflow.php @@ -34,8 +34,7 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { protected function log($message) { $console = PhutilConsole::getConsole(); - $console->getServer()->setEnableLog(true); - $console->writeLog($message."\n"); + $console->writeErr($message."\n"); } public function execute(PhutilArgumentParser $args) { @@ -154,6 +153,9 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { $this->log(pht('Found %d file(s) to atomize.', count($file_atomizers))); $futures = $this->buildAtomizerFutures($file_atomizers); + + $this->log(pht('Atomizing %d file(s).', count($file_atomizers))); + if ($futures) { $this->resolveAtomizerFutures($futures, $file_hashes); $this->log(pht("Atomization complete.")); @@ -278,21 +280,31 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { $atomizers[$atomizer][] = $file; } + $root = dirname(phutil_get_library_root('phabricator')); + $config_root = $this->getConfig('root'); + + $bar = id(new PhutilConsoleProgressBar()) + ->setTotal(count($file_atomizers)); + $futures = array(); foreach ($atomizers as $class => $files) { foreach (array_chunk($files, 32) as $chunk) { $future = new ExecFuture( '%s atomize --ugly --book %s --atomizer %s -- %Ls', - dirname(phutil_get_library_root('phabricator')).'/bin/diviner', + $root.'/bin/diviner', $this->getBookConfigPath(), $class, $chunk); - $future->setCWD($this->getConfig('root')); + $future->setCWD($config_root); $futures[] = $future; + + $bar->update(count($chunk)); } } + $bar->done(); + return $futures; } @@ -300,6 +312,8 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { assert_instances_of($futures, 'Future'); $atom_cache = $this->getAtomCache(); + $bar = id(new PhutilConsoleProgressBar()) + ->setTotal(count($futures)); foreach (Futures($futures)->limit(4) as $key => $future) { $atoms = $future->resolveJSON(); @@ -310,7 +324,10 @@ final class DivinerGenerateWorkflow extends DivinerWorkflow { } $atom_cache->addAtom($atom); } + + $bar->update(1); } + $bar->done(); } diff --git a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php index b1d5e98370..7fcd71c1cf 100644 --- a/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php +++ b/src/infrastructure/storage/patch/PhabricatorBuiltinPatchList.php @@ -1555,6 +1555,10 @@ final class PhabricatorBuiltinPatchList extends PhabricatorSQLPatchList { 'type' => 'sql', 'name' => $this->getPatchPath('20130820.releephxactions.sql'), ), + '20130826.divinernode.sql' => array( + 'type' => 'sql', + 'name' => $this->getPatchPath('20130826.divinernode.sql'), + ), ); } }