From 6eed5c251411e12acf41d23137b78e60412e6d76 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Tue, 6 Jan 2015 22:45:44 +1100
Subject: [PATCH] Create a custom exception class for missing linter
 dependencies

Summary: I feel that we are abusing `ArcanistUsageException`. Throw a more tailed exception instead. Depends on D11197.

Test Plan: `arc lint`, I guess.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: avivey, Korvin, epriestley

Differential Revision: https://secure.phabricator.com/D11205
---
 src/__phutil_library_map__.php                              | 2 ++
 src/lint/linter/ArcanistExternalLinter.php                  | 6 +++---
 src/lint/linter/ArcanistJSHintLinter.php                    | 2 +-
 src/lint/linter/ArcanistMissingLinterException.php          | 3 +++
 src/lint/linter/ArcanistPyLintLinter.php                    | 2 +-
 .../linter/__tests__/ArcanistExternalLinterTestCase.php     | 2 +-
 src/lint/linter/__tests__/ArcanistLinterTestCase.php        | 6 ++++--
 7 files changed, 15 insertions(+), 8 deletions(-)
 create mode 100644 src/lint/linter/ArcanistMissingLinterException.php

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 2844716e..aabdc232 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -125,6 +125,7 @@ phutil_register_library_map(array(
     'ArcanistMercurialParserTestCase' => 'repository/parser/__tests__/ArcanistMercurialParserTestCase.php',
     'ArcanistMergeConflictLinter' => 'lint/linter/ArcanistMergeConflictLinter.php',
     'ArcanistMergeConflictLinterTestCase' => 'lint/linter/__tests__/ArcanistMergeConflictLinterTestCase.php',
+    'ArcanistMissingLinterException' => 'lint/linter/ArcanistMissingLinterException.php',
     'ArcanistNoEffectException' => 'exception/usage/ArcanistNoEffectException.php',
     'ArcanistNoEngineException' => 'exception/usage/ArcanistNoEngineException.php',
     'ArcanistNoLintLinter' => 'lint/linter/ArcanistNoLintLinter.php',
@@ -308,6 +309,7 @@ phutil_register_library_map(array(
     'ArcanistMercurialParserTestCase' => 'ArcanistTestCase',
     'ArcanistMergeConflictLinter' => 'ArcanistLinter',
     'ArcanistMergeConflictLinterTestCase' => 'ArcanistArcanistLinterTestCase',
+    'ArcanistMissingLinterException' => 'Exception',
     'ArcanistNoEffectException' => 'ArcanistUsageException',
     'ArcanistNoEngineException' => 'ArcanistUsageException',
     'ArcanistNoLintLinter' => 'ArcanistLinter',
diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php
index bb907d57..bcb7e1af 100644
--- a/src/lint/linter/ArcanistExternalLinter.php
+++ b/src/lint/linter/ArcanistExternalLinter.php
@@ -265,7 +265,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
 
     if ($interpreter) {
       if (!Filesystem::binaryExists($interpreter)) {
-        throw new ArcanistUsageException(
+        throw new ArcanistMissingLinterException(
           pht(
             'Unable to locate interpreter "%s" to run linter %s. You may need '.
             'to install the interpreter, or adjust your linter configuration.',
@@ -273,7 +273,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
             get_class($this)));
       }
       if (!Filesystem::pathExists($binary)) {
-        throw new ArcanistUsageException(
+        throw new ArcanistMissingLinterException(
           sprintf(
             "%s\n%s",
             pht(
@@ -287,7 +287,7 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter {
       }
     } else {
       if (!Filesystem::binaryExists($binary)) {
-        throw new ArcanistUsageException(
+        throw new ArcanistMissingLinterException(
           sprintf(
             "%s\n%s",
             pht(
diff --git a/src/lint/linter/ArcanistJSHintLinter.php b/src/lint/linter/ArcanistJSHintLinter.php
index 7884ef51..ea18e771 100644
--- a/src/lint/linter/ArcanistJSHintLinter.php
+++ b/src/lint/linter/ArcanistJSHintLinter.php
@@ -148,7 +148,7 @@ final class ArcanistJSHintLinter extends ArcanistExternalLinter {
 
     if (!is_array($errors)) {
       // Something went wrong and we can't decode the output. Exit abnormally.
-      throw new ArcanistUsageException(
+      throw new RuntimeException(
         "JSHint returned unparseable output.\n".
         "stdout:\n\n{$stdout}".
         "stderr:\n\n{$stderr}");
diff --git a/src/lint/linter/ArcanistMissingLinterException.php b/src/lint/linter/ArcanistMissingLinterException.php
new file mode 100644
index 00000000..a86f56a8
--- /dev/null
+++ b/src/lint/linter/ArcanistMissingLinterException.php
@@ -0,0 +1,3 @@
+<?php
+
+final class ArcanistMissingLinterException extends Exception {}
diff --git a/src/lint/linter/ArcanistPyLintLinter.php b/src/lint/linter/ArcanistPyLintLinter.php
index b758e8e4..45ebf37f 100644
--- a/src/lint/linter/ArcanistPyLintLinter.php
+++ b/src/lint/linter/ArcanistPyLintLinter.php
@@ -108,7 +108,7 @@ final class ArcanistPyLintLinter extends ArcanistLinter {
 
       list($err) = exec_manual('which %s', $pylint_bin);
       if ($err) {
-        throw new ArcanistUsageException(
+        throw new ArcanistMissingLinterException(
           "PyLint does not appear to be installed on this system. Install it ".
           "(e.g., with 'sudo easy_install pylint') or configure ".
           "'lint.pylint.prefix' in your .arcconfig to point to the directory ".
diff --git a/src/lint/linter/__tests__/ArcanistExternalLinterTestCase.php b/src/lint/linter/__tests__/ArcanistExternalLinterTestCase.php
index 7036cf86..bd7407c1 100644
--- a/src/lint/linter/__tests__/ArcanistExternalLinterTestCase.php
+++ b/src/lint/linter/__tests__/ArcanistExternalLinterTestCase.php
@@ -8,7 +8,7 @@ abstract class ArcanistExternalLinterTestCase extends ArcanistLinterTestCase {
       $this->assertTrue(
         $version !== false,
         pht('Failed to parse version from command.'));
-    } catch (ArcanistUsageException $ex) {
+    } catch (ArcanistMissingLinterException $ex) {
       $this->assertSkipped($ex->getMessage());
     }
   }
diff --git a/src/lint/linter/__tests__/ArcanistLinterTestCase.php b/src/lint/linter/__tests__/ArcanistLinterTestCase.php
index 87a84c0c..3b019f21 100644
--- a/src/lint/linter/__tests__/ArcanistLinterTestCase.php
+++ b/src/lint/linter/__tests__/ArcanistLinterTestCase.php
@@ -145,13 +145,15 @@ abstract class ArcanistLinterTestCase extends ArcanistPhutilTestCase {
       if ($exception instanceof PhutilAggregateException) {
         $caught_exception = false;
         foreach ($exception->getExceptions() as $ex) {
-          if ($ex instanceof ArcanistUsageException) {
+          if ($ex instanceof ArcanistUsageException ||
+              $ex instanceof ArcanistMissingLinterException) {
             $this->assertSkipped($ex->getMessage());
           } else {
             $caught_exception = true;
           }
         }
-      } else if ($exception instanceof ArcanistUsageException) {
+      } else if ($exception instanceof ArcanistUsageException ||
+                 $exception instanceof ArcanistMissingLinterException) {
         $this->assertSkipped($exception->getMessage());
       }
       $exception_message = $exception->getMessage()."\n\n".