From d28c591e7450f01e47460e2a4fa8c5b5857bee70 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Mon, 10 Sep 2012 17:09:10 -0700
Subject: [PATCH] Don't fatal on duplicate field in commit message

Summary:
If the actual commit message has a duplicate field and we shouldAutoclose it then the commit message parser fails.
Put the error in `$errors` instead.

Test Plan:
Reparsed commit with duplicate field in message.
Tried to `arc diff` message with duplicate field.

Reviewers: epriestley, nh

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3470
---
 ...API_differential_parsecommitmessage_Method.php | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/applications/conduit/method/differential/ConduitAPI_differential_parsecommitmessage_Method.php b/src/applications/conduit/method/differential/ConduitAPI_differential_parsecommitmessage_Method.php
index a25d977652..bdccc8c793 100644
--- a/src/applications/conduit/method/differential/ConduitAPI_differential_parsecommitmessage_Method.php
+++ b/src/applications/conduit/method/differential/ConduitAPI_differential_parsecommitmessage_Method.php
@@ -22,6 +22,8 @@
 final class ConduitAPI_differential_parsecommitmessage_Method
   extends ConduitAPIMethod {
 
+  private $errors;
+
   public function getMethodDescription() {
     return "Parse commit messages for Differential fields.";
   }
@@ -58,13 +60,14 @@ final class ConduitAPI_differential_parsecommitmessage_Method
 
     $aux_fields = mpull($aux_fields, null, 'getCommitMessageKey');
 
+    $this->errors = array();
+
     // Build a map from labels (like "Test Plan") to field keys
     // (like "testPlan").
     $label_map = $this->buildLabelMap($aux_fields);
     $field_map = $this->parseCommitMessage($corpus, $label_map);
 
     $fields = array();
-    $errors = array();
     foreach ($field_map as $field_key => $field_value) {
       $field = $aux_fields[$field_key];
       try {
@@ -72,7 +75,8 @@ final class ConduitAPI_differential_parsecommitmessage_Method
         $field->setValueFromParsedCommitMessage($fields[$field_key]);
       } catch (DifferentialFieldParseException $ex) {
         $field_label = $field->renderLabelForCommitMessage();
-        $errors[] = "Error parsing field '{$field_label}': ".$ex->getMessage();
+        $this->errors[] =
+          "Error parsing field '{$field_label}': ".$ex->getMessage();
       }
     }
 
@@ -82,7 +86,7 @@ final class ConduitAPI_differential_parsecommitmessage_Method
           $aux_field->validateField();
         } catch (DifferentialFieldValidationException $ex) {
           $field_label = $aux_field->renderLabelForCommitMessage();
-          $errors[] =
+          $this->errors[] =
             "Invalid or missing field '{$field_label}': ".
             $ex->getMessage();
         }
@@ -90,7 +94,7 @@ final class ConduitAPI_differential_parsecommitmessage_Method
     }
 
     return array(
-      'errors' => $errors,
+      'errors' => $this->errors,
       'fields' => $fields,
     );
   }
@@ -144,8 +148,7 @@ final class ConduitAPI_differential_parsecommitmessage_Method
         $lines[$key] = trim($match['text']);
         $field = $label_map[strtolower($match['field'])];
         if (!empty($seen[$field])) {
-          throw new Exception(
-            "Field '{$field}' occurs twice in commit message!");
+          $this->errors[] = "Field '{$field}' occurs twice in commit message!";
         }
         $seen[$field] = true;
       }