CPPCheckParser with auto closed <error/> tags #82

release.sh

@@ -1,5 +1,5 @@
 #!/bin/bash
-./gradlew release -d || exit 1
+./gradlew release || exit 1
 ./build.sh
 git commit -a --amend --no-edit
 git push -f

src/main/java/se/bjurr/violations/lib/parsers/CPPCheckParser.java

@@ -3,9 +3,9 @@
 import static se.bjurr.violations.lib.model.SEVERITY.ERROR;
 import static se.bjurr.violations.lib.model.SEVERITY.INFO;
 import static se.bjurr.violations.lib.model.SEVERITY.WARN;
-import static se.bjurr.violations.lib.model.Violation.violationBuilder;
 import static se.bjurr.violations.lib.reports.Parser.CPPCHECK;
 import static se.bjurr.violations.lib.util.ViolationParserUtils.findAttribute;
+import static se.bjurr.violations.lib.util.ViolationParserUtils.findIntegerAttribute;
 import static se.bjurr.violations.lib.util.ViolationParserUtils.getAttribute;
 import static se.bjurr.violations.lib.util.ViolationParserUtils.getChunks;
 import static se.bjurr.violations.lib.util.ViolationParserUtils.getIntegerAttribute;
@@ -26,38 +26,73 @@ public List<Violation> parseReportOutput(final String string) throws Exception {
       final String errorChunk = errorChunks.get(errorIndex);
       final String severity = getAttribute(errorChunk, "severity");
       final String msg = getAttribute(errorChunk, "msg");
-      final String verbose = getAttribute(errorChunk, "verbose");
+      final Optional<String> verbose = findAttribute(errorChunk, "verbose");
       final String id = getAttribute(errorChunk, "id");
       final List<String> locationChunks = getChunks(errorChunk, "<location", "/>");
+
       for (final String locationChunk : locationChunks) {
         final Integer line = getIntegerAttribute(locationChunk, "line");
         final Optional<String> info = findAttribute(locationChunk, "info");
         final String fileString = getAttribute(errorChunk, "file");
-        String message = "";
-        if (verbose.startsWith(msg)) {
-          message = verbose;
-        } else {
-          message = msg + ". " + verbose;
-        }
-        if (info.isPresent() && !message.contains(info.get())) {
-          message = message + ". " + info.get();
-        }
+        final String message = this.constructMessage(msg, verbose, info);
         violations.add( //
-            violationBuilder() //
+            Violation.violationBuilder() //
                 .setParser(CPPCHECK) //
                 .setStartLine(line) //
                 .setFile(fileString) //
-                .setSeverity(toSeverity(severity)) //
+                .setSeverity(this.toSeverity(severity)) //
+                .setMessage(message) //
+                .setRule(id) //
+                .setGroup(Integer.toString(errorIndex)) //
+                .build() //
+            );
+      }
+    }
+
+    final List<String> errorChunksNoEndtag = getChunks(string, "<error", "\\/>");
+    for (int errorIndex = 0; errorIndex < errorChunksNoEndtag.size(); errorIndex++) {
+      final String errorChunk = errorChunksNoEndtag.get(errorIndex);
+      final String severity = getAttribute(errorChunk, "severity");
+      final String msg = getAttribute(errorChunk, "msg");
+      final Optional<String> verbose = findAttribute(errorChunk, "verbose");
+      final String id = getAttribute(errorChunk, "id");
+      final Optional<Integer> resultLine = findIntegerAttribute(errorChunk, "line");
+      final Optional<String> resultFile = findAttribute(errorChunk, "file");
+      final Optional<String> resultInfo = findAttribute(errorChunk, "info");
+
+      if (resultLine.isPresent() && resultFile.isPresent()) {
+        final String message = this.constructMessage(msg, verbose, resultInfo);
+        violations.add( //
+            Violation.violationBuilder() //
+                .setParser(CPPCHECK) //
+                .setStartLine(resultLine.get()) //
+                .setFile(resultFile.get()) //
+                .setSeverity(this.toSeverity(severity)) //
                 .setMessage(message) //
                 .setRule(id) //
                 .setGroup(Integer.toString(errorIndex)) //
                 .build() //
             );
       }
     }
+
     return violations;
   }
 
+  private String constructMessage(
+      final String msg, final Optional<String> verbose, final Optional<String> info) {
+    String message = "";
+    if (verbose.orElse("").startsWith(msg)) {
+      message = verbose.get();
+    } else {
+      message = msg + ". " + verbose.orElse("");
+    }
+    if (info.isPresent() && !message.contains(info.get())) {
+      message = message + ". " + info.get();
+    }
+    return message;
+  }
+
   public SEVERITY toSeverity(final String severity) {
     if (severity.equalsIgnoreCase("error")) {
       return ERROR;

src/test/java/se/bjurr/violations/lib/CPPCheckTest.java

@@ -136,4 +136,20 @@ public void testThatViolationsCanBeParsedByRule() {
     assertThat(secondErrorTag.get(0).getMessage()) //
         .isEqualTo("Condition 'rc' is always true");
   }
+
+  @Test
+  public void testThatViolationsCanBeParsedWhenErrorTagHasNoEndtag() {
+    final String rootFolder = getRootFolder();
+
+    final List<Violation> actual =
+        violationsApi() //
+            .withPattern(".*/cppcheck/error_without_endtag\\.xml$") //
+            .inFolder(rootFolder) //
+            .findAll(CPPCHECK) //
+            .violations();
+
+    final Violation violation0 = actual.get(0);
+    assertThat(violation0.getMessage()) //
+        .startsWith("The scope of the variable");
+  }
 }

src/test/resources/cppcheck/error_without_endtag.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<results>
+    <error file="filename.cpp" line="49" id="variableScope" severity="style" msg="The scope of the variable &apos;softwareComponentsAreOk&apos; can be reduced. Warning: Be careful when fixing this message, especially when there are inner loops. Here is an example where cppcheck will write that the scope for &apos;i&apos; can be reduced:
+void f(int x)
+{
+    int i = 0;
+    if (x) {
+        // it&apos;s safe to move &apos;int i = 0;&apos; here
+        for (int n = 0; n &lt; 10; ++n) {
+            // it is possible but not safe to move &apos;int i = 0;&apos; here
+            do_something(&amp;i);
+        }
+    }
+}
+When you see this message it is always safe to reduce the variable scope 1 level."/>
+</results>