CxxValgrindSensor: allow multiple <stack> nodes #1440

* proper parsing of multiple <stack>s and <auxwhat>s
see https://github.com/pathscale/valgrind-mmt/blob/master/docs/internals/xml-output.txt

Each stackstrace of Valgrind will be stored as a separate NewIssue.
The reason is missing formatting for error messages
(even line breaks are not allowed).
Error messages with one stack trace are already hard to read.

* revisit Valgrind*::equals() and Valgrind*::hashCode() implementation
a. equals() shouldn not be implemented through hashCode() because
   there could be hash collisions and unequal objects could return
   true
b. perform equals()/hashCode() over all fields
   I believe that SonarQube has to show the Valgrind's report as-is.
   There should be no filtering or obfuscation

cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java

@@ -68,14 +68,28 @@ protected void processReport(final SensorContext context, File report)
     saveErrors(context, parser.processReport(report));
   }
 
+  private static String createErrorMsg(ValgrindError error, ValgrindStack stack, Integer stackNr) {
+    StringBuilder errorMsg = new StringBuilder();
+    errorMsg.append(error.getText());
+    if (error.getStacks().size() > 1) {
+      errorMsg.append(" (Stack ").append(stackNr).append(")");
+    }
+    errorMsg.append("\n\n").append(stack);
+    return errorMsg.toString();
+  }
+
   void saveErrors(SensorContext context, Set<ValgrindError> valgrindErrors) {
     for (ValgrindError error : valgrindErrors) {
-      ValgrindFrame frame = error.getLastOwnFrame(context.fileSystem().baseDir().getPath());
-      if (frame != null) {
-        saveUniqueViolation(context, CxxValgrindRuleRepository.KEY,
-          frame.getPath(), frame.getLine(), error.getKind(), error.toString());
-      } else {
-        LOG.warn("Cannot find a project file to assign the valgrind error '{}' to", error);
+      Integer stackNr = 0;
+      for (ValgrindStack stack : error.getStacks()) {
+        ValgrindFrame frame = stack.getLastOwnFrame(context.fileSystem().baseDir().getPath());
+        if (frame != null) {
+          String errorMsg = createErrorMsg(error, stack, stackNr);
+          saveUniqueViolation(context, CxxValgrindRuleRepository.KEY, frame.getPath(), frame.getLine(), error.getKind(), errorMsg);
+        } else {
+          LOG.warn("Cannot find a project file to assign the valgrind error '{}' to", error);
+        }
+        ++stackNr;
       }
     }
   }

cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindError.java

@@ -19,6 +19,9 @@
  */
 package org.sonar.cxx.sensors.valgrind;
 
+import java.util.List;
+
+import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 
 /**
@@ -29,54 +32,78 @@ class ValgrindError {
 
   private final String kind;
   private final String text;
-  private final ValgrindStack stack;
+  private final List<ValgrindStack> stacks;
 
   /**
    * Constructs a ValgrindError out of the given attributes
    *
    * @param kind The kind of error, plays the role of an id
    * @param text Description of the error
-   * @param stack The associated call stack
+   * @param stacks One or more associated call stacks
    */
-  public ValgrindError(String kind, String text, ValgrindStack stack) {
+  public ValgrindError(String kind, String text, List<ValgrindStack> stacks) {
     this.kind = kind;
     this.text = text;
-    this.stack = stack;
+    this.stacks = stacks;
   }
 
+
+  /**
+   * For debug prints only; SonarQube cannot deal with long/multi-line error
+   * messages. Formats like Markdown or HTML are not supported.
+   *
+   * For the sake of readability each ValgrindStack will be saved as a separate NewIssue.
+   *
+   * See <a href=
+   * "http://javadocs.sonarsource.org/7.0/apidocs/org/sonar/api/batch/sensor/issue/NewIssueLocation.html#message-java.lang.String-">NewIssueLocation::message()</a>
+   */
   @Override
   public String toString() {
-    return text + "\n\n" + stack;
+    StringBuilder sb = new StringBuilder();
+    sb.append("ValgrindError [kind=").append(kind).append(", text=").append(text).append(", stacks=[");
+    for (ValgrindStack stack : stacks) {
+      sb.append(" ValgrindStack=[").append(stack).append("] ");
+    }
+    sb.append("] ]");
+    return sb.toString();
   }
 
+
   @Override
-  public boolean equals(Object o) {
-    if (this == o) {
+  public boolean equals(Object obj) {
+    if (this == obj)
       return true;
-    }
-    if (o == null || getClass() != o.getClass()) {
+    if (obj == null)
       return false;
-    }
-    ValgrindError other = (ValgrindError) o;
-    return hashCode() == other.hashCode();
+    if (getClass() != obj.getClass())
+      return false;
+    ValgrindError other = (ValgrindError) obj;
+    return new EqualsBuilder()
+        .append(kind, other.kind)
+        .append(text, other.text)
+        .append(stacks, other.stacks)
+        .isEquals();
   }
 
   @Override
   public int hashCode() {
     return new HashCodeBuilder()
-      .append(kind)
-      .append(stack)
-      .toHashCode();
+        .append(kind)
+        .append(text)
+        .append(stacks)
+        .toHashCode();
   }
 
   String getKind() {
     return this.kind;
   }
 
-  /**
-   * @see ValgrindStack#getLastFrame
-   */
-  public ValgrindFrame getLastOwnFrame(String basedir) {
-    return stack.getLastOwnFrame(basedir);
+  public String getText() {
+    return text;
+  }
+
+  public List<ValgrindStack> getStacks() {
+    return stacks;
   }
+
 }

cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindFrame.java

@@ -22,45 +22,35 @@
 import java.io.File;
 import javax.annotation.Nullable;
 import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
+import org.apache.commons.lang.ObjectUtils;
 
 /**
  * Represents a stack frame. Overwrites equality. Has a string serialization
  * that resembles the valgrind output in textual mode.
  */
 class ValgrindFrame {
 
-  private String ip = "???";
-  private String obj = "";
-  private String fn = "???";
-  private String dir = "";
-  private String file = "";
-  private String line = "";
+  private String ip;
+  private String obj;
+  private String fn;
+  private String dir;
+  private String file;
+  private String line;
 
   /**
    * Constructs a stack frame with given attributes. Its perfectly valid if some
    * of them are empty or don't carry meaningful information.
    */
   public ValgrindFrame(@Nullable String ip, @Nullable String obj, @Nullable String fn, @Nullable String dir,
     @Nullable String file, @Nullable String line) {
-    if (ip != null) {
-      this.ip = ip;
-    }
-    if (obj != null) {
-      this.obj = obj;
-    }
-    if (fn != null) {
-      this.fn = fn;
-    }
-    if (dir != null) {
-      this.dir = FilenameUtils.normalize(dir);
-    }
-    if (file != null) {
-      this.file = file;
-    }
-    if (line != null) {
-      this.line = line;
-    }
+    this.ip = (ip != null) ? ip : "???";
+    this.obj = (obj != null) ? obj : "";
+    this.fn = (fn != null) ? fn : "???";
+    this.dir = (dir != null) ? FilenameUtils.normalize(dir) : "";
+    this.file = (file != null) ? file : "";
+    this.line = (line != null) ? line : "";
   }
 
   @Override
@@ -77,25 +67,33 @@ public String toString() {
 
   @Override
   public boolean equals(Object o) {
-    if (this == o) {
+    if (this == o)
       return true;
-    }
-    if (o == null || getClass() != o.getClass()) {
+    if (o == null)
+      return false;
+    if (getClass() != o.getClass())
       return false;
-    }
     ValgrindFrame other = (ValgrindFrame) o;
-    return hashCode() == other.hashCode();
+    return new EqualsBuilder()
+        .append(ip, other.ip)
+        .append(obj, other.obj)
+        .append(fn, other.fn)
+        .append(dir, other.dir)
+        .append(file, other.file)
+        .append(line, other.line)
+        .isEquals();
   }
 
   @Override
   public int hashCode() {
     return new HashCodeBuilder()
-      .append(obj)
-      .append(fn)
-      .append(dir)
-      .append(file)
-      .append(line)
-      .toHashCode();
+        .append(ip)
+        .append(obj)
+        .append(fn)
+        .append(dir)
+        .append(file)
+        .append(line)
+        .toHashCode();
   }
 
   String getPath() {

cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindReportParser.java

@@ -20,8 +20,12 @@
 package org.sonar.cxx.sensors.valgrind;
 
 import java.io.File;
+import java.util.ArrayList;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
+import java.util.Stack;
+
 import javax.xml.stream.XMLStreamException;
 import org.codehaus.staxmate.in.SMHierarchicCursor;
 import org.codehaus.staxmate.in.SMInputCursor;
@@ -93,7 +97,8 @@ private static ValgrindError parseErrorTag(SMInputCursor error) throws XMLStream
 
       String kind = null;
       String text = null;
-      ValgrindStack stack = null;
+      List<String> details = new ArrayList<>();
+      List<ValgrindStack> stacks = new ArrayList<>();
       while (child.getNext() != null) {
         String tagName = child.getLocalName();
         if ("kind".equalsIgnoreCase(tagName)) {
@@ -102,17 +107,23 @@ private static ValgrindError parseErrorTag(SMInputCursor error) throws XMLStream
           text = child.childElementCursor("text").advance().getElemStringValue();
         } else if ("what".equalsIgnoreCase(tagName)) {
           text = child.getElemStringValue();
+        } else if ("auxwhat".equalsIgnoreCase(tagName)) {
+          details.add(child.getElemStringValue());
         } else if ("stack".equalsIgnoreCase(tagName)) {
-          stack = parseStackTag(child);
+          stacks.add(parseStackTag(child));
         }
       }
 
-      if (text == null || kind == null || stack == null) {
+      if (text == null || kind == null || stacks.isEmpty()) {
         String msg = "Valgrind error is incomplete: we require all of 'kind', '*what.text' and 'stack'";
         child.throwStreamException(msg);
       }
 
-      return new ValgrindError(kind, text, stack);
+      if (!details.isEmpty()) {
+        text = text + ": " + String.join("; ", details);
+      }
+
+      return new ValgrindError(kind, text, stacks);
     }
 
     /**

cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindStack.java

@@ -23,6 +23,7 @@
 import java.util.List;
 import javax.annotation.Nullable;
 import org.apache.commons.io.FilenameUtils;
+import org.apache.commons.lang.builder.EqualsBuilder;
 import org.apache.commons.lang.builder.HashCodeBuilder;
 
 /**
@@ -53,23 +54,23 @@ public String toString() {
 
   @Override
   public int hashCode() {
-    HashCodeBuilder builder = new HashCodeBuilder();
-    for (ValgrindFrame frame : frames) {
-      builder.append(frame);
-    }
-    return builder.toHashCode();
+    return new HashCodeBuilder()
+        .append(frames)
+        .toHashCode();
   }
 
   @Override
-  public boolean equals(Object o) {
-    if (this == o) {
+  public boolean equals(Object obj) {
+    if (this == obj)
       return true;
-    }
-    if (o == null || getClass() != o.getClass()) {
+    if (obj == null)
       return false;
-    }
-    ValgrindStack other = (ValgrindStack) o;
-    return hashCode() == other.hashCode();
+    if (getClass() != obj.getClass())
+      return false;
+    ValgrindStack other = (ValgrindStack) obj;
+    return new EqualsBuilder()
+        .append(frames, other.frames)
+        .isEquals();
   }
 
   /**

cxx-sensors/src/test/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensorTest.java

@@ -19,6 +19,7 @@
  */
 package org.sonar.cxx.sensors.valgrind;
 
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -81,18 +82,21 @@ public void sensorDescriptor() {
     DefaultSensorDescriptor descriptor = new DefaultSensorDescriptor();
     sensor.describe(descriptor);
 
-    SoftAssertions softly = new SoftAssertions(); 
+    SoftAssertions softly = new SoftAssertions();
     softly.assertThat(descriptor.name()).isEqualTo(language.getName() + " ValgrindSensor");
     softly.assertThat(descriptor.languages()).containsOnly(language.getKey());
     softly.assertThat(descriptor.ruleRepositories()).containsOnly(CxxValgrindRuleRepository.KEY);
     softly.assertAll();
   }
-  
+
   private ValgrindError mockValgrindError(boolean inside) {
+    ValgrindStack stack = mock(ValgrindStack.class);
+    ValgrindFrame frame = inside ? generateValgrindFrame() : null;
+    when(stack.getLastOwnFrame(anyString())).thenReturn(frame);
+
     ValgrindError error = mock(ValgrindError.class);
     when(error.getKind()).thenReturn("valgrind-error");
-    ValgrindFrame frame = inside == true ? generateValgrindFrame() : null;
-    when(error.getLastOwnFrame((anyString()))).thenReturn(frame);
+    when(error.getStacks()).thenReturn(Collections.singletonList(stack));
     return error;
   }