diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index c616c1847b8..a499b49cf82 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: June 2024 (7.3.0) --- @@ -19,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 @@ -55,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" %} @@ -80,10 +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-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" @@ -98,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.

" %} @@ -125,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" @@ -141,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. +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. +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.3.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. Recoverable 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..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: August 2023 (7.0.0) +last_updated: June 2024 (7.3.0) --- ## Overview @@ -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" @@ -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-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. 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.3.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 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.
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.3.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. @@ -390,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 occurred while processing the files. Since PMD 7.3.0." + default="true" + %} {% include custom/cli_option_row.html options="format" description="The format of the report (e.g. `csv`, `text`, `xml`)." default="text" @@ -424,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.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" description="Enables or disabled skipping of blocks like a pre-processor. See also option skipBlocksPattern." diff --git a/docs/pages/pmd/userdocs/tools/ant.md b/docs/pages/pmd/userdocs/tools/ant.md index 4c81ff7daf3..b37ea8ad5ff 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: June 2024 (7.3.0) --- ## PMD @@ -63,8 +64,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 recoverable errors occurred while analyzing files. No diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index aee8e21a14c..7a49dff62b4 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,6 +15,8 @@ This is a {{ site.pmd.release_type }} release. ### 🚀 New and noteworthy ### 🐛 Fixed Issues +* cli + * [#2827](https://github.com/pmd/pmd/issues/2827): \[cli] Consider processing errors in exit status * apex * [#5053](https://github.com/pmd/pmd/issues/5053): \[apex] CPD fails to parse string literals with escaped characters * java-bestpractices @@ -25,6 +27,42 @@ 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 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-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. + +* 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`. 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 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`. + ### ✨ External Contributions {% endtocmaker %} 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..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 @@ -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; @@ -82,6 +83,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 { @@ -99,7 +101,15 @@ 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 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); + } + + // 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); @@ -118,12 +128,20 @@ 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) { + String message = String.format("There were %d recovered errors during analysis.", errors); + if (failOnError) { + throw new BuildException(message + " Ignore these with failOnError=\"true\"."); + } else { + log(message + " Not failing build, because failOnError=\"false\".", Project.MSG_WARN); + } + } } } catch (IOException ioe) { 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 { @@ -220,6 +238,10 @@ public void setIgnoreUsings(boolean value) { this.ignoreUsings = value; } + /** + * @deprecated Use {@link #setFailOnError(boolean)} instead. + */ + @Deprecated public void setSkipLexicalErrors(boolean skipLexicalErrors) { this.skipLexicalErrors = skipLexicalErrors; } @@ -252,6 +274,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.3.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/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java b/pmd-ant/src/main/java/net/sourceforge/pmd/ant/internal/PMDTaskImpl.java index a4be75fb15f..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 errors occurred while running PMD"); + throw new BuildException("Some recoverable errors occurred while running PMD"); } } 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..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 @@ -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 + + 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..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,11 +43,17 @@ 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 f15328047b7..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", @@ -103,6 +107,7 @@ protected CPDConfiguration toConfiguration() { configuration.addRelativizeRoots(relativizeRootPaths); } configuration.setFailOnViolation(failOnViolation); + configuration.setFailOnError(failOnError); configuration.setInputFilePath(fileListPath); if (inputPaths != null) { configuration.setInputPathList(new ArrayList<>(inputPaths)); @@ -123,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; } @@ -133,6 +146,11 @@ protected CPDConfiguration toConfiguration() { MutableBoolean hasViolations = new MutableBoolean(); cpd.performAnalysis(report -> hasViolations.setValue(!report.getMatches().isEmpty())); + boolean hasErrors = configuration.getReporter().numErrors() > 0; + if (hasErrors && configuration.isFailOnError()) { + return CliExitCode.RECOVERED_ERRORS_OR_VIOLATIONS; + } + if (hasViolations.booleanValue() && configuration.isFailOnViolation()) { return CliExitCode.VIOLATIONS_FOUND; } 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..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,6 +270,7 @@ protected PMDConfiguration toConfiguration() { configuration.setSuppressMarker(suppressMarker); configuration.setThreads(threads); configuration.setFailOnViolation(failOnViolation); + configuration.setFailOnError(failOnError); configuration.setAnalysisCacheLocation(cacheLocation != null ? cacheLocation.toString() : null); configuration.setIgnoreIncrementalAnalysis(noCache); @@ -330,6 +331,8 @@ protected CliExitCode doExecute(PMDConfiguration configuration) { if (pmdReporter.numErrors() > 0) { // processing errors are ignored return CliExitCode.ERROR; + } 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 16fabc270b6..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 @@ -4,30 +4,39 @@ package net.sourceforge.pmd.cli.internal; -import net.sourceforge.pmd.PMDConfiguration; +import net.sourceforge.pmd.AbstractConfiguration; /** * The execution result of any given command. */ public enum CliExitCode { - /** No 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), /** - * 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), /** * 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 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. 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); + VIOLATIONS_FOUND(4), + /** + * 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#isFailOnError()} + * is set. It can be disabled by using CLI flag {@code --no-fail-on-error}. + */ + RECOVERED_ERRORS_OR_VIOLATIONS(5); private final int exitCode; @@ -45,6 +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 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 af1eee72aa1..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 @@ -5,6 +5,7 @@ package net.sourceforge.pmd.cli; import static net.sourceforge.pmd.cli.internal.CliExitCode.OK; +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; @@ -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(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(VIOLATIONS_FOUND, "--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")); } @@ -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" ))); @@ -231,6 +232,43 @@ void testSkipLexicalErrors() throws Exception { }); } + @Test + void testExitCodeWithLexicalErrors() throws Exception { + runCli(RECOVERED_ERRORS_OR_VIOLATIONS, + "--minimum-tokens", "10", + "-d", Paths.get(BASE_RES_PATH, "badandgood", "BadFile.java").toString(), + "--format", "text") + .verify(r -> { + r.checkStdErr(containsPattern("Skipping file: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdOut(emptyString()); + }); + } + + @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-error") + .verify(r -> { + r.checkStdErr(containsPattern("Skipping file: Lexical error in file '.*?BadFile\\.java'")); + r.checkStdOut(emptyString()); + }); + } + + @Test + void testExitCodeWithLexicalErrorsAndSkipLexical() throws Exception { + runCli(OK, + "--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 { 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..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,6 +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.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; @@ -318,6 +319,27 @@ void exitStatusWithViolations() throws Exception { )); } + @Test + 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")); + r.checkStdErr(containsString("An error occurred while executing PMD.")); + }); + } + + @Test + void exitStatusWithErrorsNoFail() throws Exception { + runCli(OK, "--use-version", "dummy-parserThrows", + "-d", srcDir.toString(), "-f", "text", "-R", RULESET_WITH_VIOLATION, + "--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.")); + }); + } + @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-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; } 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..0960a0fd482 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,8 @@ public abstract class AbstractConfiguration { private Path ignoreFilePath; private List excludes = new ArrayList<>(); private boolean collectRecursive = true; + private boolean failOnViolation = true; + private boolean failOnError = true; protected AbstractConfiguration(LanguageRegistry languageRegistry, PmdReporter messageReporter) { @@ -377,4 +379,68 @@ public boolean collectFilesRecursively() { 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.). + * + *

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

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 #isFailOnError() + * @since 6.0.0 + */ + public void setFailOnViolation(boolean failOnViolation) { + this.failOnViolation = 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 exit status is 4. See + * {@link #isFailOnViolation()}. + * + * @return failOnError + * + * @see #isFailOnViolation() + * @since 7.3.0 + */ + public boolean isFailOnError() { + return failOnError; + } + + /** + * 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 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.3.0 + */ + public void setFailOnError(boolean failOnError) { + this.failOnError = failOnError; + } } 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..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; @@ -73,8 +74,6 @@ public class CPDConfiguration extends AbstractConfiguration { private boolean help; - private boolean failOnViolation = true; - public CPDConfiguration() { this(LanguageRegistry.CPD); @@ -228,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; } @@ -260,14 +269,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)) { 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 = 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 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; } 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