From 642c24568246c482e0e4a1bbcbce57becfbc67e9 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Tue, 10 Oct 2017 18:14:39 -0300 Subject: [PATCH 01/12] Improve CC Issue representation --- build.gradle | 1 + src/main/java/cc/Config.java | 3 - src/main/java/cc/JsonReport.java | 65 +++------------ src/main/java/cc/models/CodeClimateIssue.java | 82 +++++++++++++++++++ .../java/cc/serialization/GsonFactory.java | 13 +++ .../java/integration/SanityCheckTest.java | 25 +++--- 6 files changed, 123 insertions(+), 66 deletions(-) create mode 100644 src/main/java/cc/models/CodeClimateIssue.java create mode 100644 src/main/java/cc/serialization/GsonFactory.java diff --git a/build.gradle b/build.gradle index 824f0fe..073e92e 100644 --- a/build.gradle +++ b/build.gradle @@ -42,6 +42,7 @@ dependencies { compile("org.sonarsource.java:sonar-java-plugin:4.3.0.7717") testCompile("org.assertj:assertj-core:2.8.0") + testCompile("org.skyscreamer:jsonassert:1.5.0") testCompile("junit:junit:4.12") } diff --git a/src/main/java/cc/Config.java b/src/main/java/cc/Config.java index 5b9a48d..1eb2635 100644 --- a/src/main/java/cc/Config.java +++ b/src/main/java/cc/Config.java @@ -5,10 +5,7 @@ import com.google.gson.GsonBuilder; import com.google.gson.stream.JsonReader; -import java.io.File; -import java.io.FileNotFoundException; import java.io.FileReader; -import java.util.ArrayList; import java.util.Arrays; import java.util.List; diff --git a/src/main/java/cc/JsonReport.java b/src/main/java/cc/JsonReport.java index 6600db3..bb6fd54 100644 --- a/src/main/java/cc/JsonReport.java +++ b/src/main/java/cc/JsonReport.java @@ -1,14 +1,15 @@ package cc; -import com.google.gson.JsonObject; -import com.google.gson.JsonArray; -import java.io.PrintStream; +import cc.models.CodeClimateIssue; +import cc.serialization.GsonFactory; +import com.google.gson.Gson; + import java.util.Collection; import java.util.Date; import java.util.List; import java.util.Arrays; import java.util.function.Function; -import org.sonarlint.cli.util.Logger; + import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; import org.sonarsource.sonarlint.core.client.api.common.analysis.AnalysisResults; import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; @@ -21,6 +22,11 @@ public class JsonReport implements org.sonarlint.cli.report.Reporter { "VULNERABILITY" ); + final Gson gson; + + public JsonReport() { + this.gson = new GsonFactory().create(); + } @Override public void execute(String projectName, Date date, Collection trackables, AnalysisResults result, Function ruleDescriptionProducer) { @@ -29,55 +35,8 @@ public void execute(String projectName, Date date, Collection trackab RuleDetails ruleDetails = ruleDescriptionProducer.apply(issue.getRuleKey()); if (VALID_RULE_DETAIL_TYPES.contains(ruleDetails.getType())) { - JsonObject json = new JsonObject(); - json.addProperty("type", "issue"); - json.addProperty("check_name", issue.getRuleKey()); - json.addProperty("severity", ruleDetails.getSeverity().toLowerCase()); - json.addProperty("description", issue.getMessage()); - - JsonObject content = new JsonObject(); - json.add("content", content); - content.addProperty("body", ruleDetails.getHtmlDescription()); - // // ruleDetails.getExtendedDescription(); - - JsonObject location = new JsonObject(); - json.add("location", location); - - // Code Climate CLI expects relative path to file - location.addProperty("path", issue.getInputFile().getPath().replaceFirst("^/tmp/code/", "")); - - JsonObject lines = new JsonObject(); - location.add("lines", lines); - - if (issue.getStartLine() != null) { - lines.addProperty("begin", issue.getStartLine()); - - if (issue.getEndLine() != null) { - lines.addProperty("end", issue.getEndLine()); - } else { - lines.addProperty("end", 1); - } - } else { - lines.addProperty("begin", 1); - lines.addProperty("end", 1); - } - - String category; - switch (ruleDetails.getType()) { - case "VULNERABILITY": { - category = "Security"; - break; - } - default: { - category = "Bug Risk"; - break; - } - } - JsonArray categories = new JsonArray(); - categories.add(category); - json.add("categories", categories); - - System.out.println(json.toString() + "\0"); + CodeClimateIssue codeClimateIssue = new CodeClimateIssue(issue, ruleDetails); + System.out.println(gson.toJson(codeClimateIssue) + "\0"); } } } diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java new file mode 100644 index 0000000..561e9f5 --- /dev/null +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -0,0 +1,82 @@ +package cc.models; + +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; +import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; + +import java.util.ArrayList; + +public class CodeClimateIssue { + final String type; + final String checkName; + final String severity; + final String description; + final Content content; + final Location location; + final Categories categories; + + public CodeClimateIssue(Issue issue, RuleDetails ruleDetails) { + this.type = "issue"; + this.checkName = issue.getRuleKey(); + this.description = issue.getMessage(); + this.severity = ruleDetails.getSeverity().toLowerCase(); + this.content = new Content(ruleDetails.getHtmlDescription()); + this.location = new Location(issue); + this.categories = new Categories(ruleDetails); + } + + class Content { + String body; + + public Content(String body) { + this.body = body; + } + } + + class Location { + final String path; + final Lines lines; + + public Location(Issue issue) { + this.path = issue.getInputFile().getPath().replaceFirst("^/tmp/code/", ""); + this.lines = new Lines(issue); + } + } + + class Lines { + final Integer begin; + final Integer end; + + public Lines(Issue issue) { + if (issue.getStartLine() != null) { + this.begin = issue.getStartLine(); + + if (issue.getEndLine() != null) { + this.end = issue.getEndLine(); + } else { + this.end = 1; + } + } else { + this.begin = 1; + this.end = 1; + } + + } + } + + class Categories extends ArrayList { + public Categories(RuleDetails ruleDetails) { + String category; + switch (ruleDetails.getType()) { + case "VULNERABILITY": { + category = "Security"; + break; + } + default: { + category = "Bug Risk"; + break; + } + } + add(category); + } + } +} diff --git a/src/main/java/cc/serialization/GsonFactory.java b/src/main/java/cc/serialization/GsonFactory.java new file mode 100644 index 0000000..ad30d57 --- /dev/null +++ b/src/main/java/cc/serialization/GsonFactory.java @@ -0,0 +1,13 @@ +package cc.serialization; + +import com.google.gson.FieldNamingPolicy; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; + +public class GsonFactory { + public Gson create() { + return new GsonBuilder() + .setFieldNamingPolicy(FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES) + .create(); + } +} diff --git a/src/test/java/integration/SanityCheckTest.java b/src/test/java/integration/SanityCheckTest.java index 62d979a..6d629a4 100644 --- a/src/test/java/integration/SanityCheckTest.java +++ b/src/test/java/integration/SanityCheckTest.java @@ -1,18 +1,19 @@ package integration; +import org.assertj.core.util.Strings; import org.junit.Test; +import org.skyscreamer.jsonassert.JSONAssert; import support.Shell; -import java.util.Arrays; -import java.util.List; - import static org.assertj.core.api.Assertions.assertThat; public class SanityCheckTest { @Test public void executeJavaLibFixture() throws Exception { - List expectedOutput = Arrays.asList("{\"type\":\"issue\"," + + String expectedOutput = + "[" + + "{\"type\":\"issue\"," + "\"check_name\":\"squid:S1994\"," + "\"severity\":\"major\"," + "\"description\":\"This loop's stop condition tests \\\"k\\\" but the incrementer updates \\\"i\\\".\"," + @@ -22,8 +23,8 @@ public void executeJavaLibFixture() throws Exception { "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + "\"lines\":{\"begin\":17," + "\"end\":17}}," + - "\"categories\":[\"Bug Risk\"]}\u0000", - "{\"type\":\"issue\"," + + "\"categories\":[\"Bug Risk\"]}" + + ",{\"type\":\"issue\"," + "\"check_name\":\"squid:S1994\"," + "\"severity\":\"major\"," + "\"description\":\"This loop's stop condition tests \\\"k\\\" but the incrementer updates \\\"i\\\".\"," + @@ -33,8 +34,8 @@ public void executeJavaLibFixture() throws Exception { "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + "\"lines\":{\"begin\":20," + "\"end\":20}}," + - "\"categories\":[\"Bug Risk\"]}\u0000", - "{\"type\":\"issue\"," + + "\"categories\":[\"Bug Risk\"]}" + + ",{\"type\":\"issue\"," + "\"check_name\":\"squid:S1994\"," + "\"severity\":\"major\"," + "\"description\":\"This loop's stop condition tests \\\"k\\\" but the incrementer updates \\\"i\\\".\"," + @@ -44,11 +45,15 @@ public void executeJavaLibFixture() throws Exception { "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + "\"lines\":{\"begin\":23," + "\"end\":23}}," + - "\"categories\":[\"Bug Risk\"]}\u0000"); + "\"categories\":[\"Bug Risk\"]}" + + "]"; Shell.Process process = Shell.execute("build/codeclimate-sonar fixtures/java_lib"); assertThat(process.exitCode).isEqualTo(0); - assertThat(process.stdout).contains(expectedOutput); + assertThat(process.stdout).contains("\u0000"); + + String stdoutAsJson = "[" + Strings.join(process.stdout.split("\u0000")).with(",") + "]"; + JSONAssert.assertEquals(stdoutAsJson, expectedOutput, false); } } From 670817b28b3e3bc760d4e35cbbfa162376e90046 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 09:57:23 -0300 Subject: [PATCH 02/12] Break subclasses into their own files --- src/main/java/cc/models/Categories.java | 20 +++++ src/main/java/cc/models/CodeClimateIssue.java | 86 ++++--------------- src/main/java/cc/models/Content.java | 9 ++ src/main/java/cc/models/Lines.java | 22 +++++ src/main/java/cc/models/Location.java | 11 +++ 5 files changed, 80 insertions(+), 68 deletions(-) create mode 100644 src/main/java/cc/models/Categories.java create mode 100644 src/main/java/cc/models/Content.java create mode 100644 src/main/java/cc/models/Lines.java create mode 100644 src/main/java/cc/models/Location.java diff --git a/src/main/java/cc/models/Categories.java b/src/main/java/cc/models/Categories.java new file mode 100644 index 0000000..267bd38 --- /dev/null +++ b/src/main/java/cc/models/Categories.java @@ -0,0 +1,20 @@ +package cc.models; + +import java.util.ArrayList; + +class Categories extends ArrayList { + public Categories(String ruleType) { + String category; + switch (ruleType) { + case "VULNERABILITY": { + category = "Security"; + break; + } + default: { + category = "Bug Risk"; + break; + } + } + add(category); + } +} diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index 561e9f5..d5bb737 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -3,80 +3,30 @@ import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; -import java.util.ArrayList; - public class CodeClimateIssue { - final String type; - final String checkName; - final String severity; - final String description; - final Content content; - final Location location; - final Categories categories; + public final String type = "issue"; + public final String checkName; + public final String severity; + public final String description; + public final Content content; + public final Location location; + public final Categories categories; + + public CodeClimateIssue(String checkName, String severity, String description, Content content, Location location, Categories categories) { + this.checkName = checkName; + this.severity = severity; + this.description = description; + this.content = content; + this.location = location; + this.categories = categories; + } public CodeClimateIssue(Issue issue, RuleDetails ruleDetails) { - this.type = "issue"; this.checkName = issue.getRuleKey(); this.description = issue.getMessage(); this.severity = ruleDetails.getSeverity().toLowerCase(); this.content = new Content(ruleDetails.getHtmlDescription()); - this.location = new Location(issue); - this.categories = new Categories(ruleDetails); - } - - class Content { - String body; - - public Content(String body) { - this.body = body; - } - } - - class Location { - final String path; - final Lines lines; - - public Location(Issue issue) { - this.path = issue.getInputFile().getPath().replaceFirst("^/tmp/code/", ""); - this.lines = new Lines(issue); - } - } - - class Lines { - final Integer begin; - final Integer end; - - public Lines(Issue issue) { - if (issue.getStartLine() != null) { - this.begin = issue.getStartLine(); - - if (issue.getEndLine() != null) { - this.end = issue.getEndLine(); - } else { - this.end = 1; - } - } else { - this.begin = 1; - this.end = 1; - } - - } - } - - class Categories extends ArrayList { - public Categories(RuleDetails ruleDetails) { - String category; - switch (ruleDetails.getType()) { - case "VULNERABILITY": { - category = "Security"; - break; - } - default: { - category = "Bug Risk"; - break; - } - } - add(category); - } + this.location = new Location(issue.getInputFile().getPath(), issue.getStartLine(), issue.getEndLine()); + this.categories = new Categories(ruleDetails.getType()); } } diff --git a/src/main/java/cc/models/Content.java b/src/main/java/cc/models/Content.java new file mode 100644 index 0000000..56f58e3 --- /dev/null +++ b/src/main/java/cc/models/Content.java @@ -0,0 +1,9 @@ +package cc.models; + +class Content { + String body; + + public Content(String body) { + this.body = body; + } +} diff --git a/src/main/java/cc/models/Lines.java b/src/main/java/cc/models/Lines.java new file mode 100644 index 0000000..6462ec5 --- /dev/null +++ b/src/main/java/cc/models/Lines.java @@ -0,0 +1,22 @@ +package cc.models; + +class Lines { + final Integer begin; + final Integer end; + + public Lines(Integer startLine, Integer endLine) { + if (startLine != null) { + this.begin = startLine; + + if (endLine != null) { + this.end = endLine; + } else { + this.end = 1; + } + } else { + this.begin = 1; + this.end = 1; + } + + } +} diff --git a/src/main/java/cc/models/Location.java b/src/main/java/cc/models/Location.java new file mode 100644 index 0000000..946629c --- /dev/null +++ b/src/main/java/cc/models/Location.java @@ -0,0 +1,11 @@ +package cc.models; + +class Location { + final String path; + final Lines lines; + + public Location(String path, Integer startLine, Integer endLine) { + this.path = path.replaceFirst("^/tmp/code/", ""); + this.lines = new Lines(startLine, endLine); + } +} From 892bdb36cf1e8e903bb708b96f78250233aa9744 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 10:34:03 -0300 Subject: [PATCH 03/12] Validate Severity values --- src/main/java/cc/JsonReport.java | 2 +- src/main/java/cc/models/CodeClimateIssue.java | 19 +++++---- src/main/java/cc/models/Severity.java | 20 +++++++++ .../java/cc/models/CodeClimateIssueTest.java | 41 +++++++++++++++++++ 4 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 src/main/java/cc/models/Severity.java create mode 100644 src/test/java/cc/models/CodeClimateIssueTest.java diff --git a/src/main/java/cc/JsonReport.java b/src/main/java/cc/JsonReport.java index bb6fd54..2e6cb1b 100644 --- a/src/main/java/cc/JsonReport.java +++ b/src/main/java/cc/JsonReport.java @@ -35,7 +35,7 @@ public void execute(String projectName, Date date, Collection trackab RuleDetails ruleDetails = ruleDescriptionProducer.apply(issue.getRuleKey()); if (VALID_RULE_DETAIL_TYPES.contains(ruleDetails.getType())) { - CodeClimateIssue codeClimateIssue = new CodeClimateIssue(issue, ruleDetails); + CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails); System.out.println(gson.toJson(codeClimateIssue) + "\0"); } } diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index d5bb737..862f4e8 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -6,13 +6,13 @@ public class CodeClimateIssue { public final String type = "issue"; public final String checkName; - public final String severity; + public final Severity severity; public final String description; public final Content content; public final Location location; public final Categories categories; - public CodeClimateIssue(String checkName, String severity, String description, Content content, Location location, Categories categories) { + public CodeClimateIssue(String checkName, Severity severity, String description, Content content, Location location, Categories categories) { this.checkName = checkName; this.severity = severity; this.description = description; @@ -21,12 +21,13 @@ public CodeClimateIssue(String checkName, String severity, String description, C this.categories = categories; } - public CodeClimateIssue(Issue issue, RuleDetails ruleDetails) { - this.checkName = issue.getRuleKey(); - this.description = issue.getMessage(); - this.severity = ruleDetails.getSeverity().toLowerCase(); - this.content = new Content(ruleDetails.getHtmlDescription()); - this.location = new Location(issue.getInputFile().getPath(), issue.getStartLine(), issue.getEndLine()); - this.categories = new Categories(ruleDetails.getType()); + public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails) { + String checkName = issue.getRuleKey(); + String description = issue.getMessage(); + Severity severity = Severity.from(ruleDetails.getSeverity()); + Content content = new Content(ruleDetails.getHtmlDescription()); + Location location = new Location(issue.getInputFile().getPath(), issue.getStartLine(), issue.getEndLine()); + Categories categories = new Categories(ruleDetails.getType()); + return new CodeClimateIssue(checkName, severity, description, content, location, categories); } } diff --git a/src/main/java/cc/models/Severity.java b/src/main/java/cc/models/Severity.java new file mode 100644 index 0000000..fc4cc7e --- /dev/null +++ b/src/main/java/cc/models/Severity.java @@ -0,0 +1,20 @@ +package cc.models; + +import com.google.gson.annotations.SerializedName; + +public enum Severity { + @SerializedName("major") + MAJOR, + @SerializedName("minor") + MINOR, + @SerializedName("info") + INFO, + @SerializedName("critical") + CRITICAL, + @SerializedName("blocker") + BLOCKER; + + public static Severity from(String value) { + return valueOf(value.toUpperCase()); + } +} diff --git a/src/test/java/cc/models/CodeClimateIssueTest.java b/src/test/java/cc/models/CodeClimateIssueTest.java new file mode 100644 index 0000000..c9f6adc --- /dev/null +++ b/src/test/java/cc/models/CodeClimateIssueTest.java @@ -0,0 +1,41 @@ +package cc.models; + +import cc.serialization.GsonFactory; +import com.google.gson.Gson; +import org.junit.Test; + +import static org.assertj.core.api.Assertions.assertThat; + +public class CodeClimateIssueTest { + + @Test(expected = IllegalArgumentException.class) + public void raise_error_on_invalid_severity() throws Exception { + createIssueForSeverity("unknown"); + } + + @Test + public void accepts_only_valid_severities() throws Exception { + assertThat(createIssueForSeverity("major").severity).isEqualTo(Severity.MAJOR); + assertThat(createIssueForSeverity("minor").severity).isEqualTo(Severity.MINOR); + assertThat(createIssueForSeverity("critical").severity).isEqualTo(Severity.CRITICAL); + assertThat(createIssueForSeverity("blocker").severity).isEqualTo(Severity.BLOCKER); + assertThat(createIssueForSeverity("info").severity).isEqualTo(Severity.INFO); + } + + @Test + public void properly_serialize_severity() throws Exception { + Gson gson = new GsonFactory().create(); + assertThat(gson.toJson(createIssueForSeverity("info"))).contains("\"severity\":\"info\""); + } + + private CodeClimateIssue createIssueForSeverity(String severity) { + return new CodeClimateIssue( + "check", + Severity.from(severity), + "desc", + new Content(""), + new Location("path", 0, 1), + new Categories("VULNERABILITY") + ); + } +} \ No newline at end of file From bbfb3c5b591c32273e7a38bc2ef51452cd3a4c83 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 16:29:33 -0300 Subject: [PATCH 04/12] Validate issue location --- src/main/java/cc/models/Categories.java | 6 ++ src/main/java/cc/models/CodeClimateIssue.java | 9 +- src/main/java/cc/models/Content.java | 6 ++ src/main/java/cc/models/Location.java | 14 +++ src/main/java/cc/models/Severity.java | 9 +- .../java/cc/models/CodeClimateIssueTest.java | 3 +- src/test/java/support/FakeRuleDetails.java | 54 ++++++++++++ src/test/java/support/OutputHelper.java | 23 +++++ .../support/fakes/FakeClientInputFile.java | 48 ++++++++++ src/test/java/support/fakes/FakeIssue.java | 82 +++++++++++++++++ .../java/support/fakes/FakeTrackable.java | 87 +++++++++++++++++++ 11 files changed, 334 insertions(+), 7 deletions(-) create mode 100644 src/test/java/support/FakeRuleDetails.java create mode 100644 src/test/java/support/OutputHelper.java create mode 100644 src/test/java/support/fakes/FakeClientInputFile.java create mode 100644 src/test/java/support/fakes/FakeIssue.java create mode 100644 src/test/java/support/fakes/FakeTrackable.java diff --git a/src/main/java/cc/models/Categories.java b/src/main/java/cc/models/Categories.java index 267bd38..568e39d 100644 --- a/src/main/java/cc/models/Categories.java +++ b/src/main/java/cc/models/Categories.java @@ -1,5 +1,7 @@ package cc.models; +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; + import java.util.ArrayList; class Categories extends ArrayList { @@ -17,4 +19,8 @@ public Categories(String ruleType) { } add(category); } + + public static Categories from(RuleDetails ruleDetails) { + return new Categories(ruleDetails.getType()); + } } diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index 862f4e8..6b15c46 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -24,10 +24,11 @@ public CodeClimateIssue(String checkName, Severity severity, String description, public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails) { String checkName = issue.getRuleKey(); String description = issue.getMessage(); - Severity severity = Severity.from(ruleDetails.getSeverity()); - Content content = new Content(ruleDetails.getHtmlDescription()); - Location location = new Location(issue.getInputFile().getPath(), issue.getStartLine(), issue.getEndLine()); - Categories categories = new Categories(ruleDetails.getType()); + Severity severity = Severity.from(ruleDetails); + Content content = Content.from(ruleDetails); + Location location = Location.from(issue); + Categories categories = Categories.from(ruleDetails); return new CodeClimateIssue(checkName, severity, description, content, location, categories); } + } diff --git a/src/main/java/cc/models/Content.java b/src/main/java/cc/models/Content.java index 56f58e3..1840150 100644 --- a/src/main/java/cc/models/Content.java +++ b/src/main/java/cc/models/Content.java @@ -1,9 +1,15 @@ package cc.models; +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; + class Content { String body; public Content(String body) { this.body = body; } + + public static Content from(RuleDetails ruleDetails) { + return new Content(ruleDetails.getHtmlDescription()); + } } diff --git a/src/main/java/cc/models/Location.java b/src/main/java/cc/models/Location.java index 946629c..a9b0506 100644 --- a/src/main/java/cc/models/Location.java +++ b/src/main/java/cc/models/Location.java @@ -1,5 +1,8 @@ package cc.models; +import org.sonarsource.sonarlint.core.client.api.common.analysis.ClientInputFile; +import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; + class Location { final String path; final Lines lines; @@ -8,4 +11,15 @@ public Location(String path, Integer startLine, Integer endLine) { this.path = path.replaceFirst("^/tmp/code/", ""); this.lines = new Lines(startLine, endLine); } + + public static Location from(Issue issue) { + ClientInputFile inputFile = issue.getInputFile(); + + if (inputFile == null || inputFile.getPath() == null || + issue.getStartLine() == null || issue.getEndLine() == null) { + throw new IllegalArgumentException("Impossible to identify issue's location"); + } + + return new Location(inputFile.getPath(), issue.getStartLine(), issue.getEndLine()); + } } diff --git a/src/main/java/cc/models/Severity.java b/src/main/java/cc/models/Severity.java index fc4cc7e..47c9fac 100644 --- a/src/main/java/cc/models/Severity.java +++ b/src/main/java/cc/models/Severity.java @@ -1,6 +1,7 @@ package cc.models; import com.google.gson.annotations.SerializedName; +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; public enum Severity { @SerializedName("major") @@ -14,7 +15,11 @@ public enum Severity { @SerializedName("blocker") BLOCKER; - public static Severity from(String value) { - return valueOf(value.toUpperCase()); + public static Severity from(RuleDetails ruleDetails) { + try { + return valueOf(ruleDetails.getSeverity().toUpperCase()); + } catch (Exception e) { + throw new IllegalArgumentException("Unknown severity", e); + } } } diff --git a/src/test/java/cc/models/CodeClimateIssueTest.java b/src/test/java/cc/models/CodeClimateIssueTest.java index c9f6adc..d872dd4 100644 --- a/src/test/java/cc/models/CodeClimateIssueTest.java +++ b/src/test/java/cc/models/CodeClimateIssueTest.java @@ -3,6 +3,7 @@ import cc.serialization.GsonFactory; import com.google.gson.Gson; import org.junit.Test; +import support.FakeRuleDetails; import static org.assertj.core.api.Assertions.assertThat; @@ -31,7 +32,7 @@ public void properly_serialize_severity() throws Exception { private CodeClimateIssue createIssueForSeverity(String severity) { return new CodeClimateIssue( "check", - Severity.from(severity), + Severity.from(new FakeRuleDetails(severity)), "desc", new Content(""), new Location("path", 0, 1), diff --git a/src/test/java/support/FakeRuleDetails.java b/src/test/java/support/FakeRuleDetails.java new file mode 100644 index 0000000..bb24974 --- /dev/null +++ b/src/test/java/support/FakeRuleDetails.java @@ -0,0 +1,54 @@ +package support; + +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; + +import javax.annotation.CheckForNull; + +public class FakeRuleDetails implements RuleDetails { + private String severity; + + public FakeRuleDetails(String severity) { + this.severity = severity; + } + + @Override + public String getKey() { + return null; + } + + @Override + public String getName() { + return null; + } + + @Override + public String getHtmlDescription() { + return null; + } + + @Override + public String getLanguage() { + return null; + } + + @Override + public String getSeverity() { + return severity; + } + + @CheckForNull + @Override + public String getType() { + return "BUG"; + } + + @Override + public String[] getTags() { + return new String[0]; + } + + @Override + public String getExtendedDescription() { + return null; + } +} diff --git a/src/test/java/support/OutputHelper.java b/src/test/java/support/OutputHelper.java new file mode 100644 index 0000000..ea5e7e3 --- /dev/null +++ b/src/test/java/support/OutputHelper.java @@ -0,0 +1,23 @@ +package support; + +import java.io.ByteArrayOutputStream; +import java.io.OutputStream; +import java.io.PrintStream; + +public class OutputHelper { + public final OutputStream stdout; + public final OutputStream stderr; + + public OutputHelper(OutputStream out, OutputStream err) { + this.stdout = out; + this.stderr = err; + } + + public static OutputHelper setup() { + OutputStream out = new ByteArrayOutputStream(); + OutputStream err = new ByteArrayOutputStream(); + System.setOut(new PrintStream(out)); + System.setErr(new PrintStream(err)); + return new OutputHelper(out, err); + } +} diff --git a/src/test/java/support/fakes/FakeClientInputFile.java b/src/test/java/support/fakes/FakeClientInputFile.java new file mode 100644 index 0000000..eb068e8 --- /dev/null +++ b/src/test/java/support/fakes/FakeClientInputFile.java @@ -0,0 +1,48 @@ +package support.fakes; + +import org.sonarsource.sonarlint.core.client.api.common.analysis.ClientInputFile; + +import javax.annotation.CheckForNull; +import java.io.IOException; +import java.io.InputStream; +import java.nio.charset.Charset; + +public class FakeClientInputFile implements ClientInputFile { + String path; + + public FakeClientInputFile(String path) { + this.path = path; + } + + @Override + public String getPath() { + return path; + } + + @CheckForNull + @Override + public boolean isTest() { + return false; + } + + @CheckForNull + @Override + public Charset getCharset() { + return null; + } + + @Override + public G getClientObject() { + return null; + } + + @Override + public InputStream inputStream() throws IOException { + return null; + } + + @Override + public String contents() throws IOException { + return null; + } +} diff --git a/src/test/java/support/fakes/FakeIssue.java b/src/test/java/support/fakes/FakeIssue.java new file mode 100644 index 0000000..1c25af1 --- /dev/null +++ b/src/test/java/support/fakes/FakeIssue.java @@ -0,0 +1,82 @@ +package support.fakes; + +import org.sonarsource.sonarlint.core.client.api.common.analysis.ClientInputFile; +import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; + +import javax.annotation.CheckForNull; +import java.util.List; + +public class FakeIssue implements Issue { + + String path = "/tmp/File.java"; + Integer startLine; + Integer endLine; + + public FakeIssue(String path, Integer startLine, Integer endLine) { + this.path = path; + this.startLine = startLine; + this.endLine = endLine; + } + + @Override + public String getSeverity() { + return null; + } + + @CheckForNull + @Override + public String getType() { + return null; + } + + @CheckForNull + @Override + public String getMessage() { + return null; + } + + @Override + public String getRuleKey() { + return null; + } + + @Override + public String getRuleName() { + return null; + } + + @CheckForNull + @Override + public Integer getStartLine() { + return startLine; + } + + @CheckForNull + @Override + public Integer getStartLineOffset() { + return null; + } + + @CheckForNull + @Override + public Integer getEndLine() { + return endLine; + } + + @CheckForNull + @Override + public Integer getEndLineOffset() { + return null; + } + + @Override + public List flows() { + return null; + } + + @CheckForNull + @Override + public ClientInputFile getInputFile() { + return new FakeClientInputFile(path); + } +} diff --git a/src/test/java/support/fakes/FakeTrackable.java b/src/test/java/support/fakes/FakeTrackable.java new file mode 100644 index 0000000..fa01a06 --- /dev/null +++ b/src/test/java/support/fakes/FakeTrackable.java @@ -0,0 +1,87 @@ +package support.fakes; + +import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; +import org.sonarsource.sonarlint.core.tracking.TextRange; +import org.sonarsource.sonarlint.core.tracking.Trackable; + +import javax.annotation.CheckForNull; + +public class FakeTrackable implements Trackable{ + + private FakeIssue fakeIssue; + + public FakeTrackable(FakeIssue fakeIssue) { + this.fakeIssue = fakeIssue; + } + + @Override + public Issue getIssue() { + return fakeIssue; + } + + @Override + public String getRuleKey() { + return null; + } + + @Override + public String getRuleName() { + return null; + } + + @Override + public String getSeverity() { + return null; + } + + @Override + public String getMessage() { + return null; + } + + @CheckForNull + @Override + public Integer getLine() { + return null; + } + + @CheckForNull + @Override + public Integer getLineHash() { + return null; + } + + @CheckForNull + @Override + public TextRange getTextRange() { + return null; + } + + @CheckForNull + @Override + public Integer getTextRangeHash() { + return null; + } + + @CheckForNull + @Override + public Long getCreationDate() { + return null; + } + + @CheckForNull + @Override + public String getServerIssueKey() { + return null; + } + + @Override + public boolean isResolved() { + return false; + } + + @Override + public String getAssignee() { + return null; + } +} From eb5c82bb31b70a5c952e38e5a0baed606487cb24 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 16:30:08 -0300 Subject: [PATCH 05/12] Skip issue creation with bogus data and print a warn instead --- src/main/java/cc/JsonReport.java | 14 +++++-- src/test/java/cc/JsonReportTest.java | 56 ++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 3 deletions(-) create mode 100644 src/test/java/cc/JsonReportTest.java diff --git a/src/main/java/cc/JsonReport.java b/src/main/java/cc/JsonReport.java index 2e6cb1b..73aed29 100644 --- a/src/main/java/cc/JsonReport.java +++ b/src/main/java/cc/JsonReport.java @@ -34,9 +34,17 @@ public void execute(String projectName, Date date, Collection trackab Issue issue = trackable.getIssue(); RuleDetails ruleDetails = ruleDescriptionProducer.apply(issue.getRuleKey()); - if (VALID_RULE_DETAIL_TYPES.contains(ruleDetails.getType())) { - CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails); - System.out.println(gson.toJson(codeClimateIssue) + "\0"); + String type = ruleDetails.getType(); + if (VALID_RULE_DETAIL_TYPES.contains(type)) { + try { + CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails); + System.out.println(gson.toJson(codeClimateIssue) + "\0"); + } catch (IllegalArgumentException e) { + System.err.println(e.getMessage()); + System.err.println(issue); + System.err.println(ruleDetails); + e.printStackTrace(System.err); + } } } } diff --git a/src/test/java/cc/JsonReportTest.java b/src/test/java/cc/JsonReportTest.java new file mode 100644 index 0000000..d4618e5 --- /dev/null +++ b/src/test/java/cc/JsonReportTest.java @@ -0,0 +1,56 @@ +package cc; + +import org.junit.Before; +import org.junit.Test; +import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; +import org.sonarsource.sonarlint.core.tracking.Trackable; +import support.FakeRuleDetails; +import support.OutputHelper; +import support.fakes.FakeIssue; +import support.fakes.FakeTrackable; + +import java.util.Date; +import java.util.List; + +import static java.util.Arrays.asList; +import static org.assertj.core.api.Assertions.assertThat; + +public class JsonReportTest { + + OutputHelper output; + + @Before + public void setUp() throws Exception { + output = OutputHelper.setup(); + } + + @Test + public void serialize_issue() throws Exception { + executeReport("major", new FakeIssue("file.java", 0, 1)); + assertThat(output.stdout.toString()).contains("\"type\":\"issue\""); + } + + @Test + public void does_not_create_issue_for_unknown_severity() throws Exception { + executeReport("unknown", new FakeIssue("file.java", 0, 1)); + assertThat(output.stderr.toString()).contains("Unknown severity"); + } + + @Test + public void does_not_create_issue_for_unknown_path() throws Exception { + executeReport("major", new FakeIssue(null, 1, 0)); + assertThat(output.stderr.toString()).contains("Impossible to identify issue's location"); + } + + @Test + public void does_not_create_issue_for_unknown_location() throws Exception { + executeReport("major", new FakeIssue("file.java", null, null)); + assertThat(output.stderr.toString()).contains("Impossible to identify issue's location"); + } + + void executeReport(String severity, FakeIssue issue) { + RuleDetails ruleDetails = new FakeRuleDetails(severity); + List trackables = asList(new FakeTrackable(issue)); + new JsonReport().execute("prj", new Date(0), trackables, null, _ruleKey -> ruleDetails); + } +} \ No newline at end of file From 59ac7c0392346a9f13a51145e767fe92dfc78a50 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 17:07:37 -0300 Subject: [PATCH 06/12] Report paths relative to base dir --- src/main/java/cc/JsonReport.java | 6 ++++-- src/main/java/cc/models/CodeClimateIssue.java | 4 ++-- src/main/java/cc/models/Location.java | 9 +++++---- src/main/java/org/sonarlint/cli/CustomMain.java | 2 +- src/test/java/cc/JsonReportTest.java | 8 +++++++- src/test/java/cc/models/CodeClimateIssueTest.java | 2 +- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/main/java/cc/JsonReport.java b/src/main/java/cc/JsonReport.java index 73aed29..4336ebe 100644 --- a/src/main/java/cc/JsonReport.java +++ b/src/main/java/cc/JsonReport.java @@ -23,8 +23,10 @@ public class JsonReport implements org.sonarlint.cli.report.Reporter { ); final Gson gson; + final String baseDir; - public JsonReport() { + public JsonReport(String baseDir) { + this.baseDir = baseDir; this.gson = new GsonFactory().create(); } @@ -37,7 +39,7 @@ public void execute(String projectName, Date date, Collection trackab String type = ruleDetails.getType(); if (VALID_RULE_DETAIL_TYPES.contains(type)) { try { - CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails); + CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails, baseDir); System.out.println(gson.toJson(codeClimateIssue) + "\0"); } catch (IllegalArgumentException e) { System.err.println(e.getMessage()); diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index 6b15c46..f4c289a 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -21,12 +21,12 @@ public CodeClimateIssue(String checkName, Severity severity, String description, this.categories = categories; } - public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails) { + public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails, String baseDir) { String checkName = issue.getRuleKey(); String description = issue.getMessage(); Severity severity = Severity.from(ruleDetails); Content content = Content.from(ruleDetails); - Location location = Location.from(issue); + Location location = Location.from(issue, baseDir); Categories categories = Categories.from(ruleDetails); return new CodeClimateIssue(checkName, severity, description, content, location, categories); } diff --git a/src/main/java/cc/models/Location.java b/src/main/java/cc/models/Location.java index a9b0506..f27ef56 100644 --- a/src/main/java/cc/models/Location.java +++ b/src/main/java/cc/models/Location.java @@ -7,12 +7,13 @@ class Location { final String path; final Lines lines; - public Location(String path, Integer startLine, Integer endLine) { - this.path = path.replaceFirst("^/tmp/code/", ""); + public Location(String baseDir, String path, Integer startLine, Integer endLine) { + String regex = ("^" + baseDir + "/").replace("//", "/"); + this.path = path.replaceFirst(regex, ""); this.lines = new Lines(startLine, endLine); } - public static Location from(Issue issue) { + public static Location from(Issue issue, String baseDir) { ClientInputFile inputFile = issue.getInputFile(); if (inputFile == null || inputFile.getPath() == null || @@ -20,6 +21,6 @@ public static Location from(Issue issue) { throw new IllegalArgumentException("Impossible to identify issue's location"); } - return new Location(inputFile.getPath(), issue.getStartLine(), issue.getEndLine()); + return new Location(baseDir, inputFile.getPath(), issue.getStartLine(), issue.getEndLine()); } } diff --git a/src/main/java/org/sonarlint/cli/CustomMain.java b/src/main/java/org/sonarlint/cli/CustomMain.java index fa4b029..b6d2ff0 100644 --- a/src/main/java/org/sonarlint/cli/CustomMain.java +++ b/src/main/java/org/sonarlint/cli/CustomMain.java @@ -98,7 +98,7 @@ public CustomReportFactory(Charset charset) { @Override public List createReporters(Path basePath) { - return Arrays.asList(new JsonReport()); + return Arrays.asList(new JsonReport(basePath.toString())); } } } \ No newline at end of file diff --git a/src/test/java/cc/JsonReportTest.java b/src/test/java/cc/JsonReportTest.java index d4618e5..d7304f2 100644 --- a/src/test/java/cc/JsonReportTest.java +++ b/src/test/java/cc/JsonReportTest.java @@ -30,6 +30,12 @@ public void serialize_issue() throws Exception { assertThat(output.stdout.toString()).contains("\"type\":\"issue\""); } + @Test + public void serialize_issue_relative_path() throws Exception { + executeReport("major", new FakeIssue("/tmp/dir/file.java", 0, 1)); + assertThat(output.stdout.toString()).contains("\"path\":\"dir/file.java\""); + } + @Test public void does_not_create_issue_for_unknown_severity() throws Exception { executeReport("unknown", new FakeIssue("file.java", 0, 1)); @@ -51,6 +57,6 @@ public void does_not_create_issue_for_unknown_location() throws Exception { void executeReport(String severity, FakeIssue issue) { RuleDetails ruleDetails = new FakeRuleDetails(severity); List trackables = asList(new FakeTrackable(issue)); - new JsonReport().execute("prj", new Date(0), trackables, null, _ruleKey -> ruleDetails); + new JsonReport("/tmp").execute("prj", new Date(0), trackables, null, _ruleKey -> ruleDetails); } } \ No newline at end of file diff --git a/src/test/java/cc/models/CodeClimateIssueTest.java b/src/test/java/cc/models/CodeClimateIssueTest.java index d872dd4..133188f 100644 --- a/src/test/java/cc/models/CodeClimateIssueTest.java +++ b/src/test/java/cc/models/CodeClimateIssueTest.java @@ -35,7 +35,7 @@ private CodeClimateIssue createIssueForSeverity(String severity) { Severity.from(new FakeRuleDetails(severity)), "desc", new Content(""), - new Location("path", 0, 1), + new Location("/tmp", "path", 0, 1), new Categories("VULNERABILITY") ); } From 5b3b72e96f109799ed37b8084c8d5a0f05d0a9bc Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Wed, 11 Oct 2017 17:12:54 -0300 Subject: [PATCH 07/12] Fix path relative to base dir --- src/test/java/integration/SanityCheckTest.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/java/integration/SanityCheckTest.java b/src/test/java/integration/SanityCheckTest.java index 6d629a4..171c5c8 100644 --- a/src/test/java/integration/SanityCheckTest.java +++ b/src/test/java/integration/SanityCheckTest.java @@ -20,7 +20,7 @@ public void executeJavaLibFixture() throws Exception { "\"content\":{\"body\":\"

It is almost always an error when a for loop's stop condition and incrementer don't act on the same variable. Even when it is not," + " it\\ncould confuse future maintainers of the code," + " and should be avoided.

\\n

Noncompliant Code Example

\\n
\\nfor (i = 0; i < 10; j++) {  // Noncompliant\\n  // ...\\n}\\n
\\n

Compliant Solution

\\n
\\nfor (i = 0; i < 10; i++) {\\n  // ...\\n}\\n
\"}," + - "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + + "\"location\":{\"path\":\"main/java/Library.java\"," + "\"lines\":{\"begin\":17," + "\"end\":17}}," + "\"categories\":[\"Bug Risk\"]}" + @@ -31,7 +31,7 @@ public void executeJavaLibFixture() throws Exception { "\"content\":{\"body\":\"

It is almost always an error when a for loop's stop condition and incrementer don't act on the same variable. Even when it is not," + " it\\ncould confuse future maintainers of the code," + " and should be avoided.

\\n

Noncompliant Code Example

\\n
\\nfor (i = 0; i < 10; j++) {  // Noncompliant\\n  // ...\\n}\\n
\\n

Compliant Solution

\\n
\\nfor (i = 0; i < 10; i++) {\\n  // ...\\n}\\n
\"}," + - "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + + "\"location\":{\"path\":\"main/java/Library.java\"," + "\"lines\":{\"begin\":20," + "\"end\":20}}," + "\"categories\":[\"Bug Risk\"]}" + @@ -42,7 +42,7 @@ public void executeJavaLibFixture() throws Exception { "\"content\":{\"body\":\"

It is almost always an error when a for loop's stop condition and incrementer don't act on the same variable. Even when it is not," + " it\\ncould confuse future maintainers of the code," + " and should be avoided.

\\n

Noncompliant Code Example

\\n
\\nfor (i = 0; i < 10; j++) {  // Noncompliant\\n  // ...\\n}\\n
\\n

Compliant Solution

\\n
\\nfor (i = 0; i < 10; i++) {\\n  // ...\\n}\\n
\"}," + - "\"location\":{\"path\":\"fixtures/java_lib/main/java/Library.java\"," + + "\"location\":{\"path\":\"main/java/Library.java\"," + "\"lines\":{\"begin\":23," + "\"end\":23}}," + "\"categories\":[\"Bug Risk\"]}" + From 02ea41a2ccd73eb83fe61fd750d7dd50104ff523 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Thu, 12 Oct 2017 12:10:47 -0300 Subject: [PATCH 08/12] Improve naming and packaging --- src/test/java/cc/JsonReportTest.java | 2 +- src/test/java/integration/ConfigurationOptionsTest.java | 6 +++--- .../java/support/{TestSystem.java => SystemHelper.java} | 2 +- src/test/java/support/{ => fakes}/FakeRuleDetails.java | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename src/test/java/support/{TestSystem.java => SystemHelper.java} (93%) rename src/test/java/support/{ => fakes}/FakeRuleDetails.java (97%) diff --git a/src/test/java/cc/JsonReportTest.java b/src/test/java/cc/JsonReportTest.java index d7304f2..b2c79bd 100644 --- a/src/test/java/cc/JsonReportTest.java +++ b/src/test/java/cc/JsonReportTest.java @@ -4,7 +4,7 @@ import org.junit.Test; import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; import org.sonarsource.sonarlint.core.tracking.Trackable; -import support.FakeRuleDetails; +import support.fakes.FakeRuleDetails; import support.OutputHelper; import support.fakes.FakeIssue; import support.fakes.FakeTrackable; diff --git a/src/test/java/integration/ConfigurationOptionsTest.java b/src/test/java/integration/ConfigurationOptionsTest.java index f898e92..ecab24d 100644 --- a/src/test/java/integration/ConfigurationOptionsTest.java +++ b/src/test/java/integration/ConfigurationOptionsTest.java @@ -5,7 +5,7 @@ import org.junit.BeforeClass; import org.junit.Test; import org.sonarlint.cli.SonarProperties; -import support.TestSystem; +import support.SystemHelper; import java.io.ByteArrayOutputStream; import java.io.PrintStream; @@ -16,7 +16,7 @@ public class ConfigurationOptionsTest { ByteArrayOutputStream stdout; ByteArrayOutputStream stderr; - TestSystem system; + SystemHelper system; @BeforeClass public static void beforeAll() { @@ -25,7 +25,7 @@ public static void beforeAll() { @Before public void setUp() throws Exception { - system = new TestSystem(); + system = new SystemHelper(); stdout = new ByteArrayOutputStream(); stderr = new ByteArrayOutputStream(); diff --git a/src/test/java/support/TestSystem.java b/src/test/java/support/SystemHelper.java similarity index 93% rename from src/test/java/support/TestSystem.java rename to src/test/java/support/SystemHelper.java index 687030e..9b15da6 100644 --- a/src/test/java/support/TestSystem.java +++ b/src/test/java/support/SystemHelper.java @@ -4,7 +4,7 @@ import java.util.Properties; -public class TestSystem extends System2 { +public class SystemHelper extends System2 { public int exitCode; final Properties properties = new Properties(); diff --git a/src/test/java/support/FakeRuleDetails.java b/src/test/java/support/fakes/FakeRuleDetails.java similarity index 97% rename from src/test/java/support/FakeRuleDetails.java rename to src/test/java/support/fakes/FakeRuleDetails.java index bb24974..284eecc 100644 --- a/src/test/java/support/FakeRuleDetails.java +++ b/src/test/java/support/fakes/FakeRuleDetails.java @@ -1,4 +1,4 @@ -package support; +package support.fakes; import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; From e9642fef27bacc9172065f965711c67775bb4785 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Thu, 12 Oct 2017 12:13:28 -0300 Subject: [PATCH 09/12] Relax SEVERITY validation, just avoid errors processing data --- src/main/java/cc/models/CodeClimateIssue.java | 6 ++--- src/main/java/cc/models/Severity.java | 22 +++++-------------- src/test/java/cc/JsonReportTest.java | 6 ++--- .../java/cc/models/CodeClimateIssueTest.java | 20 ++++++----------- 4 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index f4c289a..9efb827 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -6,13 +6,13 @@ public class CodeClimateIssue { public final String type = "issue"; public final String checkName; - public final Severity severity; + public final String severity; public final String description; public final Content content; public final Location location; public final Categories categories; - public CodeClimateIssue(String checkName, Severity severity, String description, Content content, Location location, Categories categories) { + public CodeClimateIssue(String checkName, String severity, String description, Content content, Location location, Categories categories) { this.checkName = checkName; this.severity = severity; this.description = description; @@ -24,7 +24,7 @@ public CodeClimateIssue(String checkName, Severity severity, String description, public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails, String baseDir) { String checkName = issue.getRuleKey(); String description = issue.getMessage(); - Severity severity = Severity.from(ruleDetails); + String severity = Severity.from(ruleDetails); Content content = Content.from(ruleDetails); Location location = Location.from(issue, baseDir); Categories categories = Categories.from(ruleDetails); diff --git a/src/main/java/cc/models/Severity.java b/src/main/java/cc/models/Severity.java index 47c9fac..5fdd33f 100644 --- a/src/main/java/cc/models/Severity.java +++ b/src/main/java/cc/models/Severity.java @@ -3,23 +3,13 @@ import com.google.gson.annotations.SerializedName; import org.sonarsource.sonarlint.core.client.api.common.RuleDetails; -public enum Severity { - @SerializedName("major") - MAJOR, - @SerializedName("minor") - MINOR, - @SerializedName("info") - INFO, - @SerializedName("critical") - CRITICAL, - @SerializedName("blocker") - BLOCKER; +public class Severity { - public static Severity from(RuleDetails ruleDetails) { - try { - return valueOf(ruleDetails.getSeverity().toUpperCase()); - } catch (Exception e) { - throw new IllegalArgumentException("Unknown severity", e); + public static String from(RuleDetails ruleDetails) { + String severity = ruleDetails.getSeverity(); + if(severity == null) { + return null; } + return severity.toLowerCase(); } } diff --git a/src/test/java/cc/JsonReportTest.java b/src/test/java/cc/JsonReportTest.java index b2c79bd..61f555a 100644 --- a/src/test/java/cc/JsonReportTest.java +++ b/src/test/java/cc/JsonReportTest.java @@ -37,9 +37,9 @@ public void serialize_issue_relative_path() throws Exception { } @Test - public void does_not_create_issue_for_unknown_severity() throws Exception { - executeReport("unknown", new FakeIssue("file.java", 0, 1)); - assertThat(output.stderr.toString()).contains("Unknown severity"); + public void does_not_include_unknown_severity() throws Exception { + executeReport(null, new FakeIssue("file.java", 0, 1)); + assertThat(output.stdout.toString()).doesNotContain("severity"); } @Test diff --git a/src/test/java/cc/models/CodeClimateIssueTest.java b/src/test/java/cc/models/CodeClimateIssueTest.java index 133188f..6e72581 100644 --- a/src/test/java/cc/models/CodeClimateIssueTest.java +++ b/src/test/java/cc/models/CodeClimateIssueTest.java @@ -3,30 +3,24 @@ import cc.serialization.GsonFactory; import com.google.gson.Gson; import org.junit.Test; -import support.FakeRuleDetails; +import support.fakes.FakeRuleDetails; import static org.assertj.core.api.Assertions.assertThat; public class CodeClimateIssueTest { - @Test(expected = IllegalArgumentException.class) - public void raise_error_on_invalid_severity() throws Exception { - createIssueForSeverity("unknown"); - } - @Test - public void accepts_only_valid_severities() throws Exception { - assertThat(createIssueForSeverity("major").severity).isEqualTo(Severity.MAJOR); - assertThat(createIssueForSeverity("minor").severity).isEqualTo(Severity.MINOR); - assertThat(createIssueForSeverity("critical").severity).isEqualTo(Severity.CRITICAL); - assertThat(createIssueForSeverity("blocker").severity).isEqualTo(Severity.BLOCKER); - assertThat(createIssueForSeverity("info").severity).isEqualTo(Severity.INFO); + public void down_case_severities() throws Exception { + assertThat(createIssueForSeverity("MAJOR").severity).isEqualTo("major"); + assertThat(createIssueForSeverity("MINOR").severity).isEqualTo("minor"); + assertThat(createIssueForSeverity("CRITICAL").severity).isEqualTo("critical"); + assertThat(createIssueForSeverity(null).severity).isNull(); } @Test public void properly_serialize_severity() throws Exception { Gson gson = new GsonFactory().create(); - assertThat(gson.toJson(createIssueForSeverity("info"))).contains("\"severity\":\"info\""); + assertThat(gson.toJson(createIssueForSeverity("INFO"))).contains("\"severity\":\"info\""); } private CodeClimateIssue createIssueForSeverity(String severity) { From a2e484ea597083f3d3d545d9cc89dee7c3777d81 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Thu, 12 Oct 2017 12:14:31 -0300 Subject: [PATCH 10/12] Relax LOCATION validation, does not include values if bogus --- src/main/java/cc/models/CodeClimateIssue.java | 2 +- src/main/java/cc/models/Lines.java | 22 +++++++++---------- src/main/java/cc/models/Location.java | 13 +++++------ src/test/java/cc/JsonReportTest.java | 8 +++---- .../java/cc/models/CodeClimateIssueTest.java | 2 +- 5 files changed, 22 insertions(+), 25 deletions(-) diff --git a/src/main/java/cc/models/CodeClimateIssue.java b/src/main/java/cc/models/CodeClimateIssue.java index 9efb827..c193296 100644 --- a/src/main/java/cc/models/CodeClimateIssue.java +++ b/src/main/java/cc/models/CodeClimateIssue.java @@ -26,7 +26,7 @@ public static CodeClimateIssue from(Issue issue, RuleDetails ruleDetails, String String description = issue.getMessage(); String severity = Severity.from(ruleDetails); Content content = Content.from(ruleDetails); - Location location = Location.from(issue, baseDir); + Location location = Location.from(baseDir, issue); Categories categories = Categories.from(ruleDetails); return new CodeClimateIssue(checkName, severity, description, content, location, categories); } diff --git a/src/main/java/cc/models/Lines.java b/src/main/java/cc/models/Lines.java index 6462ec5..f885eb3 100644 --- a/src/main/java/cc/models/Lines.java +++ b/src/main/java/cc/models/Lines.java @@ -1,22 +1,20 @@ package cc.models; +import org.sonarsource.sonarlint.core.client.api.common.analysis.Issue; + class Lines { final Integer begin; final Integer end; - public Lines(Integer startLine, Integer endLine) { - if (startLine != null) { - this.begin = startLine; + public Lines(Integer begin, Integer end) { + this.begin = begin; + this.end = end; + } - if (endLine != null) { - this.end = endLine; - } else { - this.end = 1; - } - } else { - this.begin = 1; - this.end = 1; + public static Lines from(Issue issue) { + if(issue.getStartLine() == null || issue.getEndLine() == null) { + return null; } - + return new Lines(issue.getStartLine(), issue.getEndLine()); } } diff --git a/src/main/java/cc/models/Location.java b/src/main/java/cc/models/Location.java index f27ef56..f39de09 100644 --- a/src/main/java/cc/models/Location.java +++ b/src/main/java/cc/models/Location.java @@ -7,20 +7,19 @@ class Location { final String path; final Lines lines; - public Location(String baseDir, String path, Integer startLine, Integer endLine) { + public Location(String baseDir, String path, Lines lines) { String regex = ("^" + baseDir + "/").replace("//", "/"); this.path = path.replaceFirst(regex, ""); - this.lines = new Lines(startLine, endLine); + this.lines = lines; } - public static Location from(Issue issue, String baseDir) { + public static Location from(String baseDir, Issue issue) { ClientInputFile inputFile = issue.getInputFile(); - if (inputFile == null || inputFile.getPath() == null || - issue.getStartLine() == null || issue.getEndLine() == null) { - throw new IllegalArgumentException("Impossible to identify issue's location"); + if (inputFile == null || inputFile.getPath() == null) { + return null; } - return new Location(baseDir, inputFile.getPath(), issue.getStartLine(), issue.getEndLine()); + return new Location(baseDir, inputFile.getPath(), Lines.from(issue)); } } diff --git a/src/test/java/cc/JsonReportTest.java b/src/test/java/cc/JsonReportTest.java index 61f555a..3b637b8 100644 --- a/src/test/java/cc/JsonReportTest.java +++ b/src/test/java/cc/JsonReportTest.java @@ -43,15 +43,15 @@ public void does_not_include_unknown_severity() throws Exception { } @Test - public void does_not_create_issue_for_unknown_path() throws Exception { + public void does_not_include_unknown_path() throws Exception { executeReport("major", new FakeIssue(null, 1, 0)); - assertThat(output.stderr.toString()).contains("Impossible to identify issue's location"); + assertThat(output.stdout.toString()).doesNotContain("path"); } @Test - public void does_not_create_issue_for_unknown_location() throws Exception { + public void does_not_include_unknown_location() throws Exception { executeReport("major", new FakeIssue("file.java", null, null)); - assertThat(output.stderr.toString()).contains("Impossible to identify issue's location"); + assertThat(output.stdout.toString()).doesNotContain("lines"); } void executeReport(String severity, FakeIssue issue) { diff --git a/src/test/java/cc/models/CodeClimateIssueTest.java b/src/test/java/cc/models/CodeClimateIssueTest.java index 6e72581..d26a271 100644 --- a/src/test/java/cc/models/CodeClimateIssueTest.java +++ b/src/test/java/cc/models/CodeClimateIssueTest.java @@ -29,7 +29,7 @@ private CodeClimateIssue createIssueForSeverity(String severity) { Severity.from(new FakeRuleDetails(severity)), "desc", new Content(""), - new Location("/tmp", "path", 0, 1), + new Location("/tmp", "path", new Lines(0, 1)), new Categories("VULNERABILITY") ); } From bc056a425eee78898fd27296cb1167f69e4df739 Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Thu, 12 Oct 2017 12:15:10 -0300 Subject: [PATCH 11/12] Fix relative paths on tests --- .../java/integration/ConfigurationOptionsTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/java/integration/ConfigurationOptionsTest.java b/src/test/java/integration/ConfigurationOptionsTest.java index ecab24d..0769dc8 100644 --- a/src/test/java/integration/ConfigurationOptionsTest.java +++ b/src/test/java/integration/ConfigurationOptionsTest.java @@ -42,8 +42,8 @@ public void limit_path_included_within_analysis() throws Exception { App.execute(new String[]{}, system); String output = stdout.toString(); - assertThat(output).contains("issue", "fixtures/multiple_paths/src/included/java/pkg1/HasIssue.java"); - assertThat(output).doesNotContain("fixtures/multiple_paths/src/excluded/java/pkg1/HasIssue.java"); + assertThat(output).contains("\"type\":\"issue\"", "src/included/java/pkg1/HasIssue.java"); + assertThat(output).doesNotContain("src/excluded/java/pkg1/HasIssue.java"); } @Test @@ -52,9 +52,9 @@ public void include_all_files_by_default() throws Exception { String output = stdout.toString(); assertThat(output).contains( - "issue", - "fixtures/multiple_paths/src/included/java/pkg1/HasIssue.java", - "fixtures/multiple_paths/src/excluded/java/pkg1/HasIssue.java" + "\"type\":\"issue\"", + "src/included/java/pkg1/HasIssue.java", + "src/excluded/java/pkg1/HasIssue.java" ); } } From be264579f463705f0a9dcb7e352de0d5e3c7bc8e Mon Sep 17 00:00:00 2001 From: Filipe Esperandio Date: Thu, 12 Oct 2017 12:16:05 -0300 Subject: [PATCH 12/12] Remove validation from the engine. Let it process invalid issues which will be properly validated by the CLI --- src/main/java/cc/JsonReport.java | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/main/java/cc/JsonReport.java b/src/main/java/cc/JsonReport.java index 4336ebe..833fcf6 100644 --- a/src/main/java/cc/JsonReport.java +++ b/src/main/java/cc/JsonReport.java @@ -38,15 +38,8 @@ public void execute(String projectName, Date date, Collection trackab String type = ruleDetails.getType(); if (VALID_RULE_DETAIL_TYPES.contains(type)) { - try { - CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails, baseDir); - System.out.println(gson.toJson(codeClimateIssue) + "\0"); - } catch (IllegalArgumentException e) { - System.err.println(e.getMessage()); - System.err.println(issue); - System.err.println(ruleDetails); - e.printStackTrace(System.err); - } + CodeClimateIssue codeClimateIssue = CodeClimateIssue.from(issue, ruleDetails, baseDir); + System.out.println(gson.toJson(codeClimateIssue) + "\0"); } } }