From 5fdc9bfb5b7da25ac1db1afed742f099d2dedc50 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 5 Nov 2020 10:51:39 +0100 Subject: [PATCH 1/4] [core] Add error recovery mode This can be enabled by setting system property "pmd.error_recovery" to an arbitrary value, e.g. "-Dpmd.error_recovery". --- .../sourceforge/pmd/SourceCodeProcessor.java | 9 +- .../sourceforge/pmd/internal/SystemProps.java | 28 +++++ .../lang/rule/internal/RuleApplicator.java | 14 ++- .../pmd/SourceCodeProcessorTest.java | 104 ++++++++++++++++++ .../pmd/lang/DummyLanguageModule.java | 14 +++ 5 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java create mode 100644 pmd-core/src/test/java/net/sourceforge/pmd/SourceCodeProcessorTest.java diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java index 535227fc10..4dcd6f983b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/SourceCodeProcessor.java @@ -14,6 +14,7 @@ import net.sourceforge.pmd.benchmark.TimeTracker; import net.sourceforge.pmd.benchmark.TimedOperation; import net.sourceforge.pmd.benchmark.TimedOperationCategory; import net.sourceforge.pmd.internal.RulesetStageDependencyHelper; +import net.sourceforge.pmd.internal.SystemProps; import net.sourceforge.pmd.lang.LanguageVersion; import net.sourceforge.pmd.lang.Parser; import net.sourceforge.pmd.lang.ast.Node; @@ -105,9 +106,15 @@ public class SourceCodeProcessor { } catch (ParseException pe) { configuration.getAnalysisCache().analysisFailed(ctx.getSourceCodeFile()); throw new PMDException("Error while parsing " + ctx.getSourceCodeFile(), pe); - } catch (Exception | StackOverflowError | AssertionError e) { + } catch (Exception e) { configuration.getAnalysisCache().analysisFailed(ctx.getSourceCodeFile()); throw new PMDException("Error while processing " + ctx.getSourceCodeFile(), e); + } catch (StackOverflowError | AssertionError e) { + if (SystemProps.isErrorRecoveryMode()) { + configuration.getAnalysisCache().analysisFailed(ctx.getSourceCodeFile()); + throw new PMDException("Error while processing " + ctx.getSourceCodeFile(), e); + } + throw e; } finally { ruleSets.end(ctx); } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java new file mode 100644 index 0000000000..20551fa84c --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java @@ -0,0 +1,28 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.internal; + +public final class SystemProps { + + public static final String PMD_ERROR_RECOVERY = "pmd.error_recovery"; + + private SystemProps() { + } + + /** + * In error recovery mode errors like StackOverflowError or AssetionErrors are logged + * and the execution continues. + * These exceptions mean, that something went really wrong while executing and + * depending on where the error occurred, the internal state might be corrupted + * or not. Hence, it might work to continue and "ignore" (just log) the error + * or we'll see more problems when continuing. That's why error recovery mode + * is not enabled by default. + *

+ * The System Property is called {@code pmd.error_recovery}. + */ + public static boolean isErrorRecoveryMode() { + return System.getProperty(PMD_ERROR_RECOVERY) != null; + } +} 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 75997e77a7..db8e93871f 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 @@ -16,6 +16,7 @@ import net.sourceforge.pmd.RuleSet; import net.sourceforge.pmd.benchmark.TimeTracker; import net.sourceforge.pmd.benchmark.TimedOperation; import net.sourceforge.pmd.benchmark.TimedOperationCategory; +import net.sourceforge.pmd.internal.SystemProps; import net.sourceforge.pmd.lang.ast.Node; /** Applies a set of rules to a set of ASTs. */ @@ -60,10 +61,21 @@ public class RuleApplicator { try (TimedOperation rcto = TimeTracker.startOperation(TimedOperationCategory.RULE, rule.getName())) { rule.apply(node, ctx); rcto.close(1); - } catch (RuntimeException | StackOverflowError | AssertionError e) { + } catch (RuntimeException e) { if (ctx.isIgnoreExceptions()) { ctx.getReport().addError(new ProcessingError(e, String.valueOf(ctx.getSourceCodeFile()))); + if (LOG.isLoggable(Level.WARNING)) { + LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " + + ctx.getSourceCodeFile() + ", continuing with next rule", e); + } + } else { + throw e; + } + } catch (StackOverflowError | AssertionError e) { + if (SystemProps.isErrorRecoveryMode()) { + ctx.getReport().addError(new ProcessingError(e, String.valueOf(ctx.getSourceCodeFile()))); + if (LOG.isLoggable(Level.WARNING)) { LOG.log(Level.WARNING, "Exception applying rule " + rule.getName() + " on file " + ctx.getSourceCodeFile() + ", continuing with next rule", e); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/SourceCodeProcessorTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/SourceCodeProcessorTest.java new file mode 100644 index 0000000000..1827d2a49f --- /dev/null +++ b/pmd-core/src/test/java/net/sourceforge/pmd/SourceCodeProcessorTest.java @@ -0,0 +1,104 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd; + +import java.io.StringReader; +import java.util.Arrays; +import java.util.List; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; +import org.junit.rules.TestRule; + +import net.sourceforge.pmd.internal.SystemProps; +import net.sourceforge.pmd.lang.DummyLanguageModule; +import net.sourceforge.pmd.lang.Language; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.LanguageVersion; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.rule.AbstractRule; + +public class SourceCodeProcessorTest { + + @org.junit.Rule + public TestRule restoreSystemProperties = new RestoreSystemProperties(); + + private SourceCodeProcessor processor; + private StringReader sourceCode; + private RuleContext ctx; + private List rulesets; + private LanguageVersion dummyThrows; + private LanguageVersion dummyDefault; + + @Before + public void prepare() { + Language dummyLanguage = LanguageRegistry.findLanguageByTerseName(DummyLanguageModule.TERSE_NAME); + dummyDefault = dummyLanguage.getDefaultVersion(); + dummyThrows = dummyLanguage.getVersion("1.9-throws"); + + processor = new SourceCodeProcessor(new PMDConfiguration()); + sourceCode = new StringReader("test"); + Rule rule = new RuleThatThrows(); + rulesets = Arrays.asList(RulesetsFactoryUtils.defaultFactory().createSingleRuleRuleSet(rule)); + + ctx = new RuleContext(); + } + + @Test + public void inErrorRecoveryModeErrorsShouldBeLoggedByParser() { + System.setProperty(SystemProps.PMD_ERROR_RECOVERY, ""); + ctx.setLanguageVersion(dummyThrows); + + Assert.assertThrows(PMDException.class, () -> { + processor.processSourceCode(sourceCode, new RuleSets(rulesets), ctx); + }); + // the error is actually logged by PmdRunnable + } + + @Test + public void inErrorRecoveryModeErrorsShouldBeLoggedByRule() throws Exception { + System.setProperty(SystemProps.PMD_ERROR_RECOVERY, ""); + ctx.setLanguageVersion(dummyDefault); + + processor.processSourceCode(sourceCode, new RuleSets(rulesets), ctx); + Assert.assertEquals(1, ctx.getReport().getProcessingErrors().size()); + Assert.assertSame(AssertionError.class, ctx.getReport().getProcessingErrors().get(0).getError().getClass()); + } + + @Test + public void withoutErrorRecoveryModeProcessingShouldBeAbortedByParser() { + Assert.assertNull(System.getProperty(SystemProps.PMD_ERROR_RECOVERY)); + ctx.setLanguageVersion(dummyThrows); + + Assert.assertThrows(AssertionError.class, () -> { + processor.processSourceCode(sourceCode, new RuleSets(rulesets), ctx); + }); + } + + @Test + public void withoutErrorRecoveryModeProcessingShouldBeAbortedByRule() { + Assert.assertNull(System.getProperty(SystemProps.PMD_ERROR_RECOVERY)); + ctx.setLanguageVersion(dummyDefault); + + Assert.assertThrows(AssertionError.class, () -> { + processor.processSourceCode(sourceCode, new RuleSets(rulesets), ctx); + }); + } + + private static class RuleThatThrows extends AbstractRule { + + RuleThatThrows() { + Language dummyLanguage = LanguageRegistry.findLanguageByTerseName(DummyLanguageModule.TERSE_NAME); + setLanguage(dummyLanguage); + } + + @Override + public void apply(Node target, RuleContext ctx) { + throw new AssertionError("test"); + } + } +} diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java index 0a897ef4a4..fa979beb12 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/DummyLanguageModule.java @@ -14,6 +14,7 @@ import net.sourceforge.pmd.lang.ast.DummyAstStages; import net.sourceforge.pmd.lang.ast.DummyRoot; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.ast.ParseException; +import net.sourceforge.pmd.lang.ast.RootNode; import net.sourceforge.pmd.lang.rule.ParametricRuleViolation; import net.sourceforge.pmd.lang.rule.impl.DefaultRuleViolationFactory; @@ -36,6 +37,7 @@ public class DummyLanguageModule extends BaseLanguageModule { addVersion("1.6", new Handler(), "6"); addDefaultVersion("1.7", new Handler(), "7"); addVersion("1.8", new Handler(), "8"); + addVersion("1.9-throws", new HandlerWithParserThatThrows()); } public static class Handler extends AbstractPmdLanguageVersionHandler { @@ -64,6 +66,18 @@ public class DummyLanguageModule extends BaseLanguageModule { } } + public static class HandlerWithParserThatThrows extends Handler { + @Override + public Parser getParser(ParserOptions parserOptions) { + return new AbstractParser(parserOptions) { + @Override + public RootNode parse(String fileName, Reader source) throws ParseException { + throw new AssertionError("test error while parsing"); + } + }; + } + } + public static class RuleViolationFactory extends DefaultRuleViolationFactory { @Override From 56a5ee536b8972a2086f8ed52eacfb27b0e82068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 10 Nov 2020 14:02:02 +0100 Subject: [PATCH 2/4] Deprecations for #905 --- docs/pages/release_notes.md | 3 +++ .../pmd/lang/java/ast/ASTClassOrInterfaceBody.java | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index abd3ee22d0..bda687c234 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -28,6 +28,9 @@ This is a {{ site.pmd.release_type }} release. {% jdoc !!java::lang.java.ast.ASTTypeParameter#getParameterName() %} and the corresponding XPath attributes. In both cases they're replaced with a new method `getName`, the attribute is `@Name`. +* {% jdoc !!java::lang.java.ast.ASTClassOrInterfaceBody#isAnonymousInnerClass() %}, + and {% jdoc !!java::lang.java.ast.ASTClassOrInterfaceBody#isEnumChild() %}, + refs [#905](https://github.com/pmd/pmd/issues/905) #### Internal API diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceBody.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceBody.java index 7405de514d..818aed2cc1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceBody.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTClassOrInterfaceBody.java @@ -35,10 +35,20 @@ public class ASTClassOrInterfaceBody extends AbstractJavaNode { return visitor.visit(this, data); } + /** + * @deprecated Test the parent for {@link ASTAllocationExpression}. + * This will be removed in pmd 7 as unnecessary (refs #905) + */ + @Deprecated public boolean isAnonymousInnerClass() { return getParent() instanceof ASTAllocationExpression; } + /** + * @deprecated Test the parent for {@link ASTEnumConstant}. + * This will be removed in pmd 7 as unnecessary (refs #905) + */ + @Deprecated public boolean isEnumChild() { return getParent() instanceof ASTEnumConstant; } From a5c42ddefcd4710e60f0bb03713ccc8473b4aa4f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 11 Nov 2020 15:33:38 +0100 Subject: [PATCH 3/4] Add test case for #2595 --- .../xml/AvoidReassigningLoopVariables.xml | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml index a75e19c559..b39cd59870 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml @@ -698,4 +698,33 @@ public class Foo { } ]]> + + AvoidReassigningLoopVariables detects some harmless reassigning of loop variables in foreach #2595 + firstOnly + 1 + 6 + + From 697e3ae29004b57ce9d4705999b3642e6294566b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 11 Nov 2020 21:41:25 +0100 Subject: [PATCH 4/4] Fix typo --- .../src/main/java/net/sourceforge/pmd/internal/SystemProps.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java b/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java index 20551fa84c..913760c125 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/internal/SystemProps.java @@ -12,7 +12,7 @@ public final class SystemProps { } /** - * In error recovery mode errors like StackOverflowError or AssetionErrors are logged + * In error recovery mode errors like StackOverflowError or AssertionErrors are logged * and the execution continues. * These exceptions mean, that something went really wrong while executing and * depending on where the error occurred, the internal state might be corrupted