fix CppcheckParserV2; allow multiple NewIssueLocations

cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java

@@ -20,12 +20,16 @@
 package org.sonar.cxx.sensors.cppcheck;
 
 import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
 import javax.xml.stream.XMLStreamException;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
+import org.sonar.cxx.sensors.utils.CxxReportLocation;;
 import org.sonar.cxx.sensors.utils.EmptyReportException;
 import org.sonar.cxx.sensors.utils.StaxParser;
 
@@ -77,11 +81,14 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
                 );
                 String file = null;
                 String line = null;
+                String info = null;
 
+                List<CxxReportLocation> locations = new ArrayList<>();
                 SMInputCursor locationCursor = errorCursor.childElementCursor("location");
-                if (locationCursor.getNext() != null) {
+                while (locationCursor.getNext() != null) {
                   file = locationCursor.getAttrValue("file");
                   line = locationCursor.getAttrValue("line");
+                  info = locationCursor.getAttrValue("info");
 
                   if (file != null) {
                     file = file.replace('\\', '/');
@@ -91,11 +98,19 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException {
                     // findings on project level
                     file = null;
                     line = null;
+                    info = null;
                   }
+
+                  CxxReportLocation location = new CxxReportLocation(file, line, (info == null) ? msg : info);
+                  locations.add(location);
                 }
 
                 if (isInputValid(id, msg)) {
-                  sensor.saveUniqueViolation(context, CxxCppCheckRuleRepository.KEY, file, line, id, msg);
+                  if (locations.isEmpty()) {
+                    sensor.saveUniqueViolation(context, CxxCppCheckRuleRepository.KEY, file, line, id, msg);
+                  } else {
+                    sensor.saveUniqueViolation(context, CxxCppCheckRuleRepository.KEY, id, locations);
+                  }
                 } else {
                   LOG.warn("Skipping invalid violation: '{}'", msg);
                 }

cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java

@@ -0,0 +1,49 @@
+/*
+ * Sonar C++ Plugin (Community)
+ * Copyright (C) 2010-2018 SonarOpenCommunity
+ * http://github.com/SonarOpenCommunity/sonar-cxx
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ */
+package org.sonar.cxx.sensors.utils;
+
+/**
+ * Each issues in SonarQube might have multiple locations; Encapsulate its
+ * properties in this structure
+ */
+public class CxxReportLocation {
+  final String file;
+  final String line;
+  final String msg;
+
+  public CxxReportLocation(String file, String line, String msg) {
+    super();
+    this.file = file;
+    this.line = line;
+    this.msg = msg;
+  }
+
+  public String getFile() {
+    return file;
+  }
+
+  public String getLine() {
+    return line;
+  }
+
+  public String getMsg() {
+    return msg;
+  }
+}

cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java

@@ -25,6 +25,7 @@
 import java.io.File;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -44,6 +45,7 @@
 import org.sonar.api.utils.log.Logger;
 import org.sonar.api.utils.log.Loggers;
 import org.sonar.cxx.CxxLanguage;
+import org.sonar.cxx.sensors.utils.CxxReportLocation;
 
 /**
  * This class is used as base for all sensors which import reports. It hosts common logic such as finding the reports
@@ -267,70 +269,104 @@ private static List<String> normalizeReportPaths(final File moduleBaseDir, List<
    * @param ruleId
    * @param msg
    */
-  public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey,
-    @Nullable String file, @Nullable String line, String ruleId, String msg) {
+  public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, @Nullable String file,
+      @Nullable String line, String ruleId, String msg) {
+    CxxReportLocation location = new CxxReportLocation(file, line, msg);
+    saveUniqueViolation(sensorContext, ruleRepoKey, ruleId, Collections.singletonList(location));
+  }
+
+  /**
+   * Saves code violation only if unique. Compares file, line, ruleId and msg or the first given location
+   *
+   * @param sensorContext
+   * @param ruleRepoKey
+   * @param ruleId
+   * @param locations
+   */
+  public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, String ruleId,
+      List<CxxReportLocation> locations) {
+    CxxReportLocation firstLocation = locations.get(0);
     // StringBuilder is slower
-    if (uniqueIssues.add(file + line + ruleId + msg)) {
-      saveViolation(sensorContext, ruleRepoKey, file, line, ruleId, msg);
+    if (uniqueIssues.add(firstLocation.getFile() + firstLocation.getLine() + ruleId + firstLocation.getMsg())) {
+      saveViolation(sensorContext, ruleRepoKey, ruleId, locations);
+    }
+  }
+
+  private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, NewIssue newIssue,
+      CxxReportLocation location, Map<InputFile, Integer> tmpViolationsPerFileCount) {
+    String root = sensorContext.fileSystem().baseDir().getAbsolutePath();
+    String normalPath = CxxUtils.normalizePathFull(location.getFile(), root);
+    if (normalPath != null && !notFoundFiles.contains(normalPath)) {
+      InputFile inputFile = sensorContext.fileSystem()
+          .inputFile(sensorContext.fileSystem().predicates().hasAbsolutePath(normalPath));
+      if (inputFile != null) {
+        int lines = inputFile.lines();
+        int lineNr = getLineAsInt(location.getLine(), lines);
+        NewIssueLocation newIssueLocation = newIssue.newLocation().on(inputFile)
+            .at(inputFile.selectLine(lineNr > 0 ? lineNr : 1)).message(location.getMsg());
+
+        tmpViolationsPerFileCount.merge(inputFile, 1, Integer::sum);
+
+        return newIssueLocation;
+      } else {
+        LOG.warn("Cannot find the file '{}', skipping violations", normalPath);
+        notFoundFiles.add(normalPath);
+      }
     }
+    return null;
+  }
+
+  private NewIssueLocation createNewIssueLocationModule(SensorContext sensorContext, NewIssue newIssue,
+      CxxReportLocation location) {
+    NewIssueLocation newIssueLocation = newIssue.newLocation().on(sensorContext.module()).message(location.getMsg());
+    return newIssueLocation;
   }
 
   /**
    * Saves a code violation which is detected in the given file/line and has given ruleId and message. Saves it to the
    * given project and context. Project or file-level violations can be saved by passing null for the according
    * parameters ('file' = null for project level, 'line' = null for file-level)
    */
-  private void saveViolation(SensorContext sensorContext, String ruleRepoKey,
-    @Nullable String filename, @Nullable String line, String ruleId, String msg) {
-    // handles file="" situation -- file level
-    if ((filename != null) && (!filename.isEmpty())) {
-      String root = sensorContext.fileSystem().baseDir().getAbsolutePath();
-      String normalPath = CxxUtils.normalizePathFull(filename, root);
-      if (normalPath != null && !notFoundFiles.contains(normalPath)) {
-        InputFile inputFile = sensorContext.fileSystem().inputFile(sensorContext.fileSystem()
-          .predicates().hasAbsolutePath(normalPath));
-        if (inputFile != null) {
-          try {
-            int lines = inputFile.lines();
-            int lineNr = getLineAsInt(line, lines);
-            String repoKey = ruleRepoKey + this.language.getRepositorySuffix();
-            NewIssue newIssue = sensorContext
-              .newIssue()
-              .forRule(RuleKey.of(repoKey, ruleId));
-            NewIssueLocation location = newIssue.newLocation()
-              .on(inputFile)
-              .at(inputFile.selectLine(lineNr > 0 ? lineNr : 1))
-              .message(msg);
-
-            newIssue.at(location);
-            newIssue.save();
-
-            violationsPerFileCount.merge(inputFile, 1, Integer::sum);
-            violationsPerModuleCount++;
-          } catch (RuntimeException ex) {
-            LOG.error("Could not add the issue '{}', skipping issue", ex.getMessage());
-            CxxUtils.validateRecovery(ex, this.language);
-          }
-        } else {
-          LOG.warn("Cannot find the file '{}', skipping violations", normalPath);
-          notFoundFiles.add(normalPath);
+  private void saveViolation(SensorContext sensorContext, String ruleRepoKey, String ruleId,
+      List<CxxReportLocation> locations) {
+
+    String repoKey = ruleRepoKey + this.language.getRepositorySuffix();
+    NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(repoKey, ruleId));
+
+    int tmpViolationPerModuleCount = 0;
+    Map<InputFile, Integer> tmpViolationsPerFileCount = new HashMap<>();
+    List<NewIssueLocation> newIssueLocations = new ArrayList<>();
+
+    for (CxxReportLocation location : locations) {
+      if (location.getFile() != null && !location.getFile().isEmpty()) {
+        NewIssueLocation newIssueLocation = createNewIssueLocationFile(sensorContext, newIssue, location,
+            tmpViolationsPerFileCount);
+        if (newIssueLocation != null) {
+          newIssueLocations.add(newIssueLocation);
+          tmpViolationPerModuleCount++;
         }
+      } else {
+        NewIssueLocation newIssueLocation = createNewIssueLocationModule(sensorContext, newIssue, location);
+        newIssueLocations.add(newIssueLocation);
+        tmpViolationPerModuleCount++;
       }
-    } else {
-      // project level
-      try {
-        NewIssue newIssue = sensorContext.newIssue().forRule(
-          RuleKey.of(ruleRepoKey + this.language.getRepositorySuffix(), ruleId));
-        NewIssueLocation location = newIssue.newLocation()
-          .on(sensorContext.module())
-          .message(msg);
+    }
 
-        newIssue.at(location);
+    if (!newIssueLocations.isEmpty()) {
+      try {
+        newIssue.at(newIssueLocations.get(0));
+        for (int i = 1; i < newIssueLocations.size(); i++) {
+          newIssue.addLocation(newIssueLocations.get(i));
+        }
         newIssue.save();
-        violationsPerModuleCount++;
+
+        for (Map.Entry<InputFile, Integer> entry : tmpViolationsPerFileCount.entrySet()) {
+          violationsPerFileCount.merge(entry.getKey(), entry.getValue(), Integer::sum);
+        }
+        violationsPerModuleCount += tmpViolationPerModuleCount;
       } catch (RuntimeException ex) {
-        LOG.error("Could not add the issue '{}' for rule '{}:{}', skipping issue",
-          ex.getMessage(), ruleRepoKey, ruleId);
+        LOG.error("Could not add the issue '{}' for rule '{}:{}', skipping issue", ex.getMessage(), ruleRepoKey,
+            ruleId);
         CxxUtils.validateRecovery(ex, this.language);
       }
     }

cxx-sensors/src/test/java/org/sonar/cxx/sensors/cppcheck/CxxCppCheckSensorTest.java

@@ -58,7 +58,7 @@ public void shouldReportCorrectViolations() {
     context.fileSystem().add(TestInputFileBuilder.create("ProjectKey", "sources/utils/utils.cpp")
       .setLanguage("cpp").initMetadata(new String("asd\nasdas\nasda\n")).build());
     sensor.execute(context);
-    assertThat(context.allIssues()).hasSize(9);
+    assertThat(context.allIssues()).hasSize(11);
   }
 
   @Test

cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml

@@ -10,5 +10,13 @@
              verbose="long error text" inconclusive="true">
       <location file="sources/utils/utils.cpp" line="1"/>
     </error>
+    <error id="nullPointerRedundantCheck" severity="warning" msg="Either the condition &apos;!e&apos; is redundant or there is possible null pointer dereference: e." verbose="Either the condition &apos;!e&apos; is redundant or there is possible null pointer dereference: e." cwe="476">
+      <location file="sources/utils/code_chunks.cpp" line="88" info="Null pointer dereference"/>
+      <location file="sources/utils/code_chunks.cpp" line="89" info="Assuming that condition &apos;!e&apos; is not redundant"/>
+    </error>
+    <error id="nullPointer" severity="error" msg="Null pointer dereference: ptr" verbose="Null pointer dereference: ptr" cwe="476">
+      <location file="sources/utils/code_chunks.cpp" line="92" info="Null pointer dereference"/>
+      <location file="sources/utils/code_chunks.cpp" line="91" info="Assignment &apos;ptr=nullptr&apos;, assigned value is 0"/>
+    </error>
   </errors>
 </results>

sonar-cxx-plugin/src/samples/SampleProject/sources/utils/code_chunks.cpp

@@ -1,4 +1,5 @@
 #include <stdio.h>
+#include <stdlib.h>
 #include <iostream>
 using namespace std;
 
@@ -82,6 +83,13 @@ void foo()
     //     cout << "word" << endl;
     //     cout << "word" << endl;
     // }
+
+    const char* e = getenv("PATH");
+    std::cout << *e << std::endl;
+    if ( !e ) { std::cout << "environment variable PATH was not set" << std::endl; }
+
+    int* ptr = nullptr;
+    *ptr = 1;
 }