From 468266d58df34623bc89f014d03db3156f2bde0f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 2 May 2024 18:51:16 +0200 Subject: [PATCH 01/15] [cli] CPD: Add new exit code 5: VIOLATIONS_OR_PROCESSING_ERRORS --- .../pmd/cli/commands/internal/CpdCommand.java | 5 +++ .../pmd/cli/internal/CliExitCode.java | 25 ++++++++++---- .../net/sourceforge/pmd/cli/CpdCliTest.java | 33 ++++++++++++++++--- 3 files changed, 53 insertions(+), 10 deletions(-) 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 f15328047b7..f629de248c6 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 @@ -133,6 +133,11 @@ protected CPDConfiguration toConfiguration() { MutableBoolean hasViolations = new MutableBoolean(); cpd.performAnalysis(report -> hasViolations.setValue(!report.getMatches().isEmpty())); + boolean hasProcessingErrors = configuration.getReporter().numErrors() > 0; + if (hasProcessingErrors) { + return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; + } + if (hasViolations.booleanValue() && configuration.isFailOnViolation()) { return CliExitCode.VIOLATIONS_FOUND; } 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 16fabc270b6..43e11467fa2 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 @@ -10,10 +10,10 @@ * The execution result of any given command. */ public enum CliExitCode { - /** No errors, no violations. This is exit code {@code 0}. */ + /** No errors, no processing errors, no violations. This is exit code {@code 0}. */ OK(0), /** - * Errors were detected, PMD may have not run to the end. + * Unexpected errors were detected, PMD may have not run to the end. * This is exit code {@code 1}. */ ERROR(1), @@ -23,11 +23,23 @@ public enum CliExitCode { */ USAGE_ERROR(2), /** - * No errors, but PMD found violations. This is exit code {@code 4}. - * This is only returned if {@link PMDConfiguration#isFailOnViolation()} - * is set (CLI flag {@code --failOnViolation}). + * 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()} + * is set. It can be disabled by using CLI flag {@code --no-fail-on-violation}. */ - VIOLATIONS_FOUND(4); + VIOLATIONS_FOUND(4), + /** + * PMD did run, but there were either duplications/violations + * or processing errors or both. + * + *

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.

+ */ + VIOLATIONS_OR_PROCESSING_ERRORS(5); private final int exitCode; @@ -45,6 +57,7 @@ public static CliExitCode fromInt(int i) { case 1: return ERROR; case 2: return USAGE_ERROR; case 4: return VIOLATIONS_FOUND; + case 5: return VIOLATIONS_OR_PROCESSING_ERRORS; 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 af1eee72aa1..22004ac5d37 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,6 +6,7 @@ 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.util.CollectionUtil.listOf; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.containsString; @@ -128,14 +129,14 @@ void testWrongCliOptionsDoPrintUsage() throws Exception { @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_FOUND, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "false"); + final CliExecutionResult result = runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--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_FOUND, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); + final CliExecutionResult result = runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); result.checkStdErr(containsString("No such file false")); } @@ -152,7 +153,7 @@ void testFindJavaDuplication() throws Exception { */ @Test void testIgnoreIdentifiers() throws Exception { - runCli(VIOLATIONS_FOUND, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "false", "--debug") + runCli(VIOLATIONS_FOUND, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "--debug") .verify(result -> result.checkStdOut(containsString( "Found a 14 line (89 tokens) duplication" ))); @@ -220,7 +221,7 @@ void testEncodingOption() throws Exception { */ @Test void testSkipLexicalErrors() throws Exception { - runCli(VIOLATIONS_FOUND, + runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--minimum-tokens", "10", "-d", BASE_RES_PATH + "badandgood/", "--format", "text", @@ -231,6 +232,30 @@ void testSkipLexicalErrors() throws Exception { }); } + @Test + void testExitCodeWithLexicalErrors() throws Exception { + runCli(VIOLATIONS_OR_PROCESSING_ERRORS, + "--minimum-tokens", "10", + "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), + "--format", "text") + .verify(r -> { + r.checkStdErr(containsPattern("Error while tokenizing: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdOut(emptyString()); + }); + } + + @Test + void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { + runCli(VIOLATIONS_OR_PROCESSING_ERRORS, + "--minimum-tokens", "10", + "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), + "--format", "text", + "--skip-lexical-errors") + .verify(r -> { + r.checkStdErr(containsPattern("Skipping file: Lexical error in file .*?BadFile\\.java")); + r.checkStdOut(emptyString()); + }); + } @Test void jsShouldFindDuplicatesWithDifferentFileExtensions() throws Exception { From 0fc23fc9f4cea1d9a0663ae22ab16378749020b5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 09:23:10 +0200 Subject: [PATCH 02/15] [cli] PMD: Add new exit code 5: VIOLATIONS_OR_PROCESSING_ERRORS --- .../pmd/cli/commands/internal/PmdCommand.java | 2 ++ .../test/java/net/sourceforge/pmd/cli/PmdCliTest.java | 11 +++++++++++ .../net/sourceforge/pmd/cli/RuleSetWithViolations.xml | 4 ++-- .../net/sourceforge/pmd/reporting/ReportStats.java | 6 ++++++ 4 files changed, 21 insertions(+), 2 deletions(-) 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 02a5804c98f..4e15aa0f840 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 @@ -330,6 +330,8 @@ protected CliExitCode doExecute(PMDConfiguration configuration) { if (pmdReporter.numErrors() > 0) { // processing errors are ignored return CliExitCode.ERROR; + } else if (stats.getNumErrors() > 0) { + return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; } else if (stats.getNumViolations() > 0 && configuration.isFailOnViolation()) { return CliExitCode.VIOLATIONS_FOUND; } else { 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 85958562fdf..275da2b43ba 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,6 +8,7 @@ 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.util.CollectionUtil.listOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; @@ -318,6 +319,16 @@ void exitStatusWithViolations() throws Exception { )); } + @Test + void exitStatusWithProcessingErrors() throws Exception { + runCli(VIOLATIONS_OR_PROCESSING_ERRORS, "--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")); + r.checkStdErr(containsString("An error occurred while executing PMD.")); + }); + } + @Test void testZipFileAsSource() throws Exception { Path zipArchive = createTemporaryZipArchive("sources.zip"); diff --git a/pmd-cli/src/test/resources/net/sourceforge/pmd/cli/RuleSetWithViolations.xml b/pmd-cli/src/test/resources/net/sourceforge/pmd/cli/RuleSetWithViolations.xml index 8049a8b63b7..4243101b432 100644 --- a/pmd-cli/src/test/resources/net/sourceforge/pmd/cli/RuleSetWithViolations.xml +++ b/pmd-cli/src/test/resources/net/sourceforge/pmd/cli/RuleSetWithViolations.xml @@ -3,12 +3,12 @@ xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd"> - Ruleset used by test RuleSetFactoryTest + Ruleset used by test net.sourceforge.pmd.cli.PmdCliTest + externalInfoUrl="${pmd.website.baseurl}/rules/test/RuleSetWithViolations.xml#ReportAllRootNodes"> Just for test 3 diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/ReportStats.java b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/ReportStats.java index eef7cd7dc57..9ac07837a3d 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/ReportStats.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/ReportStats.java @@ -23,10 +23,16 @@ public static ReportStats empty() { return new ReportStats(0, 0); } + /** + * Count of processing errors. + */ public int getNumErrors() { return numErrors; } + /** + * Count of found rule violations. + */ public int getNumViolations() { return numViolations; } From e667bf6773e6461721290ec8c4fa857077bceb19 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 09:33:46 +0200 Subject: [PATCH 03/15] [cli] Add new param "--no-fail-on-processing-error" --- .../internal/AbstractAnalysisPmdSubcommand.java | 6 ++++++ .../pmd/cli/commands/internal/CpdCommand.java | 3 ++- .../pmd/cli/commands/internal/PmdCommand.java | 3 ++- .../java/net/sourceforge/pmd/cli/CpdCliTest.java | 13 +++++++++++++ .../java/net/sourceforge/pmd/cli/PmdCliTest.java | 11 +++++++++++ .../net/sourceforge/pmd/AbstractConfiguration.java | 9 +++++++++ 6 files changed, 43 insertions(+), 2 deletions(-) diff --git a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java index e45086a60a6..c5535393310 100644 --- a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java +++ b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java @@ -48,6 +48,12 @@ public abstract class AbstractAnalysisPmdSubcommand relativizeRootPaths; @Option(names = { "--relativize-paths-with", "-z"}, description = "Path relative to which directories are rendered in the report. " 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 f629de248c6..b949de739ce 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 @@ -103,6 +103,7 @@ protected CPDConfiguration toConfiguration() { configuration.addRelativizeRoots(relativizeRootPaths); } configuration.setFailOnViolation(failOnViolation); + configuration.setFailOnProcessingError(failOnProcessingError); configuration.setInputFilePath(fileListPath); if (inputPaths != null) { configuration.setInputPathList(new ArrayList<>(inputPaths)); @@ -134,7 +135,7 @@ protected CPDConfiguration toConfiguration() { cpd.performAnalysis(report -> hasViolations.setValue(!report.getMatches().isEmpty())); boolean hasProcessingErrors = configuration.getReporter().numErrors() > 0; - if (hasProcessingErrors) { + if (hasProcessingErrors && configuration.isFailOnProcessingError()) { return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; } 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 4e15aa0f840..547feac371f 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 @@ -270,6 +270,7 @@ protected PMDConfiguration toConfiguration() { configuration.setSuppressMarker(suppressMarker); configuration.setThreads(threads); configuration.setFailOnViolation(failOnViolation); + configuration.setFailOnProcessingError(failOnProcessingError); configuration.setAnalysisCacheLocation(cacheLocation != null ? cacheLocation.toString() : null); configuration.setIgnoreIncrementalAnalysis(noCache); @@ -330,7 +331,7 @@ protected CliExitCode doExecute(PMDConfiguration configuration) { if (pmdReporter.numErrors() > 0) { // processing errors are ignored return CliExitCode.ERROR; - } else if (stats.getNumErrors() > 0) { + } else if (stats.getNumErrors() > 0 && configuration.isFailOnProcessingError()) { return CliExitCode.VIOLATIONS_OR_PROCESSING_ERRORS; } else if (stats.getNumViolations() > 0 && configuration.isFailOnViolation()) { return CliExitCode.VIOLATIONS_FOUND; 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 22004ac5d37..6597b4c4143 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 @@ -244,6 +244,19 @@ void testExitCodeWithLexicalErrors() throws Exception { }); } + @Test + void testExitCodeWithLexicalErrorsNoFail() throws Exception { + runCli(OK, + "--minimum-tokens", "10", + "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), + "--format", "text", + "--no-fail-on-processing-error") + .verify(r -> { + r.checkStdErr(containsPattern("Error while tokenizing: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdOut(emptyString()); + }); + } + @Test void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { runCli(VIOLATIONS_OR_PROCESSING_ERRORS, 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 275da2b43ba..36a19974a52 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 @@ -329,6 +329,17 @@ void exitStatusWithProcessingErrors() throws Exception { }); } + @Test + void exitStatusWithProcessingErrorsNoFail() throws Exception { + runCli(OK, "--use-version", "dummy-parserThrows", + "-d", srcDir.toString(), "-f", "text", "-R", RULESET_WITH_VIOLATION, + "--no-fail-on-processing-error") + .verify(r -> { + r.checkStdOut(containsString("someSource.dummy\t-\tParseException: Parse exception: ohio")); + r.checkStdErr(containsString("An error occurred while executing PMD.")); + }); + } + @Test void testZipFileAsSource() throws Exception { Path zipArchive = createTemporaryZipArchive("sources.zip"); 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 f79609ad9cf..6d737ec2199 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -46,6 +46,7 @@ public abstract class AbstractConfiguration { private Path ignoreFilePath; private List excludes = new ArrayList<>(); private boolean collectRecursive = true; + private boolean failOnProcessingError = true; protected AbstractConfiguration(LanguageRegistry languageRegistry, PmdReporter messageReporter) { @@ -377,4 +378,12 @@ public boolean collectFilesRecursively() { public void collectFilesRecursively(boolean collectRecursive) { this.collectRecursive = collectRecursive; } + + public boolean isFailOnProcessingError() { + return failOnProcessingError; + } + + public void setFailOnProcessingError(boolean failOnProcessingError) { + this.failOnProcessingError = failOnProcessingError; + } } From d0a913e50b714459d61e488967f9d928e43fb447 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 09:35:59 +0200 Subject: [PATCH 04/15] [cli] Move failOnViolation to AbstractConfiguration --- .../pmd/AbstractConfiguration.java | 22 +++++++++++++++++++ .../net/sourceforge/pmd/PMDConfiguration.java | 22 ------------------- .../sourceforge/pmd/cpd/CPDConfiguration.java | 10 --------- 3 files changed, 22 insertions(+), 32 deletions(-) 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 6d737ec2199..b4b0c68332e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -46,6 +46,7 @@ public abstract class AbstractConfiguration { private Path ignoreFilePath; private List excludes = new ArrayList<>(); private boolean collectRecursive = true; + private boolean failOnViolation = true; private boolean failOnProcessingError = true; @@ -379,6 +380,27 @@ public void collectFilesRecursively(boolean collectRecursive) { this.collectRecursive = collectRecursive; } + /** + * 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.). + * + * @return failOnViolation + */ + public boolean isFailOnViolation() { + return failOnViolation; + } + + /** + * 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 + */ + public void setFailOnViolation(boolean failOnViolation) { + this.failOnViolation = failOnViolation; + } + public boolean isFailOnProcessingError() { return failOnProcessingError; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/PMDConfiguration.java b/pmd-core/src/main/java/net/sourceforge/pmd/PMDConfiguration.java index 17a75577610..287d001f2ab 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/PMDConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/PMDConfiguration.java @@ -108,7 +108,6 @@ public class PMDConfiguration extends AbstractConfiguration { private String reportFormat; private Properties reportProperties = new Properties(); private boolean showSuppressedViolations = false; - private boolean failOnViolation = true; private AnalysisCache analysisCache = new NoopAnalysisCache(); private boolean ignoreIncrementalAnalysis; @@ -362,27 +361,6 @@ public void setReportProperties(Properties reportProperties) { this.reportProperties = reportProperties; } - /** - * 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.). - * - * @return failOnViolation - */ - public boolean isFailOnViolation() { - return failOnViolation; - } - - /** - * 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 - */ - public void setFailOnViolation(boolean failOnViolation) { - this.failOnViolation = failOnViolation; - } - /** * Retrieves the currently used analysis cache. Will never be null. * diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java index d97cc8068a8..edce7c89450 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java @@ -73,8 +73,6 @@ public class CPDConfiguration extends AbstractConfiguration { private boolean help; - private boolean failOnViolation = true; - public CPDConfiguration() { this(LanguageRegistry.CPD); @@ -260,14 +258,6 @@ public void setSkipBlocksPattern(String skipBlocksPattern) { this.skipBlocksPattern = skipBlocksPattern; } - public boolean isFailOnViolation() { - return failOnViolation; - } - - public void setFailOnViolation(boolean failOnViolation) { - this.failOnViolation = failOnViolation; - } - @Override protected void checkLanguageIsAcceptable(Language lang) throws UnsupportedOperationException { if (!(lang instanceof CpdCapableLanguage)) { From a63cfb8228b9d95a1ccd7189380b85607a8c3bc5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 10:15:10 +0200 Subject: [PATCH 05/15] [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 c616c1847b8..a74766610fe 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 58259b8a715..e9b679aa621 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 4c81ff7daf3..44f1813f30a 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 1a3ab6dc48f..4510c977df5 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 a4be75fb15f..8b160790d96 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 @@ private void doTask() { 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 b949de739ce..30b312e69e5 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 @@ protected CPDConfiguration toConfiguration() { 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 547feac371f..3684a8ca87e 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 @@ protected CliExitCode doExecute(PMDConfiguration configuration) { // 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 43e11467fa2..0bdf2f479d4 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 static CliExitCode fromInt(int i) { 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 6597b4c4143..5f76af32f8e 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 @@ 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 @@ void testWrongCliOptionsDoPrintUsage() throws Exception { @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 @@ void testEncodingOption() throws Exception { */ @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 @@ void testSkipLexicalErrors() throws Exception { @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 @@ void testExitCodeWithLexicalErrorsNoFail() throws Exception { @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 36a19974a52..361c4891afa 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.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 @@ void exitStatusWithViolations() throws Exception { @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 b4b0c68332e..1118b97d85b 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 void collectFilesRecursively(boolean collectRecursive) { * 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 boolean isFailOnViolation() { * 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; } From 95215ecde7e8539b5d6373a7ebe0d5bb95e3aa3f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 10:42:24 +0200 Subject: [PATCH 06/15] Fix checkstyle --- pmd-cli/src/test/java/net/sourceforge/pmd/cli/CpdCliTest.java | 2 +- pmd-cli/src/test/java/net/sourceforge/pmd/cli/PmdCliTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) 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 5f76af32f8e..7e41671e280 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 @@ -5,8 +5,8 @@ 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.PROCESSING_ERRORS_OR_VIOLATIONS; +import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_FOUND; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.Matchers.containsString; 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 361c4891afa..03d91efbc24 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 @@ -6,9 +6,9 @@ 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.PROCESSING_ERRORS_OR_VIOLATIONS; 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.PROCESSING_ERRORS_OR_VIOLATIONS; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; From ab82f28cf1db6a66e28a5030c478929f1d1fdab7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 10:46:33 +0200 Subject: [PATCH 07/15] Fix integration test --- .../java/net/sourceforge/pmd/dist/BinaryDistributionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/dist/BinaryDistributionIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/dist/BinaryDistributionIT.java index 50bbbfccd0e..1b5c701c542 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/dist/BinaryDistributionIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/dist/BinaryDistributionIT.java @@ -191,7 +191,7 @@ void runPMDWithError() throws Exception { ExecutionResult result = PMDExecutor.runPMDRules(createTemporaryReportFile(), tempDir, srcDir, "src/test/resources/rulesets/sample-ruleset.xml"); - result.assertExitCode(0).assertStdErr(containsString("Run in verbose mode to see a stack-trace.")); + result.assertExitCode(5).assertStdErr(containsString("Run in verbose mode to see a stack-trace.")); } @Test From 15cc64ff2413f07d656bd230981c1c67dc9e65da Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 10:52:24 +0200 Subject: [PATCH 08/15] Fix PMD errors in compat6 --- .../src/main/java/net/sourceforge/pmd/PMDConfiguration.java | 2 ++ .../src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java | 2 ++ 2 files changed, 4 insertions(+) diff --git a/pmd-compat6/src/main/java/net/sourceforge/pmd/PMDConfiguration.java b/pmd-compat6/src/main/java/net/sourceforge/pmd/PMDConfiguration.java index f058699d12b..57aaaa39aa8 100644 --- a/pmd-compat6/src/main/java/net/sourceforge/pmd/PMDConfiguration.java +++ b/pmd-compat6/src/main/java/net/sourceforge/pmd/PMDConfiguration.java @@ -445,6 +445,7 @@ public void setReportProperties(Properties reportProperties) { * * @return failOnViolation */ + @Override public boolean isFailOnViolation() { return failOnViolation; } @@ -456,6 +457,7 @@ public boolean isFailOnViolation() { * @param failOnViolation * failOnViolation */ + @Override public void setFailOnViolation(boolean failOnViolation) { this.failOnViolation = failOnViolation; } diff --git a/pmd-compat6/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java b/pmd-compat6/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java index c49b21586c1..34f4764631a 100644 --- a/pmd-compat6/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java +++ b/pmd-compat6/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java @@ -275,10 +275,12 @@ public void setSkipBlocksPattern(String skipBlocksPattern) { this.skipBlocksPattern = skipBlocksPattern; } + @Override public boolean isFailOnViolation() { return failOnViolation; } + @Override public void setFailOnViolation(boolean failOnViolation) { this.failOnViolation = failOnViolation; } From facbdd402c3c7c422e46a750dbb4c5d51d4fd646 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 3 May 2024 12:53:40 +0200 Subject: [PATCH 09/15] [doc] Update last_updated tags [skip ci] --- docs/pages/pmd/userdocs/cli_reference.md | 1 + docs/pages/pmd/userdocs/cpd/cpd.md | 2 +- docs/pages/pmd/userdocs/tools/ant.md | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index a74766610fe..c7a74d16977 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -5,6 +5,7 @@ tags: [userdocs] keywords: [command, line, options, help, formats, renderers] permalink: pmd_userdocs_cli_reference.html author: Tom Copeland , Xavier Le Vourch , Juan Martín Sotuyo Dodero +last_updated: May 2024 (7.2.0) --- diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index e9b679aa621..02775e3c785 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -4,7 +4,7 @@ tags: [cpd, userdocs] summary: "Learn how to use CPD, the copy-paste detector shipped with PMD." permalink: pmd_userdocs_cpd.html author: Tom Copeland -last_updated: August 2023 (7.0.0) +last_updated: May 2024 (7.2.0) --- ## Overview diff --git a/docs/pages/pmd/userdocs/tools/ant.md b/docs/pages/pmd/userdocs/tools/ant.md index 44f1813f30a..f2e33a330e0 100644 --- a/docs/pages/pmd/userdocs/tools/ant.md +++ b/docs/pages/pmd/userdocs/tools/ant.md @@ -6,6 +6,7 @@ author: > David Dixon-Peugh , Tom Copeland , Xavier Le Vourch +last_updated: May 2024 (7.2.0) --- ## PMD From 06efd65b167750ed3efcf2d53e1a84b82a9c4ccd Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 May 2024 14:39:09 +0200 Subject: [PATCH 10/15] [cli] Rename new parameter to be `--no-fail-on-error` --- docs/pages/pmd/userdocs/cli_reference.md | 26 ++++++-------- docs/pages/pmd/userdocs/cpd/cpd.md | 14 ++++---- docs/pages/release_notes.md | 14 ++++---- .../AbstractAnalysisPmdSubcommand.java | 12 +++---- .../pmd/cli/commands/internal/CpdCommand.java | 8 ++--- .../pmd/cli/commands/internal/PmdCommand.java | 6 ++-- .../pmd/cli/internal/CliExitCode.java | 17 +++++----- .../net/sourceforge/pmd/cli/CpdCliTest.java | 14 ++++---- .../net/sourceforge/pmd/cli/PmdCliTest.java | 10 +++--- .../pmd/AbstractConfiguration.java | 34 +++++++++---------- .../net/sourceforge/pmd/reporting/Report.java | 21 +++++++++--- 11 files changed, 92 insertions(+), 84 deletions(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index c7a74d16977..cf90f0e39b1 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -20,7 +20,6 @@ The tool comes with a rather extensive help text, simply running with `--help`! Default value Applies to - {% include custom/cli_option_row.html options="--rulesets,-R" option_arg="refs" description="Path to a ruleset xml file. The path may reference @@ -56,7 +55,6 @@ The tool comes with a rather extensive help text, simply running with `--help`! (\":\" on Linux, \";\" on Windows) is used to separate the entries. Alternatively, a single `file:` URL to a text file containing path elements on consecutive lines can be specified. -

See also [Providing the auxiliary classpath](pmd_languages_java.html#providing-the-auxiliary-classpath).

" languages="Java" %} @@ -81,15 +79,15 @@ 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-error" + description="Specifies whether PMD exits with non-zero status if recoverable errors occurred. + By default PMD exits with status 5 if recoverable errors occurred (whether there are violations or not). + Disable this option with `--no-fail-on-error` to exit with 0 instead. In any case, a report with the found violations will be written." %} {% 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. - Disable this feature with `--no-fail-on-violation` to exit with 0 instead and just output the report." + Disable this feature with `--no-fail-on-violation` to exit with 0 instead. In any case a report with the found violations will be written." %} {% include custom/cli_option_row.html options="--file-list" option_arg="filepath" @@ -104,9 +102,7 @@ The tool comes with a rather extensive help text, simply running with `--help`! by extension is disabled and PMD tries to parse all files with the given language `<lang>`. Parsing errors are ignored and unparsable files are skipped. -

Use `--use-version` to specify the language version to use, if it is not the default.

-

This option allows to use the xml language for files, that don't use xml as extension. See [example](#analyze-other-xml-formats) below.

" %} @@ -131,7 +127,7 @@ The tool comes with a rather extensive help text, simply running with `--help`! {% include custom/cli_option_row.html options="--minimum-priority" option_arg="priority" description="Rule priority threshold; rules with lower priority than configured here won't be used. - Valid values (case insensitive): High, Medium_High, Medium, Medium_Low, Low. + Valid values (case-insensitive): High, Medium_High, Medium, Medium_Low, Low. An integer between 1 (High) and 5 (Low) is also supported. See [Configuring rules](pmd_userdocs_configuring_rules.html) on how to override priorities in custom rulesets." default="Low" @@ -147,7 +143,7 @@ The tool comes with a rather extensive help text, simply running with `--help`! description="Enables / disable progress bar indicator of live analysis progress. This ie enabled by default." %} {% include custom/cli_option_row.html options="--property,-P" - option_arg="name>= -0Everything is fine, no violations found and no processing error occurred. +0Everything is fine, no violations found and no recoverable 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.

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.

+5At least one recoverable error has occurred. There might be additionally zero or more violations detected. + To ignore recoverable errors, use --no-fail-on-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 +In any case, the root cause should be investigated. If it's a problem in PMD itself, please create a bug report. Recoverable errors are usually part of the generated PMD report." %} ## Logging diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index 02775e3c785..d13416eacc7 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -150,10 +150,10 @@ 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-error" + description="Specifies whether CPD exits with non-zero status if recoverable errors occurred. + By default CPD exits with status 5 if recoverable errors occurred (whether there are duplications or not). + Disable this option with `--no-fail-on-error` to exit with 0 instead. In any case, a report with the found duplications will be written." %} {% include custom/cli_option_row.html options="--[no-]fail-on-violation" description="Specifies whether CPD exits with non-zero status if violations are found. @@ -288,12 +288,12 @@ Please note that if CPD detects duplicated source code, it will exit with status This behavior has been introduced to ease CPD integration into scripts or hooks, such as SVN hooks. - + - +
0Everything is fine, no code duplications found and no processing errors occurred.
0Everything is fine, no code duplications found and no recoverable 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.

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.

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

Since PMD 7.2.0.

{%include note.html content="If PMD exits with 5, then PMD had trouble lexing one or more files. diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b9600bd0c0d..8a90e752d53 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -53,15 +53,17 @@ Since this release, PMD will also expose any getter returning a collection of an #### 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. +* New exit code 5 introduced. PMD and CPD will exit now by default with exit code 5, if any recoverable error + (e.g. parsing exception, lexing exception or rule exception) occurred. PMD will still create a report with + all detected violations or duplications if recoverable errors occurred. Such errors mean, that the report + might be incomplete, as either violations or duplications for an entire file or for a specific rule are missing. + 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. +* New CLI parameter `--no-fail-on-error` to ignore such errors and not exit with code 5. By default, + a build with errors will now fail and with that parameter, the previous behavior can be restored. + This parameter is available for both PMD and CPD. #### Deprecated API diff --git a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java index c5535393310..02a090ab3ff 100644 --- a/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java +++ b/pmd-cli/src/main/java/net/sourceforge/pmd/cli/commands/internal/AbstractAnalysisPmdSubcommand.java @@ -43,16 +43,16 @@ public abstract class AbstractAnalysisPmdSubcommand relativizeRootPaths; 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 30b312e69e5..a02723b1b02 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 @@ -103,7 +103,7 @@ protected CPDConfiguration toConfiguration() { configuration.addRelativizeRoots(relativizeRootPaths); } configuration.setFailOnViolation(failOnViolation); - configuration.setFailOnProcessingError(failOnProcessingError); + configuration.setFailOnError(failOnError); configuration.setInputFilePath(fileListPath); if (inputPaths != null) { configuration.setInputPathList(new ArrayList<>(inputPaths)); @@ -134,9 +134,9 @@ protected CPDConfiguration toConfiguration() { MutableBoolean hasViolations = new MutableBoolean(); cpd.performAnalysis(report -> hasViolations.setValue(!report.getMatches().isEmpty())); - boolean hasProcessingErrors = configuration.getReporter().numErrors() > 0; - if (hasProcessingErrors && configuration.isFailOnProcessingError()) { - return CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; + boolean hasErrors = configuration.getReporter().numErrors() > 0; + if (hasErrors && configuration.isFailOnError()) { + return CliExitCode.RECOVERED_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 3684a8ca87e..f60632ba63e 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 @@ -270,7 +270,7 @@ protected PMDConfiguration toConfiguration() { configuration.setSuppressMarker(suppressMarker); configuration.setThreads(threads); configuration.setFailOnViolation(failOnViolation); - configuration.setFailOnProcessingError(failOnProcessingError); + configuration.setFailOnError(failOnError); configuration.setAnalysisCacheLocation(cacheLocation != null ? cacheLocation.toString() : null); configuration.setIgnoreIncrementalAnalysis(noCache); @@ -331,8 +331,8 @@ protected CliExitCode doExecute(PMDConfiguration configuration) { if (pmdReporter.numErrors() > 0) { // processing errors are ignored return CliExitCode.ERROR; - } else if (stats.getNumErrors() > 0 && configuration.isFailOnProcessingError()) { - return CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; + } else if (stats.getNumErrors() > 0 && configuration.isFailOnError()) { + return CliExitCode.RECOVERED_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 0bdf2f479d4..909f942c6c3 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 @@ -10,7 +10,7 @@ * The execution result of any given command. */ public enum CliExitCode { - /** No errors, no processing errors, no violations. This is exit code {@code 0}. */ + /** No errors, no recoverable errors, no violations, no duplications. This is exit code {@code 0}. */ OK(0), /** * Unexpected errors were detected, PMD may have not run to the end. @@ -19,25 +19,24 @@ public enum CliExitCode { ERROR(1), /** * Indicates a problem with the CLI parameters: either a required - * parameter is missing or an invalid parameter was provided. + * parameter is missing or an invalid parameter was provided. This is exit code {@code 2}. */ USAGE_ERROR(2), /** - * 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}. + * No errors, but PMD found either duplications/violations. This is exit code {@code 4}. * *

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 was at least one processing error. There + * PMD did run, but there was at least one recoverable error. There * might be additionally duplications or violations. This is exit code {@code 5}. * - *

This is only returned if {@link AbstractConfiguration#isFailOnProcessingError()} - * is set. It can be disabled by using CLI flag {@code --no-fail-on-processing-error}. + *

This is only returned if {@link AbstractConfiguration#isFailOnError()} + * is set. It can be disabled by using CLI flag {@code --no-fail-on-error}. */ - PROCESSING_ERRORS_OR_VIOLATIONS(5); + RECOVERED_ERRORS_OR_VIOLATIONS(5); private final int exitCode; @@ -55,7 +54,7 @@ public static CliExitCode fromInt(int i) { case 1: return ERROR; case 2: return USAGE_ERROR; case 4: return VIOLATIONS_FOUND; - case 5: return PROCESSING_ERRORS_OR_VIOLATIONS; + case 5: return RECOVERED_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 7e41671e280..fd8a60d3797 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 @@ -5,7 +5,7 @@ package net.sourceforge.pmd.cli; import static net.sourceforge.pmd.cli.internal.CliExitCode.OK; -import static net.sourceforge.pmd.cli.internal.CliExitCode.PROCESSING_ERRORS_OR_VIOLATIONS; +import static net.sourceforge.pmd.cli.internal.CliExitCode.RECOVERED_ERRORS_OR_VIOLATIONS; import static net.sourceforge.pmd.cli.internal.CliExitCode.VIOLATIONS_FOUND; import static net.sourceforge.pmd.util.CollectionUtil.listOf; import static org.hamcrest.CoreMatchers.startsWith; @@ -129,14 +129,14 @@ void testWrongCliOptionsDoPrintUsage() throws Exception { @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(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "34", "--dir", SRC_DIR, "--ignore-identifiers", "false"); + final CliExecutionResult result = runCli(RECOVERED_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(PROCESSING_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); + final CliExecutionResult result = runCli(RECOVERED_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "34", "--ignore-identifiers", "false", "--dir", SRC_DIR); result.checkStdErr(containsString("No such file false")); } @@ -221,7 +221,7 @@ void testEncodingOption() throws Exception { */ @Test void testSkipLexicalErrors() throws Exception { - runCli(PROCESSING_ERRORS_OR_VIOLATIONS, + runCli(RECOVERED_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "10", "-d", BASE_RES_PATH + "badandgood/", "--format", "text", @@ -234,7 +234,7 @@ void testSkipLexicalErrors() throws Exception { @Test void testExitCodeWithLexicalErrors() throws Exception { - runCli(PROCESSING_ERRORS_OR_VIOLATIONS, + runCli(RECOVERED_ERRORS_OR_VIOLATIONS, "--minimum-tokens", "10", "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text") @@ -250,7 +250,7 @@ void testExitCodeWithLexicalErrorsNoFail() throws Exception { "--minimum-tokens", "10", "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text", - "--no-fail-on-processing-error") + "--no-fail-on-error") .verify(r -> { r.checkStdErr(containsPattern("Error while tokenizing: Lexical error in file '.*?BadFile\\.java'")); r.checkStdOut(emptyString()); @@ -259,7 +259,7 @@ void testExitCodeWithLexicalErrorsNoFail() throws Exception { @Test void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { - runCli(PROCESSING_ERRORS_OR_VIOLATIONS, + runCli(RECOVERED_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 03d91efbc24..fd77c97b42f 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 @@ -6,7 +6,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.PROCESSING_ERRORS_OR_VIOLATIONS; +import static net.sourceforge.pmd.cli.internal.CliExitCode.RECOVERED_ERRORS_OR_VIOLATIONS; 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.util.CollectionUtil.listOf; @@ -320,8 +320,8 @@ void exitStatusWithViolations() throws Exception { } @Test - void exitStatusWithProcessingErrors() throws Exception { - runCli(PROCESSING_ERRORS_OR_VIOLATIONS, "--use-version", "dummy-parserThrows", + void exitStatusWithErrors() throws Exception { + runCli(RECOVERED_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")); @@ -330,10 +330,10 @@ void exitStatusWithProcessingErrors() throws Exception { } @Test - void exitStatusWithProcessingErrorsNoFail() throws Exception { + void exitStatusWithErrorsNoFail() throws Exception { runCli(OK, "--use-version", "dummy-parserThrows", "-d", srcDir.toString(), "-f", "text", "-R", RULESET_WITH_VIOLATION, - "--no-fail-on-processing-error") + "--no-fail-on-error") .verify(r -> { r.checkStdOut(containsString("someSource.dummy\t-\tParseException: Parse exception: ohio")); r.checkStdErr(containsString("An error occurred while executing PMD.")); 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 1118b97d85b..d20e5cca3bd 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -47,7 +47,7 @@ public abstract class AbstractConfiguration { private List excludes = new ArrayList<>(); private boolean collectRecursive = true; private boolean failOnViolation = true; - private boolean failOnProcessingError = true; + private boolean failOnError = true; protected AbstractConfiguration(LanguageRegistry languageRegistry, PmdReporter messageReporter) { @@ -384,12 +384,12 @@ public void collectFilesRecursively(boolean collectRecursive) { * 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()}. + *

Note: If additionally recoverable errors occurred, the exit status is 5. See + * {@link #isFailOnError()}. * * @return failOnViolation * - * @see #isFailOnProcessingError() + * @see #isFailOnError() */ public boolean isFailOnViolation() { return failOnViolation; @@ -399,12 +399,12 @@ public boolean isFailOnViolation() { * 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.). * - *

Note: If additionally processing errors occurred, the exit status is 5. See - * {@link #isFailOnProcessingError()}. + *

Note: If additionally recoverable errors occurred, the exit status is 5. See + * {@link #isFailOnError()}. * * @param failOnViolation whether to exit with 4 and fail the build if violations are found. * - * @see #isFailOnProcessingError() + * @see #isFailOnError() */ public void setFailOnViolation(boolean failOnViolation) { this.failOnViolation = failOnViolation; @@ -412,31 +412,31 @@ public void setFailOnViolation(boolean 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.). + * recoverable 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 + *

Note: If only violations are found, the exist status is 4. See * {@link #isFailOnViolation()}. * - * @return failOnProcessingError + * @return failOnError * * @see #isFailOnViolation() */ - public boolean isFailOnProcessingError() { - return failOnProcessingError; + public boolean isFailOnError() { + return failOnError; } /** * 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.). + * if recoverable 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 + *

Note: If only 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. + * @param failOnError whether to exit with 5 and fail the build if recoverable errors occurred. * * @see #isFailOnViolation() */ - public void setFailOnProcessingError(boolean failOnProcessingError) { - this.failOnProcessingError = failOnProcessingError; + public void setFailOnError(boolean failOnError) { + this.failOnError = failOnError; } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/Report.java b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/Report.java index a13d468952f..10f0058e94e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/reporting/Report.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/reporting/Report.java @@ -22,8 +22,8 @@ import net.sourceforge.pmd.util.BaseResultProducingCloseable; /** - * A {@link Report} collects all information during a PMD execution. This - * includes violations, suppressed violations, metrics, error during processing + * A {@link Report} collects all information during a PMD execution. This includes violations, + * suppressed violations, metrics, recoverable errors (that occurred during processing) * and configuration errors. * *

A report may be created by a {@link GlobalReportBuilderListener} that you @@ -50,7 +50,10 @@ private Report() { } /** - * Represents a configuration error. + * Represents a configuration error for a specific rule. + * + *

This might be a missing rule property + * or rule property with pointless values. */ public static class ConfigurationError { @@ -90,7 +93,15 @@ public String issue() { } /** - * Represents a processing error, such as a parse error. + * Represents a recovered error that occurred during analysis. + * + *

This might be a parse error or an unexpected error originating from a rule. + * Such errors are called recoverable, because PMD can just skip + * the problematic file, continue the analysis with the other files and still create a report. + * However, due to these errors, the report might be incomplete. + * + *

Some report formats, such as {@link net.sourceforge.pmd.renderers.XMLRenderer}, include these + * errors for further investigation. */ public static class ProcessingError { @@ -98,7 +109,7 @@ public static class ProcessingError { private final FileId file; /** - * Creates a new processing error + * Creates a new processing error. * * @param error * the error From b624b41673f923ba5069dfc77d6f86ca594e3494 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 May 2024 15:17:39 +0200 Subject: [PATCH 11/15] [ant] Add new CPD parameter failOnError --- docs/pages/pmd/userdocs/cpd/cpd.md | 4 ++++ docs/pages/release_notes.md | 6 +++++ .../java/net/sourceforge/pmd/ant/CPDTask.java | 18 +++++++++++++++ .../net/sourceforge/pmd/ant/CPDTaskTest.java | 22 ++++++++++++++++++- .../pmd/ant/src/sampleLexError.dummy | 1 + .../sourceforge/pmd/ant/xml/cpdtasktest.xml | 17 ++++++++++++++ .../pmd/ant/xml/expected-pmd-ant-xml.xml | 8 +++++++ 7 files changed, 75 insertions(+), 1 deletion(-) create mode 100644 pmd-ant/src/test/resources/net/sourceforge/pmd/ant/src/sampleLexError.dummy diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index d13416eacc7..7560694e1de 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -401,6 +401,10 @@ Andy Glover wrote an Ant task for CPD; here's how to use it: keep their encoding.
If not specified, CPD uses the system default encoding." %} + {% include custom/cli_option_row.html options="failOnError" + description="Whether to fail the build if any errors occur while processing the files. Since PMD 7.2.0." + default="true" + %} {% include custom/cli_option_row.html options="format" description="The format of the report (e.g. `csv`, `text`, `xml`)." default="text" diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8a90e752d53..edb8ce6debb 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -65,6 +65,12 @@ Since this release, PMD will also expose any getter returning a collection of an a build with errors will now fail and with that parameter, the previous behavior can be restored. This parameter is available for both PMD and CPD. +##### Ant + +* CPDTask has a new parameter `failOnError`. In controls, whether to fail the build if any recoverable errors occurred. + By default, the build will fail. CPD will still create a report with all detected duplications, but the report might + be incomplete. + #### Deprecated API * pmd-java diff --git a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java index 607186cb8a9..31fcd376006 100644 --- a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java +++ b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java @@ -82,6 +82,7 @@ public class CPDTask extends Task { private File outputFile; private String encoding = System.getProperty("file.encoding"); private List filesets = new ArrayList<>(); + private boolean failOnError = true; @Override public void execute() throws BuildException { @@ -118,6 +119,14 @@ public void execute() throws BuildException { long timeTaken = System.currentTimeMillis() - start; log("Done analyzing code; that took " + timeTaken + " milliseconds"); + int errors = config.getReporter().numErrors(); + if (errors > 0) { + if (failOnError) { + throw new BuildException("There were " + errors + " recovered errors during analysis. Ignore these with failOnError=\"true\"."); + } else { + log("There were " + errors + " recovered errors during analysis. Not failing build, because failOnError=\"false\".", Project.MSG_WARN); + } + } } } catch (IOException ioe) { log(ioe.toString(), Project.MSG_ERR); @@ -252,6 +261,15 @@ public void setSkipBlocksPattern(String skipBlocksPattern) { this.skipBlocksPattern = skipBlocksPattern; } + /** + * Whether to fail the build if any recoverable errors occurred while processing the files. + * + * @since 7.2.0 + */ + public void setFailOnError(boolean failOnError) { + this.failOnError = failOnError; + } + public static class FormatAttribute extends EnumeratedAttribute { private static final String[] FORMATS = new String[] { XML_FORMAT, TEXT_FORMAT, CSV_FORMAT }; diff --git a/pmd-ant/src/test/java/net/sourceforge/pmd/ant/CPDTaskTest.java b/pmd-ant/src/test/java/net/sourceforge/pmd/ant/CPDTaskTest.java index 79b50a0dee7..83b56f8781c 100644 --- a/pmd-ant/src/test/java/net/sourceforge/pmd/ant/CPDTaskTest.java +++ b/pmd-ant/src/test/java/net/sourceforge/pmd/ant/CPDTaskTest.java @@ -6,6 +6,7 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; @@ -14,6 +15,7 @@ import java.nio.file.Path; import java.nio.file.Paths; +import org.apache.tools.ant.BuildException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; @@ -34,7 +36,25 @@ void setUp() { @Test void testBasic() throws IOException { executeTarget("testBasic"); - Path report = Paths.get("target/cpd.ant.tests"); + assertReport("target/cpd.ant.tests"); + } + + @Test + void failOnErrorDefault() throws IOException { + BuildException buildException = assertThrows(BuildException.class, () -> executeTarget("failOnErrorDefault")); + assertThat(buildException.getMessage(), containsString("There were 1 recovered errors during analysis.")); + assertReport("target/cpd.ant.tests"); + } + + @Test + void failOnErrorIgnore() throws IOException { + executeTarget("failOnErrorIgnore"); + assertReport("target/cpd.ant.tests"); + assertThat(log.toString(), containsString("There were 1 recovered errors during analysis.")); + } + + private static void assertReport(String path) throws IOException { + Path report = Paths.get(path); assertTrue(Files.exists(report), "Report was not created"); String reportContent = IOUtil.readFileToString(report.toFile(), StandardCharsets.UTF_8); assertThat(reportContent, containsString("Found a 1 line (21 tokens) duplication in the following files:")); diff --git a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/src/sampleLexError.dummy b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/src/sampleLexError.dummy new file mode 100644 index 00000000000..3175abe874d --- /dev/null +++ b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/src/sampleLexError.dummy @@ -0,0 +1 @@ +:throw_lex_source_exception: diff --git a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml index df4f993f921..aef5c73621e 100644 --- a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml +++ b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml @@ -11,7 +11,24 @@ + + + + + + + + + + + + + + + + + diff --git a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/expected-pmd-ant-xml.xml b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/expected-pmd-ant-xml.xml index 60456101228..5eb60815928 100644 --- a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/expected-pmd-ant-xml.xml +++ b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/expected-pmd-ant-xml.xml @@ -16,4 +16,12 @@ Test Rule 2 Test Rule 3 + + +Test Rule 2 + + +Test Rule 3 + + From 10c78f26deeb57b8b1f3b05695b6180d2d6e1964 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 May 2024 16:16:20 +0200 Subject: [PATCH 12/15] [cpd] Deprecate skipLexicalErrors --- docs/pages/pmd/userdocs/cpd/cpd.md | 10 ++++++---- docs/pages/release_notes.md | 3 +++ .../java/net/sourceforge/pmd/ant/CPDTask.java | 15 +++++++++++++-- .../net/sourceforge/pmd/ant/xml/cpdtasktest.xml | 4 ++-- .../pmd/cli/commands/internal/CpdCommand.java | 14 +++++++++++++- .../java/net/sourceforge/pmd/cli/CpdCliTest.java | 8 ++++---- .../net/sourceforge/pmd/cpd/CPDConfiguration.java | 11 +++++++++++ .../java/net/sourceforge/pmd/cpd/CpdAnalysis.java | 3 +++ 8 files changed, 55 insertions(+), 13 deletions(-) diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index 7560694e1de..2457b13c0f0 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -132,8 +132,8 @@ exactly identical. description="Don't scan subdirectories. By default, subdirectories are considered." %} {% include custom/cli_option_row.html options="--skip-lexical-errors" - description="Skip files which can't be tokenized due to invalid characters instead of aborting CPD. - By default, CPD analysis is stopped on the first error." + description="Deprecated Skip files which can't be tokenized due to invalid characters instead of aborting CPD. + By default, CPD analysis is stopped on the first error. This is deprecated. Use `--fail-on-error` instead." %} {% include custom/cli_option_row.html options="--format,-f" option_arg="format" @@ -439,8 +439,10 @@ Andy Glover wrote an Ant task for CPD; here's how to use it: default="false" %} {% include custom/cli_option_row.html options="skipLexicalErrors" - description="Skip files which can't be tokenized due to invalid characters instead of aborting CPD." - default="false" + description="Deprecated Skip files which can't be tokenized + due to invalid characters instead of aborting CPD. This parameter is deprecated and + ignored since PMD 7.2.0. It is now by default true. Use `failOnError` instead to fail the build." + default="true" %} {% include custom/cli_option_row.html options="skipBlocks" description="Enables or disabled skipping of blocks like a pre-processor. See also option skipBlocksPattern." diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index edb8ce6debb..fd4c83473a6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -65,11 +65,14 @@ Since this release, PMD will also expose any getter returning a collection of an a build with errors will now fail and with that parameter, the previous behavior can be restored. This parameter is available for both PMD and CPD. +* The CLI parameter `--skip-lexical-errors` is deprecated. Use the new parameter `--[no-]--fail-on-error` instead. + ##### Ant * CPDTask has a new parameter `failOnError`. In controls, whether to fail the build if any recoverable errors occurred. By default, the build will fail. CPD will still create a report with all detected duplications, but the report might be incomplete. +* The parameter `skipLexicalError` in CPDTask is deprecated. Use the new parameter `failOnError` instead. #### Deprecated API diff --git a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java index 31fcd376006..423f48d41c6 100644 --- a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java +++ b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java @@ -75,6 +75,7 @@ public class CPDTask extends Task { private boolean ignoreIdentifiers; private boolean ignoreAnnotations; private boolean ignoreUsings; + @Deprecated private boolean skipLexicalErrors; private boolean skipDuplicateFiles; private boolean skipBlocks = true; @@ -100,7 +101,14 @@ public void execute() throws BuildException { config.setOnlyRecognizeLanguage(config.getLanguageRegistry().getLanguageById(language)); config.setSourceEncoding(Charset.forName(encoding)); config.setSkipDuplicates(skipDuplicateFiles); - config.setSkipLexicalErrors(skipLexicalErrors); + + if (skipLexicalErrors) { + log("skipLexicalErrors is deprecated and ignored. Lexical errors are now by default skipped. Use failOnError=\"false\" to not fail the build.", Project.MSG_WARN); + failOnError = false; + } + + // implicitly enable skipLexicalErrors, so that we can fail the build at the end. A report is created in any case. + config.setSkipLexicalErrors(true); config.setIgnoreAnnotations(ignoreAnnotations); config.setIgnoreLiterals(ignoreLiterals); @@ -132,7 +140,6 @@ public void execute() throws BuildException { log(ioe.toString(), Project.MSG_ERR); throw new BuildException("IOException during task execution", ioe); } catch (ReportException re) { - re.printStackTrace(); log(re.toString(), Project.MSG_ERR); throw new BuildException("ReportException during task execution", re); } finally { @@ -229,6 +236,10 @@ public void setIgnoreUsings(boolean value) { this.ignoreUsings = value; } + /** + * @deprecated Use {@link #setFailOnError(boolean)} instead. + */ + @Deprecated public void setSkipLexicalErrors(boolean skipLexicalErrors) { this.skipLexicalErrors = skipLexicalErrors; } diff --git a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml index aef5c73621e..922e4a2a9a5 100644 --- a/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml +++ b/pmd-ant/src/test/resources/net/sourceforge/pmd/ant/xml/cpdtasktest.xml @@ -17,7 +17,7 @@ - + @@ -25,7 +25,7 @@ - + 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 a02723b1b02..fe8a835774f 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 @@ -68,8 +68,12 @@ public class CpdCommand extends AbstractAnalysisPmdSubcommand @Option(names = "--ignore-sequences", description = "Ignore sequences of identifiers and literals") private boolean ignoreIdentifierAndLiteralSequences; + /** + * @deprecated Use {@link #failOnError} instead. + */ @Option(names = "--skip-lexical-errors", - description = "Skip files which can't be tokenized due to invalid characters, instead of aborting with an error.") + description = "Skip files which can't be tokenized due to invalid characters, instead of aborting with an error. Deprecated - use --[no-]fail-on-error instead.") + @Deprecated private boolean skipLexicalErrors; @Option(names = "--no-skip-blocks", @@ -124,6 +128,14 @@ protected CPDConfiguration toConfiguration() { configuration.setSourceEncoding(encoding.getEncoding()); configuration.setInputUri(uri); + if (skipLexicalErrors) { + configuration.getReporter().warn("--skip-lexical-errors is deprecated. Use --no-fail-on-error instead."); + configuration.setFailOnError(false); + } + + // implicitly enable skipLexicalErrors, so that we can fail the build at the end. A report is created in any case. + configuration.setSkipLexicalErrors(true); + return configuration; } 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 fd8a60d3797..c7eab5f88fa 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 @@ -221,7 +221,7 @@ void testEncodingOption() throws Exception { */ @Test void testSkipLexicalErrors() throws Exception { - runCli(RECOVERED_ERRORS_OR_VIOLATIONS, + runCli(VIOLATIONS_FOUND, "--minimum-tokens", "10", "-d", BASE_RES_PATH + "badandgood/", "--format", "text", @@ -239,7 +239,7 @@ void testExitCodeWithLexicalErrors() throws Exception { "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text") .verify(r -> { - r.checkStdErr(containsPattern("Error while tokenizing: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdErr(containsPattern("Skipping file: Lexical error in file '.*?BadFile\\.java'")); r.checkStdOut(emptyString()); }); } @@ -252,14 +252,14 @@ void testExitCodeWithLexicalErrorsNoFail() throws Exception { "--format", "text", "--no-fail-on-error") .verify(r -> { - r.checkStdErr(containsPattern("Error while tokenizing: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdErr(containsPattern("Skipping file: Lexical error in file '.*?BadFile\\.java'")); r.checkStdOut(emptyString()); }); } @Test void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { - runCli(RECOVERED_ERRORS_OR_VIOLATIONS, + runCli(OK, "--minimum-tokens", "10", "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), "--format", "text", diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java index edce7c89450..acc2a1d6ac0 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDConfiguration.java @@ -65,6 +65,7 @@ public class CPDConfiguration extends AbstractConfiguration { private boolean ignoreIdentifierAndLiteralSequences = false; + @Deprecated private boolean skipLexicalErrors = false; private boolean noSkipBlocks = false; @@ -226,10 +227,20 @@ public void setIgnoreIdentifierAndLiteralSequences(boolean ignoreIdentifierAndLi this.ignoreIdentifierAndLiteralSequences = ignoreIdentifierAndLiteralSequences; } + /** + * @deprecated This option will be removed. With {@link #isFailOnError()}, you can + * control whether lexical errors should fail the build or not. + */ + @Deprecated public boolean isSkipLexicalErrors() { return skipLexicalErrors; } + /** + * @deprecated This option will be removed. With {@link #setFailOnError(boolean)}, you can + * control whether lexical errors should fail the build or not. + */ + @Deprecated public void setSkipLexicalErrors(boolean skipLexicalErrors) { this.skipLexicalErrors = skipLexicalErrors; } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CpdAnalysis.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CpdAnalysis.java index ccc2b6c6867..fb47dc0a31c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CpdAnalysis.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/CpdAnalysis.java @@ -151,6 +151,9 @@ public void performAnalysis() { @SuppressWarnings("PMD.CloseResource") public void performAnalysis(Consumer consumer) { + if (configuration.isSkipLexicalErrors()) { + LOGGER.warn("The option skipLexicalErrors is deprecated. Use failOnError instead."); + } try (SourceManager sourceManager = new SourceManager(files.getCollectedFiles())) { Map tokenizers = From 603187e41be9458b521552cc9551c32caa78e224 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 17 May 2024 16:37:37 +0200 Subject: [PATCH 13/15] Update javadoc --- .../java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java | 2 +- .../main/java/net/sourceforge/pmd/AbstractConfiguration.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) 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 8b160790d96..5f5e1f9bd91 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 @@ private void doTask() { pmd.performAnalysis(); stats = reportStatsListener.getResult(); if (failOnError && pmd.getReporter().numErrors() > 0) { - throw new BuildException("Some processing errors occurred while running PMD"); + throw new BuildException("Some recoverable errors occurred while running PMD"); } } 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 d20e5cca3bd..35261b13572 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -390,6 +390,7 @@ public void collectFilesRecursively(boolean collectRecursive) { * @return failOnViolation * * @see #isFailOnError() + * @since 7.2.0 */ public boolean isFailOnViolation() { return failOnViolation; @@ -405,6 +406,7 @@ public boolean isFailOnViolation() { * @param failOnViolation whether to exit with 4 and fail the build if violations are found. * * @see #isFailOnError() + * @since 6.0.0 */ public void setFailOnViolation(boolean failOnViolation) { this.failOnViolation = failOnViolation; @@ -420,6 +422,7 @@ public void setFailOnViolation(boolean failOnViolation) { * @return failOnError * * @see #isFailOnViolation() + * @since 6.0.0 */ public boolean isFailOnError() { return failOnError; @@ -435,6 +438,7 @@ public boolean isFailOnError() { * @param failOnError whether to exit with 5 and fail the build if recoverable errors occurred. * * @see #isFailOnViolation() + * @since 7.2.0 */ public void setFailOnError(boolean failOnError) { this.failOnError = failOnError; From 0fe477ec06d223300ad3efe6306dcc6ee4b04a47 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 20 May 2024 09:25:06 +0200 Subject: [PATCH 14/15] Fixups from PR review - release notes: API Changes - fix javadoc since tags - improve messages in ant task for deprecated skipLexicalErrors --- docs/pages/pmd/userdocs/cpd/cpd.md | 2 +- docs/pages/pmd/userdocs/tools/ant.md | 2 +- docs/pages/release_notes.md | 16 ++++++++++++---- .../java/net/sourceforge/pmd/ant/CPDTask.java | 10 ++++++---- .../sourceforge/pmd/AbstractConfiguration.java | 4 ++-- 5 files changed, 22 insertions(+), 12 deletions(-) diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index 2457b13c0f0..0be9c12cacd 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -402,7 +402,7 @@ Andy Glover wrote an Ant task for CPD; here's how to use it: If not specified, CPD uses the system default encoding." %} {% include custom/cli_option_row.html options="failOnError" - description="Whether to fail the build if any errors occur while processing the files. Since PMD 7.2.0." + description="Whether to fail the build if any errors occurred while processing the files. Since PMD 7.2.0." default="true" %} {% include custom/cli_option_row.html options="format" diff --git a/docs/pages/pmd/userdocs/tools/ant.md b/docs/pages/pmd/userdocs/tools/ant.md index f2e33a330e0..f81e1aa5468 100644 --- a/docs/pages/pmd/userdocs/tools/ant.md +++ b/docs/pages/pmd/userdocs/tools/ant.md @@ -65,7 +65,7 @@ The examples below won't repeat this taskdef element, as this is always required failOnError - Whether or not to fail the build if any processing errors occur while analyzing files. + Whether or not to fail the build if any recoverable errors occurred while analyzing files. No diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index fd4c83473a6..78bf29e02f7 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -65,19 +65,27 @@ Since this release, PMD will also expose any getter returning a collection of an a build with errors will now fail and with that parameter, the previous behavior can be restored. This parameter is available for both PMD and CPD. -* The CLI parameter `--skip-lexical-errors` is deprecated. Use the new parameter `--[no-]--fail-on-error` instead. +* The CLI parameter `--skip-lexical-errors` is deprecated. By default, lexical errors are skipped but the + build is failed. Use the new parameter `--[no-]fail-on-error` instead to control whether to fail the build or not. ##### Ant -* CPDTask has a new parameter `failOnError`. In controls, whether to fail the build if any recoverable errors occurred. +* CPDTask has a new parameter `failOnError`. It controls, whether to fail the build if any recoverable error occurred. By default, the build will fail. CPD will still create a report with all detected duplications, but the report might be incomplete. -* The parameter `skipLexicalError` in CPDTask is deprecated. Use the new parameter `failOnError` instead. +* The parameter `skipLexicalError` in CPDTask is deprecated and ignored. Lexical errors are now always skipped. + Use the new parameter `failOnError` instead to control whether to fail the build or not. #### Deprecated API +* pmd-ant + * {% jdoc !!ant::ant.CPDTask#setSkipLexicalErrors(boolean) %}: Use {% jdoc ant::ant.CPDTask#setFailOnError(boolean) %} + instead to control, whether to ignore errors or fail the build. +* pmd-core + * {% jdoc !!core::cpd.CPDConfiguration#isSkipLexicalErrors() %} and {% jdoc core::cpd.CPDConfiguration#setSkipLexicalErrors(boolean) %}: + Use {%jdoc core::AbstractConfiguration#setFailOnError(boolean) %} to control whether to ignore errors or fail the build. * pmd-java - * {% jdoc !!java::lang.java.ast.ASTResource#getStableName() %} and the corresponding attribute `@StableName` + * {% jdoc !!java::lang.java.ast.ASTResource#getStableName() %} and the corresponding attribute `@StableName`. ### ✨ External Contributions diff --git a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java index 423f48d41c6..b82671c22fe 100644 --- a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java +++ b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java @@ -103,8 +103,9 @@ public void execute() throws BuildException { config.setSkipDuplicates(skipDuplicateFiles); if (skipLexicalErrors) { - log("skipLexicalErrors is deprecated and ignored. Lexical errors are now by default skipped. Use failOnError=\"false\" to not fail the build.", Project.MSG_WARN); - failOnError = false; + log("skipLexicalErrors is deprecated since 7.2.0 and the property is ignored. " + + "Lexical errors are now skipped by default and the build is failed. " + + "Use failOnError=\"false\" to not fail the build.", Project.MSG_WARN); } // implicitly enable skipLexicalErrors, so that we can fail the build at the end. A report is created in any case. @@ -129,10 +130,11 @@ public void execute() throws BuildException { int errors = config.getReporter().numErrors(); if (errors > 0) { + String message = String.format("There were %d recovered errors during analysis.", errors); if (failOnError) { - throw new BuildException("There were " + errors + " recovered errors during analysis. Ignore these with failOnError=\"true\"."); + throw new BuildException(message + " Ignore these with failOnError=\"true\"."); } else { - log("There were " + errors + " recovered errors during analysis. Not failing build, because failOnError=\"false\".", Project.MSG_WARN); + log(message + " Not failing build, because failOnError=\"false\".", Project.MSG_WARN); } } } 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 35261b13572..567917a98f3 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -390,7 +390,7 @@ public void collectFilesRecursively(boolean collectRecursive) { * @return failOnViolation * * @see #isFailOnError() - * @since 7.2.0 + * @since 6.0.0 */ public boolean isFailOnViolation() { return failOnViolation; @@ -422,7 +422,7 @@ public void setFailOnViolation(boolean failOnViolation) { * @return failOnError * * @see #isFailOnViolation() - * @since 6.0.0 + * @since 7.2.0 */ public boolean isFailOnError() { return failOnError; From d285082709d4dfdf81f885914640337805191290 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 21 Jun 2024 19:23:55 +0200 Subject: [PATCH 15/15] Updated doc to mention 7.3.0 --- docs/pages/pmd/userdocs/cli_reference.md | 6 +++--- docs/pages/pmd/userdocs/cpd/cpd.md | 10 +++++----- docs/pages/pmd/userdocs/tools/ant.md | 2 +- .../src/main/java/net/sourceforge/pmd/ant/CPDTask.java | 4 ++-- .../net/sourceforge/pmd/AbstractConfiguration.java | 8 ++++---- 5 files changed, 15 insertions(+), 15 deletions(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index cf90f0e39b1..a499b49cf82 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -5,7 +5,7 @@ tags: [userdocs] keywords: [command, line, options, help, formats, renderers] permalink: pmd_userdocs_cli_reference.html author: Tom Copeland , Xavier Le Vourch , Juan Martín Sotuyo Dodero -last_updated: May 2024 (7.2.0) +last_updated: June 2024 (7.3.0) --- @@ -210,7 +210,7 @@ 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) or 5 (since 7.2.0). +Please note that if PMD detects any violations, it will exit with status 4 (since 5.3) or 5 (since 7.3.0). This behavior has been introduced to ease PMD integration into scripts or hooks, such as SVN hooks. @@ -219,7 +219,7 @@ This behavior has been introduced to ease PMD integration into scripts or hooks, + To ignore recoverable errors, use --no-fail-on-error.

Since PMD 7.3.0.

2Usage error. Command-line parameters are invalid or missing.
4At least one violation has been detected, unless --no-fail-on-violation is set.

Since PMD 5.3.

5At least one recoverable error has occurred. There might be additionally zero or more violations detected. - To ignore recoverable errors, use --no-fail-on-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. diff --git a/docs/pages/pmd/userdocs/cpd/cpd.md b/docs/pages/pmd/userdocs/cpd/cpd.md index 0be9c12cacd..1bf2954c18c 100644 --- a/docs/pages/pmd/userdocs/cpd/cpd.md +++ b/docs/pages/pmd/userdocs/cpd/cpd.md @@ -4,7 +4,7 @@ tags: [cpd, userdocs] summary: "Learn how to use CPD, the copy-paste detector shipped with PMD." permalink: pmd_userdocs_cpd.html author: Tom Copeland -last_updated: May 2024 (7.2.0) +last_updated: June 2024 (7.3.0) --- ## Overview @@ -284,7 +284,7 @@ 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) or 5 (since 7.2.0). +Please note that if CPD detects duplicated source code, it will exit with status 4 (since 5.0) or 5 (since 7.3.0). This behavior has been introduced to ease CPD integration into scripts or hooks, such as SVN hooks. @@ -293,7 +293,7 @@ This behavior has been introduced to ease CPD integration into scripts or hooks, + To ignore recoverable errors, use --no-fail-on-error.

Since PMD 7.3.0.

2Usage error. Command-line parameters are invalid or missing.
4At least one code duplication has been detected unless --no-fail-on-violation is set.

Since PMD 5.0.

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

Since PMD 7.2.0.

{%include note.html content="If PMD exits with 5, then PMD had trouble lexing one or more files. @@ -402,7 +402,7 @@ Andy Glover wrote an Ant task for CPD; here's how to use it: If not specified, CPD uses the system default encoding." %} {% include custom/cli_option_row.html options="failOnError" - description="Whether to fail the build if any errors occurred while processing the files. Since PMD 7.2.0." + description="Whether to fail the build if any errors occurred while processing the files. Since PMD 7.3.0." default="true" %} {% include custom/cli_option_row.html options="format" @@ -441,7 +441,7 @@ Andy Glover wrote an Ant task for CPD; here's how to use it: {% include custom/cli_option_row.html options="skipLexicalErrors" description="Deprecated Skip files which can't be tokenized due to invalid characters instead of aborting CPD. This parameter is deprecated and - ignored since PMD 7.2.0. It is now by default true. Use `failOnError` instead to fail the build." + ignored since PMD 7.3.0. It is now by default true. Use `failOnError` instead to fail the build." default="true" %} {% include custom/cli_option_row.html options="skipBlocks" diff --git a/docs/pages/pmd/userdocs/tools/ant.md b/docs/pages/pmd/userdocs/tools/ant.md index f81e1aa5468..b37ea8ad5ff 100644 --- a/docs/pages/pmd/userdocs/tools/ant.md +++ b/docs/pages/pmd/userdocs/tools/ant.md @@ -6,7 +6,7 @@ author: > David Dixon-Peugh , Tom Copeland , Xavier Le Vourch -last_updated: May 2024 (7.2.0) +last_updated: June 2024 (7.3.0) --- ## PMD diff --git a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java index b82671c22fe..e27ca93821d 100644 --- a/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java +++ b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/CPDTask.java @@ -103,7 +103,7 @@ public void execute() throws BuildException { config.setSkipDuplicates(skipDuplicateFiles); if (skipLexicalErrors) { - log("skipLexicalErrors is deprecated since 7.2.0 and the property is ignored. " + log("skipLexicalErrors is deprecated since 7.3.0 and the property is ignored. " + "Lexical errors are now skipped by default and the build is failed. " + "Use failOnError=\"false\" to not fail the build.", Project.MSG_WARN); } @@ -277,7 +277,7 @@ public void setSkipBlocksPattern(String skipBlocksPattern) { /** * Whether to fail the build if any recoverable errors occurred while processing the files. * - * @since 7.2.0 + * @since 7.3.0 */ public void setFailOnError(boolean failOnError) { this.failOnError = failOnError; 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 567917a98f3..0960a0fd482 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/AbstractConfiguration.java @@ -416,13 +416,13 @@ public void setFailOnViolation(boolean failOnViolation) { * Whether PMD should exit with status 5 (the default behavior, true) if * recoverable errors occurred or just with 0 (to not break the build, e.g.). * - *

Note: If only violations are found, the exist status is 4. See + *

Note: If only violations are found, the exit status is 4. See * {@link #isFailOnViolation()}. * * @return failOnError * * @see #isFailOnViolation() - * @since 7.2.0 + * @since 7.3.0 */ public boolean isFailOnError() { return failOnError; @@ -432,13 +432,13 @@ public boolean isFailOnError() { * Sets whether PMD should exit with status 5 (the default behavior, true) * if recoverable errors occurred or just with 0 (to not break the build, e.g.). * - *

Note: If only violations are found, the exist status is 4. See + *

Note: If only violations are found, the exit status is 4. See * {@link #isFailOnViolation()}. * * @param failOnError whether to exit with 5 and fail the build if recoverable errors occurred. * * @see #isFailOnViolation() - * @since 7.2.0 + * @since 7.3.0 */ public void setFailOnError(boolean failOnError) { this.failOnError = failOnError;