Move child loading into DivinerAtomQuery and collect/organize more data on class Atoms

Summary:
Ref T988. This is //extremely// rough looking in the UI, but gets most of the information we need into the right places.

The controller rendering code is super rough too, I'm going to break that up shortly.

  - Add `needChildren()` to `DivinerAtomQuery`.
  - Compose and organize class methods when rendering classes. The old Diviner was not smart enough to do this, so a class would only document methods which the class itself implemented, not methods inherited from parents. I'd like to show those too to provide a more complete understanding of how to use a class (but they'll be marked with "inherited" or somesuch). This code walks the "extends" list and builds all of the class methods, annotating them with where they are defined and where they are implemented.
  - Coompose and organize "tasks". The old Diviner was not smart enough to do this, but I want to reduce the amount of duplicate/busy work involved in documenting subclasses. In particular, I want them to inherit "@task" declarations from parents so that class trees are more cohesive. They now do so.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T988

Differential Revision: https://secure.phabricator.com/D6823

src/applications/diviner/controller/DivinerAtomController.php

@@ -47,6 +47,7 @@ final class DivinerAtomController extends DivinerController {
       ->withIndexes(array($this->atomIndex))
       ->needAtoms(true)
       ->needExtends(true)
+      ->needChildren(true)
       ->executeOne();
 
     if (!$symbol) {
@@ -54,20 +55,6 @@ 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(
@@ -170,14 +157,63 @@ final class DivinerAtomController extends DivinerController {
           ->setReturn($return));
     }
 
-    if ($children) {
+    $methods = $this->composeMethods($symbol);
+    if ($methods) {
+
+      $tasks = $this->composeTasks($symbol);
+
+      if ($tasks) {
+        $methods_by_task = igroup($methods, 'task');
+
+        $document->appendChild(
+          id(new PhabricatorHeaderView())
+            ->setHeader(pht('Tasks')));
+
+        if (isset($methods_by_task[''])) {
+          $tasks[''] = array(
+            'name' => '',
+            'title' => pht('Other Methods'),
+            'defined' => $symbol,
+          );
+        }
+
+        foreach ($tasks as $spec) {
+          $document->appendChild(
+            id(new PhabricatorHeaderView())
+              ->setHeader($spec['title']));
+
+          $task_methods = idx($methods_by_task, $spec['name'], array());
+          if ($task_methods) {
+            $document->appendChild(hsprintf('<ul>'));
+            foreach ($task_methods as $task_method) {
+              $atom = last($task_method['atoms']);
+              $document->appendChild(
+                hsprintf('<li>%s()</li>', $atom->getName()));
+            }
+            $document->appendChild(hsprintf('</ul>'));
+          } else {
+            $document->appendChild("No methods for this task.");
+          }
+        }
+      }
+
       $document->appendChild(
         id(new PhabricatorHeaderView())
           ->setHeader(pht('Methods')));
-      foreach ($children as $child) {
+      foreach ($methods as $spec) {
-        $document->appendChild(
+        $method_header = id(new PhabricatorHeaderView())
-          id(new PhabricatorHeaderView())
+          ->setHeader(last($spec['atoms'])->getName());
-            ->setHeader($child->getName()));
+
+        $inherited = $spec['inherited'];
+        if ($inherited) {
+          $method_header->addTag(
+            id(new PhabricatorTagView())
+              ->setType(PhabricatorTagView::TYPE_STATE)
+              ->setBackgroundColor(PhabricatorTagView::COLOR_GREY)
+              ->setName(pht('Inherited')));
+        }
+
+        $document->appendChild($method_header);
       }
     }
 
@@ -301,4 +337,88 @@ final class DivinerAtomController extends DivinerController {
     $view->addProperty(pht('Defined'), $defined);
   }
 
+  private function composeMethods(DivinerLiveSymbol $symbol) {
+    $methods = $this->findMethods($symbol);
+    if (!$methods) {
+      return $methods;
+    }
+
+    foreach ($methods as $name => $method) {
+      // Check for "@task" on each parent, to find the most recently declared
+      // "@task".
+      $task = null;
+      foreach ($method['atoms'] as $key => $method_symbol) {
+        $atom = $method_symbol->getAtom();
+        if ($atom->getDocblockMetaValue('task')) {
+          $task = $atom->getDocblockMetaValue('task');
+        }
+      }
+      $methods[$name]['task'] = $task;
+
+      // Set 'inherited' if this atom has no implementation of the method.
+      if (last($method['implementations']) !== $symbol) {
+        $methods[$name]['inherited'] = true;
+      } else {
+        $methods[$name]['inherited'] = false;
+      }
+    }
+
+    return $methods;
+  }
+
+  private function findMethods(DivinerLiveSymbol $symbol) {
+    $child_specs = array();
+    foreach ($symbol->getExtends() as $extends) {
+      if ($extends->getType() == DivinerAtom::TYPE_CLASS) {
+        $child_specs = $this->findMethods($extends);
+      }
+    }
+
+    foreach ($symbol->getChildren() as $child) {
+      if ($child->getType() == DivinerAtom::TYPE_METHOD) {
+        $name = $child->getName();
+        if (isset($child_specs[$name])) {
+          $child_specs[$name]['atoms'][] = $child;
+          $child_specs[$name]['implementations'][] = $symbol;
+        } else {
+          $child_specs[$name] = array(
+            'atoms' => array($child),
+            'defined' => $symbol,
+            'implementations' => array($symbol),
+          );
+        }
+      }
+    }
+
+    return $child_specs;
+  }
+
+  private function composeTasks(DivinerLiveSymbol $symbol) {
+    $extends_task_specs = array();
+    foreach ($symbol->getExtends() as $extends) {
+      $extends_task_specs += $this->composeTasks($extends);
+    }
+
+    $task_specs = array();
+
+    $tasks = $symbol->getAtom()->getDocblockMetaValue('task');
+    if (strlen($tasks)) {
+      $tasks = phutil_split_lines($tasks, $retain_endings = false);
+
+      foreach ($tasks as $task) {
+        list($name, $title) = explode(' ', $task, 2);
+        $name = trim($name);
+        $title = trim($title);
+
+        $task_specs[$name] = array(
+          'name'        => $name,
+          'title'       => $title,
+          'defined'     => $symbol,
+        );
+      }
+    }
+
+    return $task_specs + $extends_task_specs;
+  }
+
 }

src/applications/diviner/publisher/DivinerLivePublisher.php

@@ -126,12 +126,19 @@ final class DivinerLivePublisher extends DivinerPublisher {
 
       $symbol->save();
 
-      if ($is_documentable) {
+      // TODO: We probably need a finer-grained sense of what "documentable"
-        $storage = $this->loadAtomStorageForSymbol($symbol)
+      // atoms are. Neither files nor methods are currently considered
-          ->setAtomData($atom->toDictionary())
+      // documentable, but for different reasons: files appear nowhere, while
-          ->setContent(null)
+      // methods just don't appear at the top level. These are probably
-          ->save();
+      // separate concepts. Since we need atoms in order to build method
-      }
+      // documentation, we insert them here. This also means we insert files,
+      // which are unnecessary and unused. Make sure this makes sense, but then
+      // probably introduce separate "isTopLevel" and "isDocumentable" flags?
+
+      $storage = $this->loadAtomStorageForSymbol($symbol)
+        ->setAtomData($atom->toDictionary())
+        ->setContent(null)
+        ->save();
     }
   }
 

src/applications/diviner/query/DivinerAtomQuery.php

@@ -16,6 +16,7 @@ final class DivinerAtomQuery
 
   private $needAtoms;
   private $needExtends;
+  private $needChildren;
 
   public function withIDs(array $ids) {
     $this->ids = $ids;
@@ -62,6 +63,11 @@ final class DivinerAtomQuery
     return $this;
   }
 
+  public function needChildren($need) {
+    $this->needChildren = $need;
+    return $this;
+  }
+
 
   /**
    * Include "ghosts", which are symbols which used to exist but do not exist
@@ -130,8 +136,6 @@ final class DivinerAtomQuery
       $atom->attachBook($book);
     }
 
-    $need_atoms = $this->needAtoms;
-
     if ($this->needAtoms) {
       $atom_data = id(new DivinerLiveAtom())->loadAllWhere(
         'symbolPHID IN (%Ls)',
@@ -170,6 +174,7 @@ final class DivinerAtomQuery
           ->withNames($names)
           ->needExtends(true)
           ->needAtoms(true)
+          ->needChildren($this->needChildren)
           ->execute();
         $xatoms = mgroup($xatoms, 'getName', 'getType', 'getBookPHID');
       } else {
@@ -222,6 +227,25 @@ final class DivinerAtomQuery
       }
     }
 
+    if ($this->needChildren) {
+      $child_hashes = $this->getAllChildHashes($atoms, $this->needExtends);
+
+      if ($child_hashes) {
+        $children = id(new DivinerAtomQuery())
+          ->setViewer($this->getViewer())
+          ->withIncludeUndocumentable(true)
+          ->withNodeHashes($child_hashes)
+          ->needAtoms($this->needAtoms)
+          ->execute();
+
+        $children = mpull($children, null, 'getNodeHash');
+      } else {
+        $children = array();
+      }
+
+      $this->attachAllChildren($atoms, $children, $this->needExtends);
+    }
+
     return $atoms;
   }
 
@@ -322,4 +346,63 @@ final class DivinerAtomQuery
     return $this->formatWhereClause($where);
   }
 
+
+  /**
+   * Walk a list of atoms and collect all the node hashes of the atoms'
+   * children. When recursing, also walk up the tree and collect children of
+   * atoms they extend.
+   *
+   * @param list<DivinerLiveSymbol> List of symbols to collect child hashes of.
+   * @param bool                    True to collect children of extended atoms,
+   *                                as well.
+   * @return map<string, string>    Hashes of atoms' children.
+   */
+  private function getAllChildHashes(array $symbols, $recurse_up) {
+    assert_instances_of($symbols, 'DivinerLiveSymbol');
+
+    $hashes = array();
+    foreach ($symbols as $symbol) {
+      foreach ($symbol->getAtom()->getChildHashes() as $hash) {
+        $hashes[$hash] = $hash;
+      }
+      if ($recurse_up) {
+        $hashes += $this->getAllChildHashes($symbol->getExtends(), true);
+      }
+    }
+
+    return $hashes;
+  }
+
+
+  /**
+   * Attach child atoms to existing atoms. In recursive mode, also attach child
+   * atoms to atoms that these atoms extend.
+   *
+   * @param list<DivinerLiveSymbol> List of symbols to attach childeren to.
+   * @param map<string, DivinerLiveSymbol> Map of symbols, keyed by node hash.
+   * @param bool True to attach children to extended atoms, as well.
+   * @return void
+   */
+  private function attachAllChildren(
+    array $symbols,
+    array $children,
+    $recurse_up) {
+
+    assert_instances_of($symbols, 'DivinerLiveSymbol');
+    assert_instances_of($children, 'DivinerLiveSymbol');
+
+    foreach ($symbols as $symbol) {
+      $symbol_children = array();
+      foreach ($symbol->getAtom()->getChildHashes() as $hash) {
+        if (isset($children[$hash])) {
+          $symbol_children[] = $children[$hash];
+        }
+      }
+      $symbol->attachChildren($symbol_children);
+      if ($recurse_up) {
+        $this->attachAllChildren($symbol->getExtends(), $children, true);
+      }
+    }
+  }
+
 }

src/applications/diviner/storage/DivinerLiveSymbol.php

@@ -21,6 +21,7 @@ final class DivinerLiveSymbol extends DivinerDAO
   private $book = self::ATTACHABLE;
   private $atom = self::ATTACHABLE;
   private $extends = self::ATTACHABLE;
+  private $children = self::ATTACHABLE;
 
   public function getConfiguration() {
     return array(
@@ -115,6 +116,16 @@ final class DivinerLiveSymbol extends DivinerDAO
     return $this->assertAttached($this->extends);
   }
 
+  public function attachChildren(array $children) {
+    assert_instances_of($children, 'DivinerLiveSymbol');
+    $this->children = $children;
+    return $this;
+  }
+
+  public function getChildren() {
+    return $this->assertAttached($this->children);
+  }
+
 
 /* -(  PhabricatorPolicyInterface  )----------------------------------------- */