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 8b9f27f277..bb1afd7fa3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/RuleContext.java @@ -12,9 +12,11 @@ import org.checkerframework.checker.nullness.qual.NonNull; import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.Report.SuppressedViolation; +import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; import net.sourceforge.pmd.lang.rule.RuleViolationFactory; +import net.sourceforge.pmd.processor.AbstractPMDProcessor; import net.sourceforge.pmd.processor.FileAnalysisListener; /** @@ -24,11 +26,11 @@ import net.sourceforge.pmd.processor.FileAnalysisListener; * the {@link ViolationSuppressor}s for the language. */ public final class RuleContext implements AutoCloseable { + // Rule contexts do not need to be thread-safe, within PmdRunnable + // they are stack-local private static final Object[] NO_ARGS = new Object[0]; - private boolean ignoreExceptions = true; - private final FileAnalysisListener listener; private RuleContext(FileAnalysisListener listener) { @@ -36,11 +38,16 @@ public final class RuleContext implements AutoCloseable { } + /** + * Close the listener. + */ @Override public void close() throws Exception { listener.close(); } + // TODO document + public void reportError(ProcessingError error) { listener.onError(error); } @@ -93,32 +100,16 @@ public final class RuleContext implements AutoCloseable { // Escape PMD specific variable message format, specifically the { // in the ${, so MessageFormat doesn't bitch. final String escapedMessage = StringUtils.replace(message, "${", "$'{'"); - return MessageFormat.format(escapedMessage, args != null ? args : NO_ARGS); + return MessageFormat.format(escapedMessage, args); } - /** - * Gets the configuration whether to skip failing rules (true) - * or whether to throw a a RuntimeException and abort the processing for the - * first failing rule. - * - * @return true when failing rules are skipped, - * false otherwise. - * - * TODO this looks only useful in unit tests... + * Create a new RuleContext. This is internal API owned by {@link AbstractPMDProcessor} + * (can likely be hidden when everything relevant is moved into rule package). */ - public boolean isIgnoreExceptions() { - return ignoreExceptions; - } - - + @InternalApi 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/rule/internal/RuleApplicator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java index 3f4a8e1bad..6696cd1517 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/internal/RuleApplicator.java @@ -61,16 +61,12 @@ public class RuleApplicator { rule.apply(node, ctx); rcto.close(1); } catch (RuntimeException e) { - if (ctx.isIgnoreExceptions()) { - String filename = node.getSourceCodeFile(); - ctx.reportError(new ProcessingError(e, filename)); + String filename = node.getSourceCodeFile(); + ctx.reportError(new ProcessingError(e, filename)); - if (LOG.isLoggable(Level.WARNING)) { - LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " - + filename + ", continuing with next rule", e); - } - } else { - throw e; + if (LOG.isLoggable(Level.WARNING)) { + LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " + + filename + ", continuing with next rule", e); } } } 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 a37da034d0..79dc376573 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 @@ -10,7 +10,9 @@ import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import net.sourceforge.pmd.Report.ConfigurationError; +import net.sourceforge.pmd.Report.ProcessingError; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.lang.ast.FileAnalysisException; import net.sourceforge.pmd.util.CollectionUtil; import net.sourceforge.pmd.util.datasource.DataSource; @@ -123,4 +125,38 @@ public interface GlobalAnalysisListener extends AutoCloseable { } + /** + * A listener that throws processing errors when they occur. They + * are all thrown as {@link FileAnalysisException}s. Config errors + * are ignored. + */ + static GlobalAnalysisListener exceptionThrower() { + return new GlobalAnalysisListener() { + @Override + public FileAnalysisListener startFileAnalysis(DataSource file) { + return new FileAnalysisListener() { + @Override + public void onRuleViolation(RuleViolation violation) { + // do nothing + } + + @Override + public void onError(ProcessingError error) throws FileAnalysisException { + Throwable cause = error.getError(); + if (cause instanceof FileAnalysisException) { + throw (FileAnalysisException) cause; + } + throw new FileAnalysisException(cause); + } + }; + } + + @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 ff1ac2533b..94c7c86dbc 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 @@ -102,15 +102,7 @@ class PmdRunnable implements Runnable { } catch (Exception e) { configuration.getAnalysisCache().analysisFailed(file); ruleCtx.reportError(new Report.ProcessingError(e, file.getPath())); - - if (ruleCtx.isIgnoreExceptions()) { - LOG.log(Level.FINE, "Exception while processing file: " + file, e); - } else { - if (e instanceof FileAnalysisException) { // NOPMD AvoidInstanceofChecksInCatchClause - throw (FileAnalysisException) e; - } - throw new FileAnalysisException(e); - } + LOG.log(Level.FINE, "Exception while processing file: " + file, 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 aaabaeec43..9d4fdeda6d 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/RuleSetTest.java @@ -38,7 +38,6 @@ import net.sourceforge.pmd.lang.ast.DummyRoot; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.rule.RuleReference; import net.sourceforge.pmd.lang.rule.RuleTargetSelector; -import net.sourceforge.pmd.processor.FileAnalysisListener; public class RuleSetTest { @@ -490,19 +489,6 @@ public class RuleSetTest { assertTrue("Should be a RuntimeException", errors.get(0).getError() instanceof RuntimeException); } - @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 = RuleContext.createThrowingExceptions(FileAnalysisListener.noop()); - ruleset.apply(makeCompilationUnits(), context); - } @Test public void ruleExceptionShouldNotStopProcessingFile() throws Exception { 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 739720e85c..6a4faf2715 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 @@ -47,6 +47,7 @@ import net.sourceforge.pmd.lang.Language; import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.processor.AbstractPMDProcessor; +import net.sourceforge.pmd.processor.GlobalAnalysisListener; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.renderers.TextRenderer; import net.sourceforge.pmd.util.datasource.DataSource; @@ -289,19 +290,21 @@ public abstract class RuleTst { }); } - GlobalReportBuilder builder = new GlobalReportBuilder(); - + GlobalReportBuilder reportBuilder = new GlobalReportBuilder(); + // Add a listener that throws when an error occurs: + // this replaces ruleContext.setIgnoreExceptions(false) + GlobalAnalysisListener listener = GlobalAnalysisListener.tee(listOf(GlobalAnalysisListener.exceptionThrower(), reportBuilder)); AbstractPMDProcessor.runSingleFile( listOf(RulesetsFactoryUtils.defaultFactory().createSingleRuleRuleSet(rule)), DataSource.forString(code, "test." + languageVersion.getLanguage().getExtensions().get(0)), - builder, + listener, config ); - builder.close(); + reportBuilder.close(); - return builder.getReport(); + return reportBuilder.getReport(); } catch (Exception e) { throw new RuntimeException(e); }