From 46812f68c3fcf1bf1e8ae100f7b073f1436abd27 Mon Sep 17 00:00:00 2001 From: kailiu Date: Thu, 13 Feb 2014 17:48:11 -0800 Subject: [PATCH] Improve/fix bugs for the cpp linter Summary: Previous our new `arc lint` has two annoying bugs: * Keeping sending false alarm that we'd put c++ system files first -- even though we've already done that. - this problem is caused by our linter, which doesn't give the underlying cpplint.py right file path (it gives "-" as file name), making cpplint.py work incorrectly. * Only works in rocksdb's root dir; Otherwise it'll throw exception saying "cannot find cpplint.py". I copied open source ArcanistCpplintLinter and modifiy it for our use. Test Plan: Ran arc lint and made sure the above-mentioned problem won't occur. Reviewers: haobo, sdong, igor, ljin, yhchiang, dhruba CC: leveldb Differential Revision: https://reviews.facebook.net/D16137 --- .arcconfig | 3 +- linters/__phutil_library_map__.php | 1 + linters/cpp_linter/ArcanistCpplintLinter.php | 113 +++++++++++++++++++ linters/{ => cpp_linter}/cpplint.py | 0 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 linters/cpp_linter/ArcanistCpplintLinter.php rename linters/{ => cpp_linter}/cpplint.py (100%) diff --git a/.arcconfig b/.arcconfig index 0c4c85e69..85ca38f25 100644 --- a/.arcconfig +++ b/.arcconfig @@ -6,6 +6,5 @@ "linters" ], "lint.engine" : "FacebookFbcodeLintEngine", - "lint.engine.single.linter" : "FbcodeCppLinter", - "lint.cpplint.prefix" : "linters" + "lint.engine.single.linter" : "FbcodeCppLinter" } diff --git a/linters/__phutil_library_map__.php b/linters/__phutil_library_map__.php index cb10bed69..7808dc1a4 100644 --- a/linters/__phutil_library_map__.php +++ b/linters/__phutil_library_map__.php @@ -13,6 +13,7 @@ phutil_register_library_map(array( 'FacebookFbcodeLintEngine' => 'lint_engine/FacebookFbcodeLintEngine.php', 'FbcodeCppLinter' => 'cpp_linter/FbcodeCppLinter.php', 'PfffCppLinter' => 'cpp_linter/PfffCppLinter.php', + 'ArcanistCpplintLinter' => 'cpp_linter/ArcanistCpplintLinter.php', ), 'function' => array( diff --git a/linters/cpp_linter/ArcanistCpplintLinter.php b/linters/cpp_linter/ArcanistCpplintLinter.php new file mode 100644 index 000000000..cb7842248 --- /dev/null +++ b/linters/cpp_linter/ArcanistCpplintLinter.php @@ -0,0 +1,113 @@ +getEngine()->getConfigurationManager(); + $options = $config->getConfigFromAnySource('lint.cpplint.options', ''); + + return $options; + } + + public function getLintPath() { + $config = $this->getEngine()->getConfigurationManager(); + $prefix = $config->getConfigFromAnySource('lint.cpplint.prefix'); + $bin = $config->getConfigFromAnySource('lint.cpplint.bin', 'cpplint.py'); + + if ($prefix !== null) { + if (!Filesystem::pathExists($prefix.'/'.$bin)) { + throw new ArcanistUsageException( + "Unable to find cpplint.py binary in a specified directory. Make ". + "sure that 'lint.cpplint.prefix' and 'lint.cpplint.bin' keys are ". + "set correctly. If you'd rather use a copy of cpplint installed ". + "globally, you can just remove these keys from your .arcconfig."); + } + + $bin = csprintf("%s/%s", $prefix, $bin); + + return $bin; + } + + // Search under current dir + list($err) = exec_manual('which %s/%s', $this->linterDir(), $bin); + if (!$err) { + return $this->linterDir().'/'.$bin; + } + + // Look for globally installed cpplint.py + list($err) = exec_manual('which %s', $bin); + if ($err) { + throw new ArcanistUsageException( + "cpplint.py does not appear to be installed on this system. Install ". + "it (e.g., with 'wget \"http://google-styleguide.googlecode.com/". + "svn/trunk/cpplint/cpplint.py\"') or configure 'lint.cpplint.prefix' ". + "in your .arcconfig to point to the directory where it resides. ". + "Also don't forget to chmod a+x cpplint.py!"); + } + + return $bin; + } + + public function lintPath($path) { + $bin = $this->getLintPath(); + $options = $this->getLintOptions(); + $path = $this->rocksdbDir().'/'.$path; + + $f = new ExecFuture("%C %C $path", $bin, $options); + + list($err, $stdout, $stderr) = $f->resolve(); + + if ($err === 2) { + throw new Exception("cpplint failed to run correctly:\n".$stderr); + } + + $lines = explode("\n", $stderr); + $messages = array(); + foreach ($lines as $line) { + $line = trim($line); + $matches = null; + $regex = '/^[^:]+:(\d+):\s*(.*)\s*\[(.*)\] \[(\d+)\]$/'; + if (!preg_match($regex, $line, $matches)) { + continue; + } + foreach ($matches as $key => $match) { + $matches[$key] = trim($match); + } + $message = new ArcanistLintMessage(); + $message->setPath($path); + $message->setLine($matches[1]); + $message->setCode($matches[3]); + $message->setName($matches[3]); + $message->setDescription($matches[2]); + $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + $this->addLintMessage($message); + } + } + + // The path of this linter + private function linterDir() { + return dirname(__FILE__); + } + + // TODO(kaili) a quick and dirty way to figure out rocksdb's root dir. + private function rocksdbDir() { + return $this->linterDir()."/../.."; + } +} diff --git a/linters/cpplint.py b/linters/cpp_linter/cpplint.py similarity index 100% rename from linters/cpplint.py rename to linters/cpp_linter/cpplint.py