Fix pmd#3427 - Stop printing usage test on error

pmd-core/src/main/java/net/sourceforge/pmd/PMD.java

@@ -35,6 +35,7 @@
 import net.sourceforge.pmd.cache.NoopAnalysisCache;
 import net.sourceforge.pmd.cli.PMDCommandLineInterface;
 import net.sourceforge.pmd.cli.PmdParametersParseResult;
+import net.sourceforge.pmd.cli.internal.CliMessages;
 import net.sourceforge.pmd.lang.Language;
 import net.sourceforge.pmd.lang.LanguageFilenameFilter;
 import net.sourceforge.pmd.lang.LanguageVersion;
@@ -252,18 +253,24 @@ public static int doPMD(PMDConfiguration configuration) {
             try (TimedOperation rto = TimeTracker.startOperation(TimedOperationCategory.REPORTING)) {
                 renderer.end();
                 renderer.flush();
-                return report.getViolations().size();
             }
+
+            if (!report.getProcessingErrors().isEmpty()) {
+                printErrorDetected(report.getProcessingErrors().size());
+            }
+
+            return report.getViolations().size();
+
         } catch (Exception e) {
             String message = e.getMessage();
             if (message != null) {
                 LOG.severe(message);
+                LOG.log(Level.FINE, "Exception during processing", e);
             } else {
                 LOG.log(Level.SEVERE, "Exception during processing", e);
             }
-            LOG.log(Level.FINE, "Exception during processing", e);
-            LOG.info(PMDCommandLineInterface.buildUsageText());
-            return PMDCommandLineInterface.NO_ERRORS_STATUS;
+            printErrorDetected(1);
+            return PMDCommandLineInterface.NO_ERRORS_STATUS; // fixme?
         } finally {
             /*
              * Make sure it's our own classloader before attempting to close it....
@@ -552,13 +559,18 @@ public static StatusCode runPmd(String... args) {
             System.out.println(PMDCommandLineInterface.buildUsageText());
             return StatusCode.OK;
         } else if (parseResult.isError()) {
-            System.out.println(PMDCommandLineInterface.buildUsageText());
             System.err.println(parseResult.getError().getMessage());
+            System.err.println(CliMessages.runWithHelpFlagMessage());
             return StatusCode.ERROR;
         }
         return runPmd(parseResult.toConfiguration());
     }
 
+    private static void printErrorDetected(int errors) {
+        String msg = CliMessages.errorDetectedMessage(errors, "PMD");
+        LOG.severe(msg);
+    }
+
     /**
      * Execute PMD from a configuration. Returns the status code without
      * exiting the VM. This is the main entry point to run a full PMD run
@@ -591,8 +603,6 @@ public static StatusCode runPmd(PMDConfiguration configuration) {
                 status = StatusCode.OK;
             }
         } catch (Exception e) {
-            System.out.println(PMDCommandLineInterface.buildUsageText());
-            System.out.println();
             System.err.println(e.getMessage());
             status = StatusCode.ERROR;
         } finally {

pmd-core/src/main/java/net/sourceforge/pmd/cli/internal/CliMessages.java

@@ -0,0 +1,29 @@
+/*
+ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
+ */
+
+package net.sourceforge.pmd.cli.internal;
+
+/**
+ * @author Clément Fournier
+ */
+public final class CliMessages {
+
+    private CliMessages() {
+        // utility class
+    }
+
+    public static String errorDetectedMessage(int errors, String program) {
+        String anError = errors == 1 ? "An error" : errors + " errors";
+        return anError + " occurred while executing " + program + ".\n"
+            + "Run with --debug to see a stack-trace.\n"
+            + "If you think this is a bug in " + program
+            + ", please report this issue at https://github.com/pmd/pmd/issues/new/choose\n"
+            + "If you do so, please include a stack-trace, the code sample\n"
+            + " causing the issue, and details about your run configuration.";
+    }
+
+    public static String runWithHelpFlagMessage() {
+        return "Run with --help for command line help.";
+    }
+}

pmd-core/src/main/java/net/sourceforge/pmd/cpd/CPDCommandLineInterface.java

@@ -22,6 +22,7 @@
 
 import net.sourceforge.pmd.PMD;
 import net.sourceforge.pmd.PMDVersion;
+import net.sourceforge.pmd.cli.internal.CliMessages;
 import net.sourceforge.pmd.util.FileUtil;
 import net.sourceforge.pmd.util.database.DBURI;
 
@@ -76,9 +77,8 @@ public static void main(String[] args) {
                 return;
             }
         } catch (ParameterException e) {
-            jcommander.usage();
-            System.out.println(buildUsageText());
-            System.err.println(" " + e.getMessage());
+            System.err.println(e.getMessage());
+            System.err.println(CliMessages.runWithHelpFlagMessage());
             setStatusCodeOrExit(ERROR_STATUS);
             return;
         }
@@ -117,6 +117,7 @@ public static void main(String[] args) {
             }
         } catch (IOException | RuntimeException e) {
             e.printStackTrace();
+            LOGGER.severe(CliMessages.errorDetectedMessage(1, "CPD"));
             setStatusCodeOrExit(ERROR_STATUS);
         }
     }

pmd-core/src/test/java/net/sourceforge/pmd/cli/CoreCliTest.java

@@ -4,6 +4,9 @@
 
 package net.sourceforge.pmd.cli;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.containsStringIgnoringCase;
+import static org.hamcrest.Matchers.not;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotEquals;
@@ -17,13 +20,17 @@
 import java.nio.file.Path;
 import java.util.logging.Logger;
 
+import org.hamcrest.Matcher;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.contrib.java.lang.system.RestoreSystemProperties;
+import org.junit.contrib.java.lang.system.SystemErrRule;
+import org.junit.contrib.java.lang.system.SystemOutRule;
 import org.junit.rules.TemporaryFolder;
 
 import net.sourceforge.pmd.PMD;
+import net.sourceforge.pmd.PMD.StatusCode;
 import net.sourceforge.pmd.junit.JavaUtilLoggingRule;
 
 /**
@@ -40,6 +47,10 @@ public class CoreCliTest {
     public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
     @Rule
     public JavaUtilLoggingRule loggingRule = new JavaUtilLoggingRule(PMD.class.getPackage().getName()).mute();
+    @Rule
+    public final SystemOutRule outStreamCaptor = new SystemOutRule();
+    @Rule
+    public final SystemErrRule errStreamCaptor = new SystemErrRule();
 
     private Path srcDir;
 
@@ -164,6 +175,29 @@ public void testRelativeReportFileLongOption() throws IOException {
     }
 
 
+    @Test
+    public void testWrongCliOptionsDoNotPrintUsage() {
+        String[] args = { "-invalid" };
+        PmdParametersParseResult params = PmdParametersParseResult.extractParameters(args);
+        assertTrue("Expected invalid args", params.isError());
+
+        startCapturingErrAndOut();
+        StatusCode code = PMD.runPmd(args);
+        assertEquals(StatusCode.ERROR, code);
+        assertThatErrAndOut(not(containsStringIgnoringCase("Available report formats and")));
+    }
+
+    private void assertThatErrAndOut(Matcher<String> matcher) {
+        assertThat("stdout", outStreamCaptor.getLog(), matcher);
+        assertThat("stderr", errStreamCaptor.getLog(), matcher);
+    }
+
+    private void startCapturingErrAndOut() {
+        outStreamCaptor.enableLog();
+        errStreamCaptor.enableLog();
+        outStreamCaptor.mute();
+        errStreamCaptor.mute();
+    }
 
     // utilities
 

pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java

@@ -18,6 +18,7 @@
 import org.junit.Test;
 
 import net.sourceforge.pmd.PMDVersion;
+import net.sourceforge.pmd.cli.internal.CliMessages;
 
 public class BinaryDistributionIT extends AbstractBinaryDistributionTest {
 
@@ -80,7 +81,7 @@ public void runPMD() throws Exception {
         ExecutionResult result;
 
         result = PMDExecutor.runPMD(tempDir); // without any argument, display usage help and error
-        result.assertExecutionResult(1, SUPPORTED_LANGUAGES_PMD);
+        result.assertExecutionResultErrOutput(1, CliMessages.runWithHelpFlagMessage());
 
         result = PMDExecutor.runPMD(tempDir, "-h");
         result.assertExecutionResult(0, SUPPORTED_LANGUAGES_PMD);
@@ -97,14 +98,22 @@ public void runPMD() throws Exception {
         result.assertExecutionResult(4, "");
     }
 
+    @Test
+    public void runPMDWithError() throws Exception {
+        String srcDir = new File(".", "src/test/resources/sample-source/unparsable/").getAbsolutePath();
+
+        ExecutionResult result = PMDExecutor.runPMDRules(folder.newFile().toPath(), tempDir, srcDir, "src/test/resources/rulesets/sample-ruleset.xml");
+        result.assertExecutionResultErrOutput(0, "Run with --debug to see a stack-trace.");
+    }
+
     @Test
     public void runCPD() throws Exception {
         String srcDir = new File(".", "src/test/resources/sample-source-cpd/").getAbsolutePath();
 
         ExecutionResult result;
 
         result = CpdExecutor.runCpd(tempDir); // without any argument, display usage help and error
-        result.assertExecutionResult(1, SUPPORTED_LANGUAGES_CPD);
+        result.assertExecutionResultErrOutput(1, CliMessages.runWithHelpFlagMessage());
 
         result = CpdExecutor.runCpd(tempDir, "-h");
         result.assertExecutionResult(0, SUPPORTED_LANGUAGES_CPD);

pmd-dist/src/test/java/net/sourceforge/pmd/it/CpdExecutor.java

@@ -4,11 +4,9 @@
 
 package net.sourceforge.pmd.it;
 
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
 import java.util.Arrays;
 
-import org.apache.commons.io.IOUtils;
 import org.apache.commons.lang3.SystemUtils;
 
 import net.sourceforge.pmd.PMDVersion;
@@ -25,30 +23,6 @@ private CpdExecutor() {
         // this is a helper class only
     }
 
-    private static ExecutionResult runCpdUnix(Path tempDir, String... arguments) throws Exception {
-        String cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/run.sh").toAbsolutePath().toString();
-        ProcessBuilder pb = new ProcessBuilder(cmd, "cpd");
-        pb.command().addAll(Arrays.asList(arguments));
-        pb.redirectErrorStream(true);
-        Process process = pb.start();
-        String output = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8);
-
-        int result = process.waitFor();
-        return new ExecutionResult(result, output, null, null);
-    }
-
-    private static ExecutionResult runCpdWindows(Path tempDir, String... arguments) throws Exception {
-        String cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/cpd.bat").toAbsolutePath().toString();
-        ProcessBuilder pb = new ProcessBuilder(cmd);
-        pb.command().addAll(Arrays.asList(arguments));
-        pb.redirectErrorStream(true);
-        Process process = pb.start();
-        String output = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8);
-
-        int result = process.waitFor();
-        return new ExecutionResult(result, output, null, null);
-    }
-
     /**
      * Executes CPD found in tempDir with the given command line arguments.
      * @param tempDir the directory, to which the binary distribution has been extracted
@@ -57,10 +31,12 @@ private static ExecutionResult runCpdWindows(Path tempDir, String... arguments)
      * @throws Exception if the execution fails for any reason (executable not found, ...)
      */
     public static ExecutionResult runCpd(Path tempDir, String... arguments) throws Exception {
+        String cmd;
         if (SystemUtils.IS_OS_WINDOWS) {
-            return runCpdWindows(tempDir, arguments);
+            cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/cpd.bat").toAbsolutePath().toString();
         } else {
-            return runCpdUnix(tempDir, arguments);
+            cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/run.sh").toAbsolutePath().toString();
         }
+        return PMDExecutor.runCommand(cmd, Arrays.asList(arguments), null);
     }
 }

pmd-dist/src/test/java/net/sourceforge/pmd/it/ExecutionResult.java

@@ -68,10 +68,38 @@ public void assertExecutionResult(int expectedExitCode, String expectedOutput) {
      * generated report.
      *
      * @param expectedExitCode the exit code, e.g. 0 if no rule violations are expected, or 4 if violations are found
-     * @param expectedOutput the output to search for
-     * @param expectedReport the string to search for tin the report
+     * @param expectedOutput   the output to search for
+     * @param expectedReport   the string to search for tin the report
      */
     public void assertExecutionResult(int expectedExitCode, String expectedOutput, String expectedReport) {
+        assertExecResultImpl(expectedExitCode, output, expectedOutput, expectedReport);
+    }
+
+    /**
+     * Asserts that the command exited with the expected exit code and that the given expected
+     * output is contained in the actual command ERROR output, and the given expected report is in the
+     * generated report.
+     *
+     * @param expectedExitCode    the exit code, e.g. 0 if no rule violations are expected, or 4 if violations are found
+     * @param expectedErrorOutput the output to search for in stderr
+     * @param expectedReport      the string to search for tin the report
+     */
+    public void assertExecutionResultErrOutput(int expectedExitCode, String expectedErrorOutput, String expectedReport) {
+        assertExecResultImpl(expectedExitCode, errorOutput, expectedErrorOutput, expectedReport);
+    }
+
+    /**
+     * Asserts that the command exited with the expected exit code and that the given expected
+     * output is contained in the actual command ERROR output.
+     *
+     * @param expectedExitCode    the exit code, e.g. 0 if no rule violations are expected, or 4 if violations are found
+     * @param expectedErrorOutput the output to search for in stderr
+     */
+    public void assertExecutionResultErrOutput(int expectedExitCode, String expectedErrorOutput) {
+        assertExecResultImpl(expectedExitCode, errorOutput, expectedErrorOutput, null);
+    }
+
+    private void assertExecResultImpl(int expectedExitCode, String output, String expectedOutput, String expectedReport) {
         assertEquals("Command exited with wrong code.\nComplete result:\n\n" + this, expectedExitCode, exitCode);
         assertNotNull("No output found", output);
         if (expectedOutput != null && !expectedOutput.isEmpty()) {
@@ -83,7 +111,7 @@ public void assertExecutionResult(int expectedExitCode, String expectedOutput, S
         }
         if (expectedReport != null && !expectedReport.isEmpty()) {
             assertTrue("Expected report '" + expectedReport + "'.\nComplete result:\n\n" + this,
-                    report.contains(expectedReport));
+                       report.contains(expectedReport));
         }
     }
 

pmd-dist/src/test/java/net/sourceforge/pmd/it/PMDExecutor.java

@@ -39,15 +39,15 @@ private static ExecutionResult runPMDUnix(Path tempDir, Path reportFile, String.
         List<String> args = new ArrayList<>();
         args.add("pmd");
         args.addAll(Arrays.asList(arguments));
-        return runPMD(cmd, args, reportFile);
+        return runCommand(cmd, args, reportFile);
     }
 
     private static ExecutionResult runPMDWindows(Path tempDir, Path reportFile, String... arguments) throws Exception {
         String cmd = tempDir.resolve(AbstractBinaryDistributionTest.PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/pmd.bat").toAbsolutePath().toString();
-        return runPMD(cmd, Arrays.asList(arguments), reportFile);
+        return runCommand(cmd, Arrays.asList(arguments), reportFile);
     }
 
-    private static ExecutionResult runPMD(String cmd, List<String> arguments, Path reportFile) throws Exception {
+    static ExecutionResult runCommand(String cmd, List<String> arguments, Path reportFile) throws Exception {
         ProcessBuilder pb = new ProcessBuilder(cmd);
         pb.command().addAll(arguments);
         pb.redirectErrorStream(false);

pmd-dist/src/test/resources/sample-source/unparsable/cannot_be_parsed.java

@@ -0,0 +1,3 @@
+
+
+{} // not a valid file