From a63cfb8228b9d95a1ccd7189380b85607a8c3bc5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 10:15:10 +0200 Subject: [PATCH] [doc] New exit code 5, --no-fail-on-processing-error Fixes #2827 --- docs/pages/pmd/userdocs/cli_reference.md | 18 ++++++++-- docs/pages/pmd/userdocs/cpd/cpd.md | 17 +++++++-- docs/pages/pmd/userdocs/tools/ant.md | 4 +-- docs/pages/release_notes.md | 14 ++++++++ .../pmd/ant/internal/PMDTaskImpl.java | 2 +- .../pmd/cli/commands/internal/CpdCommand.java | 2 +- .../pmd/cli/commands/internal/PmdCommand.java | 2 +- .../pmd/cli/internal/CliExitCode.java | 18 +++++----- .../net/sourceforge/pmd/cli/CpdCliTest.java | 12 +++---- .../net/sourceforge/pmd/cli/PmdCliTest.java | 4 +-- .../pmd/AbstractConfiguration.java | 35 +++++++++++++++++-- 11 files changed, 97 insertions(+), 31 deletions(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index c616c1847b..a74766610f 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -80,6 +80,11 @@ The tool comes with a rather extensive help text, simply running with `--help`! The valid values are the standard character sets of `java.nio.charset.Charset`." default="UTF-8" %} + {% include custom/cli_option_row.html options="--[no-]fail-on-processing-error" + description="Specifies whether PMD exits with non-zero status if processing errors occurred. + By default PMD exits with status 5 if processing errors or violations are found. + Disable this option with `--no-fail-on-processing-error` to exit with 0 instead and just write the report." + %} {% include custom/cli_option_row.html options="--[no-]fail-on-violation" description="Specifies whether PMD exits with non-zero status if violations are found. By default PMD exits with status 4 if violations are found. @@ -208,16 +213,23 @@ Or you can set the environment variable `CLASSPATH` before starting PMD, e.g. ## Exit Status -Please note that if PMD detects any violations, it will exit with status 4 (since 5.3). +Please note that if PMD detects any violations, it will exit with status 4 (since 5.3) or 5 (since 7.2.0). This behavior has been introduced to ease PMD integration into scripts or hooks, such as SVN hooks. - + - + +
0Everything is fine, no violations found.
0Everything is fine, no violations found and no processing error occurred.
1PMD exited with an exception.
2Usage error. Command-line parameters are invalid or missing.
4At least one violation has been detected, unless --no-fail-on-violation is set.
4At least one violation has been detected, unless --no-fail-on-violation is set.

Since PMD 5.3.

5At least one processing error has occurred. There might be additionally zero or more violations detected. + To ignore processing errors, use --no-fail-on-processing-error.

Since PMD 7.2.0.

+{%include note.html content="If PMD exits with 5, then PMD had either trouble parsing one or more files or a rule failed with an exception. +That means, that either no violations for the entire file or for that rule are reported. These cases can be considered as false-negatives. +In any case, the root cause should be investigated. If it's a problem in PMD itself, please create a bug report. Processing errors +are usually part of the generated PMD report." %} + ## Logging PMD internally uses [slf4j](https://www.slf4j.org/) and ships with slf4j-simple as the logging implementation. diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index 58259b8a71..e9b679aa62 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -150,6 +150,11 @@ exactly identical. If the root path is mentioned (e.g. \"/\" or \"C:\\\"), then the paths will be rendered as absolute." %} + {% include custom/cli_option_row.html options="--[no-]fail-on-processing-error" + description="Specifies whether CPD exits with non-zero status if processing errors occurred. + By default CPD exits with status 5 if processing errors or violations are found. + Disable this option with `--no-fail-on-processing-error` to exit with 0 instead and just write the report." + %} {% include custom/cli_option_row.html options="--[no-]fail-on-violation" description="Specifies whether CPD exits with non-zero status if violations are found. By default CPD exits with status 4 if violations are found. @@ -279,16 +284,22 @@ If you specify a source directory but don't want to scan the sub-directories, yo ### Exit status -Please note that if CPD detects duplicated source code, it will exit with status 4 (since 5.0). +Please note that if CPD detects duplicated source code, it will exit with status 4 (since 5.0) or 5 (since 7.2.0). This behavior has been introduced to ease CPD integration into scripts or hooks, such as SVN hooks. - + - + +
0Everything is fine, no code duplications found.
0Everything is fine, no code duplications found and no processing errors occurred.
1CPD exited with an exception.
2Usage error. Command-line parameters are invalid or missing.
4At least one code duplication has been detected unless --no-fail-on-violation is set.
4At least one code duplication has been detected unless --no-fail-on-violation is set.

Since PMD 5.0.

5At least one processing error has occurred. There might be additionally zero or more duplications detected. + To ignore processing errors, use --no-fail-on-processing-error.

Since PMD 7.2.0.

+{%include note.html content="If PMD exits with 5, then PMD had trouble lexing one or more files. +That means, that no duplications for the entire file are reported. This can be considered as false-negative. +In any case, the root cause should be investigated. If it's a problem in PMD itself, please create a bug report." %} + ## Logging PMD internally uses [slf4j](https://www.slf4j.org/) and ships with slf4j-simple as the logging implementation. diff --git a/docs/pages/pmd/userdocs/tools/ant.md b/docs/pages/pmd/userdocs/tools/ant.md index 4c81ff7daf..44f1813f30 100644 --- a/docs/pages/pmd/userdocs/tools/ant.md +++ b/docs/pages/pmd/userdocs/tools/ant.md @@ -63,8 +63,8 @@ The examples below won't repeat this taskdef element, as this is always required Yes, unless the ruleset nested element is used - failonerror - Whether or not to fail the build if any errors occur while processing the files + failOnError + Whether or not to fail the build if any processing errors occur while analyzing files. No diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 1a3ab6dc48..4510c977df 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 +* cli + * [#2827](https://github.com/pmd/pmd/issues/2827): \[cli] Consider processing errors in exit status * core * [#4978](https://github.com/pmd/pmd/issues/4978): \[core] Referenced Rulesets do not emit details on validation errors * [#4983](https://github.com/pmd/pmd/pull/4983): \[cpd] Fix CPD crashes about unicode escapes @@ -33,6 +35,18 @@ This is a {{ site.pmd.release_type }} release. ### 🚨 API Changes +#### CLI + +* New exit code 5 introduced. PMD and CPD will exit now by default with exit code 5, if any processing error + (e.g. parsing exception, lexing exception or rule exception) occurred. Such processing errors mean, that + either no violations or duplication for the entire file or for that rule are reported. These cases can be + considered as false-negatives. + + In any case, the root cause should be investigated. If it's a problem in PMD itself, please create a bug report. + +* New CLI parameter `--no-fail-on-processing-error` to ignore processing errors and not exit with code 5. By default, + a build with processing errors will now fail and with that parameter, the previous behavior can be restored. + #### Deprecated API * pmd-java diff --git a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index a4be75fb15..8b160790d9 100644 --- a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java +++ b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java @@ -144,7 +144,7 @@ public class PMDTaskImpl { pmd.performAnalysis(); stats = reportStatsListener.getResult(); if (failOnError && pmd.getReporter().numErrors() > 0) { - throw new BuildException("Some errors occurred while running PMD"); + throw new BuildException("Some processing errors occurred while running PMD"); } } diff --git a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java index b949de739c..30b312e69e 100644 --- a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java +++ b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/CpdCommand.java @@ -136,7 +136,7 @@ public class CpdCommand extends AbstractAnalysisPmdSubcommand boolean hasProcessingErrors = configuration.getReporter().numErrors() > 0; if (hasProcessingErrors && configuration.isFailOnProcessingError()) { - return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; + return CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; } if (hasViolations.booleanValue() && configuration.isFailOnViolation()) { diff --git a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java index 547feac371..3684a8ca87 100644 --- a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java +++ b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/PmdCommand.java @@ -332,7 +332,7 @@ public class PmdCommand extends AbstractAnalysisPmdSubcommand // processing errors are ignored return CliExitCode.ERROR; } else if (stats.getNumErrors() > 0 && configuration.isFailOnProcessingError()) { - return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; + return CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; } else if (stats.getNumViolations() > 0 && configuration.isFailOnViolation()) { return CliExitCode.VIOLATIONS_FOUND; } else { diff --git a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/internal/CliExitCode.java b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/internal/CliExitCode.java index 43e11467fa..0bdf2f479d 100644 --- a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/internal/CliExitCode.java +++ b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/internal/CliExitCode.java @@ -4,7 +4,7 @@ package net.sourceforge.pmd.cli.internal; -import net.sourceforge.pmd.PMDConfiguration; +import net.sourceforge.pmd.AbstractConfiguration; /** * The execution result of any given command. @@ -26,20 +26,18 @@ public enum CliExitCode { * No errors, but PMD found either duplications/violations or couldn't analyze all * files due to parsing/lexing problems. This is exit code {@code 4}. * - *

This is only returned if {@link PMDConfiguration#isFailOnViolation()} + *

This is only returned if {@link AbstractConfiguration#isFailOnViolation()} * is set. It can be disabled by using CLI flag {@code --no-fail-on-violation}. */ VIOLATIONS_FOUND(4), /** - * PMD did run, but there were either duplications/violations - * or processing errors or both. + * PMD did run, but there was at least one processing error. There + * might be additionally duplications or violations. This is exit code {@code 5}. * - *

In case you find processing errors: Please report them

- * - *

If cli flag --no-fail-on-processing-errors is used, then this - * exit code is not used. In such case, either 0 or 4 is returned.

+ *

This is only returned if {@link AbstractConfiguration#isFailOnProcessingError()} + * is set. It can be disabled by using CLI flag {@code --no-fail-on-processing-error}. */ - VIOLATIONS_OR_PROCESSING_ERRORS(5); + PROCESSING_ERRORS_OR_VIOLATIONS(5); private final int exitCode; @@ -57,7 +55,7 @@ public enum CliExitCode { case 1: return ERROR; case 2: return USAGE_ERROR; case 4: return VIOLATIONS_FOUND; - case 5: return VIOLATIONS_OR_PROCESSING_ERRORS; + case 5: return PROCESSING_ERRORS_OR_VIOLATIONS; default: throw new IllegalArgumentException("Not a known exit code: " + i); } diff --git a/pmd-cli/src/test/java/net/sourceforge/pmd/cli/CpdCliTest.java b/pmd-cli/src/test/java/net/sourceforge/pmd/cli/CpdCliTest.java index 6597b4c414..5f76af32f8 100644 --- a/pmd-cli/src/test/java/net/sourceforge/pmd/cli/CpdCliTest.java +++ b/pmd-cli/src/test/java/net/sourceforge/pmd/cli/CpdCliTest.java @@ -6,7 +6,7 @@ package net.sourceforge.pmd.cli; import static net.sourceforge.pmd.cli.internal.CliExitCode.OK; import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_FOUND; -import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; +import static net.sourceforge.pmd.cli.internal.CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.containsString; @@ -129,14 +129,14 @@ class CpdCliTest extends BaseCliTest { @Test void testWrongCliOptionResultsInErrorLoggingAfterDir() throws Exception { // --ignore-identifiers doesn't take an argument anymore - it is interpreted as a file for inputPaths - final CliExecutionResult result = runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "false"); + final CliExecutionResult result = runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "false"); result.checkStdErr(containsString("No such file false")); } @Test void testWrongCliOptionResultsInErrorLoggingBeforeDir() throws Exception { // --ignore-identifiers doesn't take an argument anymore - it is interpreted as a file for inputPaths - final CliExecutionResult result = runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); + final CliExecutionResult result = runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); result.checkStdErr(containsString("No such file false")); } @@ -221,7 +221,7 @@ class CpdCliTest extends BaseCliTest { */ @Test void testSkipLexicalErrors() throws Exception { - runCli(VIOLATIONS_OR_PROCESSING_ERRORS, + runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "10", "-d", BASE_RES_PATH + "badandgood/", "--format", "text", @@ -234,7 +234,7 @@ class CpdCliTest extends BaseCliTest { @Test void testExitCodeWithLexicalErrors() throws Exception { - runCli(VIOLATIONS_OR_PROCESSING_ERRORS, + runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "10", "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text") @@ -259,7 +259,7 @@ class CpdCliTest extends BaseCliTest { @Test void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { - runCli(VIOLATIONS_OR_PROCESSING_ERRORS, + runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "10", "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text", diff --git a/pmd-cli/src/test/java/net/sourceforge/pmd/cli/PmdCliTest.java b/pmd-cli/src/test/java/net/sourceforge/pmd/cli/PmdCliTest.java index 36a19974a5..361c4891af 100644 --- a/pmd-cli/src/test/java/net/sourceforge/pmd/cli/PmdCliTest.java +++ b/pmd-cli/src/test/java/net/sourceforge/pmd/cli/PmdCliTest.java @@ -8,7 +8,7 @@ import static net.sourceforge.pmd.cli.internal.CliExitCode.ERROR; import static net.sourceforge.pmd.cli.internal.CliExitCode.OK; import static net.sourceforge.pmd.cli.internal.CliExitCode.USAGE_ERROR; import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_FOUND; -import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; +import static net.sourceforge.pmd.cli.internal.CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -321,7 +321,7 @@ class PmdCliTest extends BaseCliTest { @Test void exitStatusWithProcessingErrors() throws Exception { - runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--use-version", "dummy-parserThrows", + runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--use-version", "dummy-parserThrows", "-d", srcDir.toString(), "-f", "text", "-R", RULESET_WITH_VIOLATION) .verify(r -> { r.checkStdOut(containsString("someSource.dummy\t-\tParseException: Parse exception: ohio")); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java index b4b0c68332..1118b97d85 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -384,7 +384,12 @@ public abstract class AbstractConfiguration { * Whether PMD should exit with status 4 (the default behavior, true) if * violations are found or just with 0 (to not break the build, e.g.). * + *

Note: If additionally processing errors occurred, the exit status is 5. See + * {@link #isFailOnProcessingError()}. + * * @return failOnViolation + * + * @see #isFailOnProcessingError() */ public boolean isFailOnViolation() { return failOnViolation; @@ -394,17 +399,43 @@ public abstract class AbstractConfiguration { * Sets whether PMD should exit with status 4 (the default behavior, true) * if violations are found or just with 0 (to not break the build, e.g.). * - * @param failOnViolation - * failOnViolation + *

Note: If additionally processing errors occurred, the exit status is 5. See + * {@link #isFailOnProcessingError()}. + * + * @param failOnViolation whether to exit with 4 and fail the build if violations are found. + * + * @see #isFailOnProcessingError() */ public void setFailOnViolation(boolean failOnViolation) { this.failOnViolation = failOnViolation; } + /** + * Whether PMD should exit with status 5 (the default behavior, true) if + * processing errors occurred or just with 0 (to not break the build, e.g.). + * + *

Note: If additionally violations are found, the exist status is 4. See + * {@link #isFailOnViolation()}. + * + * @return failOnProcessingError + * + * @see #isFailOnViolation() + */ public boolean isFailOnProcessingError() { return failOnProcessingError; } + /** + * Sets whether PMD should exit with status 5 (the default behavior, true) + * if processing errors occurred or just with 0 (to not break the build, e.g.). + * + *

Note: If additionally violations are found, the exist status is 4. See + * {@link #isFailOnViolation()}. + * + * @param failOnProcessingError whether to exit with 5 and fail the build if processing errors occurred. + * + * @see #isFailOnViolation() + */ public void setFailOnProcessingError(boolean failOnProcessingError) { this.failOnProcessingError = failOnProcessingError; }