From d09eb881a368f41723588822bacdf99a01fcd6c0 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Sat, 20 Oct 2012 04:42:53 -0700
Subject: [PATCH] Print lint results before throwing

Summary:
When some linter throws then we don't print any result.
This is bad in case when the linter threw e.g. because of syntax error in some file which some other linter will tell us about.

Test Plan: Threw from a linter, made lint error in a file, saw error then exception.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3756
---
 src/lint/engine/ArcanistLintEngine.php | 73 +++++++++++++++-----------
 src/workflow/ArcanistLintWorkflow.php  | 13 ++++-
 2 files changed, 55 insertions(+), 31 deletions(-)

diff --git a/src/lint/engine/ArcanistLintEngine.php b/src/lint/engine/ArcanistLintEngine.php
index 59a05d2c..8ea0755b 100644
--- a/src/lint/engine/ArcanistLintEngine.php
+++ b/src/lint/engine/ArcanistLintEngine.php
@@ -196,44 +196,49 @@ abstract class ArcanistLintEngine {
       throw new ArcanistNoEffectException("No paths are lintable.");
     }
 
+    $exceptions = array();
     foreach ($linters as $linter) {
-      $linter->setEngine($this);
-      if (!$linter->canRun()) {
-        continue;
-      }
-      $paths = $linter->getPaths();
-
-      foreach ($paths as $key => $path) {
-        // Make sure each path has a result generated, even if it is empty
-        // (i.e., the file has no lint messages).
-        $result = $this->getResultForPath($path);
-        if (isset($stopped[$path])) {
-          unset($paths[$key]);
+      try {
+        $linter->setEngine($this);
+        if (!$linter->canRun()) {
+          continue;
         }
-      }
-      $paths = array_values($paths);
+        $paths = $linter->getPaths();
 
-      if ($paths) {
-        $linter->willLintPaths($paths);
-        foreach ($paths as $path) {
-          $linter->willLintPath($path);
-          $linter->lintPath($path);
-          if ($linter->didStopAllLinters()) {
-            $stopped[$path] = true;
+        foreach ($paths as $key => $path) {
+          // Make sure each path has a result generated, even if it is empty
+          // (i.e., the file has no lint messages).
+          $result = $this->getResultForPath($path);
+          if (isset($stopped[$path])) {
+            unset($paths[$key]);
           }
         }
-      }
+        $paths = array_values($paths);
 
-      $minimum = $this->minimumSeverity;
-      foreach ($linter->getLintMessages() as $message) {
-        if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) {
-          continue;
+        if ($paths) {
+          $linter->willLintPaths($paths);
+          foreach ($paths as $path) {
+            $linter->willLintPath($path);
+            $linter->lintPath($path);
+            if ($linter->didStopAllLinters()) {
+              $stopped[$path] = true;
+            }
+          }
         }
-        if (!$this->isRelevantMessage($message)) {
-          continue;
+
+        $minimum = $this->minimumSeverity;
+        foreach ($linter->getLintMessages() as $message) {
+          if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) {
+            continue;
+          }
+          if (!$this->isRelevantMessage($message)) {
+            continue;
+          }
+          $result = $this->getResultForPath($message->getPath());
+          $result->addMessage($message);
         }
-        $result = $this->getResultForPath($message->getPath());
-        $result->addMessage($message);
+      } catch (Exception $ex) {
+        $exceptions[] = $ex;
       }
     }
 
@@ -258,6 +263,14 @@ abstract class ArcanistLintEngine {
       }
     }
 
+    if ($exceptions) {
+      throw new PhutilAggregateException('Some linters failed:', $exceptions);
+    }
+
+    return $this->results;
+  }
+
+  public function getResults() {
     return $this->results;
   }
 
diff --git a/src/workflow/ArcanistLintWorkflow.php b/src/workflow/ArcanistLintWorkflow.php
index 06e284a0..4c9c11a2 100644
--- a/src/workflow/ArcanistLintWorkflow.php
+++ b/src/workflow/ArcanistLintWorkflow.php
@@ -201,7 +201,14 @@ EOTEXT
       $engine->setEnableAsyncLint(false);
     }
 
-    $results = $engine->run();
+    $failed = null;
+    try {
+      $engine->run();
+    } catch (Exception $ex) {
+      $failed = $ex;
+    }
+
+    $results = $engine->getResults();
 
     // It'd be nice to just return a single result from the run method above
     // which contains both the lint messages and the postponed linters.
@@ -309,6 +316,10 @@ EOTEXT
       }
     }
 
+    if ($failed) {
+      throw $failed;
+    }
+
     $repository_api = $this->getRepositoryAPI();
     if ($wrote_to_disk &&
         ($repository_api instanceof ArcanistGitAPI) &&