Page MenuHomePhabricator
Files F20877070

D9068.diff
No OneTemporary
Actions

D9068.diff
View Options

diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -45,6 +45,7 @@
'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 @@
'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
--- a/src/lint/linter/ArcanistCppcheckLinter.php
+++ b/src/lint/linter/ArcanistCppcheckLinter.php
@@ -1,121 +1,110 @@
<?php
/**
- * Uses cppcheck to do basic checks in a cpp file
- *
- * You can get it here:
- * http://cppcheck.sourceforge.net/
- *
- * @group linter
+ * Uses Cppcheck to do basic checks in a C++ file.
*/
-final class ArcanistCppcheckLinter extends ArcanistLinter {
+final class ArcanistCppcheckLinter extends ArcanistExternalLinter {
+
+ public function getInfoName() {
+ return 'Cppcheck';
+ }
+
+ public function getInfoURI() {
+ return 'http://cppcheck.sourceforge.net';
+ }
+
+ public function getInfoDescription() {
+ return pht('Use `cppcheck` to perform static analysis on C/C++ code.');
+ }
public function getLinterName() {
return 'cppcheck';
}
- public function getLintOptions() {
- $config = $this->getEngine()->getConfigurationManager();
- // You will for sure want some options. The below default tends to be ok
- return $config->getConfigFromAnySource(
- 'lint.cppcheck.options',
- '-j2 --inconclusive --enable=performance,style,portability,information');
+ public function getLinterConfigurationName() {
+ return 'cppcheck';
}
- 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.");
- }
+ public function getDefaultBinary() {
+ $prefix = $this->getDeprecatedConfiguration('lint.cppcheck.prefix');
+ $bin = $this->getDeprecatedConfiguration('lint.cppcheck.bin', 'cppcheck');
- return csprintf("%s/%s", $prefix, $bin);
+ if ($prefix) {
+ return $prefix.'/'.$bin;
+ } else {
+ return $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."
- );
- }
+ public function getVersion() {
+ list($stdout) = execx('%C --version', $this->getExecutableCommand());
- return $bin;
+ $matches = array();
+ $regex = '/^Cppcheck (?P<version>\d+\.\d+)$/';
+ if (preg_match($regex, $stdout, $matches)) {
+ return $matches['version'];
+ } else {
+ return false;
+ }
}
- public function lintPath($path) {
- $bin = $this->getLintPath();
- $options = $this->getLintOptions();
+ public function getInstallInstructions() {
+ return pht('Install Cpplint using `apt-get install cpplint` or similar.');
+ }
- list($rc, $stdout, $stderr) = exec_manual(
- "%C %C --inline-suppr --xml-version=2 -q %s",
- $bin,
- $options,
- $this->getEngine()->getFilePathOnDisk($path));
+ protected function getMandatoryFlags() {
+ return array(
+ '--quiet',
+ '--inline-suppr',
+ '--xml',
+ '--xml-version=2',
+ );
+ }
- if ($rc === 1) {
- throw new Exception("cppcheck failed to run correctly:\n".$stderr);
- }
+ protected function getDefaultFlags() {
+ return $this->getDeprecatedConfiguration(
+ 'lint.cppcheck.options',
+ array('-j2', '--enable=performance,style,portability,information'));
+ }
+ 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;
+ 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'));
+
+ switch ($error->getAttribute('severity')) {
+ case 'error':
+ $message->setSeverity(ArcanistLintSeverity::SEVERITY_ERROR);
+ break;
+
+ default:
+ if ($error->getAttribute('inconclusive')) {
+ $message->setSeverity(ArcanistLintSeverity::SEVERITY_ADVICE);
+ } else {
+ $message->setSeverity(ArcanistLintSeverity::SEVERITY_WARNING);
+ }
+ break;
+ }
+
+ $messages[] = $message;
}
- $location = $loc_node->item(0);
- if (!$location) {
- continue;
- }
- $file = $location->getAttribute('file');
- if ($file != Filesystem::resolvePath($path)) {
- continue;
- }
- $line = $location->getAttribute('line');
-
- $id = $error->getAttribute('id');
- $severity = $error->getAttribute('severity');
- $msg = $error->getAttribute('msg');
- $inconclusive = $error->getAttribute('inconclusive');
- $verbose_msg = $error->getAttribute('verbose');
-
- $severity_code = ArcanistLintSeverity::SEVERITY_WARNING;
- if ($inconclusive) {
- $severity_code = ArcanistLintSeverity::SEVERITY_ADVICE;
- } else if (stripos($severity, 'error') !== false) {
- $severity_code = ArcanistLintSeverity::SEVERITY_ERROR;
- }
-
- $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
--- /dev/null
+++ b/src/lint/linter/__tests__/ArcanistCppcheckLinterTestCase.php
@@ -0,0 +1,12 @@
+<?php
+
+final class ArcanistCppcheckLinterTestCase
+ extends ArcanistArcanistLinterTestCase {
+
+ public function testCppcheckLint() {
+ return $this->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
--- /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
--- /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
--- /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
--- /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:

File Metadata

Mime Type
text/plain
Expires
Sat, Jun 27, 11:52 AM (21 h, 54 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
16305914
Default Alt Text
D9068.diff (10 KB)

Event Timeline

Phabricator · Built by Phacility