Skip to content

Commit

Permalink
Merge pull request #1461 from ivangalkin/valgrindstacks
Browse files Browse the repository at this point in the history 
CxxValgrindSensor: allow multiple <stack> nodes #1440
  • Loading branch information
@guwirth
guwirth authored Apr 20, 2018
2 parents 14089b8 + 663e698 commit 2eeb8ca
Show file tree
Hide file tree
Showing 11 changed files with 502 additions and 100 deletions.
26 changes: 20 additions & 6 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down
67 changes: 47 additions & 20 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindError.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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;
}

}
68 changes: 33 additions & 35 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindFrame.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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() {
Expand Down
19 changes: 15 additions & 4 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindReportParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand All @@ -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);
}

/**
Expand Down
25 changes: 13 additions & 12 deletions cxx-sensors/src/main/java/org/sonar/cxx/sensors/valgrind/ValgrindStack.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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();
}

/**
Expand Down
12 changes: 8 additions & 4 deletions cxx-sensors/src/test/java/org/sonar/cxx/sensors/valgrind/CxxValgrindSensorTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit 2eeb8ca

Please sign in to comment.