From 6b2dde878ad62018a0cd87a4fdbccf9a3981be24 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 17 Feb 2022 10:25:06 +0100 Subject: [PATCH 1/2] [core] Fix SARIF formatter reporting multiple locations ... when it should report multiple results instead. Now one violation is mapped to exactly one result. Fixes #3768 --- docs/pages/release_notes.md | 2 + .../internal/sarif/SarifLogBuilder.java | 65 ++++----- .../pmd/renderers/SarifRendererTest.java | 56 ++++++-- .../expected-multiple-locations.sarif.json | 130 ++++++++++++++++++ .../sarif/expected-multiple.sarif.json | 16 +-- 5 files changed, 212 insertions(+), 57 deletions(-) create mode 100644 pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple-locations.sarif.json diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 957dbd0bec4..ab32d865ff9 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,6 +16,8 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* core + * [#3768](https://github.com/pmd/pmd/issues/3768): \[core] SARIF formatter reports multiple locations when it should report multiple results * misc * [#3759](https://github.com/pmd/pmd/issues/3759): \[lang-test] Upgrade dokka maven plugin to 1.4.32 diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/internal/sarif/SarifLogBuilder.java b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/internal/sarif/SarifLogBuilder.java index a2b0826a4b1..22461c6a82d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/renderers/internal/sarif/SarifLogBuilder.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/renderers/internal/sarif/SarifLogBuilder.java @@ -4,38 +4,36 @@ package net.sourceforge.pmd.renderers.internal.sarif; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ArtifactLocation; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.AssociatedRule; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Component; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Exception; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Invocation; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Location; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Message; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.MultiformatMessage; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.PhysicalLocation; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.PropertyBag; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Region; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ReportingDescriptor; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Result; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Run; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Tool; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ToolConfigurationNotification; -import static net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ToolExecutionNotification; - import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import net.sourceforge.pmd.PMDVersion; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ArtifactLocation; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.AssociatedRule; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Component; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Exception; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Invocation; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Location; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Message; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.MultiformatMessage; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.PhysicalLocation; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.PropertyBag; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Region; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ReportingDescriptor; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Result; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Run; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.Tool; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ToolConfigurationNotification; +import net.sourceforge.pmd.renderers.internal.sarif.SarifLog.ToolExecutionNotification; public class SarifLogBuilder { - private final Map> locationsByRule = new HashMap<>(); + private final List rules = new ArrayList<>(); + private final List results = new ArrayList<>(); private final List toolConfigurationNotifications = new ArrayList<>(); private final List toolExecutionNotifications = new ArrayList<>(); @@ -45,11 +43,15 @@ public static SarifLogBuilder sarifLogBuilder() { public SarifLogBuilder add(RuleViolation violation) { final ReportingDescriptor ruleDescriptor = getReportingDescriptor(violation); - final Location location = getRuleViolationLocation(violation); + int ruleIndex = rules.indexOf(ruleDescriptor); + if (ruleIndex == -1) { + rules.add(ruleDescriptor); + ruleIndex = rules.size() - 1; + } - final List ruleLocation = locationsByRule.containsKey(ruleDescriptor) ? locationsByRule.get(ruleDescriptor) : new ArrayList(); - ruleLocation.add(location); - locationsByRule.put(ruleDescriptor, ruleLocation); + final Location location = getRuleViolationLocation(violation); + final Result result = resultFrom(ruleDescriptor, ruleIndex, location); + results.add(result); return this; } @@ -105,15 +107,6 @@ public SarifLogBuilder addConfigurationError(Report.ConfigurationError error) { } public SarifLog build() { - final List rules = new ArrayList<>(locationsByRule.keySet()); - - final List results = new ArrayList<>(); - for (int i = 0, size = rules.size(); i < size; i++) { - ReportingDescriptor rule = rules.get(i); - List locations = locationsByRule.get(rule); - results.add(resultFrom(rule, i, locations)); - } - final Component driver = getDriverComponent().toBuilder().rules(rules).build(); final Tool tool = Tool.builder().driver(driver).build(); final Invocation invocation = Invocation.builder() @@ -136,7 +129,7 @@ private boolean isExecutionSuccessful() { return toolExecutionNotifications.isEmpty() && toolConfigurationNotifications.isEmpty(); } - private Result resultFrom(ReportingDescriptor rule, Integer ruleIndex, List locations) { + private Result resultFrom(ReportingDescriptor rule, Integer ruleIndex, Location location) { final Result result = Result.builder() .ruleId(rule.getId()) .ruleIndex(ruleIndex) @@ -147,7 +140,7 @@ private Result resultFrom(ReportingDescriptor rule, Integer ruleIndex, List(rule, ctx, node, "blah"); + } + private RuleViolation newRuleViolation(int endColumn, String ruleName) { DummyNode node = createNode(endColumn); RuleContext ctx = new RuleContext(); diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple-locations.sarif.json b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple-locations.sarif.json new file mode 100644 index 00000000000..7a545006b02 --- /dev/null +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple-locations.sarif.json @@ -0,0 +1,130 @@ +{ + "$schema": "https://raw.githubusercontent.com/oasis-tcs/sarif-spec/master/Schemata/sarif-schema-2.1.0.json", + "version": "2.1.0", + "runs": [ + { + "tool": { + "driver": { + "name": "PMD", + "version": "unknown", + "informationUri": "https://pmd.github.io/pmd/", + "rules": [ + { + "id": "Foo", + "shortDescription": { + "text": "blah" + }, + "fullDescription": { + "text": "desc" + }, + "help": { + "text": "desc" + }, + "properties": { + "ruleset": "RuleSet", + "priority": 5, + "tags": [ + "RuleSet" + ] + } + }, + { + "id": "Boo", + "shortDescription": { + "text": "blah" + }, + "fullDescription": { + "text": "desc" + }, + "help": { + "text": "desc" + }, + "properties": { + "ruleset": "RuleSet", + "priority": 1, + "tags": [ + "RuleSet" + ] + } + } + ] + } + }, + "results": [ + { + "ruleId": "Foo", + "ruleIndex": 0, + "message": { + "text": "blah" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "notAvailable.ext" + }, + "region": { + "startLine": 1, + "startColumn": 1, + "endLine": 1, + "endColumn": 10 + } + } + } + ] + }, + { + "ruleId": "Boo", + "ruleIndex": 1, + "message": { + "text": "blah" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "notAvailable.ext" + }, + "region": { + "startLine": 2, + "startColumn": 2, + "endLine": 3, + "endColumn": 1 + } + } + } + ] + }, + { + "ruleId": "Foo", + "ruleIndex": 0, + "message": { + "text": "blah" + }, + "locations": [ + { + "physicalLocation": { + "artifactLocation": { + "uri": "notAvailable.ext" + }, + "region": { + "startLine": 5, + "startColumn": 1, + "endLine": 5, + "endColumn": 11 + } + } + } + ] + } + ], + "invocations": [ + { + "executionSuccessful": true, + "toolConfigurationNotifications": [], + "toolExecutionNotifications": [] + } + ] + } + ] +} \ No newline at end of file diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple.sarif.json b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple.sarif.json index e89a3927dfe..108ac1578f8 100644 --- a/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple.sarif.json +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/sarif/expected-multiple.sarif.json @@ -10,7 +10,7 @@ "informationUri": "https://pmd.github.io/pmd/", "rules": [ { - "id": "Boo", + "id": "Foo", "shortDescription": { "text": "blah" }, @@ -22,14 +22,14 @@ }, "properties": { "ruleset": "RuleSet", - "priority": 1, + "priority": 5, "tags": [ "RuleSet" ] } }, { - "id": "Foo", + "id": "Boo", "shortDescription": { "text": "blah" }, @@ -41,7 +41,7 @@ }, "properties": { "ruleset": "RuleSet", - "priority": 5, + "priority": 1, "tags": [ "RuleSet" ] @@ -52,7 +52,7 @@ }, "results": [ { - "ruleId": "Boo", + "ruleId": "Foo", "ruleIndex": 0, "message": { "text": "blah" @@ -67,14 +67,14 @@ "startLine": 1, "startColumn": 1, "endLine": 1, - "endColumn": 2 + "endColumn": 1 } } } ] }, { - "ruleId": "Foo", + "ruleId": "Boo", "ruleIndex": 1, "message": { "text": "blah" @@ -89,7 +89,7 @@ "startLine": 1, "startColumn": 1, "endLine": 1, - "endColumn": 1 + "endColumn": 2 } } } From 3eed1164b48da0802d15834f69502ba50ff4a14d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 17 Feb 2022 11:17:14 +0100 Subject: [PATCH 2/2] [core] Refactoring renderer tests --- .../pmd/renderers/AbstractRendererTest.java | 51 ++++++++++---- .../pmd/renderers/CSVRendererTest.java | 6 +- .../renderers/CodeClimateRendererTest.java | 17 ++--- .../pmd/renderers/JsonRendererTest.java | 3 +- .../pmd/renderers/PapariTextRendererTest.java | 6 +- .../pmd/renderers/RenderersTests.java | 3 + .../pmd/renderers/SarifRendererTest.java | 66 +++---------------- .../renderers/SummaryHTMLRendererTest.java | 3 +- .../pmd/renderers/TextPadRendererTest.java | 6 +- .../pmd/renderers/TextRendererTest.java | 2 +- .../pmd/renderers/VBHTMLRendererTest.java | 4 -- .../pmd/renderers/XMLRendererTest.java | 2 +- .../pmd/renderers/YAHTMLRendererTest.java | 18 ++--- .../pmd/renderers/json/expected-multiple.json | 2 +- 14 files changed, 74 insertions(+), 115 deletions(-) diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java index bff13f27ef9..097c1c80b30 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/AbstractRendererTest.java @@ -15,6 +15,7 @@ import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.ReportTest; +import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RulePriority; import net.sourceforge.pmd.RuleViolation; @@ -64,26 +65,53 @@ public void testNullPassedIn() throws Exception { protected Report reportOneViolation() { Report report = new Report(); - report.addRuleViolation(newRuleViolation(1)); + report.addRuleViolation(newRuleViolation(1, 1, 1, 1, createFooRule())); return report; } - private Report reportTwoViolations() { + protected Report reportTwoViolations() { Report report = new Report(); - RuleViolation informationalRuleViolation = newRuleViolation(1); - informationalRuleViolation.getRule().setPriority(RulePriority.LOW); + RuleViolation informationalRuleViolation = newRuleViolation(1, 1, 1, 1, createFooRule()); report.addRuleViolation(informationalRuleViolation); - RuleViolation severeRuleViolation = newRuleViolation(2); - severeRuleViolation.getRule().setPriority(RulePriority.HIGH); + RuleViolation severeRuleViolation = newRuleViolation(1, 1, 1, 2, createBooRule()); report.addRuleViolation(severeRuleViolation); return report; } - protected RuleViolation newRuleViolation(int endColumn) { - DummyNode node = createNode(endColumn); + protected DummyNode createNode(int beginLine, int beginColumn, int endLine, int endColumn) { + DummyNode node = new DummyNode(1); + node.testingOnlySetBeginLine(beginLine); + node.testingOnlySetBeginColumn(beginColumn); + node.testingOnlySetEndLine(endLine); + node.testingOnlySetEndColumn(endColumn); + return node; + } + + protected RuleViolation newRuleViolation(int beginLine, int beginColumn, int endLine, int endColumn, Rule rule) { + DummyNode node = createNode(beginLine, beginColumn, endLine, endColumn); RuleContext ctx = new RuleContext(); ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - return new ParametricRuleViolation(new FooRule(), ctx, node, "blah"); + return new ParametricRuleViolation(rule, ctx, node, "blah"); + } + + /** + * Creates a new rule instance with name "Boo" and priority {@link RulePriority#HIGH}. + */ + protected Rule createBooRule() { + Rule booRule = new FooRule(); + booRule.setName("Boo"); + booRule.setPriority(RulePriority.HIGH); + return booRule; + } + + /** + * Creates a new rule instance with name "Foo" and priority {@link RulePriority#LOW}. + */ + protected Rule createFooRule() { + Rule fooRule = new FooRule(); + fooRule.setName("Foo"); + fooRule.setPriority(RulePriority.LOW); + return fooRule; } protected static DummyNode createNode(int endColumn) { @@ -97,14 +125,11 @@ protected static DummyNode createNode(int endColumn) { @Test public void testRuleWithProperties() throws Exception { - DummyNode node = createNode(1); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); Report report = new Report(); RuleWithProperties theRule = new RuleWithProperties(); theRule.setProperty(RuleWithProperties.STRING_PROPERTY_DESCRIPTOR, "the string value\nsecond line with \"quotes\""); - report.addRuleViolation(new ParametricRuleViolation(theRule, ctx, node, "blah")); + report.addRuleViolation(newRuleViolation(1, 1, 1, 1, theRule)); String rendered = ReportTest.render(getRenderer(), report); assertEquals(filter(getExpectedWithProperties()), filter(rendered)); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CSVRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CSVRendererTest.java index 5ef24218fee..4b0fdf30243 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CSVRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CSVRendererTest.java @@ -30,7 +30,7 @@ public String getExpectedEmpty() { public String getExpectedMultiple() { return getHeader() + "\"1\",\"\",\"" + getSourceCodeFilename() + "\",\"5\",\"1\",\"blah\",\"RuleSet\",\"Foo\"" + PMD.EOL - + "\"2\",\"\",\"" + getSourceCodeFilename() + "\",\"1\",\"1\",\"blah\",\"RuleSet\",\"Foo\"" + PMD.EOL; + + "\"2\",\"\",\"" + getSourceCodeFilename() + "\",\"1\",\"1\",\"blah\",\"RuleSet\",\"Boo\"" + PMD.EOL; } @Override @@ -46,8 +46,4 @@ public String getExpectedError(ConfigurationError error) { private String getHeader() { return "\"Problem\",\"Package\",\"File\",\"Priority\",\"Line\",\"Description\",\"Rule set\",\"Rule\"" + PMD.EOL; } - - public static junit.framework.Test suite() { - return new junit.framework.JUnit4TestAdapter(CSVRendererTest.class); - } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java index ca24c5a9095..2d6ec8be4e6 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/CodeClimateRendererTest.java @@ -6,17 +6,11 @@ import static org.junit.Assert.assertEquals; -import java.io.File; - import org.junit.Test; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.lang.ast.DummyNode; -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; import net.sourceforge.pmd.lang.rule.XPathRule; public class CodeClimateRendererTest extends AbstractRendererTest { @@ -75,8 +69,8 @@ public String getExpectedMultiple() { + "violationSuppressRegex | | Suppress violations with messages matching a regular expression\\n" + "violationSuppressXPath | | Suppress violations on nodes which match a given relative XPath expression.\\n" + "\"},\"categories\":[\"Style\"],\"location\":{\"path\":\"" + getSourceCodeFilename() + "\",\"lines\":{\"begin\":1,\"end\":1}},\"severity\":\"info\",\"remediation_points\":50000}" - + "\u0000" + PMD.EOL + "{\"type\":\"issue\",\"check_name\":\"Foo\",\"description\":\"blah\"," - + "\"content\":{\"body\":\"## Foo\\n\\nSince: PMD null\\n\\nPriority: High\\n\\n" + + "\u0000" + PMD.EOL + "{\"type\":\"issue\",\"check_name\":\"Boo\",\"description\":\"blah\"," + + "\"content\":{\"body\":\"## Boo\\n\\nSince: PMD null\\n\\nPriority: High\\n\\n" + "[Categories](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#categories): Style\\n\\n" + "[Remediation Points](https://github.com/codeclimate/platform/blob/master/spec/analyzers/SPEC.md#remediation-points): 50000\\n\\n" + "desc\\n\\n" @@ -90,18 +84,15 @@ public String getExpectedMultiple() { @Test public void testXPathRule() throws Exception { - DummyNode node = createNode(1); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); Report report = new Report(); + XPathRule theRule = new XPathRule(); theRule.setProperty(XPathRule.XPATH_DESCRIPTOR, "//dummyNode"); - // Setup as FooRule theRule.setDescription("desc"); theRule.setName("Foo"); - report.addRuleViolation(new ParametricRuleViolation(theRule, ctx, node, "blah")); + report.addRuleViolation(newRuleViolation(1, 1, 1, 2, theRule)); String rendered = ReportTest.render(getRenderer(), report); // Output should be the exact same as for non xpath rules diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/JsonRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/JsonRendererTest.java index 524277fe21f..a913af96a81 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/JsonRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/JsonRendererTest.java @@ -15,6 +15,7 @@ import org.junit.Assert; import org.junit.Test; +import net.sourceforge.pmd.FooRule; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; @@ -90,7 +91,7 @@ public void suppressedViolations() throws IOException { Map suppressedLines = new HashMap<>(); suppressedLines.put(1, "test"); rep.suppress(suppressedLines); - rep.addRuleViolation(newRuleViolation(1)); + rep.addRuleViolation(newRuleViolation(1, 1, 1, 1, new FooRule())); String actual = ReportTest.render(getRenderer(), rep); String expected = readFile("expected-suppressed.json"); Assert.assertEquals(filter(expected), filter(actual)); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/PapariTextRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/PapariTextRendererTest.java index 43d96515955..1cbf0222b6b 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/PapariTextRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/PapariTextRendererTest.java @@ -42,7 +42,7 @@ public String getExpectedEmpty() { public String getExpectedMultiple() { return "* file: " + getSourceCodeFilename() + PMD.EOL + " src: " + getSourceCodeFilename() + ":1:1" + PMD.EOL + " rule: Foo" + PMD.EOL + " msg: blah" + PMD.EOL + " code: public class Foo {}" + PMD.EOL + PMD.EOL + " src: " - + getSourceCodeFilename() + ":1:1" + PMD.EOL + " rule: Foo" + PMD.EOL + " msg: blah" + PMD.EOL + + getSourceCodeFilename() + ":1:1" + PMD.EOL + " rule: Boo" + PMD.EOL + " msg: blah" + PMD.EOL + " code: public class Foo {}" + PMD.EOL + PMD.EOL + PMD.EOL + PMD.EOL + "Summary:" + PMD.EOL + PMD.EOL + " : 2" + PMD.EOL + "* warnings: 2" + PMD.EOL; } @@ -67,8 +67,4 @@ public String getExpectedError(ConfigurationError error) { + " err: a configuration error" + PMD.EOL + PMD.EOL + "* errors: 1" + PMD.EOL + "* warnings: 0" + PMD.EOL; } - - public static junit.framework.Test suite() { - return new junit.framework.JUnit4TestAdapter(PapariTextRendererTest.class); - } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/RenderersTests.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/RenderersTests.java index b97d81e0e25..ce0ae361f77 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/RenderersTests.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/RenderersTests.java @@ -20,9 +20,12 @@ EmacsRendererTest.class, HTMLRendererTest.class, IDEAJRendererTest.class, + JsonRendererTest.class, PapariTextRendererTest.class, + SarifRendererTest.class, SummaryHTMLRendererTest.class, TextPadRendererTest.class, + TextRendererTest.class, VBHTMLRendererTest.class, XMLRendererTest.class, XSLTRendererTest.class, diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SarifRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SarifRendererTest.java index 7b803db33d3..26899a73847 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SarifRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SarifRendererTest.java @@ -6,7 +6,6 @@ import static org.junit.Assert.assertEquals; -import java.io.File; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -16,17 +15,9 @@ import org.json.JSONObject; import org.junit.Test; -import net.sourceforge.pmd.FooRule; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.ReportTest; import net.sourceforge.pmd.Rule; -import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.RulePriority; -import net.sourceforge.pmd.RuleViolation; -import net.sourceforge.pmd.lang.ast.DummyNode; -import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.rule.AbstractRule; -import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; public class SarifRendererTest extends AbstractRendererTest { @Override @@ -76,28 +67,16 @@ public String getExpectedErrorWithoutMessage(Report.ProcessingError error) { @Override public String filter(String expected) { - return expected.replaceAll("\r\n", "\n"); // make the test run on Windows, too - } - - @Override - @Test - public void testRendererMultiple() throws Exception { - Report rep = reportTwoViolations(); - String actual = ReportTest.render(getRenderer(), rep); - assertEquals(filter(getExpectedMultiple()), filter(actual)); - } - - private Report reportTwoViolations() { - Report report = new Report(); - RuleViolation informationalRuleViolation = newRuleViolation(1, "Foo"); - informationalRuleViolation.getRule().setPriority(RulePriority.LOW); - report.addRuleViolation(informationalRuleViolation); - RuleViolation severeRuleViolation = newRuleViolation(2, "Boo"); - severeRuleViolation.getRule().setPriority(RulePriority.HIGH); - report.addRuleViolation(severeRuleViolation); - return report; + return expected.replaceAll("\r\n", "\n") // make the test run on Windows, too + .replaceAll("\"version\": \".+\",", "\"version\": \"unknown\","); } + /** + * Multiple occurrences of the same rule should be reported as individual results. + * + * @see [core] SARIF formatter reports multiple locations + * when it should report multiple results #3768 + */ @Test public void testRendererMultipleLocations() throws Exception { Report rep = reportThreeViolationsTwoRules(); @@ -110,12 +89,8 @@ public void testRendererMultipleLocations() throws Exception { } private Report reportThreeViolationsTwoRules() { - Rule fooRule = new FooRule(); - fooRule.setName("Foo"); - fooRule.setPriority(RulePriority.LOW); - Rule booRule = new FooRule(); - booRule.setName("Boo"); - booRule.setPriority(RulePriority.HIGH); + Rule fooRule = createFooRule(); + Rule booRule = createBooRule(); Report report = new Report(); report.addRuleViolation(newRuleViolation(1, 1, 1, 10, fooRule)); @@ -124,27 +99,6 @@ private Report reportThreeViolationsTwoRules() { return report; } - private RuleViolation newRuleViolation(int beginLine, int beginColumn, int endLine, int endColumn, Rule rule) { - DummyNode node = new DummyNode(1); - node.testingOnlySetBeginLine(beginLine); - node.testingOnlySetBeginColumn(beginColumn); - node.testingOnlySetEndLine(endLine); - node.testingOnlySetEndColumn(endColumn); - - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - return new ParametricRuleViolation(rule, ctx, node, "blah"); - } - - private RuleViolation newRuleViolation(int endColumn, String ruleName) { - DummyNode node = createNode(endColumn); - RuleContext ctx = new RuleContext(); - ctx.setSourceCodeFile(new File(getSourceCodeFilename())); - AbstractRule fooRule = new FooRule(); - fooRule.setName(ruleName); - return new ParametricRuleViolation(fooRule, ctx, node, "blah"); - } - private String readFile(String name) { try (InputStream in = SarifRendererTest.class.getResourceAsStream("sarif/" + name)) { String asd = IOUtils.toString(in, StandardCharsets.UTF_8); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java index bcbb301431a..b4201019c0d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/SummaryHTMLRendererTest.java @@ -71,7 +71,8 @@ public String getExpectedMultiple() { return "PMD" + PMD.EOL + "

Summary

" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL - + "" + PMD.EOL + "
Rule nameNumber of violations
Foo2
" + PMD.EOL + + "Boo1" + PMD.EOL + + "Foo1" + PMD.EOL + "" + PMD.EOL + "

Detail

" + PMD.EOL + "

PMD report

Problems found

" diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextPadRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextPadRendererTest.java index c96d73c2f52..ac45b8a55b9 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextPadRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextPadRendererTest.java @@ -25,10 +25,6 @@ public String getExpectedEmpty() { @Override public String getExpectedMultiple() { - return getSourceCodeFilename() + "(1, Foo): blah" + PMD.EOL + getSourceCodeFilename() + "(1, Foo): blah" + PMD.EOL; - } - - public static junit.framework.Test suite() { - return new junit.framework.JUnit4TestAdapter(TextPadRendererTest.class); + return getSourceCodeFilename() + "(1, Foo): blah" + PMD.EOL + getSourceCodeFilename() + "(1, Boo): blah" + PMD.EOL; } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextRendererTest.java index 69983b5f852..2e85785db2d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/TextRendererTest.java @@ -28,7 +28,7 @@ public String getExpectedEmpty() { @Override public String getExpectedMultiple() { return getSourceCodeFilename() + ":1:\tFoo:\tblah" + PMD.EOL - + getSourceCodeFilename() + ":1:\tFoo:\tblah" + PMD.EOL; + + getSourceCodeFilename() + ":1:\tBoo:\tblah" + PMD.EOL; } @Override diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/VBHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/VBHTMLRendererTest.java index e2c1e15023e..7bc7118adbf 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/VBHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/VBHTMLRendererTest.java @@ -93,8 +93,4 @@ public String getExpectedError(ConfigurationError error) { + "-->

 Configuration problems found
" + error.rule().getName() + "" + error.issue() + "
" + PMD.EOL; } - - public static junit.framework.Test suite() { - return new junit.framework.JUnit4TestAdapter(VBHTMLRendererTest.class); - } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java index 6e11717f084..e5ef41c5edd 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/XMLRendererTest.java @@ -63,7 +63,7 @@ public String getExpectedMultiple() { return getHeader() + "" + PMD.EOL + "" + PMD.EOL + "blah" + PMD.EOL + "" + PMD.EOL - + "" + + "" + PMD.EOL + "blah" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL + "" + PMD.EOL; } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java index 30dadb9359d..0b0355f5d18 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/renderers/YAHTMLRendererTest.java @@ -16,7 +16,6 @@ import org.apache.commons.io.IOUtils; import org.junit.Before; -import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -26,6 +25,7 @@ import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.ReportTest; +import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.lang.ast.DummyNode; @@ -36,7 +36,7 @@ public class YAHTMLRendererTest extends AbstractRendererTest { private File outputDir; - @Rule + @org.junit.Rule public TemporaryFolder folder = new TemporaryFolder(); @Before @@ -44,8 +44,8 @@ public void setUp() throws IOException { outputDir = folder.newFolder("pmdtest"); } - private RuleViolation newRuleViolation(int endColumn, final String packageNameArg, final String classNameArg) { - DummyNode node = createNode(endColumn); + private RuleViolation newRuleViolation(int beginLine, int beginColumn, int endLine, int endColumn, final String packageNameArg, final String classNameArg) { + DummyNode node = createNode(beginLine, beginColumn, endLine, endColumn); RuleContext ctx = new RuleContext(); ctx.setSourceCodeFile(new File(getSourceCodeFilename())); return new ParametricRuleViolation(new FooRule(), ctx, node, "blah") { @@ -57,16 +57,16 @@ private RuleViolation newRuleViolation(int endColumn, final String packageNameAr } @Override - protected RuleViolation newRuleViolation(int endColumn) { - return newRuleViolation(endColumn, "net.sf.pmd.test", "YAHTMLSampleClass"); + protected RuleViolation newRuleViolation(int beginLine, int beginColumn, int endLine, int endColumn, Rule rule) { + return newRuleViolation(beginLine, beginColumn, endLine, endColumn, "net.sf.pmd.test", "YAHTMLSampleClass"); } @Test public void testReportMultipleViolations() throws Exception { Report report = new Report(); - report.addRuleViolation(newRuleViolation(1, "net.sf.pmd.test", "YAHTMLSampleClass1")); - report.addRuleViolation(newRuleViolation(2, "net.sf.pmd.test", "YAHTMLSampleClass1")); - report.addRuleViolation(newRuleViolation(1, "net.sf.pmd.other", "YAHTMLSampleClass2")); + report.addRuleViolation(newRuleViolation(1, 1, 1, 1, "net.sf.pmd.test", "YAHTMLSampleClass1")); + report.addRuleViolation(newRuleViolation(1, 1, 1, 2, "net.sf.pmd.test", "YAHTMLSampleClass1")); + report.addRuleViolation(newRuleViolation(1, 1, 1, 1, "net.sf.pmd.other", "YAHTMLSampleClass2")); String actual = ReportTest.render(getRenderer(), report); assertEquals(filter(getExpected()), filter(actual)); diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/json/expected-multiple.json b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/json/expected-multiple.json index 9ab3704f1e1..127a1d535af 100644 --- a/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/json/expected-multiple.json +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/renderers/json/expected-multiple.json @@ -22,7 +22,7 @@ "endline": 1, "endcolumn": 2, "description": "blah", - "rule": "Foo", + "rule": "Boo", "ruleset": "RuleSet", "priority": 1 }