Introduce arc lint --severity warning

Summary: I plan to exclude advices from our saved results.

Test Plan:
  $ arc lint src/lint/engine/PhutilLintEngine.php
  $ arc lint --severity advice src/lint/engine/PhutilLintEngine.php
  $ arc lint --severity error src/lint/engine/PhutilLintEngine.php

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2038

Differential Revision: https://secure.phabricator.com/D3936

src/lint/ArcanistLintSeverity.php

@@ -13,14 +13,18 @@ final class ArcanistLintSeverity {
   const SEVERITY_ERROR        = 'error';
   const SEVERITY_DISABLED     = 'disabled';
 
-  public static function getStringForSeverity($severity_code) {
+  public static function getLintSeverities() {
-    static $map = array(
+    return array(
       self::SEVERITY_ADVICE   => 'Advice',
       self::SEVERITY_AUTOFIX  => 'Auto-Fix',
       self::SEVERITY_WARNING  => 'Warning',
       self::SEVERITY_ERROR    => 'Error',
       self::SEVERITY_DISABLED => 'Disabled',
     );
+  }
+
+  public static function getStringForSeverity($severity_code) {
+    $map = self::getLintSeverities();
 
     if (!array_key_exists($severity_code, $map)) {
       throw new Exception("Unknown lint severity '{$severity_code}'!");

src/repository/api/ArcanistGitAPI.php

@@ -500,7 +500,7 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
     $lines = array();
     foreach (explode("\n", $status) as $line) {
       if ($line) {
-        $lines[] = preg_split("/[ \t]/", $line);
+        $lines[] = preg_split("/[ \t]/", $line, 6);
       }
     }
 
@@ -535,17 +535,9 @@ final class ArcanistGitAPI extends ArcanistRepositoryAPI {
 
   public function getChangedFiles($since_commit) {
     list($stdout) = $this->execxLocal(
-      'diff --name-status -z %s',
+      'diff --name-status --raw %s',
       $since_commit);
-    $return = array();
+    return $this->parseGitStatus($stdout);
-    foreach (array_chunk(explode("\0", $stdout), 2) as $val) {
-      if (count($val) != 2) {
-        break;
-      }
-      list($status, $path) = $val;
-      $return[$path] = ($status == 'D' ? false : true);
-    }
-    return $return;
   }
 
   public function getBlame($path) {

src/repository/api/ArcanistMercurialAPI.php

@@ -253,17 +253,9 @@ final class ArcanistMercurialAPI extends ArcanistRepositoryAPI {
 
   public function getChangedFiles($since_commit) {
     list($stdout) = $this->execxLocal(
-      'status --rev %s -0',
+      'status --rev %s',
       $since_commit);
-    $return = array();
+    return ArcanistMercurialParser::parseMercurialStatus($stdout);
-    foreach (explode("\0", $stdout) as $val) {
-      $match = null;
-      if (preg_match('/^(.) (.+)/', $val, $match)) {
-        list(, $status, $path) = $match;
-        $return[$path] = ($status == 'R' ? false : true);
-      }
-    }
-    return $return;
   }
 
   public function getBlame($path) {

src/repository/api/ArcanistSubversionAPI.php

@@ -102,45 +102,9 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
             throw new Exception("Unrecognized property status '{$props}'.");
         }
 
-        switch ($item) {
+        $mask |= $this->parseSVNStatus($item);
-          case 'normal':
+        if ($item == 'external') {
-            break;
+          $externals[] = $path;
-          case 'external':
-            $mask |= self::FLAG_EXTERNALS;
-            $externals[] = $path;
-            break;
-          case 'unversioned':
-            $mask |= self::FLAG_UNTRACKED;
-            break;
-          case 'obstructed':
-            $mask |= self::FLAG_OBSTRUCTED;
-            break;
-          case 'missing':
-            $mask |= self::FLAG_MISSING;
-            break;
-          case 'added':
-            $mask |= self::FLAG_ADDED;
-            break;
-          case 'replaced':
-            // This is the result of "svn rm"-ing a file, putting another one
-            // in place of it, and then "svn add"-ing the new file. Just treat
-            // this as equivalent to "modified".
-            $mask |= self::FLAG_MODIFIED;
-            break;
-          case 'modified':
-            $mask |= self::FLAG_MODIFIED;
-            break;
-          case 'deleted':
-            $mask |= self::FLAG_DELETED;
-            break;
-          case 'conflicted':
-            $mask |= self::FLAG_CONFLICT;
-            break;
-          case 'incomplete':
-            $mask |= self::FLAG_INCOMPLETE;
-            break;
-          default:
-            throw new Exception("Unrecognized item status '{$item}'.");
         }
 
         // This is new in or around Subversion 1.6.
@@ -175,6 +139,38 @@ final class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
     return $status;
   }
 
+  private function parseSVNStatus($item) {
+    switch ($item) {
+      case 'normal':
+        return 0;
+      case 'external':
+        return self::FLAG_EXTERNALS;
+      case 'unversioned':
+        return self::FLAG_UNTRACKED;
+      case 'obstructed':
+        return self::FLAG_OBSTRUCTED;
+      case 'missing':
+        return self::FLAG_MISSING;
+      case 'added':
+        return self::FLAG_ADDED;
+      case 'replaced':
+        // This is the result of "svn rm"-ing a file, putting another one
+        // in place of it, and then "svn add"-ing the new file. Just treat
+        // this as equivalent to "modified".
+        return self::FLAG_MODIFIED;
+      case 'modified':
+        return self::FLAG_MODIFIED;
+      case 'deleted':
+        return self::FLAG_DELETED;
+      case 'conflicted':
+        return self::FLAG_CONFLICT;
+      case 'incomplete':
+        return self::FLAG_INCOMPLETE;
+      default:
+        throw new Exception("Unrecognized item status '{$item}'.");
+    }
+  }
+
   public function getSVNProperty($path, $property) {
     list($stdout) = execx(
       'svn propget %s %s@',
@@ -476,15 +472,13 @@ EODIFF;
   public function getChangedFiles($since_commit) {
     // TODO: Handle paths with newlines.
     list($stdout) = $this->execxLocal(
-      'diff --revision %s:HEAD',
+      '--xml diff --revision %s:HEAD --summarize',
       $since_commit);
+    $xml = new SimpleXMLElement($stdout);
+
     $return = array();
-    foreach (explode("\n", $stdout) as $val) {
+    foreach ($xml->paths[0]->path as $path) {
-      $match = null;
+      $return[(string)$path] = $this->parseSVNStatus($path['item']);
-      if (preg_match('/^(.)\S*\s+(.+)/', $val, $match)) {
-        list(, $status, $path) = $match;
-        $return[$path] = ($status == 'D' ? false : true);
-      }
     }
     return $return;
   }

src/workflow/ArcanistLintWorkflow.php

@@ -13,6 +13,8 @@ class ArcanistLintWorkflow extends ArcanistBaseWorkflow {
   const RESULT_SKIP       = 3;
   const RESULT_POSTPONED  = 4;
 
+  const DEFAULT_SEVERITY = ArcanistLintSeverity::SEVERITY_ADVICE;
+
   private $unresolvedMessages;
   private $shouldAmendChanges = false;
   private $shouldAmendWithoutPrompt = false;
@@ -109,6 +111,15 @@ EOTEXT
           'When linting git repositories, amend HEAD with autofix '.
           'patches suggested by lint without prompting.',
       ),
+      'severity' => array(
+        'param' => 'string',
+        'help' =>
+          "Set minimum message severity. One of: '".
+          implode(
+            "', '",
+            array_keys(ArcanistLintSeverity::getLintSeverities())).
+          "'. Defaults to '".self::DEFAULT_SEVERITY."'.",
+      ),
       '*' => 'paths',
     );
   }
@@ -162,7 +173,8 @@ EOTEXT
     $this->engine = $engine;
     $engine->setWorkingCopy($working_copy);
 
-    $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_ADVICE);
+    $engine->setMinimumSeverity(
+      $this->getArgument('severity', self::DEFAULT_SEVERITY));
 
     // Propagate information about which lines changed to the lint engine.
     // This is used so that the lint engine can drop warning messages
@@ -300,10 +312,6 @@ EOTEXT
       }
     }
 
-    if ($failed) {
-      throw $failed;
-    }
-
     $repository_api = $this->getRepositoryAPI();
     if ($wrote_to_disk &&
         ($repository_api instanceof ArcanistGitAPI) &&
@@ -330,6 +338,15 @@ EOTEXT
       }
     }
 
+    if ($this->getArgument('output') == 'json') {
+      // NOTE: Required by save_lint.php in Phabricator.
+      return 0;
+    }
+
+    if ($failed) {
+      throw $failed;
+    }
+
     $unresolved = array();
     $has_warnings = false;
     $has_errors = false;