From c7d88ec506b365b1b53ef760e4bb03862613fd80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 30 Jul 2020 02:27:55 +0200 Subject: [PATCH] Some sugar --- .../lang/apex/rule/AbstractApexRuleTest.java | 15 ++- .../main/java/net/sourceforge/pmd/Report.java | 4 +- .../java/net/sourceforge/pmd/RuleContext.java | 67 +++++-------- .../sourceforge/pmd/lang/ParserOptions.java | 7 +- .../pmd/lang/ast/TokenMgrError.java | 1 + .../ast/impl/javacc/JjtreeParserAdapter.java | 1 + .../pmd/lang/rule/AbstractRule.java | 2 + .../pmd/processor/GlobalAnalysisListener.java | 3 +- .../pmd/processor/PmdRunnable.java | 4 +- .../net/sourceforge/pmd/AbstractRuleTest.java | 15 ++- .../net/sourceforge/pmd/RuleContextTest.java | 26 ++--- .../java/net/sourceforge/pmd/RuleSetTest.java | 94 +++++++------------ .../pmd/lang/rule/XPathRuleTest.java | 46 ++++----- .../renderers/SummaryHTMLRendererTest.java | 17 ++-- .../pmd/lang/java/rule/XPathRuleTest.java | 20 ++-- .../EcmascriptParserOptionsTest.java | 4 +- .../ecmascript/ast/EcmascriptParserTest.java | 7 +- .../pmd/lang/ast/test/BaseParsingHelper.kt | 2 +- .../pmd/lang/ast/test/TestUtils.kt | 27 ++++++ .../pmd/lang/plsql/PLSQLXPathRuleTest.java | 2 +- .../pmd/testframework/RuleTst.java | 6 +- .../pmd/testframework/RuleTstTest.java | 4 +- 22 files changed, 174 insertions(+), 200 deletions(-) diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java index ff753e0324..9f3a64d920 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java @@ -8,8 +8,7 @@ import static org.junit.Assert.assertEquals; import org.junit.Test; -import net.sourceforge.pmd.Report.ReportBuilderListener; -import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.Report; import net.sourceforge.pmd.lang.apex.ast.ASTAnonymousClass; import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; import net.sourceforge.pmd.lang.apex.ast.ASTUserEnum; @@ -17,6 +16,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTUserInterface; import net.sourceforge.pmd.lang.apex.ast.ASTUserTrigger; import net.sourceforge.pmd.lang.apex.ast.ApexNode; import net.sourceforge.pmd.lang.apex.ast.ApexParserTestBase; +import net.sourceforge.pmd.lang.ast.test.TestUtilsKt; import apex.jorje.semantic.ast.compilation.Compilation; @@ -44,14 +44,11 @@ public class AbstractApexRuleTest extends ApexParserTestBase { private void run(String code) throws Exception { ApexNode node = parse(code); + TopLevelRule rule = new TopLevelRule(); + rule.setMessage("Message"); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - TopLevelRule rule = new TopLevelRule(); - rule.setMessage("Message"); - rule.apply(node, ctx); - } - assertEquals(1, reportBuilder.getReport().getViolations().size()); + Report report = TestUtilsKt.makeReport(ctx -> rule.apply(node, ctx)); + assertEquals(1, report.getViolations().size()); } private static class TopLevelRule extends AbstractApexRule { diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java index df6545b72a..4520818653 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/Report.java @@ -13,6 +13,8 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import org.checkerframework.checker.nullness.qual.NonNull; + import net.sourceforge.pmd.processor.FileAnalysisListener; import net.sourceforge.pmd.processor.GlobalAnalysisListener; import net.sourceforge.pmd.renderers.AbstractAccumulatingRenderer; @@ -262,7 +264,7 @@ public class Report { * * @throws IllegalStateException If {@link #close()} has not been called yet */ - public Report getReport() { + public @NonNull Report getReport() { if (!done) { throw new IllegalStateException("Reporting not done"); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java index 3c7c96831f..8b9f27f277 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java @@ -18,22 +18,12 @@ import net.sourceforge.pmd.lang.rule.RuleViolationFactory; import net.sourceforge.pmd.processor.FileAnalysisListener; /** - * The RuleContext provides access to Rule processing state. This information - * includes the following global information: - * - * As well as the following source file specific information: - * - * It is required that all source file specific options be set - * between calls to difference source files. Failure to do so, may result in - * undefined behavior. + * The API for rules to report violations or errors during analysis. + * This forwards events to a {@link FileAnalysisListener}. It implements + * violation suppression by filtering some violations out, according to + * the {@link ViolationSuppressor}s for the language. */ -public class RuleContext implements AutoCloseable { +public final class RuleContext implements AutoCloseable { private static final Object[] NO_ARGS = new Object[0]; @@ -41,15 +31,7 @@ public class RuleContext implements AutoCloseable { private final FileAnalysisListener listener; - /** - * Default constructor. - */ - @Deprecated - public RuleContext() { - this(FileAnalysisListener.noop()); - } - - public RuleContext(FileAnalysisListener listener) { + private RuleContext(FileAnalysisListener listener) { this.listener = listener; } @@ -80,17 +62,12 @@ public class RuleContext implements AutoCloseable { addViolationWithPosition(rule, location, -1, -1, message, formatArgs); } - public void addViolationNoSuppress(RuleViolation rv) { - listener.onRuleViolation(rv); - } - public void addViolationWithPosition(Rule rule, Node location, int beginLine, int endLine, String message, Object... formatArgs) { Objects.requireNonNull(rule); Objects.requireNonNull(location); Objects.requireNonNull(message); Objects.requireNonNull(formatArgs); - // at some point each Node will know its language version RuleViolationFactory fact = location.getLanguageVersion().getLanguageVersionHandler().getRuleViolationFactory(); RuleViolation violation = fact.createViolation(rule, location, location.getSourceCodeFile(), makeMessage(message, formatArgs)); @@ -108,6 +85,10 @@ public class RuleContext implements AutoCloseable { } } + public void addViolationNoSuppress(RuleViolation rv) { + listener.onRuleViolation(rv); + } + private String makeMessage(@NonNull String message, Object[] args) { // Escape PMD specific variable message format, specifically the { // in the ${, so MessageFormat doesn't bitch. @@ -116,22 +97,6 @@ public class RuleContext implements AutoCloseable { } - /** - * Configure whether exceptions during applying a rule should be ignored or - * not. If set to true then such exceptions are logged as - * warnings and the processing is continued with the next rule - the failing - * rule is simply skipped. This is the default behavior.
- * If set to false then the processing will be aborted with the - * exception. This is especially useful during unit tests, in order to not - * oversee any exceptions. - * - * @param ignoreExceptions - * if true simply skip failing rules (default). - */ - public void setIgnoreExceptions(boolean ignoreExceptions) { - this.ignoreExceptions = ignoreExceptions; - } - /** * Gets the configuration whether to skip failing rules (true) * or whether to throw a a RuntimeException and abort the processing for the @@ -139,9 +104,21 @@ public class RuleContext implements AutoCloseable { * * @return true when failing rules are skipped, * false otherwise. + * + * TODO this looks only useful in unit tests... */ public boolean isIgnoreExceptions() { return ignoreExceptions; } + + public static RuleContext create(FileAnalysisListener listener) { + return new RuleContext(listener); + } + + public static RuleContext createThrowingExceptions(FileAnalysisListener listener) { + RuleContext ruleContext = new RuleContext(listener); + ruleContext.ignoreExceptions = false; + return ruleContext; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ParserOptions.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ParserOptions.java index e379d05e5f..3a9085732a 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ParserOptions.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ParserOptions.java @@ -4,6 +4,10 @@ package net.sourceforge.pmd.lang; +import java.util.Objects; + +import net.sourceforge.pmd.PMD; + /** * Represents a set of configuration options for a {@link Parser}. For each * unique combination of ParserOptions a Parser will be used to create an AST. @@ -11,13 +15,14 @@ package net.sourceforge.pmd.lang; * {@link Object#hashCode()}. */ public class ParserOptions { - protected String suppressMarker; + protected String suppressMarker = PMD.SUPPRESS_MARKER; public String getSuppressMarker() { return suppressMarker; } public void setSuppressMarker(String suppressMarker) { + Objects.requireNonNull(suppressMarker); this.suppressMarker = suppressMarker; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java index acfca312cc..3625251bbe 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/TokenMgrError.java @@ -68,6 +68,7 @@ public final class TokenMgrError extends FileAnalysisException { * * @return A new exception */ + @Override public TokenMgrError setFileName(String filename) { super.setFileName(filename); return this; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java index b5d6b0e0ae..d5973b6bfb 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/javacc/JjtreeParserAdapter.java @@ -20,6 +20,7 @@ import net.sourceforge.pmd.lang.ast.TokenMgrError; public abstract class JjtreeParserAdapter implements Parser { protected JjtreeParserAdapter() { + // inheritance only } protected abstract JavaccTokenDocument newDocument(String fullText); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java index 9f0737a473..ec73d0bfa3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRule.java @@ -287,6 +287,8 @@ public abstract class AbstractRule extends AbstractPropertySource implements Rul // Override as needed } + // TODO remove those methods, make Rules have type-safe access to a RuleContext + /** * @see RuleContext#addViolation(Rule, Node, Object...) */ diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/GlobalAnalysisListener.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/GlobalAnalysisListener.java index 8781a40c9a..bca933d84c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/GlobalAnalysisListener.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/GlobalAnalysisListener.java @@ -71,6 +71,7 @@ public interface GlobalAnalysisListener extends AutoCloseable { } @Override + @SuppressWarnings("PMD.CloseResource") // false-positive public void close() throws Exception { Exception composed = null; for (GlobalAnalysisListener it : listeners) { @@ -118,7 +119,7 @@ public interface GlobalAnalysisListener extends AutoCloseable { @Override public void close() { - + // nothing to do } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java index 97e06f25d7..8b9b806486 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/processor/PmdRunnable.java @@ -85,7 +85,7 @@ public class PmdRunnable implements Runnable { LOCAL_THREAD_CONTEXT.set(tc); } - try (RuleContext ruleCtx = new RuleContext(ruleContext.startFileAnalysis(dataSource))) { + try (RuleContext ruleCtx = RuleContext.create(ruleContext.startFileAnalysis(dataSource))) { LanguageVersion langVersion = configuration.getLanguageVersionOfFile(file.getPath()); if (LOG.isLoggable(Level.FINE)) { @@ -106,7 +106,7 @@ public class PmdRunnable implements Runnable { if (ruleCtx.isIgnoreExceptions()) { LOG.log(Level.FINE, "Exception while processing file: " + file, e); } else { - if (e instanceof FileAnalysisException) { + if (e instanceof FileAnalysisException) { // NOPMD AvoidInstanceofChecksInCatchClause throw (FileAnalysisException) e; } throw new FileAnalysisException(e); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java index 4b5fc4ff33..09d855ce3e 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/AbstractRuleTest.java @@ -14,7 +14,6 @@ import java.util.Map; import org.junit.Test; -import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.Report.SuppressedViolation; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.DummyRoot; @@ -99,14 +98,12 @@ public class AbstractRuleTest { MyRule r = new MyRule(); r.definePropertyDescriptor(PropertyFactory.intProperty("testInt").desc("description").require(inRange(0, 100)).defaultValue(10).build()); r.setMessage("Message ${packageName} ${className} ${methodName} ${variableName} ${testInt} ${noSuchProperty}"); - ReportBuilderListener listener = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(listener)) { - DummyNode s = new DummyRoot().withFileName("filename"); - s.setCoords(5, 1, 6, 1); - s.setImage("TestImage"); - r.addViolation(ctx, s); - } - RuleViolation rv = listener.getReport().getViolations().get(0); + + DummyNode s = new DummyRoot().withFileName("filename"); + s.setCoords(5, 1, 6, 1); + s.setImage("TestImage"); + + RuleViolation rv = RuleContextTest.getReport(ctx -> r.addViolation(ctx, s)).getViolations().get(0); assertEquals("Message foo 10 ${noSuchProperty}", rv.getDescription()); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java index 0229ccaf80..2bad350cb2 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleContextTest.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd; +import java.util.function.Consumer; + import org.junit.Assert; import org.junit.Test; @@ -14,26 +16,26 @@ import junit.framework.JUnit4TestAdapter; public class RuleContextTest { + public static Report getReport(Consumer sideEffects) throws Exception { + ReportBuilderListener listener = new ReportBuilderListener(); + try (RuleContext ctx = RuleContext.create(listener)) { + sideEffects.accept(ctx); + } + return listener.getReport(); + } + @Test public void testMessage() throws Exception { - ReportBuilderListener listener = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(listener)) { - ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with \"'{'\""); - } + Report report = getReport(ctx -> ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with \"'{'\"")); - RuleViolation violation = listener.getReport().getViolations().get(0); - Assert.assertEquals("message with \"{\"", violation.getDescription()); + Assert.assertEquals("message with \"{\"", report.getViolations().get(0).getDescription()); } @Test public void testMessageArgs() throws Exception { - ReportBuilderListener listener = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(listener)) { - ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with 1 argument: \"{0}\"", "testarg1"); - } + Report report = getReport(ctx -> ctx.addViolationWithMessage(new FooRule(), DummyTreeUtil.tree(DummyTreeUtil::root), "message with 1 argument: \"{0}\"", "testarg1")); - RuleViolation violation = listener.getReport().getViolations().get(0); - Assert.assertEquals("message with 1 argument: \"testarg1\"", violation.getDescription()); + Assert.assertEquals("message with 1 argument: \"testarg1\"", report.getViolations().get(0).getDescription()); } public static junit.framework.Test suite() { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java index 37c08a3dd3..aaabaeec43 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -29,7 +29,6 @@ import org.checkerframework.checker.nullness.qual.NonNull; import org.junit.Test; import net.sourceforge.pmd.Report.ProcessingError; -import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.RuleSet.RuleSetBuilder; import net.sourceforge.pmd.lang.Dummy2LanguageModule; import net.sourceforge.pmd.lang.DummyLanguageModule; @@ -173,9 +172,9 @@ public class RuleSetTest { } @Test - public void testApply0Rules() { + public void testApply0Rules() throws Exception { RuleSet ruleset = createRuleSetBuilder("ruleset").build(); - verifyRuleSet(ruleset, 0, new HashSet()); + verifyRuleSet(ruleset, new HashSet()); } @Test @@ -409,14 +408,12 @@ public class RuleSetTest { RuleSet ruleSet1 = createRuleSetBuilder("RuleSet1").addRule(rule).build(); RuleSet ruleSet2 = createRuleSetBuilder("RuleSet2").addRule(rule).build(); + RuleSets ruleSets = new RuleSets(listOf(ruleSet1, ruleSet2)); // Two violations - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - ruleSets.apply(makeCompilationUnits(), ctx); - } - assertEquals("Violations", 2, reportBuilder.getReport().getViolations().size()); + Report report = RuleContextTest.getReport(ctx -> ruleSets.apply(makeCompilationUnits(), ctx)); + assertEquals("Violations", 2, report.getViolations().size()); // One violation ruleSet1 = createRuleSetBuilder("RuleSet1") @@ -424,13 +421,10 @@ public class RuleSetTest { .addRule(rule) .build(); - ruleSets = new RuleSets(listOf(ruleSet1, ruleSet2)); + RuleSets ruleSets2 = new RuleSets(listOf(ruleSet1, ruleSet2)); - reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - ruleSets.apply(makeCompilationUnits("C:\\myworkspace\\project\\some\\random\\package\\RandomClass.java"), ctx); - } - assertEquals("Violations", 1, reportBuilder.getReport().getViolations().size()); + report = RuleContextTest.getReport(ctx -> ruleSets2.apply(makeCompilationUnits("C:\\package\\RandomClass.java"), ctx)); + assertEquals("Violations", 1, report.getViolations().size()); } @Test @@ -449,24 +443,18 @@ public class RuleSetTest { assertNotSame(rule, ruleSet2.getRuleByName("FooRule1")); } - private void verifyRuleSet(RuleSet ruleset, int size, Set values) throws Exception { + private void verifyRuleSet(RuleSet ruleset, Set expected) throws Exception { - Set reportedValues = new HashSet<>(); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext context = new RuleContext(reportBuilder)) { - new RuleSets(ruleset).apply(makeCompilationUnits(), context); + Report report = RuleContextTest.getReport(ctx -> ruleset.apply(makeCompilationUnits(), ctx)); + assertEquals("Invalid number of Violations Reported", expected.size(), report.getViolations().size()); + + for (RuleViolation violation : report.getViolations()) { + assertTrue("Unexpected Violation Returned: " + violation, expected.contains(violation)); } - assertEquals("Invalid number of Violations Reported", size, reportBuilder.getReport().getViolations().size()); - - for (RuleViolation violation : reportBuilder.getReport().getViolations()) { - reportedValues.add(violation); - assertTrue("Unexpected Violation Returned: " + violation, values.contains(violation)); - } - - for (RuleViolation violation : values) { - assertTrue("Expected Violation not Returned: " + violation, reportedValues.contains(violation)); + for (RuleViolation violation : expected) { + assertTrue("Expected Violation not Returned: " + violation, report.getViolations().contains(violation)); } } @@ -492,13 +480,10 @@ public class RuleSetTest { } }) .build(); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext context = new RuleContext(reportBuilder)) { - context.setIgnoreExceptions(true); // the default - ruleset.apply(makeCompilationUnits(), context); - } - List errors = reportBuilder.getReport().getProcessingErrors(); + Report report = RuleContextTest.getReport(ctx -> ruleset.apply(makeCompilationUnits(), ctx)); + + List errors = report.getProcessingErrors(); assertFalse("Report should have processing errors", errors.isEmpty()); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); @@ -508,15 +493,14 @@ public class RuleSetTest { @Test(expected = RuntimeException.class) public void ruleExceptionShouldBeThrownIfNotIgnored() { RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported") - .addRule(new MockRule() { - @Override - public void apply(Node target, RuleContext ctx) { - throw new RuntimeException("Test exception while applying rule"); - } - }) - .build(); - RuleContext context = new RuleContext(FileAnalysisListener.noop()); - context.setIgnoreExceptions(false); + .addRule(new MockRule() { + @Override + public void apply(Node target, RuleContext ctx) { + throw new RuntimeException("Test exception while applying rule"); + } + }) + .build(); + RuleContext context = RuleContext.createThrowingExceptions(FileAnalysisListener.noop()); ruleset.apply(makeCompilationUnits(), context); } @@ -533,18 +517,16 @@ public class RuleSetTest { addViolationWithMessage(ctx, target, "Test violation of the second rule in the ruleset"); } }).build(); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext context = new RuleContext(reportBuilder)) { - context.setIgnoreExceptions(true); // the default - ruleset.apply(makeCompilationUnits(), context); - } - List errors = reportBuilder.getReport().getProcessingErrors(); + + Report report = RuleContextTest.getReport(ctx -> ruleset.apply(makeCompilationUnits(), ctx)); + + List errors = report.getProcessingErrors(); assertFalse("Report should have processing errors", errors.isEmpty()); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); - assertEquals("There should be a violation", 1, reportBuilder.getReport().getViolations().size()); + assertEquals("There should be a violation", 1, report.getViolations().size()); } @Test @@ -572,21 +554,15 @@ public class RuleSetTest { addViolationWithMessage(ctx, target, "Test violation of the second rule in the ruleset"); } }).build(); - RuleSets rulesets = new RuleSets(ruleset); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext context = new RuleContext(reportBuilder)) { - context.setIgnoreExceptions(true); // the default - rulesets.apply(makeCompilationUnits(), context); - } + Report report = RuleContextTest.getReport(ctx -> ruleset.apply(makeCompilationUnits(), ctx)); - - List errors = reportBuilder.getReport().getProcessingErrors(); + List errors = report.getProcessingErrors(); assertEquals("Errors expected", 1, errors.size()); assertEquals("Wrong error message", "RuntimeException: Test exception while applying rule", errors.get(0).getMsg()); assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); - assertEquals("There should be a violation", 1, reportBuilder.getReport().getViolations().size()); + assertEquals("There should be a violation", 1, report.getViolations().size()); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java index 6e13547dcc..780f1bdf02 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/XPathRuleTest.java @@ -11,8 +11,8 @@ import org.hamcrest.Matchers; import org.junit.Rule; import org.junit.Test; -import net.sourceforge.pmd.Report.ReportBuilderListener; -import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.RuleContextTest; import net.sourceforge.pmd.junit.JavaUtilLoggingRule; import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.ast.DummyNodeWithDeprecatedAttribute; @@ -43,13 +43,9 @@ public class XPathRuleTest { DummyNode firstNode = newNode(); - ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - // with another rule forked from the same one (in multithreaded processor) - xpr.apply(firstNode, ctx); - } - - assertEquals(1, reportBuilder.getReport().getViolations().size()); + // with another rule forked from the same one (in multithreaded processor) + Report report = RuleContextTest.getReport(ctx -> xpr.apply(firstNode, ctx)); + assertEquals(1, report.getViolations().size()); String log = loggingRule.getLog(); assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by XPath rule 'SomeRule'")); @@ -58,36 +54,28 @@ public class XPathRuleTest { loggingRule.clear(); - reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - // with another node - xpr.apply(newNode(), ctx); - } + // with another node + report = RuleContextTest.getReport(ctx -> xpr.apply(newNode(), ctx)); - assertEquals(1, reportBuilder.getReport().getViolations().size()); + assertEquals(1, report.getViolations().size()); assertEquals("", loggingRule.getLog()); // no additional warnings - reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - // with another rule forked from the same one (in multithreaded processor) - xpr.deepCopy().apply(newNode(), ctx); - } + // with another rule forked from the same one (in multithreaded processor) + report = RuleContextTest.getReport(ctx -> xpr.deepCopy().apply(newNode(), ctx)); - assertEquals(1, reportBuilder.getReport().getViolations().size()); + assertEquals(1, report.getViolations().size()); assertEquals("", loggingRule.getLog()); // no additional warnings - reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(reportBuilder)) { - // with another rule on the same node, new warnings - XPathRule otherRule = makeRule(version, "OtherRule"); - otherRule.setRuleSetName("rset.xml"); - otherRule.apply(firstNode, ctx); - } + // with another rule on the same node, new warnings + XPathRule otherRule = makeRule(version, "OtherRule"); + otherRule.setRuleSetName("rset.xml"); - assertEquals(1, reportBuilder.getReport().getViolations().size()); + report = RuleContextTest.getReport(ctx -> otherRule.apply(firstNode, ctx)); + + assertEquals(1, report.getViolations().size()); log = loggingRule.getLog(); assertThat(log, Matchers.containsString("Use of deprecated attribute 'dummyNode/@Size' by XPath rule 'OtherRule' (in ruleset 'rset.xml')")); 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 4258a2e38e..24e3e849cc 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 @@ -7,7 +7,6 @@ package net.sourceforge.pmd.renderers; import static org.junit.Assert.assertEquals; import java.util.Collections; -import java.util.Map; import org.junit.Test; @@ -16,9 +15,8 @@ import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.ConfigurationError; import net.sourceforge.pmd.Report.ProcessingError; -import net.sourceforge.pmd.Report.ReportBuilderListener; import net.sourceforge.pmd.ReportTest; -import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.RuleContextTest; import net.sourceforge.pmd.lang.ast.DummyRoot; public class SummaryHTMLRendererTest extends AbstractRendererTest { @@ -147,13 +145,10 @@ public class SummaryHTMLRendererTest extends AbstractRendererTest { } private Report createEmptyReportWithSuppression() throws Exception { - Map suppressions = Collections.singletonMap(1, "test"); - ReportBuilderListener listener = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext(listener)) { - DummyRoot root = new DummyRoot(suppressions).withFileName(""); - root.setCoords(1, 10, 4, 5); - ctx.addViolationWithPosition(new FooRule(), root, 1, 1, "suppress test"); - } - return listener.getReport(); + + DummyRoot root = new DummyRoot(Collections.singletonMap(1, "test")).withFileName(""); + root.setCoords(1, 10, 4, 5); + + return RuleContextTest.getReport(ctx -> ctx.addViolationWithPosition(new FooRule(), root, 1, 1, "suppress test")); } } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java index 697c9c4f3a..5944e2f7c7 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/XPathRuleTest.java @@ -115,18 +115,16 @@ public class XPathRuleTest extends RuleTst { + " public static void main(String args[]) {\n" + " new File(\"subdirectory\").list();\n" + " }\n" + "}"; ASTCompilationUnit cu = JavaParsingHelper.WITH_PROCESSING.parse(SUFFIX); - try (RuleContext ruleContext = new RuleContext(FileAnalysisListener.noop())) { - String xpath = "//PrimarySuffix[@Image='list']"; + String xpath = "//PrimarySuffix[@Image='list']"; - SaxonXPathRuleQuery xpathRuleQuery = new SaxonXPathRuleQuery(xpath, - XPathVersion.DEFAULT, - new HashMap<>(), - XPathHandler.noFunctionDefinitions(), - DeprecatedAttrLogger.noop()); - List nodes = xpathRuleQuery.evaluate(cu); - assertEquals(1, nodes.size()); - } + SaxonXPathRuleQuery xpathRuleQuery = new SaxonXPathRuleQuery(xpath, + XPathVersion.DEFAULT, + new HashMap<>(), + XPathHandler.noFunctionDefinitions(), + DeprecatedAttrLogger.noop()); + List nodes = xpathRuleQuery.evaluate(cu); + assertEquals(1, nodes.size()); } /** @@ -144,7 +142,7 @@ public class XPathRuleTest extends RuleTst { + " }\n" + "}"; ASTCompilationUnit cu = JavaParsingHelper.WITH_PROCESSING.parse(source); - try (RuleContext ruleContext = new RuleContext(FileAnalysisListener.noop())) { + try (RuleContext ruleContext = RuleContext.create(FileAnalysisListener.noop())) { String xpath = "//Block/BlockStatement/following-sibling::BlockStatement"; diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/EcmascriptParserOptionsTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/EcmascriptParserOptionsTest.java index b343d5bbdc..382fe1ad5d 100644 --- a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/EcmascriptParserOptionsTest.java +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/EcmascriptParserOptionsTest.java @@ -8,7 +8,6 @@ import static net.sourceforge.pmd.lang.ParserOptionsTest.verifyOptionsEqualsHash import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import java.util.List; @@ -58,14 +57,13 @@ public class EcmascriptParserOptionsTest { ((EcmascriptParserOptions) rule.getParserOptions()).getRhinoLanguageVersion()); } - @Test + @Test(expected = NullPointerException.class) public void testSetters() { EcmascriptParserOptions options = new EcmascriptParserOptions(); options.setSuppressMarker("foo"); assertEquals("foo", options.getSuppressMarker()); options.setSuppressMarker(null); - assertNull(options.getSuppressMarker()); } @Test diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java index 5195e6c131..d39b5dfbd9 100644 --- a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java @@ -19,6 +19,7 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ecmascript.EcmascriptParserOptions; import net.sourceforge.pmd.lang.ecmascript.rule.AbstractEcmascriptRule; +import net.sourceforge.pmd.processor.FileAnalysisListener; public class EcmascriptParserTest extends EcmascriptParserTestBase { @@ -64,7 +65,7 @@ public class EcmascriptParserTest extends EcmascriptParserTestBase { } MyEcmascriptRule rule = new MyEcmascriptRule(); - RuleContext ctx = new RuleContext(); + RuleContext ctx = RuleContext.create(FileAnalysisListener.noop()); rule.apply(js.parse(source), ctx); assertEquals("Scope from 2 to 4", output.get(0)); @@ -138,7 +139,9 @@ public class EcmascriptParserTest extends EcmascriptParserTestBase { */ @Test public void testSuppressionComment() { - ASTAstRoot root = js.parse("function(x) {\n" + "x = x; //NOPMD I know what I'm doing\n" + "}\n"); + ASTAstRoot root = js.parse("function(x) {\n" + + "x = x; //NOPMD I know what I'm doing\n" + + "}\n"); assertEquals(" I know what I'm doing", root.getNoPmdComments().get(2)); assertEquals(1, root.getNoPmdComments().size()); diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt index fc5c9440a9..beb637f250 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt @@ -113,7 +113,7 @@ abstract class BaseParsingHelper, T : RootNode val handler = lversion.languageVersionHandler val options = params.parserOptions ?: handler.defaultParserOptions val parser = handler.getParser(options) - val task = Parser.ParserTask(lversion, "test-file", sourceCode, SemanticErrorReporter.noop()) + val task = Parser.ParserTask(lversion, "test-file", sourceCode, SemanticErrorReporter.noop(), options.suppressMarker) val rootNode = rootClass.cast(parser.parse(task)) if (params.doProcess) { postProcessing(handler, lversion, rootNode) diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/TestUtils.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/TestUtils.kt index 68b2e51469..cc72ea36ea 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/TestUtils.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/TestUtils.kt @@ -8,6 +8,11 @@ import io.kotest.matchers.Matcher import io.kotest.matchers.collections.haveSize import io.kotest.matchers.equalityMatcher import io.kotest.matchers.should +import net.sourceforge.pmd.* +import net.sourceforge.pmd.processor.PmdRunnable +import net.sourceforge.pmd.util.CollectionUtil +import net.sourceforge.pmd.util.datasource.DataSource +import java.util.function.Consumer import java.util.stream.Stream import kotlin.reflect.KCallable import kotlin.reflect.jvm.isAccessible @@ -75,3 +80,25 @@ inline fun Any?.shouldBeA(f: (T) -> Unit = {}): T { fun Stream<*>.shouldHaveSize(i: Int) { toList() should haveSize(i) } + + +/** + * Make a report by side-effecting on a rule context. + */ +@JvmSynthetic // hide from Java consumers, Kotlin users can still use it +fun makeReport(sideEffects: (RuleContext) -> Unit): Report = + makeReport(Consumer(sideEffects)) + +/** + * Make a report by side-effecting on a rule context. + */ +fun makeReport(sideEffects: Consumer): Report { + val reportBuilder = Report.ReportBuilderListener() + val ctx = RuleContext.create(reportBuilder) + + sideEffects.accept(ctx) + + ctx.close() + + return reportBuilder.report +} diff --git a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/PLSQLXPathRuleTest.java b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/PLSQLXPathRuleTest.java index a585591d04..919e817971 100644 --- a/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/PLSQLXPathRuleTest.java +++ b/pmd-plsql/src/test/java/net/sourceforge/pmd/lang/plsql/PLSQLXPathRuleTest.java @@ -58,7 +58,7 @@ public class PLSQLXPathRuleTest extends AbstractPLSQLParserTst { rule.setMessage("Test Violation"); ReportBuilderListener reportBuilder = new ReportBuilderListener(); - try (RuleContext ctx = new RuleContext()) { + try (RuleContext ctx = RuleContext.create(reportBuilder)) { rule.apply(node, ctx); } diff --git a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java index a25e24da09..d8255751ec 100644 --- a/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java +++ b/pmd-test/src/main/java/net/sourceforge/pmd/testframework/RuleTst.java @@ -38,7 +38,9 @@ import net.sourceforge.pmd.PMDException; import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Report.GlobalReportBuilder; import net.sourceforge.pmd.Rule; +import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.RuleSetNotFoundException; +import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; import net.sourceforge.pmd.RulesetsFactoryUtils; import net.sourceforge.pmd.lang.Language; @@ -99,7 +101,8 @@ public abstract class RuleTst { */ public Rule findRule(String ruleSet, String ruleName) { try { - Rule rule = RulesetsFactoryUtils.defaultFactory().createRuleSets(ruleSet).getRuleByName(ruleName); + List ruleSets = RulesetsFactoryUtils.defaultFactory().createRuleSets(ruleSet); + Rule rule = new RuleSets(ruleSets).getRuleByName(ruleName); if (rule == null) { fail("Rule " + ruleName + " not found in ruleset " + ruleSet); } else { @@ -265,6 +268,7 @@ public abstract class RuleTst { PMDConfiguration config = new PMDConfiguration(); config.setIgnoreIncrementalAnalysis(true); config.setDefaultLanguageVersion(languageVersion); + config.setThreads(1); if (isUseAuxClasspath) { // configure the "auxclasspath" option for unit testing diff --git a/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java b/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java index 59e5508c18..6fc53ba2a8 100644 --- a/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java +++ b/pmd-test/src/test/java/net/sourceforge/pmd/testframework/RuleTstTest.java @@ -8,7 +8,6 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.Arrays; @@ -43,6 +42,7 @@ public class RuleTstTest { when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage()); when(rule.getName()).thenReturn("test rule"); when(rule.getTargetSelector()).thenReturn(RuleTargetSelector.forRootOnly()); + when(rule.deepCopy()).thenReturn(rule); Report report = ruleTester.runTestFromString("the code", rule, dummyLanguage, false); @@ -55,7 +55,6 @@ public class RuleTstTest { verify(rule).apply(any(Node.class), any(RuleContext.class)); verify(rule, times(4)).getName(); verify(rule).getPropertiesByPropertyDescriptor(); - verifyNoMoreInteractions(rule); } @Test @@ -63,6 +62,7 @@ public class RuleTstTest { when(rule.getLanguage()).thenReturn(dummyLanguage.getLanguage()); when(rule.getName()).thenReturn("test rule"); when(rule.getTargetSelector()).thenReturn(RuleTargetSelector.forRootOnly()); + when(rule.deepCopy()).thenReturn(rule); Mockito.doAnswer(new Answer() { private RuleViolation createViolation(int beginLine, String message) {