diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a7ca5c09..b5613651 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -45,6 +45,7 @@ phutil_register_library_map(array( 'ArcanistConfigurationManager' => 'configuration/ArcanistConfigurationManager.php', 'ArcanistCoverWorkflow' => 'workflow/ArcanistCoverWorkflow.php', 'ArcanistCppcheckLinter' => 'lint/linter/ArcanistCppcheckLinter.php', + 'ArcanistCppcheckLinterTestCase' => 'lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php', 'ArcanistCpplintLinter' => 'lint/linter/ArcanistCpplintLinter.php', 'ArcanistCpplintLinterTestCase' => 'lint/linter/__tests__/ArcanistCpplintLinterTestCase.php', 'ArcanistDiffChange' => 'parser/diff/ArcanistDiffChange.php', @@ -225,7 +226,8 @@ phutil_register_library_map(array( 'ArcanistConduitLinter' => 'ArcanistLinter', 'ArcanistConfigurationDrivenLintEngine' => 'ArcanistLintEngine', 'ArcanistCoverWorkflow' => 'ArcanistBaseWorkflow', - 'ArcanistCppcheckLinter' => 'ArcanistLinter', + 'ArcanistCppcheckLinter' => 'ArcanistExternalLinter', + 'ArcanistCppcheckLinterTestCase' => 'ArcanistArcanistLinterTestCase', 'ArcanistCpplintLinter' => 'ArcanistExternalLinter', 'ArcanistCpplintLinterTestCase' => 'ArcanistArcanistLinterTestCase', 'ArcanistDiffParserTestCase' => 'ArcanistTestCase', diff --git a/src/lint/linter/ArcanistCppcheckLinter.php b/src/lint/linter/ArcanistCppcheckLinter.php index 471a24ca..50f39950 100644 --- a/src/lint/linter/ArcanistCppcheckLinter.php +++ b/src/lint/linter/ArcanistCppcheckLinter.php @@ -1,121 +1,110 @@ getEngine()->getConfigurationManager(); - // You will for sure want some options. The below default tends to be ok - return $config->getConfigFromAnySource( + public function getLinterConfigurationName() { + return 'cppcheck'; + } + + public function getDefaultBinary() { + $prefix = $this->getDeprecatedConfiguration('lint.cppcheck.prefix'); + $bin = $this->getDeprecatedConfiguration('lint.cppcheck.bin', 'cppcheck'); + + if ($prefix) { + return $prefix.'/'.$bin; + } else { + return $bin; + } + } + + public function getVersion() { + list($stdout) = execx('%C --version', $this->getExecutableCommand()); + + $matches = array(); + $regex = '/^Cppcheck (?P\d+\.\d+)$/'; + if (preg_match($regex, $stdout, $matches)) { + return $matches['version']; + } else { + return false; + } + } + + public function getInstallInstructions() { + return pht('Install Cpplint using `apt-get install cpplint` or similar.'); + } + + protected function getMandatoryFlags() { + return array( + '--quiet', + '--inline-suppr', + '--xml', + '--xml-version=2', + ); + } + + protected function getDefaultFlags() { + return $this->getDeprecatedConfiguration( 'lint.cppcheck.options', - '-j2 --inconclusive --enable=performance,style,portability,information'); + array('-j2', '--enable=performance,style,portability,information')); } - public function getLintPath() { - $config = $this->getEngine()->getConfigurationManager(); - $prefix = $config->getConfigFromAnySource('lint.cppcheck.prefix'); - $bin = $config->getConfigFromAnySource('lint.cppcheck.bin', 'cppcheck'); - - if ($prefix !== null) { - if (!Filesystem::pathExists($prefix.'/'.$bin)) { - throw new ArcanistUsageException( - "Unable to find cppcheck binary in a specified directory. Make ". - "sure that 'lint.cppcheck.prefix' and 'lint.cppcheck.bin' keys are ". - "set correctly. If you'd rather use a copy of cppcheck installed ". - "globally, you can just remove these keys from your .arcconfig."); - } - - return csprintf("%s/%s", $prefix, $bin); - } - - // Look for globally installed cppcheck - list($err) = exec_manual('which %s', $bin); - if ($err) { - throw new ArcanistUsageException( - "cppcheck does not appear to be installed on this system. Install ". - "it (from http://cppcheck.sourceforge.net/) or configure ". - "'lint.cppcheck.prefix' in your .arcconfig to point to the ". - "directory where it resides." - ); - } - - return $bin; - } - - public function lintPath($path) { - $bin = $this->getLintPath(); - $options = $this->getLintOptions(); - - list($rc, $stdout, $stderr) = exec_manual( - "%C %C --inline-suppr --xml-version=2 -q %s", - $bin, - $options, - $this->getEngine()->getFilePathOnDisk($path)); - - if ($rc === 1) { - throw new Exception("cppcheck failed to run correctly:\n".$stderr); - } - + protected function parseLinterOutput($path, $err, $stdout, $stderr) { $dom = new DOMDocument(); - libxml_clear_errors(); - if ($dom->loadXML($stderr) === false || libxml_get_errors()) { - throw new ArcanistUsageException('cppcheck Linter failed to load ' . - 'output. Something happened when running cppcheck. ' . - "Output:\n$stderr" . - "\nTry running lint with --trace flag to get more details."); - } + $ok = @$dom->loadXML($stderr); + if (!$ok) { + return false; + } $errors = $dom->getElementsByTagName('error'); + $messages = array(); foreach ($errors as $error) { - $loc_node = $error->getElementsByTagName('location'); - if (!$loc_node) { - continue; - } - $location = $loc_node->item(0); - if (!$location) { - continue; - } - $file = $location->getAttribute('file'); - if ($file != Filesystem::resolvePath($path)) { - continue; - } - $line = $location->getAttribute('line'); + foreach ($error->getElementsByTagName('location') as $location) { + $message = new ArcanistLintMessage(); + $message->setPath($location->getAttribute('file')); + $message->setLine($location->getAttribute('line')); + $message->setCode('Cppcheck'); + $message->setName($error->getAttribute('id')); + $message->setDescription($error->getAttribute('msg')); - $id = $error->getAttribute('id'); - $severity = $error->getAttribute('severity'); - $msg = $error->getAttribute('msg'); - $inconclusive = $error->getAttribute('inconclusive'); - $verbose_msg = $error->getAttribute('verbose'); + switch ($error->getAttribute('severity')) { + case 'error': + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR); + break; - $severity_code = ArcanistLintSeverity::SEVERITY_WARNING; - if ($inconclusive) { - $severity_code = ArcanistLintSeverity::SEVERITY_ADVICE; - } else if (stripos($severity, 'error') !== false) { - $severity_code = ArcanistLintSeverity::SEVERITY_ERROR; + default: + if ($error->getAttribute('inconclusive')) { + $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE); + } else { + $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING); + } + break; + } + + $messages[] = $message; } - - $message = new ArcanistLintMessage(); - $message->setPath($path); - $message->setLine($line); - $message->setCode($severity); - $message->setName($id); - $message->setDescription($msg); - $message->setSeverity($severity_code); - - $this->addLintMessage($message); } + + return $messages; } } diff --git a/src/lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php b/src/lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php new file mode 100644 index 00000000..a8993900 --- /dev/null +++ b/src/lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php @@ -0,0 +1,12 @@ +executeTestsInDirectory( + dirname(__FILE__).'/cppcheck/', + new ArcanistCppcheckLinter()); + } + +} diff --git a/src/lint/linter/__tests__/cppcheck/file1.lint-test b/src/lint/linter/__tests__/cppcheck/file1.lint-test new file mode 100644 index 00000000..377277b8 --- /dev/null +++ b/src/lint/linter/__tests__/cppcheck/file1.lint-test @@ -0,0 +1,9 @@ +int main() +{ + char a[10]; + a[10] = 0; + return 0; +} +~~~~~~~~~~ +error:4: +warning:4: diff --git a/src/lint/linter/__tests__/cppcheck/inline-suppr.lint-test b/src/lint/linter/__tests__/cppcheck/inline-suppr.lint-test new file mode 100644 index 00000000..63e7d51f --- /dev/null +++ b/src/lint/linter/__tests__/cppcheck/inline-suppr.lint-test @@ -0,0 +1,7 @@ +void f() { + char arr[5]; + // cppcheck-suppress arrayIndexOutOfBounds + // cppcheck-suppress unreadVariable + arr[10] = 0; +} +~~~~~~~~~~ diff --git a/src/lint/linter/__tests__/cppcheck/ok.lint-test b/src/lint/linter/__tests__/cppcheck/ok.lint-test new file mode 100644 index 00000000..e209332b --- /dev/null +++ b/src/lint/linter/__tests__/cppcheck/ok.lint-test @@ -0,0 +1,5 @@ +int main() +{ + return 0; +} +~~~~~~~~~~ diff --git a/src/lint/linter/__tests__/cppcheck/zblair.lint-test b/src/lint/linter/__tests__/cppcheck/zblair.lint-test new file mode 100644 index 00000000..efe85a9a --- /dev/null +++ b/src/lint/linter/__tests__/cppcheck/zblair.lint-test @@ -0,0 +1,20 @@ +/** + * Taken from http://www.slideshare.net/zblair/cppcheck-10316379 + */ +void foo(char* str) { + char* buf = new char[8]; + strcpy(buf, str); + + FILE* file = fopen("out.txt", "w"); + if (!file) + return; + + for (char* c = buf; *c; ++c) + fputc((int)*c, file); + + delete buf; +} +~~~~~~~~~~ +error:10: +error:15: +error:16: