[MPMD-395] Build doesn't fail for invalid CPD format

src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java

@@ -24,25 +24,19 @@
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutputStream;
-import java.io.OutputStreamWriter;
-import java.io.Writer;
 import java.nio.charset.Charset;
 import java.util.Objects;
-import java.util.function.Predicate;
 
 import net.sourceforge.pmd.cpd.CPDConfiguration;
-import net.sourceforge.pmd.cpd.CPDReport;
 import net.sourceforge.pmd.cpd.CPDReportRenderer;
 import net.sourceforge.pmd.cpd.CSVRenderer;
 import net.sourceforge.pmd.cpd.CpdAnalysis;
-import net.sourceforge.pmd.cpd.Match;
 import net.sourceforge.pmd.cpd.SimpleRenderer;
 import net.sourceforge.pmd.cpd.XMLRenderer;
 import net.sourceforge.pmd.lang.Language;
 import org.apache.maven.plugin.MojoExecutionException;
 import org.apache.maven.plugins.pmd.ExcludeDuplicationsFromFile;
 import org.apache.maven.reporting.MavenReportException;
-import org.codehaus.plexus.util.FileUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -152,6 +146,8 @@ private CpdResult run() throws MavenReportException {
         cpdConfiguration.setIgnoreAnnotations(request.isIgnoreAnnotations());
         cpdConfiguration.setIgnoreLiterals(request.isIgnoreLiterals());
         cpdConfiguration.setIgnoreIdentifiers(request.isIgnoreIdentifiers());
+        // we are not running CPD through CLI and deal with any errors during analysis on our own
+        cpdConfiguration.setSkipLexicalErrors(true);
 
         String languageId = request.getLanguage();
         if ("javascript".equals(languageId)) {
@@ -167,64 +163,24 @@ private CpdResult run() throws MavenReportException {
         request.getFiles().forEach(f -> cpdConfiguration.addInputPath(f.toPath()));
 
         LOG.debug("Executing CPD...");
-
-        // always create XML format. we need to output it even if the file list is empty or we have no duplications
-        // so the "check" goals can check for violations
         try (CpdAnalysis cpd = CpdAnalysis.create(cpdConfiguration)) {
-            cpd.performAnalysis(report -> {
-                try {
-                    writeXmlReport(report);
-
-                    // html format is handled by maven site report, xml format has already been rendered
-                    String format = request.getFormat();
-                    if (!"html".equals(format) && !"xml".equals(format)) {
-                        writeFormattedReport(report);
-                    }
-                } catch (MavenReportException e) {
-                    LOG.error("Error while writing CPD report", e);
-                }
-            });
+            CpdReportConsumer reportConsumer = new CpdReportConsumer(request, excludeDuplicationsFromFile);
+            cpd.performAnalysis(reportConsumer);
         } catch (IOException e) {
-            LOG.error("Error while executing CPD", e);
+            throw new MavenReportException("Error while executing CPD", e);
         }
         LOG.debug("CPD finished.");
 
-        return new CpdResult(new File(request.getTargetDirectory(), "cpd.xml"), request.getOutputEncoding());
-    }
-
-    private void writeXmlReport(CPDReport cpd) throws MavenReportException {
-        File targetFile = writeReport(cpd, new XMLRenderer(request.getOutputEncoding()), "xml");
-        if (request.isIncludeXmlInSite()) {
-            File siteDir = new File(request.getReportOutputDirectory());
-            siteDir.mkdirs();
-            try {
-                FileUtils.copyFile(targetFile, new File(siteDir, "cpd.xml"));
-            } catch (IOException e) {
-                throw new MavenReportException(e.getMessage(), e);
-            }
-        }
-    }
-
-    private File writeReport(CPDReport cpd, CPDReportRenderer r, String extension) throws MavenReportException {
-        if (r == null) {
-            return null;
+        // in constrast to pmd goal, we don't have a parameter for cpd like "skipPmdError" - if there
+        // are any errors during CPD analysis, the maven build fails.
+        int cpdErrors = cpdConfiguration.getReporter().numErrors();
+        if (cpdErrors == 1) {
+            throw new MavenReportException("There was 1 error while executing CPD");
+        } else if (cpdErrors > 1) {
+            throw new MavenReportException("There were " + cpdErrors + " errors while executing CPD");
         }
 
-        File targetDir = new File(request.getTargetDirectory());
-        targetDir.mkdirs();
-        File targetFile = new File(targetDir, "cpd." + extension);
-        try (Writer writer = new OutputStreamWriter(new FileOutputStream(targetFile), request.getOutputEncoding())) {
-            r.render(cpd.filterMatches(filterMatches()), writer);
-            writer.flush();
-        } catch (IOException ioe) {
-            throw new MavenReportException(ioe.getMessage(), ioe);
-        }
-        return targetFile;
-    }
-
-    private void writeFormattedReport(CPDReport cpd) throws MavenReportException {
-        CPDReportRenderer r = createRenderer(request.getFormat(), request.getOutputEncoding());
-        writeReport(cpd, r, request.getFormat());
+        return new CpdResult(new File(request.getTargetDirectory(), "cpd.xml"), request.getOutputEncoding());
     }
 
     /**
@@ -255,18 +211,4 @@ public static CPDReportRenderer createRenderer(String format, String outputEncod
 
         return renderer;
     }
-
-    private Predicate<Match> filterMatches() {
-        return (Match match) -> {
-            LOG.debug("Filtering duplications. Using " + excludeDuplicationsFromFile.countExclusions()
-                    + " configured exclusions.");
-
-            if (excludeDuplicationsFromFile.isExcludedFromFailure(match)) {
-                LOG.debug("Excluded " + match + " duplications.");
-                return false;
-            } else {
-                return true;
-            }
-        };
-    }
 }

src/main/java/org/apache/maven/plugins/pmd/exec/CpdReportConsumer.java

@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.maven.plugins.pmd.exec;
+
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.util.function.Consumer;
+import java.util.function.Predicate;
+
+import net.sourceforge.pmd.cpd.CPDReport;
+import net.sourceforge.pmd.cpd.CPDReportRenderer;
+import net.sourceforge.pmd.cpd.Match;
+import net.sourceforge.pmd.cpd.XMLRenderer;
+import org.apache.maven.plugins.pmd.ExcludeDuplicationsFromFile;
+import org.apache.maven.reporting.MavenReportException;
+import org.codehaus.plexus.util.FileUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+class CpdReportConsumer implements Consumer<CPDReport> {
+    private static final Logger LOG = LoggerFactory.getLogger(CpdReportConsumer.class);
+
+    private final CpdRequest request;
+    private final ExcludeDuplicationsFromFile excludeDuplicationsFromFile;
+
+    CpdReportConsumer(CpdRequest request, ExcludeDuplicationsFromFile excludeDuplicationsFromFile) {
+        this.request = request;
+        this.excludeDuplicationsFromFile = excludeDuplicationsFromFile;
+    }
+
+    @Override
+    public void accept(CPDReport report) {
+        try {
+            // always create XML format. we need to output it even if the file list is empty or we have no
+            // duplications so that the "check" goals can check for violations
+            writeXmlReport(report);
+
+            // HTML format is handled by maven site report, XML format has already been rendered.
+            // a renderer is only needed for other formats
+            String format = request.getFormat();
+            if (!"html".equals(format) && !"xml".equals(format)) {
+                writeFormattedReport(report);
+            }
+        } catch (IOException | MavenReportException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
+    private void writeXmlReport(CPDReport cpd) throws IOException {
+        File targetFile = writeReport(cpd, new XMLRenderer(request.getOutputEncoding()), "xml");
+        if (request.isIncludeXmlInSite()) {
+            File siteDir = new File(request.getReportOutputDirectory());
+            if (!siteDir.exists() && !siteDir.mkdirs()) {
+                throw new IOException("Couldn't create report output directory: " + siteDir);
+            }
+            FileUtils.copyFile(targetFile, new File(siteDir, "cpd.xml"));
+        }
+    }
+
+    private void writeFormattedReport(CPDReport cpd) throws IOException, MavenReportException {
+        CPDReportRenderer r = CpdExecutor.createRenderer(request.getFormat(), request.getOutputEncoding());
+        writeReport(cpd, r, request.getFormat());
+    }
+
+    private File writeReport(CPDReport cpd, CPDReportRenderer renderer, String extension) throws IOException {
+        if (renderer == null) {
+            return null;
+        }
+
+        File targetDir = new File(request.getTargetDirectory());
+        if (!targetDir.exists() && !targetDir.mkdirs()) {
+            throw new IOException("Couldn't create report output directory: " + targetDir);
+        }
+
+        File targetFile = new File(targetDir, "cpd." + extension);
+        try (Writer writer = new OutputStreamWriter(new FileOutputStream(targetFile), request.getOutputEncoding())) {
+            renderer.render(cpd.filterMatches(filterMatches()), writer);
+            writer.flush();
+        }
+        return targetFile;
+    }
+
+    private Predicate<Match> filterMatches() {
+        return (Match match) -> {
+            LOG.debug(
+                    "Filtering duplications. Using {} configured exclusions.",
+                    excludeDuplicationsFromFile.countExclusions());
+
+            if (excludeDuplicationsFromFile.isExcludedFromFailure(match)) {
+                LOG.debug("Excluded {} duplications.", match);
+                return false;
+            } else {
+                return true;
+            }
+        };
+    }
+}

src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java

@@ -26,6 +26,7 @@
 
 import org.apache.commons.io.FileUtils;
 import org.apache.commons.lang3.StringUtils;
+import org.apache.maven.reporting.MavenReportException;
 import org.w3c.dom.Document;
 
 /**
@@ -65,8 +66,7 @@ public void testDefaultConfiguration() throws Exception {
         assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, i + 1);"));
 
         // the version should be logged
-        String output = CapturingPrintStream.getOutput();
-        assertTrue(output.contains("PMD version: " + AbstractPmdReport.getPmdVersion()));
+        assertLogOutputContains("PMD version: " + AbstractPmdReport.getPmdVersion());
     }
 
     /**
@@ -130,7 +130,8 @@ public void testInvalidFormat() throws Exception {
 
             fail("MavenReportException must be thrown");
         } catch (Exception e) {
-            assertTrue(true);
+            assertMavenReportException("There was 1 error while executing CPD", e);
+            assertLogOutputContains("Can't find CPD custom format xhtml");
         }
     }
 
@@ -240,4 +241,30 @@ public void testExclusionsConfiguration() throws Exception {
         String str = readFile(generatedFile);
         assertEquals(0, StringUtils.countMatches(str, "<duplication"));
     }
+
+    public void testWithCpdErrors() throws Exception {
+        try {
+            generateReport("cpd", "CpdReportTest/with-cpd-errors/pom.xml");
+            fail("MavenReportException must be thrown");
+        } catch (Exception e) {
+            assertMavenReportException("There was 1 error while executing CPD", e);
+            assertLogOutputContains("Lexical error in file");
+            assertLogOutputContains("BadFile.java");
+        }
+    }
+
+    private static void assertMavenReportException(String expectedMessage, Exception exception) {
+        // The maven report exception might be wrapped in a RuntimeException
+        assertTrue(
+                "Expected MavenReportException, but was: " + exception,
+                exception instanceof MavenReportException || exception.getCause() instanceof MavenReportException);
+        assertTrue(
+                "Wrong message: expected: " + expectedMessage + ", but was: " + exception.toString(),
+                exception.toString().contains(expectedMessage));
+    }
+
+    private static void assertLogOutputContains(String expectedMessage) {
+        String log = CapturingPrintStream.getOutput();
+        assertTrue("Expected '" + expectedMessage + "' in log, but was:\n" + log, log.contains(expectedMessage));
+    }
 }

src/test/java/org/apache/maven/plugins/pmd/exec/ExecutorTest.java

@@ -23,9 +23,9 @@
 import java.net.URL;
 import java.net.URLClassLoader;
 
-import junit.framework.Assert;
 import junit.framework.TestCase;
 import org.apache.commons.lang3.SystemUtils;
+import org.junit.Assert;
 
 public class ExecutorTest extends TestCase {
     public void testBuildClasspath() throws MalformedURLException {

src/test/resources/unit/CpdReportTest/with-cpd-errors/pom.xml

@@ -0,0 +1,52 @@
+<!--
+Licensed to the Apache Software Foundation (ASF) under one
+or more contributor license agreements.  See the NOTICE file
+distributed with this work for additional information
+regarding copyright ownership.  The ASF licenses this file
+to you under the Apache License, Version 2.0 (the
+"License"); you may not use this file except in compliance
+with the License.  You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing,
+software distributed under the License is distributed on an
+"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+KIND, either express or implied.  See the License for the
+specific language governing permissions and limitations
+under the License.
+-->
+
+<project>
+    <modelVersion>4.0.0</modelVersion>
+    <groupId>unit.CpdReportTest</groupId>
+    <artifactId>cpd-configuration-with-pmd-errors</artifactId>
+    <packaging>jar</packaging>
+    <version>1.0-SNAPSHOT</version>
+    <inceptionYear>2006</inceptionYear>
+    <name>Maven CPD Plugin Test for failing build when CPD errors occurred</name>
+    <url>http://maven.apache.org</url>
+    <build>
+        <finalName>with-cpd-errors</finalName>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-pmd-plugin</artifactId>
+                <configuration>
+                    <project implementation="org.apache.maven.plugins.pmd.stubs.DefaultConfigurationMavenProjectStub"/>
+                    <outputDirectory>${basedir}/target/test/unit/CpdReportTest/with-cpd-errors/target/site</outputDirectory>
+                    <targetDirectory>${basedir}/target/test/unit/CpdReportTest/with-cpd-errors/target</targetDirectory>
+                    <localRepository>${localRepository}</localRepository>
+                    <format>xml</format>
+                    <linkXRef>false</linkXRef>
+                    <xrefLocation>${basedir}/target/test/unit/CpdReportTest/with-cpd-errors/target/site/xref</xrefLocation>
+                    <minimumTokens>100</minimumTokens>
+                    <compileSourceRoots>
+                        <compileSourceRoot>${basedir}/src/test/resources/unit/CpdReportTest/with-cpd-errors/src/main/java</compileSourceRoot>
+                    </compileSourceRoots>
+                    <inputEncoding>UTF-8</inputEncoding>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
+</project>

src/test/resources/unit/CpdReportTest/with-cpd-errors/src/main/java/sample/BadFile.java

@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package sample;
+
+public class BadFile {
+    public void foo() {
+        // this is a bad character � it's U+FFFD REPLACEMENT CHARACTER
+        int a�b = 1;
+    }
+}