From 77af6fb35df8ab7162d5784287e021249ea32f88 Mon Sep 17 00:00:00 2001
From: vrana <jakubv@fb.com>
Date: Sat, 20 Oct 2012 05:45:22 -0700
Subject: [PATCH] Set all lint paths at once

Summary:
This code is much more readable to me.
It should also be faster as building the array at once should be faster than one by one.

Test Plan: Made license and XHPAST errors in PHP file, made spelling error in JS file.

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Differential Revision: https://secure.phabricator.com/D3758
---
 src/lint/engine/ArcanistSingleLintEngine.php |   4 +-
 src/lint/engine/ComprehensiveLintEngine.php  | 114 +++----------------
 src/lint/engine/PhutilLintEngine.php         |  76 +++----------
 src/lint/linter/ArcanistLinter.php           |   5 +
 4 files changed, 40 insertions(+), 159 deletions(-)

diff --git a/src/lint/engine/ArcanistSingleLintEngine.php b/src/lint/engine/ArcanistSingleLintEngine.php
index f133043e..d08e830a 100644
--- a/src/lint/engine/ArcanistSingleLintEngine.php
+++ b/src/lint/engine/ArcanistSingleLintEngine.php
@@ -71,9 +71,7 @@ final class ArcanistSingleLintEngine extends ArcanistLintEngine {
     }
 
     $linter = newv($linter_name, array());
-    foreach ($paths as $path) {
-      $linter->addPath($path);
-    }
+    $linter->setPaths($paths);
 
     return array($linter);
   }
diff --git a/src/lint/engine/ComprehensiveLintEngine.php b/src/lint/engine/ComprehensiveLintEngine.php
index 014f27d4..a0b4afcc 100644
--- a/src/lint/engine/ComprehensiveLintEngine.php
+++ b/src/lint/engine/ComprehensiveLintEngine.php
@@ -39,114 +39,32 @@ final class ComprehensiveLintEngine extends ArcanistLintEngine {
       }
     }
 
-    $generated_linter = new ArcanistGeneratedLinter();
-    $linters[] = $generated_linter;
+    $text_paths = preg_grep('/\.(php|css|hpp|cpp|l|y)$/', $paths);
+    $linters[] = id(new ArcanistGeneratedLinter())->setPaths($text_paths);
+    $linters[] = id(new ArcanistNoLintLinter())->setPaths($text_paths);
+    $linters[] = id(new ArcanistTextLinter())->setPaths($text_paths);
 
-    $nolint_linter = new ArcanistNoLintLinter();
-    $linters[] = $nolint_linter;
+    $linters[] = id(new ArcanistFilenameLinter())->setPaths($paths);
 
-    $text_linter = new ArcanistTextLinter();
-    $linters[] = $text_linter;
-    foreach ($paths as $path) {
-      $is_text = false;
-      if (preg_match('/\.(php|css|hpp|cpp|l|y)$/', $path)) {
-        $is_text = true;
-      }
-      if ($is_text) {
-        $generated_linter->addPath($path);
-        $generated_linter->addData($path, $this->loadData($path));
+    $linters[] = id(new ArcanistXHPASTLinter())
+      ->setPaths(preg_grep('/\.php$/', $paths));
 
-        $nolint_linter->addPath($path);
-        $nolint_linter->addData($path, $this->loadData($path));
+    $linters[] = id(new ArcanistApacheLicenseLinter())
+      ->setPaths(preg_grep('/\.(php|cpp|hpp|l|y)$/', $paths));
 
-        $text_linter->addPath($path);
-        $text_linter->addData($path, $this->loadData($path));
-      }
-    }
+    $py_paths = preg_grep('/\.py$/', $paths);
+    $linters[] = id(new ArcanistPyFlakesLinter())->setPaths($py_paths);
+    $linters[] = id(new ArcanistPEP8Linter())->setPaths($py_paths);
 
-    $name_linter = new ArcanistFilenameLinter();
-    $linters[] = $name_linter;
-    foreach ($paths as $path) {
-      $name_linter->addPath($path);
-    }
+    $linters[] = id(new ArcanistRubyLinter())
+      ->setPaths(preg_grep('/\.rb$/', $paths));
 
-    $xhpast_linter = new ArcanistXHPASTLinter();
-    $linters[] = $xhpast_linter;
-    foreach ($paths as $path) {
-      if (preg_match('/\.php$/', $path)) {
-        $xhpast_linter->addPath($path);
-        $xhpast_linter->addData($path, $this->loadData($path));
-      }
-    }
+    $linters[] = id(new ArcanistJSHintLinter())
+      ->setPaths(preg_grep('/\.js$/', $paths));
 
-    $linters = array_merge($linters, $this->buildLicenseLinters($paths));
-    $linters = array_merge($linters, $this->buildPythonLinters($paths));
-    $linters = array_merge($linters, $this->buildRubyLinters($paths));
     $linters = array_merge($linters, $this->buildJSLinters($paths));
 
     return $linters;
   }
 
-  public function buildLicenseLinters($paths) {
-    $license_linter = new ArcanistApacheLicenseLinter();
-
-    $linters = array();
-    $linters[] = $license_linter;
-    foreach ($paths as $path) {
-      if (preg_match('/\.(php|cpp|hpp|l|y)$/', $path)) {
-        if (!preg_match('@^externals/@', $path)) {
-          $license_linter->addPath($path);
-          $license_linter->addData($path, $this->loadData($path));
-        }
-      }
-    }
-    return $linters;
-  }
-
-  public function buildPythonLinters($paths) {
-    $pyflakes_linter = new ArcanistPyFlakesLinter();
-    $pep8_linter = new ArcanistPEP8Linter();
-
-    $linters = array();
-    $linters[] = $pyflakes_linter;
-    $linters[] = $pep8_linter;
-    foreach ($paths as $path) {
-      if (preg_match('/\.py$/', $path)) {
-        $pyflakes_linter->addPath($path);
-        $pyflakes_linter->addData($path, $this->loadData($path));
-        $pep8_linter->addPath($path);
-        $pep8_linter->addData($path, $this->loadData($path));
-      }
-    }
-    return $linters;
-  }
-
-  public function buildRubyLinters($paths) {
-    $ruby_linter = new ArcanistRubyLinter();
-
-    $linters = array();
-    $linters[] = $ruby_linter;
-      foreach ($paths as $path) {
-        if (preg_match('/\.rb$/', $path)) {
-          $ruby_linter->addPath($path);
-          $ruby_linter->addData($path, $this->loadData($path));
-        }
-      }
-    return $linters;
-  }
-
-  public function buildJSLinters($paths) {
-    $js_linter = new ArcanistJSHintLinter();
-
-    $linters = array();
-    $linters[] = $js_linter;
-      foreach ($paths as $path) {
-        if (preg_match('/\.js$/', $path)) {
-          $js_linter->addPath($path);
-          $js_linter->addData($path, $this->loadData($path));
-        }
-      }
-    return $linters;
-  }
-
 }
diff --git a/src/lint/engine/PhutilLintEngine.php b/src/lint/engine/PhutilLintEngine.php
index e4d35184..fd9be1e0 100644
--- a/src/lint/engine/PhutilLintEngine.php
+++ b/src/lint/engine/PhutilLintEngine.php
@@ -30,11 +30,7 @@ class PhutilLintEngine extends ArcanistLintEngine {
 
     $paths = $this->getPaths();
 
-    $library_linter = new ArcanistPhutilLibraryLinter();
-    $linters[] = $library_linter;
-    foreach ($paths as $path) {
-      $library_linter->addPath($path);
-    }
+    $linters[] = id(new ArcanistPhutilLibraryLinter())->setPaths($paths);
 
     // Remaining linters operate on file contents and ignore removed files.
     foreach ($paths as $key => $path) {
@@ -48,66 +44,30 @@ class PhutilLintEngine extends ArcanistLintEngine {
       }
     }
 
-    $name_linter = new ArcanistFilenameLinter();
-    $linters[] = $name_linter;
+    $linters[] = id(new ArcanistFilenameLinter())->setPaths($paths);
+
+    // Skip directories and lint only regular files in remaining linters.
     foreach ($paths as $key => $path) {
-      $name_linter->addPath($path);
-      if (!$this->getCommitHookMode() &&
-          !is_file($this->getFilePathOnDisk($path))) {
+      if ($this->getCommitHookMode()) {
+        continue;
+      }
+      if (!is_file($this->getFilePathOnDisk($path))) {
         unset($paths[$key]);
       }
     }
 
-    $generated_linter = new ArcanistGeneratedLinter();
-    $linters[] = $generated_linter;
+    $text_paths = preg_grep('/\.(php|css|js|hpp|cpp|l|y)$/', $paths);
+    $linters[] = id(new ArcanistGeneratedLinter())->setPaths($text_paths);
+    $linters[] = id(new ArcanistNoLintLinter())->setPaths($text_paths);
+    $linters[] = id(new ArcanistTextLinter())->setPaths($text_paths);
+    $linters[] = id(new ArcanistSpellingLinter())->setPaths($text_paths);
 
-    $nolint_linter = new ArcanistNoLintLinter();
-    $linters[] = $nolint_linter;
+    $linters[] = id(new ArcanistXHPASTLinter())
+      ->setCustomSeverityMap($this->getXHPASTSeverityMap())
+      ->setPaths(preg_grep('/\.php$/', $paths));
 
-    $text_linter = new ArcanistTextLinter();
-    $linters[] = $text_linter;
-
-    $spelling_linter = new ArcanistSpellingLinter();
-    $linters[] = $spelling_linter;
-    foreach ($paths as $path) {
-      $is_text = false;
-      if (preg_match('/\.(php|css|js|hpp|cpp|l|y)$/', $path)) {
-        $is_text = true;
-      }
-      if ($is_text) {
-        $generated_linter->addPath($path);
-        $generated_linter->addData($path, $this->loadData($path));
-
-        $nolint_linter->addPath($path);
-        $nolint_linter->addData($path, $this->loadData($path));
-
-        $text_linter->addPath($path);
-        $text_linter->addData($path, $this->loadData($path));
-
-        $spelling_linter->addPath($path);
-        $spelling_linter->addData($path, $this->loadData($path));
-      }
-    }
-
-    $xhpast_linter = new ArcanistXHPASTLinter();
-    $xhpast_map = $this->getXHPASTSeverityMap();
-    $xhpast_linter->setCustomSeverityMap($xhpast_map);
-    $linters[] = $xhpast_linter;
-    foreach ($paths as $path) {
-      if (preg_match('/\.php$/', $path)) {
-        $xhpast_linter->addPath($path);
-        $xhpast_linter->addData($path, $this->loadData($path));
-      }
-    }
-
-    $license_linter = new ArcanistApacheLicenseLinter();
-    $linters[] = $license_linter;
-    foreach ($paths as $path) {
-      if (preg_match('/\.(php|cpp|hpp|l|y)$/', $path)) {
-        $license_linter->addPath($path);
-        $license_linter->addData($path, $this->loadData($path));
-      }
-    }
+    $linters[] = id(new ArcanistApacheLicenseLinter())
+      ->setPaths(preg_grep('/\.(php|cpp|hpp|l|y)$/', $paths));
 
     return $linters;
   }
diff --git a/src/lint/linter/ArcanistLinter.php b/src/lint/linter/ArcanistLinter.php
index fbda2ba6..662c897c 100644
--- a/src/lint/linter/ArcanistLinter.php
+++ b/src/lint/linter/ArcanistLinter.php
@@ -57,6 +57,11 @@ abstract class ArcanistLinter {
     return $this;
   }
 
+  public function setPaths(array $paths) {
+    $this->paths = $paths;
+    return $this;
+  }
+
   public function getPaths() {
     return array_values($this->paths);
   }