Modernize RocksDB linters

Summary:
This was motivated by t7518166. checkCpp crashes on db_test.cc because the file is too big :(

Couple of changes:
* Added clang-format linter. Now we can catch all code that is not formatted correctly.
* Added Howtoeven in our list of linters
* Replaced cpplint with flint
* Removed checkCpp lint. Nobody ownes it and it doesn't work on db_test.cc

Test Plan: Made a random lint error and `arc lint`. Saw an error.

Reviewers: yhchiang, kradhakrishnan, anthony, sdong

Reviewed By: sdong

Subscribers: dhruba, leveldb

Differential Revision: https://reviews.facebook.net/D41949

arcanist_util/__phutil_library_map__.php

@@ -11,11 +11,14 @@ phutil_register_library_map(array(
   'class' =>
   array(
     'ArcanistCpplintLinter' => 'cpp_linter/ArcanistCpplintLinter.php',
+    'BaseDirectoryScopedFormatLinter' => 'cpp_linter/BaseDirectoryScopedFormatLinter.php',
     'FacebookArcanistConfiguration' => 'config/FacebookArcanistConfiguration.php',
     'FacebookFbcodeLintEngine' => 'lint_engine/FacebookFbcodeLintEngine.php',
     'FacebookFbcodeUnitTestEngine' => 'unit_engine/FacebookFbcodeUnitTestEngine.php',
+    'FacebookHowtoevenLintEngine' => 'lint_engine/FacebookHowtoevenLintEngine.php',
+    'FacebookHowtoevenLinter' => 'cpp_linter/FacebookHowtoevenLinter.php',
+    'FbcodeClangFormatLinter' => 'cpp_linter/FbcodeClangFormatLinter.php',
     'FbcodeCppLinter' => 'cpp_linter/FbcodeCppLinter.php',
-    'PfffCppLinter' => 'cpp_linter/PfffCppLinter.php',
   ),
   'function' =>
   array(
@@ -23,10 +26,13 @@ phutil_register_library_map(array(
   'xmap' =>
   array(
     'ArcanistCpplintLinter' => 'ArcanistLinter',
+    'BaseDirectoryScopedFormatLinter' => 'ArcanistLinter',
     'FacebookArcanistConfiguration' => 'ArcanistConfiguration',
     'FacebookFbcodeLintEngine' => 'ArcanistLintEngine',
     'FacebookFbcodeUnitTestEngine' => 'ArcanistBaseUnitTestEngine',
+    'FacebookHowtoevenLintEngine' => 'ArcanistLintEngine',
+    'FacebookHowtoevenLinter' => 'ArcanistLinter',
+    'FbcodeClangFormatLinter' => 'BaseDirectoryScopedFormatLinter',
     'FbcodeCppLinter' => 'ArcanistLinter',
-    'PfffCppLinter' => 'ArcanistLinter',
   ),
 ));

arcanist_util/config/FacebookArcanistConfiguration.php

@@ -1,5 +1,8 @@
 <?php
 // Copyright 2004-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
 
 class FacebookArcanistConfiguration extends ArcanistConfiguration {
 

arcanist_util/cpp_linter/BaseDirectoryScopedFormatLinter.php

@@ -0,0 +1,74 @@
+<?php
+// Copyright 2004-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+
+abstract class BaseDirectoryScopedFormatLinter extends ArcanistLinter {
+
+  const LINT_FORMATTING = 1;
+
+  private $changedLines = array();
+  private $rawLintOutput = array();
+
+  abstract protected function getPathsToLint();
+
+  protected function shouldLintPath($path) {
+    foreach ($this->getPathsToLint() as $p) {
+      // check if $path starts with $p
+      if (strncmp($path, $p, strlen($p)) === 0) {
+        return true;
+      }
+    }
+    return false;
+  }
+
+  // API to tell this linter which lines were changed
+  final public function setPathChangedLines($path, $changed) {
+    $this->changedLines[$path] = $changed;
+  }
+
+  final public function willLintPaths(array $paths) {
+    $futures = array();
+    foreach ($paths as $path) {
+      if (!$this->shouldLintPath($path)) {
+        continue;
+      }
+
+      $changed = $this->changedLines[$path];
+      if (!isset($changed)) {
+        // do not run linter if there are no changes
+        continue;
+      }
+
+      $futures[$path] = $this->getFormatFuture($path, $changed);
+    }
+
+    foreach (Futures($futures)->limit(8) as $p => $f) {
+      $this->rawLintOutput[$p] = $f->resolvex();
+    }
+  }
+
+  abstract protected function getFormatFuture($path, array $changed);
+  abstract protected function getLintMessage($diff);
+
+  final public function lintPath($path) {
+    if (!isset($this->rawLintOutput[$path])) {
+      return;
+    }
+
+    list($new_content) = $this->rawLintOutput[$path];
+    $old_content = $this->getData($path);
+
+    if ($new_content != $old_content) {
+      $diff = ArcanistDiffUtils::renderDifferences($old_content, $new_content);
+      $this->raiseLintAtOffset(
+        0,
+        self::LINT_FORMATTING,
+        $this->getLintMessage($diff),
+        $old_content,
+        $new_content);
+    }
+  }
+
+}

arcanist_util/cpp_linter/FacebookHowtoevenLinter.php

@@ -0,0 +1,223 @@
+<?php
+// Copyright 2015-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+
+final class FacebookHowtoevenLinter extends ArcanistLinter {
+
+  const VERSION = 'fd9192f324c36d28136d14380f0b552a1385b59b';
+
+  private $parsedTargets = array();
+
+  public function getLinterName() {
+    return 'Howtoeven';
+  }
+
+  protected function getSeverity($code) {
+    $severities = array(
+      ArcanistLintSeverity::SEVERITY_DISABLED,
+      ArcanistLintSeverity::SEVERITY_ADVICE,
+      ArcanistLintSeverity::SEVERITY_WARNING,
+      ArcanistLintSeverity::SEVERITY_ERROR,
+    );
+    return idx($severities, $code, ArcanistLintSeverity::SEVERITY_WARNING);
+  }
+
+  public function willLintPaths(array $paths) {
+    // Cleanup previous runs.
+    $this->localExecx("rm -rf _build/_lint");
+
+    // Build compilation database.
+    $lintable_paths = $this->getLintablePaths($paths);
+    $interesting_paths = $this->getInterestingPaths($lintable_paths);
+
+    if (!$lintable_paths) {
+      return;
+    }
+
+    // Run lint.
+    try {
+      $this->localExecx(
+        "%C %C -p _build/dev/ %Ls",
+        $this->getBinaryPath(),
+        $this->getFilteredIssues(),
+        $lintable_paths);
+    } catch (CommandException $exception) {
+      PhutilConsole::getConsole()->writeErr($exception->getMessage());
+    }
+
+    // Load results.
+    $result = id(
+      new SQLite3(
+        $this->getProjectRoot().'/_build/_lint/lint.db',
+        SQLITE3_OPEN_READONLY))
+      ->query("SELECT * FROM raised_issues");
+
+    while ($issue = $result->fetchArray(SQLITE3_ASSOC)) {
+      // Skip issues not part of the linted file.
+      if (in_array($issue['file'], $interesting_paths)) {
+        $this->addLintMessage(id(new ArcanistLintMessage())
+          ->setPath($issue['file'])
+          ->setLine($issue['line'])
+          ->setChar($issue['column'])
+          ->setCode('Howtoeven')
+          ->setSeverity($this->getSeverity($issue['severity']))
+          ->setName('Hte-'.$issue['name'])
+          ->setDescription(
+            sprintf(
+              "%s\n\n%s",
+              ($issue['message']) ? $issue['message'] : $issue['description'],
+              $issue['explanation']))
+          ->setOriginalText(idx($issue, 'original', ''))
+          ->setReplacementText(idx($issue, 'replacement', '')));
+      }
+    }
+  }
+
+  public function lintPath($path) {
+  }
+
+  /**
+   * Get the paths that we know how to lint.
+   *
+   * The strategy is to first look whether there's an existing compilation
+   * database and use that if it's exhaustive. We generate our own only if
+   * necessary.
+   */
+  private function getLintablePaths($paths) {
+    // Replace headers with existing sources.
+    for ($i = 0; $i < count($paths); $i++) {
+      if (preg_match("/\.h$/", $paths[$i])) {
+        $header = preg_replace("/\.h$/", ".cpp", $paths[$i]);
+        if (file_exists($header)) {
+          $paths[$i] = $header;
+        }
+      }
+    }
+
+    // Check if database exists and is exhaustive.
+    $available_paths = $this->getAvailablePaths();
+    $lintable_paths = array_intersect($paths, $available_paths);
+    if ($paths === $lintable_paths) {
+      return $lintable_paths;
+    }
+
+    // Generate our own database.
+    $targets = $this->getTargetsFor($paths);
+    if (!$targets) {
+      PhutilConsole::getConsole()->writeErr(
+        "No build targets found for %s\n",
+        implode(', ', $paths));
+      return array();
+    }
+
+    $this->localExecx("./tools/build/bin/fbconfig.par -r %Ls", $targets);
+    $this->localExecx("./tools/build/bin/fbmake.par gen_cdb");
+
+    $available_paths = $this->getAvailablePaths();
+    $lintable_paths = array_intersect($paths, $available_paths);
+    if ($paths != $lintable_paths) {
+      PhutilConsole::getConsole()->writeErr(
+        "Can't lint %s\n",
+        implode(', ', array_diff($paths, $available_paths)));
+    }
+
+    // Return what we know how to lint.
+    return $lintable_paths;
+  }
+
+  /**
+   * Get the available paths in the current compilation database.
+   */
+  private function getAvailablePaths() {
+    $database_path = $this->getProjectRoot()
+      .'/_build/dev/compile_commands.json';
+    if (!file_exists($database_path)) {
+      return array();
+    }
+
+    $entries = json_decode(file_get_contents($database_path), true);
+    $paths = array();
+    foreach ($entries as $entry) {
+      $paths[] = $entry['file'];
+    }
+    return $paths;
+  }
+
+  /**
+   * Search for the targets directories for the given files.
+   */
+  private static function getTargetsFor($paths) {
+    $targets = array();
+    foreach ($paths as $path) {
+      while (($path = dirname($path)) !== '.') {
+        if (in_array('TARGETS', scandir($path))) {
+          $contents = file_get_contents($path.'/TARGETS');
+          if (strpos($contents, 'cpp_binary') !== false) {
+            $targets[] = $path;
+            break;
+          }
+        }
+      }
+    }
+    return array_unique($targets);
+  }
+
+  /**
+   * The paths that we actually want to report on.
+   */
+  private function getInterestingPaths($paths) {
+    $headers = array();
+    foreach ($paths as $path) {
+      $headers[] = preg_replace("/\.cpp$/", ".h", $path);
+    }
+    return array_merge($paths, $headers);
+  }
+
+  /**
+   * The path where the binary is located. Will return the current dewey binary
+   * unless the `HOWTOEVEN_BUILD` environment variable is set.
+   */
+  private function getBinaryPath() {
+    $path = sprintf(
+      "/mnt/dewey/fbcode/.commits/%s/builds/howtoeven/client",
+      self::VERSION);
+
+    $build = getenv('HOWTOEVEN_BUILD');
+    if ($build) {
+      $path = sprintf(
+        "./_build/%s/tools/howtoeven/client",
+        $build);
+      if (!file_exists($path)) {
+        PhutilConsole::getConsole()->writeErr(">> %s does not exist\n", $path);
+        exit(1);
+      }
+    }
+
+    return $path;
+  }
+
+  /**
+   * Execute the command in the root directory.
+   */
+  private function localExecx($command /* , ... */) {
+    $arguments = func_get_args();
+    return newv('ExecFuture', $arguments)
+      ->setCWD($this->getProjectRoot())
+      ->resolvex();
+  }
+
+  /**
+   * The root of the project.
+   */
+  private function getProjectRoot() {
+    return $this->getEngine()->getWorkingCopy()->getProjectRoot();
+  }
+
+  private function getFilteredIssues() {
+    $issues = getenv('HOWTOEVEN_ISSUES');
+    return ($issues) ? csprintf('-issues %s', $issues) : '';
+  }
+
+}

arcanist_util/cpp_linter/FbcodeClangFormatLinter.php

@@ -0,0 +1,52 @@
+<?php
+// Copyright 2004-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+
+final class FbcodeClangFormatLinter extends BaseDirectoryScopedFormatLinter {
+
+  const LINT_FORMATTING = 1;
+  const CLANG_FORMAT_BINARY = '/mnt/vol/engshare/admin/scripts/clang-format';
+
+  protected function getPathsToLint() {
+    return array('');
+  }
+
+  public function getLinterName() {
+    return 'CLANG_FORMAT';
+  }
+
+  public function getLintSeverityMap() {
+    return array(
+      self::LINT_FORMATTING => ArcanistLintSeverity::SEVERITY_ADVICE,
+    );
+  }
+
+  public function getLintNameMap() {
+    return array(
+      self::LINT_FORMATTING => pht('Changes are not clang-formatted'),
+    );
+  }
+
+  protected function getFormatFuture($path, array $changed) {
+    $args = "";
+    foreach ($changed as $key => $value) {
+      $args .= " --lines=$key:$key";
+    }
+
+    return new ExecFuture(
+      "%s %s $args",
+      self::CLANG_FORMAT_BINARY,
+      $this->getEngine()->getFilePathOnDisk($path));
+  }
+
+  protected function getLintMessage($diff) {
+    $link_to_clang_format =
+      "[[ http://fburl.com/clang-format | clang-format ]]";
+    return <<<LINT_MSG
+Changes in this file were not formatted using $link_to_clang_format.
+Please run build_tools/format-diff.sh or `make format`
+LINT_MSG;
+  }
+}

arcanist_util/cpp_linter/FbcodeCppLinter.php

@@ -1,24 +1,17 @@
 <?php
+// Copyright 2004-present Facebook.  All rights reserved.
 
 class FbcodeCppLinter extends ArcanistLinter {
-  const CPPLINT      = "/home/engshare/tools/cpplint";
+  const FLINT      = "/home/engshare/tools/flint";
   const LINT_ERROR   = 1;
   const LINT_WARNING = 2;
+  const LINT_ADVICE  = 3;
   const C_FLAG = "--c_mode=true";
+
   private $rawLintOutput = array();
 
   public function willLintPaths(array $paths) {
     $futures = array();
-    $ret_value = 0;
-    $last_line = system("which cpplint", $ret_value);
-    $CPP_LINT = false;
-    if ($ret_value == 0) {
-      $CPP_LINT = $last_line;
-    } else if (file_exists(self::CPPLINT)) {
-      $CPP_LINT = self::CPPLINT;
-    }
-
-    if ($CPP_LINT) {
     foreach ($paths as $p) {
       $lpath = $this->getEngine()->getFilePathOnDisk($p);
       $lpath_file = file($lpath);
@@ -27,18 +20,18 @@ class FbcodeCppLinter extends ArcanistLinter {
           preg_match('/vim(:.*)*:\s*(set\s+)?filetype=c\s*:/', $lpath_file[0])
           ) {
         $futures[$p] = new ExecFuture("%s %s %s 2>&1",
-                             $CPP_LINT, self::C_FLAG,
+                           self::FLINT, self::C_FLAG,
                            $this->getEngine()->getFilePathOnDisk($p));
       } else {
         $futures[$p] = new ExecFuture("%s %s 2>&1",
-            $CPP_LINT, $this->getEngine()->getFilePathOnDisk($p));
+          self::FLINT, $this->getEngine()->getFilePathOnDisk($p));
       }
     }
 
     foreach (Futures($futures)->limit(8) as $p => $f) {
       $this->rawLintOutput[$p] = $f->resolvex();
     }
-    }
+
     return;
   }
 
@@ -47,44 +40,70 @@ class FbcodeCppLinter extends ArcanistLinter {
   }
 
   public function lintPath($path) {
+    $this->runCppLint($path);
+  }
+
+  private function runCppLint($path) {
     $msgs = $this->getCppLintOutput($path);
     foreach ($msgs as $m) {
       $this->raiseLintAtLine($m['line'], 0, $m['severity'], $m['msg']);
     }
   }
 
+  private function adviseOnEachPattern(
+    $path,
+    $regex,
+    $message,
+    $lint_type = self::LINT_ADVICE,
+    $match_idx = 0) {
+      $file_data = $this->getData($path);
+      $matches = array();
+      if (!preg_match_all($regex, $file_data, $matches, PREG_OFFSET_CAPTURE)) {
+        return;
+      }
+
+      foreach ($matches[$match_idx] as $match) {
+        list($match_str, $offset) = $match;
+        $this->raiseLintAtOffset($offset, $lint_type, $message, $match_str);
+      }
+  }
+
   public function getLintSeverityMap() {
     return array(
       self::LINT_WARNING => ArcanistLintSeverity::SEVERITY_WARNING,
+      self::LINT_ADVICE  => ArcanistLintSeverity::SEVERITY_ADVICE,
       self::LINT_ERROR   => ArcanistLintSeverity::SEVERITY_ERROR
     );
   }
 
   public function getLintNameMap() {
     return array(
+      self::LINT_ADVICE   => "CppLint Advice",
       self::LINT_WARNING  => "CppLint Warning",
       self::LINT_ERROR    => "CppLint Error"
     );
   }
 
   private function getCppLintOutput($path) {
-    if (!array_key_exists($path, $this->rawLintOutput)) {
-      return array();
-    }
     list($output) = $this->rawLintOutput[$path];
 
     $msgs = array();
     $current = null;
+    $matches = array();
     foreach (explode("\n", $output) as $line) {
-      if (preg_match('/[^:]*\((\d+)\):(.*)$/', $line, $matches)) {
+      if (preg_match('/.*?:(\d+):(.*)/', $line, $matches)) {
         if ($current) {
           $msgs[] = $current;
         }
         $line = $matches[1];
         $text = $matches[2];
-        $sev  = preg_match('/.*Warning.*/', $text)
-                  ? self::LINT_WARNING
-                  : self::LINT_ERROR;
+        if (preg_match('/.*Warning.*/', $text)) {
+          $sev = self::LINT_WARNING;
+        } else if (preg_match('/.*Advice.*/', $text)) {
+          $sev = self::LINT_ADVICE;
+        } else {
+          $sev = self::LINT_ERROR;
+        }
         $current = array('line'     => $line,
                          'msg'      => $text,
                          'severity' => $sev);
@@ -99,4 +118,3 @@ class FbcodeCppLinter extends ArcanistLinter {
     return $msgs;
   }
 }
-

arcanist_util/cpp_linter/PfffCppLinter.php

@@ -1,68 +0,0 @@
-<?php
-// Copyright 2004-present Facebook.  All rights reserved.
-
-class PfffCppLinter extends ArcanistLinter {
-  const PROGRAM      = "/home/engshare/tools/checkCpp";
-
-  public function getLinterName() {
-    return "checkCpp";
-  }
-  public function getLintNameMap() {
-    return array(
-    );
-  }
-
-  public function getLintSeverityMap() {
-    return array(
-    );
-  }
-
-  public function willLintPaths(array $paths) {
-    $program = false;
-    $ret_value = 0;
-    $last_line = system("which checkCpp", $ret_value);
-    if ($ret_value == 0) {
-      $program = $last_line;
-    } else if (file_exists(self::PROGRAM)) {
-      $program = self::PROGRAM;
-    }
-    if ($program) {
-      $futures = array();
-      foreach ($paths as $p) {
-        $futures[$p] = new ExecFuture("%s --lint %s 2>&1",
-          $program, $this->getEngine()->getFilePathOnDisk($p));
-      }
-      foreach (Futures($futures)->limit(8) as $p => $f) {
-
-        list($stdout, $stderr) = $f->resolvex();
-        $raw = json_decode($stdout, true);
-        if (!is_array($raw)) {
-          throw new Exception(
-            "checkCpp returned invalid JSON!".
-            "Stdout: {$stdout} Stderr: {$stderr}"
-          );
-        }
-        foreach($raw as $err) {
-          $this->addLintMessage(
-            ArcanistLintMessage::newFromDictionary(
-              array(
-                'path' => $err['file'],
-                'line' => $err['line'],
-                'char' => 0,
-                'name' => $err['name'],
-                'description' => $err['info'],
-                'code' => $this->getLinterName(),
-                'severity' => ArcanistLintSeverity::SEVERITY_WARNING,
-              )
-            )
-          );
-        }
-      }
-    }
-    return;
-  }
-
-  public function lintPath($path) {
-    return;
-  }
-}

arcanist_util/lint_engine/FacebookFbcodeLintEngine.php

@@ -1,5 +1,8 @@
 <?php
 // Copyright 2004-present Facebook.  All rights reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
 
 class FacebookFbcodeLintEngine extends ArcanistLintEngine {
 
@@ -39,14 +42,13 @@ class FacebookFbcodeLintEngine extends ArcanistLintEngine {
     $python_linter = new ArcanistPEP8Linter();
     $linters[] = $python_linter;
 
-   // Currently we can't run cpplint in commit hook mode, because it
-    // depends on having access to the working directory.
     if (!$this->getCommitHookMode()) {
       $cpp_linters = array();
-      $google_linter = new ArcanistCpplintLinter();
-      $cpp_linters[] = $linters[] = $google_linter;
+      $cpp_linters[] = $linters[] = new ArcanistCpplintLinter();
       $cpp_linters[] = $linters[] = new FbcodeCppLinter();
-      $cpp_linters[] = $linters[] = new PfffCppLinter();
+
+      $clang_format_linter = new FbcodeClangFormatLinter();
+      $linters[] = $clang_format_linter;
     }
 
     $spelling_linter = new ArcanistSpellingLinter();
@@ -93,6 +95,11 @@ class FacebookFbcodeLintEngine extends ArcanistLintEngine {
           $linter->addPath($path);
           $linter->addData($path, $this->loadData($path));
         }
+
+        $clang_format_linter->addPath($path);
+        $clang_format_linter->addData($path, $this->loadData($path));
+        $clang_format_linter->setPathChangedLines(
+          $path, $this->getPathChangedLines($path));
       }
 
       // Match *.py and contbuild config files

arcanist_util/lint_engine/FacebookHowtoevenLintEngine.php

@@ -0,0 +1,27 @@
+<?php
+// Copyright 2015-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
+
+final class FacebookHowtoevenLintEngine extends ArcanistLintEngine {
+
+  public function buildLinters() {
+    $paths = array();
+
+    foreach ($this->getPaths() as $path) {
+      // Don't try to lint deleted files or changed directories.
+      if (!Filesystem::pathExists($path) || is_dir($path)) {
+        continue;
+      }
+
+      if (preg_match('/\.(cpp|c|cc|cxx|h|hh|hpp|hxx|tcc)$/', $path)) {
+        $paths[] = $path;
+      }
+    }
+
+    $howtoeven = new FacebookHowtoevenLinter();
+    $howtoeven->setPaths($paths);
+    return array($howtoeven);
+  }
+}

arcanist_util/unit_engine/FacebookFbcodeUnitTestEngine.php

@@ -1,5 +1,8 @@
 <?php
 // Copyright 2004-present Facebook. All Rights Reserved.
+// This source code is licensed under the BSD-style license found in the
+// LICENSE file in the root directory of this source tree. An additional grant
+// of patent rights can be found in the PATENTS file in the same directory.
 
 class FacebookFbcodeUnitTestEngine extends ArcanistBaseUnitTestEngine {