From d0934a8de232294491f2e5a36beac0996ab90adf Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Thu, 22 Mar 2018 00:33:04 +0100 Subject: [PATCH 1/6] fix CppcheckParserV2; allow multiple NewIssueLocations --- .../sensors/cppcheck/CppcheckParserV2.java | 20 ++- .../cxx/sensors/utils/CxxReportLocation.java | 49 +++++++ .../cxx/sensors/utils/CxxReportSensor.java | 136 +++++++++++------- .../cppcheck-result-SAMPLE-V2.xml | 8 ++ 4 files changed, 158 insertions(+), 55 deletions(-) create mode 100644 cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java index d775283c9e..609b4fbbc4 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java +++ b/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 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,20 @@ 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); } diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java new file mode 100644 index 0000000000..595e8c9eac --- /dev/null +++ b/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; + } +} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java index 1434bfe710..13418cd96d 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java +++ b/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,98 @@ private static List 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)); + } + + public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, String ruleId, + List 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 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) + * 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 locations) { + + String repoKey = ruleRepoKey + this.language.getRepositorySuffix(); + NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(repoKey, ruleId)); + + int tmpViolationPerModuleCount = 0; + Map tmpViolationsPerFileCount = new HashMap<>(); + List 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 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); } } diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml index c83f5af80c..56b1a41559 100644 --- a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml +++ b/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"> + + + + + + + + \ No newline at end of file From 452f640d3db63fe2d8b1a6b8fcdee42cd5635309 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Thu, 22 Mar 2018 00:33:04 +0100 Subject: [PATCH 2/6] fix CppcheckParserV2; allow multiple NewIssueLocations --- .../cppcheck-result-SAMPLE-V2.xml | 16 ++++++++-------- .../SampleProject/sources/utils/code_chunks.cpp | 8 ++++++++ 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml index 56b1a41559..e0d238b749 100644 --- a/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml +++ b/cxx-sensors/src/test/resources/org/sonar/cxx/sensors/reports-project/cppcheck-reports/cppcheck-result-SAMPLE-V2.xml @@ -10,13 +10,13 @@ verbose="long error text" inconclusive="true"> - - - - - - - - + + + + + + + + \ No newline at end of file diff --git a/sonar-cxx-plugin/src/samples/SampleProject/sources/utils/code_chunks.cpp b/sonar-cxx-plugin/src/samples/SampleProject/sources/utils/code_chunks.cpp index de5f82e9e2..22e7418c22 100644 --- a/sonar-cxx-plugin/src/samples/SampleProject/sources/utils/code_chunks.cpp +++ b/sonar-cxx-plugin/src/samples/SampleProject/sources/utils/code_chunks.cpp @@ -1,4 +1,5 @@ #include +#include #include 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; } From c8865392116bc054216924a059b3e2d7c0809f86 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Thu, 22 Mar 2018 00:33:04 +0100 Subject: [PATCH 3/6] fix CppcheckParserV2; allow multiple NewIssueLocations [fix 2] --- .../org/sonar/cxx/sensors/cppcheck/CxxCppCheckSensorTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/cppcheck/CxxCppCheckSensorTest.java b/cxx-sensors/src/test/java/org/sonar/cxx/sensors/cppcheck/CxxCppCheckSensorTest.java index a46db4d375..3a06aef290 100644 --- a/cxx-sensors/src/test/java/org/sonar/cxx/sensors/cppcheck/CxxCppCheckSensorTest.java +++ b/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 From 4a640d4534b068e78dfb24ce7beb3550c484553f Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Thu, 22 Mar 2018 14:52:14 +0100 Subject: [PATCH 4/6] fix CppcheckParserV2; allow multiple NewIssueLocations [formatting] --- .../cxx/sensors/cppcheck/CppcheckParserV2.java | 5 ++--- .../sonar/cxx/sensors/utils/CxxReportSensor.java | 16 +++++++++++----- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java index 609b4fbbc4..32fb53a0e6 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java @@ -100,9 +100,8 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException { line = null; info = null; } - - CxxReportLocation location = new CxxReportLocation(file, line, - (info == null) ? msg : info); + + CxxReportLocation location = new CxxReportLocation(file, line, (info == null) ? msg : info); locations.add(location); } diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java index 13418cd96d..fda38f1652 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java @@ -275,6 +275,14 @@ public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, 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 locations) { CxxReportLocation firstLocation = locations.get(0); @@ -315,11 +323,9 @@ private NewIssueLocation createNewIssueLocationModule(SensorContext sensorContex } /** - * 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) + * 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, String ruleId, List locations) { From e04cb27be3790516dc34a0232aa8d6e37a88ec58 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Thu, 22 Mar 2018 15:09:35 +0100 Subject: [PATCH 5/6] fix CppcheckParserV2; allow multiple NewIssueLocations [fix 3] --- .../java/org/sonar/cxx/sensors/utils/CxxReportSensor.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java index fda38f1652..e5bf7ed9ba 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java @@ -308,10 +308,10 @@ private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, tmpViolationsPerFileCount.merge(inputFile, 1, Integer::sum); return newIssueLocation; + } else { + LOG.warn("Cannot find the file '{}', skipping violations", normalPath); + notFoundFiles.add(normalPath); } - } else { - LOG.warn("Cannot find the file '{}', skipping violations", normalPath); - notFoundFiles.add(normalPath); } return null; } From 6a9347ee3c09a7e814b923190052add78b4794a8 Mon Sep 17 00:00:00 2001 From: Ivan Galkin Date: Wed, 28 Mar 2018 01:41:45 +0200 Subject: [PATCH 6/6] fix CppcheckParserV2; allow multiple NewIssueLocations [fix 4] * create first-class citizen entities CxxReportIssue and CxxReportLocation (1:n) (CxxReportIssue::equals() allows clear distinguishing if issue was processed already) * CppcheckParserV2: better implementation for primary and secondary locations * CxxReportSensor: increase issues-per-module counter only once per CxxReportIssue, increase issues-per-file counter only once per affected file TODO (?): tests for CxxReportIssue and CxxReportLocation TODO (?): use the new function CxxReportSensor::saveUniqueViolation(SensorContext, CxxReportIssue) --- .../sensors/cppcheck/CppcheckParserV2.java | 46 +++++++---- .../cxx/sensors/utils/CxxReportIssue.java | 82 +++++++++++++++++++ .../cxx/sensors/utils/CxxReportLocation.java | 34 ++++++-- .../cxx/sensors/utils/CxxReportSensor.java | 48 +++++------ 4 files changed, 162 insertions(+), 48 deletions(-) create mode 100644 cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java index 32fb53a0e6..96b2413340 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/cppcheck/CppcheckParserV2.java @@ -22,6 +22,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import javax.xml.stream.XMLStreamException; import org.codehaus.staxmate.in.SMHierarchicCursor; @@ -29,6 +30,7 @@ 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.CxxReportIssue; import org.sonar.cxx.sensors.utils.CxxReportLocation;; import org.sonar.cxx.sensors.utils.EmptyReportException; import org.sonar.cxx.sensors.utils.StaxParser; @@ -74,21 +76,18 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException { parsed = true; SMInputCursor errorCursor = errorsCursor.childElementCursor("error"); while (errorCursor.getNext() != null) { - String id = errorCursor.getAttrValue("id"); - String msg = createMsg( - errorCursor.getAttrValue("inconclusive"), - errorCursor.getAttrValue("msg") - ); - String file = null; - String line = null; - String info = null; + String id = Objects.requireNonNull(errorCursor.getAttrValue("id"), + "Missing mandatory attribute /results/errors/error[@id]"); + String msg = Objects.requireNonNull(errorCursor.getAttrValue("msg"), + "Missing mandatory attribute /results/errors/error[@msg]"); + msg = createMsg(errorCursor.getAttrValue("inconclusive"), errorCursor.getAttrValue("msg")); List locations = new ArrayList<>(); SMInputCursor locationCursor = errorCursor.childElementCursor("location"); while (locationCursor.getNext() != null) { - file = locationCursor.getAttrValue("file"); - line = locationCursor.getAttrValue("line"); - info = locationCursor.getAttrValue("info"); + String file = locationCursor.getAttrValue("file"); + String line = locationCursor.getAttrValue("line"); + String info = locationCursor.getAttrValue("info"); if (file != null) { file = file.replace('\\', '/'); @@ -101,15 +100,32 @@ public void stream(SMHierarchicCursor rootCursor) throws XMLStreamException { info = null; } - CxxReportLocation location = new CxxReportLocation(file, line, (info == null) ? msg : info); - locations.add(location); + if (locations.isEmpty()) { + CxxReportLocation primaryLocation = new CxxReportLocation(file, line, msg); + locations.add(primaryLocation); + + // add the same file:line second time if there is additional + // information about the flow/analysis + if (info != null && !msg.equals(info)) { + CxxReportLocation primaryLocationWithMoreInfo = new CxxReportLocation(file, line, info); + locations.add(primaryLocationWithMoreInfo); + } + } else if (info != null) { + CxxReportLocation secondaryLocation = new CxxReportLocation(file, line, info); + locations.add(secondaryLocation); + } } if (isInputValid(id, msg)) { if (locations.isEmpty()) { - sensor.saveUniqueViolation(context, CxxCppCheckRuleRepository.KEY, file, line, id, msg); + // no tags: issue raised on the whole + // module/project + CxxReportLocation moduleLocation = new CxxReportLocation(null, null, msg); + CxxReportIssue moduleIssue = new CxxReportIssue(CxxCppCheckRuleRepository.KEY, id, moduleLocation); + sensor.saveUniqueViolation(context, moduleIssue); } else { - sensor.saveUniqueViolation(context, CxxCppCheckRuleRepository.KEY, id, locations); + CxxReportIssue issue = new CxxReportIssue(CxxCppCheckRuleRepository.KEY, id, locations); + sensor.saveUniqueViolation(context, issue); } } else { LOG.warn("Skipping invalid violation: '{}'", msg); diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java new file mode 100644 index 0000000000..5ad1f35cbf --- /dev/null +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportIssue.java @@ -0,0 +1,82 @@ +/* + * 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; + +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.stream.Collectors; + +/** + * Issue with one or multiple locations + */ +public class CxxReportIssue { + final String ruleRepoKey; + final String ruleId; + final List locations; + + public CxxReportIssue(String ruleRepoKey, String ruleId, CxxReportLocation primaryLocation) { + this(ruleRepoKey, ruleId, Collections.singletonList(primaryLocation)); + } + + public CxxReportIssue(String ruleRepoKey, String ruleId, List locations) { + super(); + this.ruleRepoKey = ruleRepoKey; + this.ruleId = ruleId; + this.locations = locations; + } + + public String getRuleRepoKey() { + return ruleRepoKey; + } + + public String getRuleId() { + return ruleId; + } + + public List getLocations() { + return Collections.unmodifiableList(locations); + } + + @Override + public String toString() { + String locationsToString = locations.stream().map(Object::toString).collect(Collectors.joining(", ")); + return "CxxReportIssue [ruleRepoKey=" + ruleRepoKey + ", ruleId=" + ruleId + ", locations=" + locationsToString + + "]"; + } + + @Override + public int hashCode() { + return Objects.hash(locations, ruleId, ruleRepoKey); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + CxxReportIssue other = (CxxReportIssue) obj; + return Objects.equals(locations, other.locations) && Objects.equals(ruleId, other.ruleId) + && Objects.equals(ruleRepoKey, other.ruleRepoKey); + } +} diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java index 595e8c9eac..0ace340f9a 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportLocation.java @@ -19,6 +19,8 @@ */ package org.sonar.cxx.sensors.utils; +import java.util.Objects; + /** * Each issues in SonarQube might have multiple locations; Encapsulate its * properties in this structure @@ -26,13 +28,13 @@ public class CxxReportLocation { final String file; final String line; - final String msg; + final String info; - public CxxReportLocation(String file, String line, String msg) { + public CxxReportLocation(String file, String line, String info) { super(); this.file = file; this.line = line; - this.msg = msg; + this.info = info; } public String getFile() { @@ -43,7 +45,29 @@ public String getLine() { return line; } - public String getMsg() { - return msg; + public String getInfo() { + return info; + } + + @Override + public String toString() { + return "CxxReportLocation [file=" + file + ", line=" + line + ", info=" + info + "]"; + } + + @Override + public int hashCode() { + return Objects.hash(file, info, line); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) + return true; + if (obj == null) + return false; + if (getClass() != obj.getClass()) + return false; + CxxReportLocation other = (CxxReportLocation) obj; + return Objects.equals(file, other.file) && Objects.equals(info, other.info) && Objects.equals(line, other.line); } } diff --git a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java index e5bf7ed9ba..a890f8a4bc 100644 --- a/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java +++ b/cxx-sensors/src/main/java/org/sonar/cxx/sensors/utils/CxxReportSensor.java @@ -55,7 +55,7 @@ public abstract class CxxReportSensor implements Sensor { private static final Logger LOG = Loggers.get(CxxReportSensor.class); private final Set notFoundFiles = new HashSet<>(); - private final Set uniqueIssues = new HashSet<>(); + private final Set uniqueIssues = new HashSet<>(); private final Map violationsPerFileCount = new HashMap<>(); private int violationsPerModuleCount; protected final CxxLanguage language; @@ -272,7 +272,8 @@ private static List normalizeReportPaths(final File moduleBaseDir, List< 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)); + CxxReportIssue issue = new CxxReportIssue(ruleRepoKey, ruleId, location); + saveUniqueViolation(sensorContext, issue); } /** @@ -283,17 +284,14 @@ public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, * @param ruleId * @param locations */ - public void saveUniqueViolation(SensorContext sensorContext, String ruleRepoKey, String ruleId, - List locations) { - CxxReportLocation firstLocation = locations.get(0); - // StringBuilder is slower - if (uniqueIssues.add(firstLocation.getFile() + firstLocation.getLine() + ruleId + firstLocation.getMsg())) { - saveViolation(sensorContext, ruleRepoKey, ruleId, locations); + public void saveUniqueViolation(SensorContext sensorContext, CxxReportIssue issue) { + if (uniqueIssues.add(issue)) { + saveViolation(sensorContext, issue); } } private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, NewIssue newIssue, - CxxReportLocation location, Map tmpViolationsPerFileCount) { + CxxReportLocation location, Set affectedFiles) { String root = sensorContext.fileSystem().baseDir().getAbsolutePath(); String normalPath = CxxUtils.normalizePathFull(location.getFile(), root); if (normalPath != null && !notFoundFiles.contains(normalPath)) { @@ -303,9 +301,9 @@ private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, 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()); + .at(inputFile.selectLine(lineNr > 0 ? lineNr : 1)).message(location.getInfo()); - tmpViolationsPerFileCount.merge(inputFile, 1, Integer::sum); + affectedFiles.add(inputFile); return newIssueLocation; } else { @@ -318,7 +316,7 @@ private NewIssueLocation createNewIssueLocationFile(SensorContext sensorContext, private NewIssueLocation createNewIssueLocationModule(SensorContext sensorContext, NewIssue newIssue, CxxReportLocation location) { - NewIssueLocation newIssueLocation = newIssue.newLocation().on(sensorContext.module()).message(location.getMsg()); + NewIssueLocation newIssueLocation = newIssue.newLocation().on(sensorContext.module()).message(location.getInfo()); return newIssueLocation; } @@ -327,28 +325,23 @@ private NewIssueLocation createNewIssueLocationModule(SensorContext sensorContex * 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, String ruleId, - List locations) { + private void saveViolation(SensorContext sensorContext, CxxReportIssue issue) { + String repoKey = issue.getRuleRepoKey() + this.language.getRepositorySuffix(); + NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(repoKey, issue.getRuleId())); - String repoKey = ruleRepoKey + this.language.getRepositorySuffix(); - NewIssue newIssue = sensorContext.newIssue().forRule(RuleKey.of(repoKey, ruleId)); - - int tmpViolationPerModuleCount = 0; - Map tmpViolationsPerFileCount = new HashMap<>(); + Set affectedFiles = new HashSet<>(); List newIssueLocations = new ArrayList<>(); - for (CxxReportLocation location : locations) { + for (CxxReportLocation location : issue.getLocations()) { if (location.getFile() != null && !location.getFile().isEmpty()) { NewIssueLocation newIssueLocation = createNewIssueLocationFile(sensorContext, newIssue, location, - tmpViolationsPerFileCount); + affectedFiles); if (newIssueLocation != null) { newIssueLocations.add(newIssueLocation); - tmpViolationPerModuleCount++; } } else { NewIssueLocation newIssueLocation = createNewIssueLocationModule(sensorContext, newIssue, location); newIssueLocations.add(newIssueLocation); - tmpViolationPerModuleCount++; } } @@ -360,13 +353,12 @@ private void saveViolation(SensorContext sensorContext, String ruleRepoKey, Stri } newIssue.save(); - for (Map.Entry entry : tmpViolationsPerFileCount.entrySet()) { - violationsPerFileCount.merge(entry.getKey(), entry.getValue(), Integer::sum); + for (InputFile affectedFile : affectedFiles) { + violationsPerFileCount.merge(affectedFile, 1, Integer::sum); } - violationsPerModuleCount += tmpViolationPerModuleCount; + violationsPerModuleCount++; } 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 '{}':{}', skipping issue", issue.toString(), ex.getMessage()); CxxUtils.validateRecovery(ex, this.language); } }