From 006a877996aace8046ea3935bea84cf89ccddb3e Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 5 Jun 2015 17:52:58 +1000 Subject: [PATCH] Allow ghost atoms to be rendered Summary: Ref T4558. Allow ghost atoms to be rendered in #diviner. This functionality didn't exist previously, but was hinted at by the TODO comments. Test Plan: Generated #diviner documentation for rARC and then removed a class (before re-generating the documentation). Navigated to the documentation for the removed class and saw "This atom no longer exists". Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley Maniphest Tasks: T4558 Differential Revision: https://secure.phabricator.com/D13114 --- .../controller/DivinerAtomController.php | 110 ++++++++++-------- .../diviner/query/DivinerAtomQuery.php | 39 +++++-- .../diviner/storage/DivinerLiveSymbol.php | 12 +- 3 files changed, 102 insertions(+), 59 deletions(-) diff --git a/src/applications/diviner/controller/DivinerAtomController.php b/src/applications/diviner/controller/DivinerAtomController.php index 10ebcccb3e..a332b63fab 100644 --- a/src/applications/diviner/controller/DivinerAtomController.php +++ b/src/applications/diviner/controller/DivinerAtomController.php @@ -35,11 +35,6 @@ 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())) @@ -47,7 +42,6 @@ final class DivinerAtomController extends DivinerController { ->withNames(array($this->atomName)) ->withContexts(array($this->atomContext)) ->withIndexes(array($this->atomIndex)) - ->withGhosts(false) ->withIsDocumentable(true) ->needAtoms(true) ->needExtends(true) @@ -66,9 +60,9 @@ final class DivinerAtomController extends DivinerController { $book->getShortTitle(), '/book/'.$book->getName().'/'); - $atom_short_title = $atom->getDocblockMetaValue( - 'short', - $symbol->getTitle()); + $atom_short_title = $atom + ? $atom->getDocblockMetaValue('short', $symbol->getTitle()) + : $symbol->getTitle(); $crumbs->addTextCrumb($atom_short_title); @@ -78,26 +72,38 @@ final class DivinerAtomController extends DivinerController { id(new PHUITagView()) ->setType(PHUITagView::TYPE_STATE) ->setBackgroundColor(PHUITagView::COLOR_BLUE) - ->setName(DivinerAtom::getAtomTypeNameString($atom->getType()))); + ->setName(DivinerAtom::getAtomTypeNameString( + $atom ? $atom->getType() : $symbol->getType()))); $properties = id(new PHUIPropertyListView()); - $group = $atom->getProperty('group'); + $group = $atom ? $atom->getProperty('group') : $symbol->getGroupName(); if ($group) { $group_name = $book->getGroupName($group); } else { $group_name = null; } - $this->buildDefined($properties, $symbol); - $this->buildExtendsAndImplements($properties, $symbol); + $document = id(new PHUIDocumentView()) + ->setBook($book->getTitle(), $group_name) + ->setHeader($header) + ->addClass('diviner-view') + ->setFontKit(PHUIDocumentView::FONT_SOURCE_SANS) + ->appendChild($properties); - $warnings = $atom->getWarnings(); - if ($warnings) { - $warnings = id(new PHUIInfoView()) - ->setErrors($warnings) - ->setTitle(pht('Documentation Warnings')) - ->setSeverity(PHUIInfoView::SEVERITY_WARNING); + if ($atom) { + $this->buildDefined($properties, $symbol); + $this->buildExtendsAndImplements($properties, $symbol); + + $warnings = $atom->getWarnings(); + if ($warnings) { + $warnings = id(new PHUIInfoView()) + ->setErrors($warnings) + ->setTitle(pht('Documentation Warnings')) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING); + } + + $document->appendChild($warnings); } $methods = $this->composeMethods($symbol); @@ -113,7 +119,10 @@ final class DivinerAtomController extends DivinerController { } $engine->process(); - $content = $this->renderDocumentationText($symbol, $engine); + if ($atom) { + $content = $this->renderDocumentationText($symbol, $engine); + $document->appendChild($content); + } $toc = $engine->getEngineMetadata( $symbol, @@ -121,16 +130,18 @@ final class DivinerAtomController extends DivinerController { PhutilRemarkupHeaderBlockRule::KEY_HEADER_TOC, array()); - $document = id(new PHUIDocumentView()) - ->setBook($book->getTitle(), $group_name) - ->setHeader($header) - ->addClass('diviner-view') - ->setFontKit(PHUIDocumentView::FONT_SOURCE_SANS) - ->appendChild($properties) - ->appendChild($warnings) - ->appendChild($content); + if (!$atom) { + $document->appendChild( + id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) + ->appendChild( + pht( + 'This atom no longer exists.'))); + } - $document->appendChild($this->buildParametersAndReturn(array($symbol))); + if ($atom) { + $document->appendChild($this->buildParametersAndReturn(array($symbol))); + } if ($methods) { $tasks = $this->composeTasks($symbol); @@ -202,7 +213,7 @@ final class DivinerAtomController extends DivinerController { } $section = id(new DivinerSectionView()) - ->setHeader(pht('Methods')); + ->setHeader(pht('Methods')); foreach ($methods as $spec) { $matom = last($spec['atoms']); @@ -473,20 +484,23 @@ final class DivinerAtomController extends DivinerController { $atom = $symbol->getAtom(); $out = array(); - if ($atom->getProperty('final')) { - $out[] = 'final'; - } - if ($atom->getProperty('abstract')) { - $out[] = 'abstract'; - } + if ($atom) { + if ($atom->getProperty('final')) { + $out[] = 'final'; + } - if ($atom->getProperty('access')) { - $out[] = $atom->getProperty('access'); - } + if ($atom->getProperty('abstract')) { + $out[] = 'abstract'; + } - if ($atom->getProperty('static')) { - $out[] = 'static'; + if ($atom->getProperty('access')) { + $out[] = $atom->getProperty('access'); + } + + if ($atom->getProperty('static')) { + $out[] = 'static'; + } } switch ($symbol->getType()) { @@ -530,13 +544,15 @@ final class DivinerAtomController extends DivinerController { $out = phutil_implode_html(' ', $out); - $parameters = $atom->getProperty('parameters'); - if ($parameters !== null) { - $pout = array(); - foreach ($parameters as $parameter) { - $pout[] = idx($parameter, 'name', '...'); + if ($atom) { + $parameters = $atom->getProperty('parameters'); + if ($parameters !== null) { + $pout = array(); + foreach ($parameters as $parameter) { + $pout[] = idx($parameter, 'name', '...'); + } + $out = array($out, '('.implode(', ', $pout).')'); } - $out = array($out, '('.implode(', ', $pout).')'); } return phutil_tag( diff --git a/src/applications/diviner/query/DivinerAtomQuery.php b/src/applications/diviner/query/DivinerAtomQuery.php index 4af0f1b1fa..e01df2022d 100644 --- a/src/applications/diviner/query/DivinerAtomQuery.php +++ b/src/applications/diviner/query/DivinerAtomQuery.php @@ -151,10 +151,6 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { foreach ($atoms as $key => $atom) { $data = idx($atom_data, $atom->getPHID()); - if (!$data) { - unset($atoms[$key]); - continue; - } $atom->attachAtom($data); } } @@ -170,6 +166,10 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { $names = array(); foreach ($atoms as $atom) { + if (!$atom->getAtom()) { + continue; + } + foreach ($atom->getAtom()->getExtends() as $xref) { $names[] = $xref->getName(); } @@ -189,10 +189,17 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { } foreach ($atoms as $atom) { - $alang = $atom->getAtom()->getLanguage(); - $extends = array(); - foreach ($atom->getAtom()->getExtends() as $xref) { + $atom_lang = null; + $atom_extends = array(); + if ($atom->getAtom()) { + $atom_lang = $atom->getAtom()->getLanguage(); + $atom_extends = $atom->getAtom()->getExtends(); + } + + $extends = array(); + + foreach ($atom_extends 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()])) { @@ -216,7 +223,7 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { // classes can not implement JS classes. $same_lang = array(); foreach ($maybe as $xatom) { - if ($xatom->getAtom()->getLanguage() == $alang) { + if ($xatom->getAtom()->getLanguage() == $atom_lang) { $same_lang[] = $xatom; } } @@ -396,7 +403,13 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { $hashes = array(); foreach ($symbols as $symbol) { - foreach ($symbol->getAtom()->getChildHashes() as $hash) { + $child_hashes = array(); + + if ($symbol->getAtom()) { + $child_hashes = $symbol->getAtom()->getChildHashes(); + } + + foreach ($child_hashes as $hash) { $hashes[$hash] = $hash; } if ($recurse_up) { @@ -426,8 +439,14 @@ final class DivinerAtomQuery extends PhabricatorCursorPagedPolicyAwareQuery { assert_instances_of($children, 'DivinerLiveSymbol'); foreach ($symbols as $symbol) { + $child_hashes = array(); $symbol_children = array(); - foreach ($symbol->getAtom()->getChildHashes() as $hash) { + + if ($symbol->getAtom()) { + $child_hashes = $symbol->getAtom()->getChildHashes(); + } + + foreach ($child_hashes as $hash) { if (isset($children[$hash])) { $symbol_children[] = $children[$hash]; } diff --git a/src/applications/diviner/storage/DivinerLiveSymbol.php b/src/applications/diviner/storage/DivinerLiveSymbol.php index ba35eed255..dc53b24f08 100644 --- a/src/applications/diviner/storage/DivinerLiveSymbol.php +++ b/src/applications/diviner/storage/DivinerLiveSymbol.php @@ -98,8 +98,12 @@ final class DivinerLiveSymbol extends DivinerDAO return $this->assertAttached($this->atom); } - public function attachAtom(DivinerLiveAtom $atom) { - $this->atom = DivinerAtom::newFromDictionary($atom->getAtomData()); + public function attachAtom(DivinerLiveAtom $atom = null) { + if ($atom === null) { + $this->atom = null; + } else { + $this->atom = DivinerAtom::newFromDictionary($atom->getAtomData()); + } return $this; } @@ -229,6 +233,10 @@ final class DivinerLiveSymbol extends DivinerDAO public function getMarkupText($field) { + if (!$this->getAtom()) { + return; + } + return $this->getAtom()->getDocblockText(); }