From 8244b3582accb15ba6386fb626af48c1bbfcaa52 Mon Sep 17 00:00:00 2001 From: Joshua Spence Date: Fri, 22 May 2015 07:29:36 +1000 Subject: [PATCH] Improve behavior for detecting the failure to parse external linter output Summary: Currently, a lot of `ArcanistExternalLinter` subclasses have something like this: ```lang=php if ($err && !$messages) { return false; } ``` We can avoid this code duplication by moving this check to the `ArcanistExternalLinter` class. Test Plan: `arc unit` Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D11321 --- src/lint/linter/ArcanistCoffeeLintLinter.php | 4 ---- src/lint/linter/ArcanistCpplintLinter.php | 4 ---- src/lint/linter/ArcanistExternalLinter.php | 7 +++++++ src/lint/linter/ArcanistFlake8Linter.php | 4 ---- src/lint/linter/ArcanistJSONLintLinter.php | 4 ---- src/lint/linter/ArcanistJscsLinter.php | 4 ---- src/lint/linter/ArcanistLesscLinter.php | 4 ---- src/lint/linter/ArcanistPEP8Linter.php | 4 ---- src/lint/linter/ArcanistPuppetLintLinter.php | 4 ---- src/lint/linter/ArcanistPyFlakesLinter.php | 4 ---- src/lint/linter/ArcanistPyLintLinter.php | 4 ---- src/lint/linter/ArcanistRubyLinter.php | 4 ---- 12 files changed, 7 insertions(+), 44 deletions(-) diff --git a/src/lint/linter/ArcanistCoffeeLintLinter.php b/src/lint/linter/ArcanistCoffeeLintLinter.php index 3301356a..d744146a 100644 --- a/src/lint/linter/ArcanistCoffeeLintLinter.php +++ b/src/lint/linter/ArcanistCoffeeLintLinter.php @@ -120,10 +120,6 @@ final class ArcanistCoffeeLintLinter extends ArcanistExternalLinter { } } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistCpplintLinter.php b/src/lint/linter/ArcanistCpplintLinter.php index f4a9123c..0680d474 100644 --- a/src/lint/linter/ArcanistCpplintLinter.php +++ b/src/lint/linter/ArcanistCpplintLinter.php @@ -56,10 +56,6 @@ final class ArcanistCpplintLinter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistExternalLinter.php b/src/lint/linter/ArcanistExternalLinter.php index 83586081..c10cbfdc 100644 --- a/src/lint/linter/ArcanistExternalLinter.php +++ b/src/lint/linter/ArcanistExternalLinter.php @@ -354,6 +354,13 @@ abstract class ArcanistExternalLinter extends ArcanistFutureLinter { $messages = $this->parseLinterOutput($path, $err, $stdout, $stderr); + if ($err && $this->shouldExpectCommandErrors() && !$messages) { + // We assume that if the future exits with a non-zero status and we + // failed to parse any linter messages, then something must've gone wrong + // during parsing. + $messages = false; + } + if ($messages === false) { if ($err) { $future->resolvex(); diff --git a/src/lint/linter/ArcanistFlake8Linter.php b/src/lint/linter/ArcanistFlake8Linter.php index dabc0b42..db8e9177 100644 --- a/src/lint/linter/ArcanistFlake8Linter.php +++ b/src/lint/linter/ArcanistFlake8Linter.php @@ -78,10 +78,6 @@ final class ArcanistFlake8Linter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistJSONLintLinter.php b/src/lint/linter/ArcanistJSONLintLinter.php index 2ac0dbbe..d6b13103 100644 --- a/src/lint/linter/ArcanistJSONLintLinter.php +++ b/src/lint/linter/ArcanistJSONLintLinter.php @@ -79,10 +79,6 @@ final class ArcanistJSONLintLinter extends ArcanistExternalLinter { } } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistJscsLinter.php b/src/lint/linter/ArcanistJscsLinter.php index 6d8f2537..d1c0b3c4 100644 --- a/src/lint/linter/ArcanistJscsLinter.php +++ b/src/lint/linter/ArcanistJscsLinter.php @@ -129,10 +129,6 @@ final class ArcanistJscsLinter extends ArcanistExternalLinter { } } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistLesscLinter.php b/src/lint/linter/ArcanistLesscLinter.php index f51660e2..4032b1ed 100644 --- a/src/lint/linter/ArcanistLesscLinter.php +++ b/src/lint/linter/ArcanistLesscLinter.php @@ -177,10 +177,6 @@ final class ArcanistLesscLinter extends ArcanistExternalLinter { } } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistPEP8Linter.php b/src/lint/linter/ArcanistPEP8Linter.php index a021ba83..f1cb21ee 100644 --- a/src/lint/linter/ArcanistPEP8Linter.php +++ b/src/lint/linter/ArcanistPEP8Linter.php @@ -70,10 +70,6 @@ final class ArcanistPEP8Linter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistPuppetLintLinter.php b/src/lint/linter/ArcanistPuppetLintLinter.php index ea5b66cf..8336cd42 100644 --- a/src/lint/linter/ArcanistPuppetLintLinter.php +++ b/src/lint/linter/ArcanistPuppetLintLinter.php @@ -133,10 +133,6 @@ final class ArcanistPuppetLintLinter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistPyFlakesLinter.php b/src/lint/linter/ArcanistPyFlakesLinter.php index 11ede0c3..3a79e02e 100644 --- a/src/lint/linter/ArcanistPyFlakesLinter.php +++ b/src/lint/linter/ArcanistPyFlakesLinter.php @@ -77,10 +77,6 @@ final class ArcanistPyFlakesLinter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistPyLintLinter.php b/src/lint/linter/ArcanistPyLintLinter.php index 46be2f45..111cc27e 100644 --- a/src/lint/linter/ArcanistPyLintLinter.php +++ b/src/lint/linter/ArcanistPyLintLinter.php @@ -142,10 +142,6 @@ final class ArcanistPyLintLinter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; } diff --git a/src/lint/linter/ArcanistRubyLinter.php b/src/lint/linter/ArcanistRubyLinter.php index 3a9845d4..bb29f381 100644 --- a/src/lint/linter/ArcanistRubyLinter.php +++ b/src/lint/linter/ArcanistRubyLinter.php @@ -81,10 +81,6 @@ final class ArcanistRubyLinter extends ArcanistExternalLinter { $messages[] = $message; } - if ($err && !$messages) { - return false; - } - return $messages; }