Support "none()" in Differential to find revisions with no (un-resigned) reviewers

Summary:
Ref T13289. In Maniphest, you can currently search for "Owner: none()" to find tasks with no owner, but there's no way to search for "Reviewers: none()" in Differential right now.

Add support for this, since it's consistent and reasonable and doesn't seem too weird or niche.

Test Plan: Searched for "Reviewers: none()", found revisions with no reviewers. Searched for "Reviewers: alice, none()", "Reviewers: alice", and "Reviewers: <no constraint>" and got sensible results.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13289

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

src/__phutil_library_map__.php

@@ -538,6 +538,7 @@ phutil_register_library_map(array(
     'DifferentialMailEngineExtension' => 'applications/differential/engineextension/DifferentialMailEngineExtension.php',
     'DifferentialMailView' => 'applications/differential/mail/DifferentialMailView.php',
     'DifferentialManiphestTasksField' => 'applications/differential/customfield/DifferentialManiphestTasksField.php',
+    'DifferentialNoReviewersDatasource' => 'applications/differential/typeahead/DifferentialNoReviewersDatasource.php',
     'DifferentialParseCacheGarbageCollector' => 'applications/differential/garbagecollector/DifferentialParseCacheGarbageCollector.php',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'applications/differential/conduit/DifferentialParseCommitMessageConduitAPIMethod.php',
     'DifferentialParseRenderTestCase' => 'applications/differential/__tests__/DifferentialParseRenderTestCase.php',
@@ -561,6 +562,7 @@ phutil_register_library_map(array(
     'DifferentialReviewer' => 'applications/differential/storage/DifferentialReviewer.php',
     'DifferentialReviewerDatasource' => 'applications/differential/typeahead/DifferentialReviewerDatasource.php',
     'DifferentialReviewerForRevisionEdgeType' => 'applications/differential/edge/DifferentialReviewerForRevisionEdgeType.php',
+    'DifferentialReviewerFunctionDatasource' => 'applications/differential/typeahead/DifferentialReviewerFunctionDatasource.php',
     'DifferentialReviewerStatus' => 'applications/differential/constants/DifferentialReviewerStatus.php',
     'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'applications/differential/herald/DifferentialReviewersAddBlockingReviewersHeraldAction.php',
     'DifferentialReviewersAddBlockingSelfHeraldAction' => 'applications/differential/herald/DifferentialReviewersAddBlockingSelfHeraldAction.php',
@@ -6197,6 +6199,7 @@ phutil_register_library_map(array(
     'DifferentialMailEngineExtension' => 'PhabricatorMailEngineExtension',
     'DifferentialMailView' => 'Phobject',
     'DifferentialManiphestTasksField' => 'DifferentialCoreCustomField',
+    'DifferentialNoReviewersDatasource' => 'PhabricatorTypeaheadDatasource',
     'DifferentialParseCacheGarbageCollector' => 'PhabricatorGarbageCollector',
     'DifferentialParseCommitMessageConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialParseRenderTestCase' => 'PhabricatorTestCase',
@@ -6220,6 +6223,7 @@ phutil_register_library_map(array(
     'DifferentialReviewer' => 'DifferentialDAO',
     'DifferentialReviewerDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialReviewerForRevisionEdgeType' => 'PhabricatorEdgeType',
+    'DifferentialReviewerFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
     'DifferentialReviewerStatus' => 'Phobject',
     'DifferentialReviewersAddBlockingReviewersHeraldAction' => 'DifferentialReviewersHeraldAction',
     'DifferentialReviewersAddBlockingSelfHeraldAction' => 'DifferentialReviewersHeraldAction',

src/applications/differential/query/DifferentialRevisionQuery.php

@@ -26,6 +26,7 @@ final class DifferentialRevisionQuery
   private $isOpen;
   private $createdEpochMin;
   private $createdEpochMax;
+  private $noReviewers;
 
   const ORDER_MODIFIED      = 'order-modified';
   const ORDER_CREATED       = 'order-created';
@@ -98,7 +99,31 @@ final class DifferentialRevisionQuery
    * @task config
    */
   public function withReviewers(array $reviewer_phids) {
-    $this->reviewers = $reviewer_phids;
+    if ($reviewer_phids === array()) {
+      throw new Exception(
+        pht(
+          'Empty "withReviewers()" constraint is invalid. Provide one or '.
+          'more values, or remove the constraint.'));
+    }
+
+    $with_none = false;
+
+    foreach ($reviewer_phids as $key => $phid) {
+      switch ($phid) {
+        case DifferentialNoReviewersDatasource::FUNCTION_TOKEN:
+          $with_none = true;
+          unset($reviewer_phids[$key]);
+          break;
+        default:
+          break;
+      }
+    }
+
+    $this->noReviewers = $with_none;
+    if ($reviewer_phids) {
+      $this->reviewers = array_values($reviewer_phids);
+    }
+
     return $this;
   }
 
@@ -572,7 +597,7 @@ final class DifferentialRevisionQuery
     if ($this->reviewers) {
       $joins[] = qsprintf(
         $conn,
-        'JOIN %T reviewer ON reviewer.revisionPHID = r.phid
+        'LEFT JOIN %T reviewer ON reviewer.revisionPHID = r.phid
           AND reviewer.reviewerStatus != %s
           AND reviewer.reviewerPHID in (%Ls)',
         id(new DifferentialReviewer())->getTableName(),
@@ -580,6 +605,15 @@ final class DifferentialRevisionQuery
         $this->reviewers);
     }
 
+    if ($this->noReviewers) {
+      $joins[] = qsprintf(
+        $conn,
+        'LEFT JOIN %T no_reviewer ON no_reviewer.revisionPHID = r.phid
+          AND no_reviewer.reviewerStatus != %s',
+        id(new DifferentialReviewer())->getTableName(),
+        DifferentialReviewerStatus::STATUS_RESIGNED);
+    }
+
     if ($this->draftAuthors) {
       $joins[] = qsprintf(
         $conn,
@@ -715,6 +749,24 @@ final class DifferentialRevisionQuery
         $statuses);
     }
 
+    $reviewer_subclauses = array();
+
+    if ($this->noReviewers) {
+      $reviewer_subclauses[] = qsprintf(
+        $conn,
+        'no_reviewer.reviewerPHID IS NULL');
+    }
+
+    if ($this->reviewers) {
+      $reviewer_subclauses[] = qsprintf(
+        $conn,
+        'reviewer.reviewerPHID IS NOT NULL');
+    }
+
+    if ($reviewer_subclauses) {
+      $where[] = qsprintf($conn, '%LO', $reviewer_subclauses);
+    }
+
     $where[] = $this->buildWhereClauseParts($conn);
 
     return $this->formatWhereClause($conn, $where);
@@ -735,6 +787,10 @@ final class DifferentialRevisionQuery
       return true;
     }
 
+    if ($this->noReviewers) {
+      return true;
+    }
+
     return parent::shouldGroupQueryResultRows();
   }
 

src/applications/differential/query/DifferentialRevisionSearchEngine.php

@@ -73,7 +73,7 @@ final class DifferentialRevisionSearchEngine
         ->setLabel(pht('Reviewers'))
         ->setKey('reviewerPHIDs')
         ->setAliases(array('reviewer', 'reviewers', 'reviewerPHID'))
-        ->setDatasource(new DiffusionAuditorFunctionDatasource())
+        ->setDatasource(new DifferentialReviewerFunctionDatasource())
         ->setDescription(
           pht('Find revisions with specific reviewers.')),
       id(new PhabricatorSearchDatasourceField())

src/applications/differential/typeahead/DifferentialNoReviewersDatasource.php

@@ -0,0 +1,78 @@
+<?php
+
+final class DifferentialNoReviewersDatasource
+  extends PhabricatorTypeaheadDatasource {
+
+  const FUNCTION_TOKEN = 'none()';
+
+  public function getBrowseTitle() {
+    return pht('Browse No Reviewers');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type "none"...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorDifferentialApplication';
+  }
+
+  public function getDatasourceFunctions() {
+    return array(
+      'none' => array(
+        'name' => pht('No Reviewers'),
+        'summary' => pht('Find results which have no reviewers.'),
+        'description' => pht(
+          "This function includes results which have no reviewers. Use a ".
+          "query like this to find results with no reviewers:\n\n%s\n\n".
+          "If you combine this function with other functions, the query will ".
+          "return results which match the other selectors //or// have no ".
+          "reviewers. For example, this query will find results which have ".
+          "`alincoln` as a reviewer, and will also find results which have ".
+          "no reviewers:".
+          "\n\n%s",
+          '> none()',
+          '> alincoln, none()'),
+      ),
+    );
+  }
+
+  public function loadResults() {
+    $results = array(
+      $this->buildNoReviewersResult(),
+    );
+    return $this->filterResultsAgainstTokens($results);
+  }
+
+  protected function evaluateFunction($function, array $argv_list) {
+    $results = array();
+
+    foreach ($argv_list as $argv) {
+      $results[] = self::FUNCTION_TOKEN;
+    }
+
+    return $results;
+  }
+
+  public function renderFunctionTokens($function, array $argv_list) {
+    $results = array();
+    foreach ($argv_list as $argv) {
+      $results[] = PhabricatorTypeaheadTokenView::newFromTypeaheadResult(
+        $this->buildNoReviewersResult());
+    }
+    return $results;
+  }
+
+  private function buildNoReviewersResult() {
+    $name = pht('No Reviewers');
+
+    return $this->newFunctionResult()
+      ->setName($name.' none')
+      ->setDisplayName($name)
+      ->setIcon('fa-ban')
+      ->setPHID('none()')
+      ->setUnique(true)
+      ->addAttribute(pht('Select results with no reviewers.'));
+  }
+
+}

src/applications/differential/typeahead/DifferentialReviewerFunctionDatasource.php

@@ -0,0 +1,26 @@
+<?php
+
+final class DifferentialReviewerFunctionDatasource
+  extends PhabricatorTypeaheadCompositeDatasource {
+
+  public function getBrowseTitle() {
+    return pht('Browse Reviewers');
+  }
+
+  public function getPlaceholderText() {
+    return pht('Type a user, project, package name or function...');
+  }
+
+  public function getDatasourceApplicationClass() {
+    return 'PhabricatorDifferentialApplication';
+  }
+
+  public function getComponentDatasources() {
+    return array(
+      new PhabricatorProjectOrUserFunctionDatasource(),
+      new PhabricatorOwnersPackageFunctionDatasource(),
+      new DifferentialNoReviewersDatasource(),
+    );
+  }
+
+}

src/applications/differential/view/DifferentialRevisionListView.php

@@ -60,6 +60,16 @@ final class DifferentialRevisionListView extends AphrontView {
     $handle_phids = array();
     foreach ($this->revisions as $key => $revision) {
       $reviewers = $revision->getReviewers();
+
+      // Don't show reviewers who have resigned. The "Reviewers" constraint
+      // does not respect these reviewers and they largely don't count as
+      // reviewers.
+      foreach ($reviewers as $reviewer_key => $reviewer) {
+        if ($reviewer->isResigned()) {
+          unset($reviewers[$reviewer_key]);
+        }
+      }
+
       if (count($reviewers) > $reviewer_limit) {
         $reviewers = array_slice($reviewers, 0, $reviewer_limit);
         $reviewer_more[$key] = true;