Prevent "Differential Revision: ..." from counting as a mention in commit messages

Summary:
Ref T13290. Ref T13291. Now that a full URI is a "mention", the full URI in "Differential Revision: ..." also triggers a mention.

Stop it from doing that, since these mentions are silly/redundant/unintended.

The API here is also slightly odd; simplify it a little bit to get rid of doing "append" with "get + append + set".

Test Plan: Used `bin/repository reparse --publish` to republish commits with "Differential Revision: ..." and verified that the revision PHID was properly dropped from the mention list.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13291, T13290

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

src/applications/audit/editor/PhabricatorAuditEditor.php

@@ -239,7 +239,7 @@ final class PhabricatorAuditEditor
           $object);
         if ($request) {
           $xactions[] = $request;
-          $this->setUnmentionablePHIDMap($request->getNewValue());
+          $this->addUnmentionablePHIDs($request->getNewValue());
         }
         break;
       default:
@@ -360,7 +360,6 @@ final class PhabricatorAuditEditor
     $flat_blocks = mpull($changes, 'getNewValue');
     $huge_block = implode("\n\n", $flat_blocks);
     $phid_map = array();
-    $phid_map[] = $this->getUnmentionablePHIDMap();
     $monograms = array();
 
     $task_refs = id(new ManiphestCustomFieldStatusParser())
@@ -385,7 +384,6 @@ final class PhabricatorAuditEditor
       ->execute();
     $phid_map[] = mpull($objects, 'getPHID', 'getPHID');
 
-
     $reverts_refs = id(new DifferentialCustomFieldRevertsParser())
       ->parseCorpus($huge_block);
     $reverts = array_mergev(ipull($reverts_refs, 'monograms'));
@@ -408,7 +406,7 @@ final class PhabricatorAuditEditor
     }
 
     $phid_map = array_mergev($phid_map);
-    $this->setUnmentionablePHIDMap($phid_map);
+    $this->addUnmentionablePHIDs($phid_map);
 
     return $result;
   }

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -846,13 +846,9 @@ final class DifferentialTransactionEditor
       $revert_phids = array();
     }
 
-    // See PHI574. Respect any unmentionable PHIDs which were set on the
+    $this->addUnmentionablePHIDs($task_phids);
-    // Editor by the caller.
+    $this->addUnmentionablePHIDs($rev_phids);
-    $unmentionable_map = $this->getUnmentionablePHIDMap();
+    $this->addUnmentionablePHIDs($revert_phids);
-    $unmentionable_map += $task_phids;
-    $unmentionable_map += $rev_phids;
-    $unmentionable_map += $revert_phids;
-    $this->setUnmentionablePHIDMap($unmentionable_map);
 
     $result = array();
     foreach ($edges as $type => $specs) {

src/applications/diffusion/worker/DiffusionUpdateObjectAfterCommitWorker.php

@@ -139,10 +139,7 @@ final class DiffusionUpdateObjectAfterCommitWorker
       ->setContentSource($content_source)
       ->setContinueOnNoEffect(true)
       ->setContinueOnMissingFields(true)
-      ->setUnmentionablePHIDMap(
+      ->addUnmentionablePHIDs(array($commit_phid));
-        array(
-          $commit_phid => $commit_phid,
-        ));
 
     $editor->applyTransactions($task, $xactions);
   }

src/applications/repository/worker/PhabricatorRepositoryCommitPublishWorker.php

@@ -62,12 +62,25 @@ final class PhabricatorRepositoryCommitPublishWorker
     $acting_phid = $this->getPublishAsPHID($commit);
     $content_source = $this->newContentSource();
 
+    $revision = DiffusionCommitRevisionQuery::loadRevisionForCommit(
+      $viewer,
+      $commit);
+
+    // Prevent the commit from generating a mention of the associated
+    // revision, if one exists, so we don't double up because of the URI
+    // in the commit message.
+    $unmentionable_phids = array();
+    if ($revision) {
+      $unmentionable_phids[] = $revision->getPHID();
+    }
+
     $editor = $commit->getApplicationTransactionEditor()
       ->setActor($viewer)
       ->setActingAsPHID($acting_phid)
       ->setContinueOnNoEffect(true)
       ->setContinueOnMissingFields(true)
-      ->setContentSource($content_source);
+      ->setContentSource($content_source)
+      ->addUnmentionablePHIDs($unmentionable_phids);
 
     try {
       $raw_patch = $this->loadRawPatchText($repository, $commit);

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -260,12 +260,14 @@ abstract class PhabricatorApplicationTransactionEditor
     return $this->isHeraldEditor;
   }
 
-  public function setUnmentionablePHIDMap(array $map) {
+  public function addUnmentionablePHIDs(array $phids) {
-    $this->unmentionablePHIDMap = $map;
+    foreach ($phids as $phid) {
+      $this->unmentionablePHIDMap[$phid] = true;
+    }
     return $this;
   }
 
-  public function getUnmentionablePHIDMap() {
+  private function getUnmentionablePHIDMap() {
     return $this->unmentionablePHIDMap;
   }
 
@@ -2090,12 +2092,14 @@ abstract class PhabricatorApplicationTransactionEditor
       ->withPHIDs($mentioned_phids)
       ->execute();
 
+    $unmentionable_map = $this->getUnmentionablePHIDMap();
+
     $mentionable_phids = array();
     if ($this->shouldEnableMentions($object, $xactions)) {
       foreach ($mentioned_objects as $mentioned_object) {
         if ($mentioned_object instanceof PhabricatorMentionableInterface) {
           $mentioned_phid = $mentioned_object->getPHID();
-          if (idx($this->getUnmentionablePHIDMap(), $mentioned_phid)) {
+          if (isset($unmentionable_map[$mentioned_phid])) {
             continue;
           }
           // don't let objects mention themselves