From 6c1bae437e30f914f909da3f657741fa396d992b Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@joshuaspence.com>
Date: Sun, 18 May 2014 10:49:22 -0700
Subject: [PATCH] Modernize `ArcanistScalaSBTLinter`.

Summary: Ref T2039. Convert the `ArcanistScalaSBTLinter` into an `ArcanistExternalLinter` and make it compatible with `.arclint`.

Test Plan: I can't really test this because I don't have any Scala projects and we don't have any test cases.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: codeblock, epriestley, Korvin

Maniphest Tasks: T2039

Differential Revision: https://secure.phabricator.com/D9097
---
 src/__phutil_library_map__.php             |   2 +-
 src/lint/linter/ArcanistScalaSBTLinter.php | 114 +++++++++++++--------
 2 files changed, 74 insertions(+), 42 deletions(-)

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
index 6956fd0b..f60b447d 100644
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -311,7 +311,7 @@ phutil_register_library_map(array(
     'ArcanistRevertWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistRubyLinter' => 'ArcanistExternalLinter',
     'ArcanistRubyLinterTestCase' => 'ArcanistArcanistLinterTestCase',
-    'ArcanistScalaSBTLinter' => 'ArcanistLinter',
+    'ArcanistScalaSBTLinter' => 'ArcanistExternalLinter',
     'ArcanistScriptAndRegexLinter' => 'ArcanistLinter',
     'ArcanistSetConfigWorkflow' => 'ArcanistBaseWorkflow',
     'ArcanistShellCompleteWorkflow' => 'ArcanistBaseWorkflow',
diff --git a/src/lint/linter/ArcanistScalaSBTLinter.php b/src/lint/linter/ArcanistScalaSBTLinter.php
index 5e24f358..80483310 100644
--- a/src/lint/linter/ArcanistScalaSBTLinter.php
+++ b/src/lint/linter/ArcanistScalaSBTLinter.php
@@ -3,60 +3,78 @@
 /**
  * Uses `sbt compile` to detect various warnings/errors in Scala code.
  */
-final class ArcanistScalaSBTLinter extends ArcanistLinter {
+final class ArcanistScalaSBTLinter extends ArcanistExternalLinter {
+
+  public function getInfoName() {
+    return 'sbt';
+  }
+
+  public function getInfoURI() {
+    return 'http://www.scala-sbt.org';
+  }
+
+  public function getInfoDescription() {
+    return pht('sbt is a build tool for Scala, Java, and more.');
+  }
 
   public function getLinterName() {
     return 'ScalaSBT';
   }
 
-  public function canRun() {
-    // Check if this looks like a SBT project. If it doesn't, throw, because
-    // we rely fairly heavily on `sbt compile` working, below. We don't want
-    // to call out to scalac ourselves, because then we'll end up in Class Path
-    // Hell. We let the build system handle this for us.
-    if (!Filesystem::pathExists('project/Build.scala') &&
-      !Filesystem::pathExists('build.sbt')) {
+  public function getLinterConfigurationName() {
+    return 'scala-sbt';
+  }
+
+  public function getDefaultBinary() {
+    $prefix = $this->getDeprecatedConfiguration('lint.scala_sbt.prefix');
+    $bin = 'sbt';
+
+    if ($prefix) {
+      return $prefix.'/'.$bin;
+    } else {
+      return $bin;
+    }
+  }
+
+  public function getVersion() {
+    // NOTE: `sbt --version` returns a non-zero exit status.
+    list($err, $stdout) = exec_manual(
+      '%C --version',
+      $this->getExecutableCommand());
+
+    $matches = array();
+    $regex = '/^sbt launcher version (?P<version>\d+\.\d+\.\d+)$/';
+    if (preg_match($regex, $stdout, $matches)) {
+      return $matches['version'];
+    } else {
       return false;
     }
+  }
+
+  public function getInstallInstructions() {
+    return pht(
+      'Check <http://www.scala-sbt.org> for installation instructions.');
+  }
+
+  public function shouldExpectCommandErrors() {
     return true;
   }
 
-  private function getSBTPath() {
-    $sbt_bin = "sbt";
-
-    $prefix = $this->getDeprecatedConfiguration('lint.scala_sbt.prefix');
-    if ($prefix !== null) {
-      $sbt_bin = $prefix . $sbt_bin;
-    }
-
-    return $sbt_bin;
+  protected function getMandatoryFlags() {
+    return array(
+      '-Dsbt.log.noformat=true',
+      'compile',
+    );
   }
 
-  private function getMessageCodeSeverity($type_of_error) {
-    switch ($type_of_error) {
-      case 'warn':
-        return ArcanistLintSeverity::SEVERITY_WARNING;
-      case 'error':
-        return ArcanistLintSeverity::SEVERITY_ERROR;
-    }
-  }
-
-  public function lintPath($path) {
-    $sbt = $this->getSBTPath();
-
-    // Tell SBT to not use color codes so our regex life is easy.
-    // TODO: Should this be "clean compile" instead of "compile"?
-    $f = new ExecFuture("%s -Dsbt.log.noformat=true compile", $sbt);
-    list($err, $stdout, $stderr) = $f->resolve();
-
-    $lines = explode("\n", $stdout);
+  protected function parseLinterOutput($path, $err, $stdout, $stderr) {
+    $lines = phutil_split_lines($stdout, false);
     $messages = array();
+
     foreach ($lines as $line) {
       $matches = null;
-      if (!preg_match(
-        "/\[(warn|error)\] (.*?):(\d+): (.*?)$/",
-        $line,
-        $matches)) {
+      $regex = '/\[(warn|error)\] (.*?):(\d+): (.*?)$/';
+      if (!preg_match($regex, $line, $matches)) {
         continue;
       }
       foreach ($matches as $key => $match) {
@@ -68,9 +86,23 @@ final class ArcanistScalaSBTLinter extends ArcanistLinter {
       $message->setLine($matches[3]);
       $message->setCode($this->getLinterName());
       $message->setDescription($matches[4]);
-      $message->setSeverity($this->getMessageCodeSeverity($matches[1]));
-      $this->addLintMessage($message);
+
+      switch ($matches[1]) {
+        case 'error':
+          $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
+          break;
+        case 'warn':
+          $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING);
+          break;
+        default:
+          $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE);
+          break;
+      }
+
+      $messages[] = $message;
     }
+
+    return $messages;
   }
 
 }