Modernize ArcanistCppcheckLinter.

Summary: Ref T2039. Make the `ArcanistCppcheckLinter` compatible with `.arclint`.

Test Plan: Wrote and executed unit tests.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Maniphest Tasks: T2039

Differential Revision: https://secure.phabricator.com/D9068

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',

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');
+  public function getDefaultBinary() {
+    $prefix = $this->getDeprecatedConfiguration('lint.cppcheck.prefix');
+    $bin = $this->getDeprecatedConfiguration('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.");
+    if ($prefix) {
+      return $prefix.'/'.$bin;
+    } else {
+      return $bin;
+    }
   }
 
-      return csprintf("%s/%s", $prefix, $bin);
+  public function getVersion() {
+    list($stdout) = execx('%C --version', $this->getExecutableCommand());
+
+    $matches = array();
+    $regex = '/^Cppcheck (?P<version>\d+\.\d+)$/';
+    if (preg_match($regex, $stdout, $matches)) {
+      return $matches['version'];
+    } else {
+      return false;
+    }
   }
 
-    // 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 getInstallInstructions() {
+    return pht('Install Cpplint using `apt-get install cpplint` or similar.');
+  }
+
+  protected function getMandatoryFlags() {
+    return array(
+      '--quiet',
+      '--inline-suppr',
+      '--xml',
+      '--xml-version=2',
     );
   }
 
-    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 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;
-      }
-      $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;
-      }
-
+      foreach ($error->getElementsByTagName('location') as $location) {
         $message = new ArcanistLintMessage();
-      $message->setPath($path);
-      $message->setLine($line);
-      $message->setCode($severity);
-      $message->setName($id);
-      $message->setDescription($msg);
-      $message->setSeverity($severity_code);
+        $message->setPath($location->getAttribute('file'));
+        $message->setLine($location->getAttribute('line'));
+        $message->setCode('Cppcheck');
+        $message->setName($error->getAttribute('id'));
+        $message->setDescription($error->getAttribute('msg'));
 
-      $this->addLintMessage($message);
+        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;
       }
     }
 
+    return $messages;
+  }
+
 }

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());
+  }
+
+}

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:

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;
+}
+~~~~~~~~~~

src/lint/linter/__tests__/cppcheck/ok.lint-test

@@ -0,0 +1,5 @@
+int main()
+{
+    return 0;
+}
+~~~~~~~~~~

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: