Show lint warnings for non-"line-able" files

Summary:
Currently, lint messages are only included if they are errors or
if they affect lines which the diff changed.  The implementation
of this caused issues for non-text files (e.g. binaries), as line
change information is not available, and the corresponding lint
messages were dropped (for non-errors).

In this diff, only lint messages concerning text files are dropped
based on this line filtering.

Test Plan: arc lint with binary file

Reviewers: jungejason, epriestley

Reviewed By: epriestley

CC: aran, aravindn, andrewjcg, liat, epriestley

Differential Revision: 1061

src/lint/engine/base/ArcanistLintEngine.php

@@ -68,6 +68,7 @@ abstract class ArcanistLintEngine {
   private $minimumSeverity = ArcanistLintSeverity::SEVERITY_DISABLED;
 
   private $changedLines = array();
+  private $textChanges = array();
   private $commitHookMode = false;
 
   public function __construct() {
@@ -101,6 +102,15 @@ abstract class ArcanistLintEngine {
     return idx($this->changedLines, $path);
   }
 
+  public function setTextChange($path) {
+    $this->textChanges[$path] = true;
+    return $this;
+  }
+
+  public function isTextChange($path) {
+    return idx($this->textChanges, $path, false);
+  }
+
   public function setFileData($data) {
     $this->fileData = $data + $this->fileData;
     return $this;
@@ -193,11 +203,14 @@ abstract class ArcanistLintEngine {
         if (!ArcanistLintSeverity::isAtLeastAsSevere($message, $minimum)) {
           continue;
         }
-        // When a user runs "arc diff", we default to raising only warnings on
+        // When a user runs "arc lint", we default to raising only warnings on
         // lines they have changed (errors are still raised anywhere in the
         // file).
+        $text_change = $this->isTextChange($message->getPath());
         $changed = $this->getPathChangedLines($message->getPath());
-        if ($changed !== null && !$message->isError()) {
+        if ($text_change === true &&
+            $changed !== null &&
+            !$message->isError()) {
           if (empty($changed[$message->getLine()])) {
             continue;
           }

src/workflow/base/ArcanistBaseWorkflow.php

@@ -754,6 +754,11 @@ class ArcanistBaseWorkflow {
     return $bundle;
   }
 
+  protected function isTextChange($path) {
+    $change = $this->getChange($path);
+    return $change->getFileType() == ArcanistDiffChangeType::FILE_TEXT;
+  }
+
   protected function getChangedLines($path, $mode) {
     if (is_dir($path)) {
       return array();

src/workflow/base/__init__.php

@@ -13,6 +13,7 @@ phutil_require_module('arcanist', 'exception/usage/userabort');
 phutil_require_module('arcanist', 'parser/bundle');
 phutil_require_module('arcanist', 'parser/diff');
 phutil_require_module('arcanist', 'parser/diff/change');
+phutil_require_module('arcanist', 'parser/diff/changetype');
 
 phutil_require_module('phutil', 'conduit/client');
 phutil_require_module('phutil', 'console');

src/workflow/lint/ArcanistLintWorkflow.php

@@ -169,12 +169,20 @@ EOTEXT
       $engine->setMinimumSeverity(ArcanistLintSeverity::SEVERITY_WARNING);
     }
 
+    // Propagate information about which lines changed to the lint engine.
+    // This is used so that the lint engine can drop messages concerning
+    // lines that weren't in the change.
     $engine->setPaths($paths);
     if (!$should_lint_all) {
       foreach ($paths as $path) {
-        $engine->setPathChangedLines(
-          $path,
-          $this->getChangedLines($path, 'new'));
+        // Explicitly flag text changes, as line information doesn't apply
+        // to non-text files.
+        if ($this->isTextChange($path)) {
+          $engine->setTextChange($path);
+          $engine->setPathChangedLines(
+            $path,
+            $this->getChangedLines($path, 'new'));
+        }
       }
     }