Skip to content

Commit

Permalink
Respecting source directory in CodeNarc #53
Browse files Browse the repository at this point in the history
  • Loading branch information
@tomasbjerre
tomasbjerre committed Jan 8, 2019
1 parent 606fb72 commit 2fbb203
Show file tree
Hide file tree
Showing 3 changed files with 1,532 additions and 28 deletions.
68 changes: 48 additions & 20 deletions src/main/java/se/bjurr/violations/lib/parsers/CodeNarcParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import static se.bjurr.violations.lib.util.ViolationParserUtils.getIntegerAttribute;

import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import javax.xml.stream.XMLInputFactory;
import javax.xml.stream.XMLStreamException;
import javax.xml.stream.XMLStreamReader;
import se.bjurr.violations.lib.model.SEVERITY;
import se.bjurr.violations.lib.model.Violation;
Expand All @@ -35,25 +37,10 @@ private SEVERITY getSeverity(Integer from) {
@Override
public List<Violation> parseReportOutput(String string) throws Exception {
final List<Violation> violations = new ArrayList<>();
final Map<String, String> rules = new HashMap<>();
try (InputStream input = new ByteArrayInputStream(string.getBytes("UTF-8"))) {
final XMLInputFactory factory = XMLInputFactory.newInstance();
final XMLStreamReader xmlr = factory.createXMLStreamReader(input);
String name = null;
String description = null;
while (xmlr.hasNext()) {
final int eventType = xmlr.next();
if (eventType == START_ELEMENT) {
if (xmlr.getLocalName().equalsIgnoreCase("Rule")) {
name = getAttribute(xmlr, "name");
}
if (xmlr.getLocalName().equalsIgnoreCase("Description")) {
description = xmlr.getElementText().trim();
rules.put(name, description);
}
}
}
}

final Map<String, String> rules = getRules(string);

final String sourceDirectory = getSourceDirectory(string);

try (InputStream input = new ByteArrayInputStream(string.getBytes("UTF-8"))) {

Expand Down Expand Up @@ -89,7 +76,7 @@ public List<Violation> parseReportOutput(String string) throws Exception {
final Violation violation =
violationBuilder() //
.setParser(CODENARC) //
.setFile(path + "/" + name) //
.setFile((sourceDirectory + "/" + path + "/" + name).replace("//", "/")) //
.setMessage(message) //
.setRule(ruleName) //
.setSeverity(getSeverity(priority)) //
Expand All @@ -102,4 +89,45 @@ public List<Violation> parseReportOutput(String string) throws Exception {
}
return violations;
}

private String getSourceDirectory(final String string) throws Exception {
try (InputStream input = new ByteArrayInputStream(string.getBytes("UTF-8"))) {
final XMLInputFactory factory = XMLInputFactory.newInstance();
final XMLStreamReader xmlr = factory.createXMLStreamReader(input);
String name = null;
String description = null;
while (xmlr.hasNext()) {
final int eventType = xmlr.next();
if (eventType == START_ELEMENT) {
if (xmlr.getLocalName().equalsIgnoreCase("SourceDirectory")) {
return xmlr.getElementText().trim();
}
}
}
}
return "";
}

private Map<String, String> getRules(final String string) throws IOException, XMLStreamException {
final Map<String, String> rules = new HashMap<>();
try (InputStream input = new ByteArrayInputStream(string.getBytes("UTF-8"))) {
final XMLInputFactory factory = XMLInputFactory.newInstance();
final XMLStreamReader xmlr = factory.createXMLStreamReader(input);
String name = null;
String description = null;
while (xmlr.hasNext()) {
final int eventType = xmlr.next();
if (eventType == START_ELEMENT) {
if (xmlr.getLocalName().equalsIgnoreCase("Rule")) {
name = getAttribute(xmlr, "name");
}
if (xmlr.getLocalName().equalsIgnoreCase("Description")) {
description = xmlr.getElementText().trim();
rules.put(name, description);
}
}
}
}
return rules;
}
}
48 changes: 40 additions & 8 deletions src/test/java/se/bjurr/violations/lib/CodeNarcTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static org.assertj.core.api.Assertions.assertThat;
import static se.bjurr.violations.lib.TestUtils.getRootFolder;
import static se.bjurr.violations.lib.ViolationsApi.violationsApi;
import static se.bjurr.violations.lib.model.SEVERITY.INFO;
import static se.bjurr.violations.lib.model.SEVERITY.WARN;
import static se.bjurr.violations.lib.reports.Parser.CODENARC;

Expand All @@ -18,29 +19,60 @@ public void testThatViolationsCanBeParsed() {

final List<Violation> actual =
violationsApi() //
.withPattern(".*/codenarc/.*\\.xml$") //
.withPattern(".*/codenarc/CodeNarc.*\\.xml$") //
.inFolder(rootFolder) //
.findAll(CODENARC) //
.violations();

assertThat(actual) //
.hasSize(32);

assertThat(actual.get(0).getMessage()) //
final Violation violation0 = actual.get(0);
assertThat(violation0.getMessage()) //
.isEqualTo("In most cases, exceptions should not be caught and ignored (swallowed).");
assertThat(actual.get(0).getFile()) //
.isEqualTo("foo/bar/Test.groovy");
assertThat(actual.get(0).getSeverity()) //
assertThat(violation0.getFile()) //
.isEqualTo("/foo/bar/Test.groovy");
assertThat(violation0.getSeverity()) //
.isEqualTo(WARN);
assertThat(actual.get(0).getRule()) //
assertThat(violation0.getRule()) //
.isEqualTo("EmptyCatchBlock");
assertThat(actual.get(0).getStartLine()) //
assertThat(violation0.getStartLine()) //
.isEqualTo(192);
assertThat(actual.get(0).getEndLine()) //
assertThat(violation0.getEndLine()) //
.isEqualTo(192);

assertThat(actual.get(2).getMessage()) //
.isEqualTo(
"Catching Exception is often too broad or general. It should usually be restricted to framework or infrastructure code, rather than application code.");
}

@Test
public void testThatViolationsCanBeParsed2() {
final String rootFolder = getRootFolder();

final List<Violation> actual =
violationsApi() //
.withPattern(".*/codenarc/SampleCodeNarc.*\\.xml$") //
.inFolder(rootFolder) //
.findAll(CODENARC) //
.violations();

assertThat(actual) //
.hasSize(79);

final Violation violation0 = actual.get(0);
assertThat(violation0.getMessage()) //
.isEqualTo(
"Violations are triggered when an excessive set of consecutive statements all reference the same variable. This can be made more readable by using a with or identity block.");
assertThat(violation0.getFile()) //
.isEqualTo("src/test/groovy/org/codenarc/rule/AbstractAstVisitorRuleTest.groovy");
assertThat(violation0.getSeverity()) //
.isEqualTo(INFO);
assertThat(violation0.getRule()) //
.isEqualTo("UnnecessaryObjectReferences");
assertThat(violation0.getStartLine()) //
.isEqualTo(184);
assertThat(violation0.getEndLine()) //
.isEqualTo(184);
}
}
Loading

0 comments on commit 2fbb203

Please sign in to comment.