From f7735814595d9c0b0f19264e3c46e5680f10bcf7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 13 Aug 2018 11:15:57 +0200 Subject: [PATCH] [core] PMD stops processing file completely, if one rule in a rule chain fails Fixes #1300 --- docs/pages/release_notes.md | 1 + .../lang/rule/AbstractRuleChainVisitor.java | 16 +++++ .../java/net/sourceforge/pmd/RuleSetTest.java | 72 +++++++++++++++++++ .../xpath/XPathMetricFunctionTest.java | 1 + 4 files changed, 90 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b341ed6703..7b78a1a024 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -36,6 +36,7 @@ This is a minor release. * core * [#1191](https://github.com/pmd/pmd/issues/1191): \[core] Test Framework: Sort violations by line/column * [#1288](https://github.com/pmd/pmd/issues/1288): \[core] No supported build listeners found with Gradle + * [#1300](https://github.com/pmd/pmd/issues/1300): \[core] PMD stops processing file completely, if one rule in a rule chain fails * java-bestpractices * [#1267](https://github.com/pmd/pmd/pull/1267): \[java] MissingOverrideRule: Avoid NoClassDefFoundError with incomplete classpath * java-codestyle diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleChainVisitor.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleChainVisitor.java index f08c44635a..bb3c5fb3d8 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleChainVisitor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/AbstractRuleChainVisitor.java @@ -12,7 +12,10 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.logging.Level; +import java.util.logging.Logger; +import net.sourceforge.pmd.Report; import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.RuleSet; @@ -27,6 +30,8 @@ import net.sourceforge.pmd.lang.ast.Node; * expressed interest in. */ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { + private static final Logger LOG = Logger.getLogger(AbstractRuleChainVisitor.class.getName()); + /** * These are all the rules participating in the RuleChain, grouped by * RuleSet. @@ -93,6 +98,17 @@ public abstract class AbstractRuleChainVisitor implements RuleChainVisitor { visits += ns.size(); } rcto.close(visits); + } catch (RuntimeException e) { + if (ctx.isIgnoreExceptions()) { + ctx.getReport().addError(new Report.ProcessingError(e, ctx.getSourceCodeFilename())); + + if (LOG.isLoggable(Level.WARNING)) { + LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " + + ctx.getSourceCodeFilename() + ", continuing with next rule", e); + } + } else { + throw e; + } } } } 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 429136f67b..3d47494e86 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -551,4 +551,76 @@ public class RuleSetTest { context.setIgnoreExceptions(false); ruleset.apply(makeCompilationUnits(), context); } + + @Test + public void ruleExceptionShouldNotStopProcessingFile() { + RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported").addRule(new MockRule() { + @Override + public void apply(List nodes, RuleContext ctx) { + throw new RuntimeException("Test exception while applying rule"); + } + }).addRule(new MockRule() { + @Override + public void apply(List nodes, RuleContext ctx) { + for (Node node : nodes) { + addViolationWithMessage(ctx, node, "Test violation of the second rule in the ruleset"); + } + } + }).build(); + RuleContext context = new RuleContext(); + context.setReport(new Report()); + context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setSourceCodeFilename(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported"); + context.setIgnoreExceptions(true); // the default + ruleset.apply(makeCompilationUnits(), context); + + assertTrue("Report should have processing errors", context.getReport().hasErrors()); + List errors = CollectionUtil.toList(context.getReport().errors()); + assertEquals("Errors expected", 1, errors.size()); + assertEquals("Wrong error message", "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, context.getReport().size()); + } + + @Test + public void ruleExceptionShouldNotStopProcessingFileWithRuleChain() { + RuleSet ruleset = createRuleSetBuilder("ruleExceptionShouldBeReported").addRule(new MockRule() { + { + addRuleChainVisit("dummyNode"); + } + + @Override + public void apply(List nodes, RuleContext ctx) { + throw new RuntimeException("Test exception while applying rule"); + } + }).addRule(new MockRule() { + { + addRuleChainVisit("dummyNode"); + } + + @Override + public void apply(List nodes, RuleContext ctx) { + for (Node node : nodes) { + addViolationWithMessage(ctx, node, "Test violation of the second rule in the ruleset"); + } + } + }).build(); + RuleContext context = new RuleContext(); + context.setReport(new Report()); + context.setLanguageVersion(LanguageRegistry.getLanguage(DummyLanguageModule.NAME).getDefaultVersion()); + context.setSourceCodeFilename(RuleSetTest.class.getName() + ".ruleExceptionShouldBeReported"); + context.setIgnoreExceptions(true); // the default + RuleSets rulesets = new RuleSets(ruleset); + rulesets.apply(makeCompilationUnits(), context, LanguageRegistry.getLanguage(DummyLanguageModule.NAME)); + + assertTrue("Report should have processing errors", context.getReport().hasErrors()); + List errors = CollectionUtil.toList(context.getReport().errors()); + assertEquals("Errors expected", 1, errors.size()); + assertEquals("Wrong error message", "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, context.getReport().size()); + } + } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/xpath/XPathMetricFunctionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/xpath/XPathMetricFunctionTest.java index 7cdaa12da2..5a64ad0efe 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/xpath/XPathMetricFunctionTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/metrics/xpath/XPathMetricFunctionTest.java @@ -53,6 +53,7 @@ public class XPathMetricFunctionTest { Report report = new Report(); ctx.setReport(report); ctx.setSourceCodeFilename("n/a"); + ctx.setIgnoreExceptions(false); // for test, we want immediate exceptions thrown and not collect them RuleSet rules = new RuleSetFactory().createSingleRuleRuleSet(rule); p.getSourceCodeProcessor().processSourceCode(new StringReader(code), new RuleSets(rules), ctx); return report.iterator();