[ 
https://issues.apache.org/jira/browse/MPMD-395?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17851709#comment-17851709
 ] 

ASF GitHub Bot commented on MPMD-395:
-------------------------------------

elharo commented on code in PR #150:
URL: https://github.com/apache/maven-pmd-plugin/pull/150#discussion_r1624715117


##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdReportConsumer.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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 (MavenReportException e) {
+            // Exceptions happening during this consumer are logged by PMD
+            // through the CPDConfiguration's reporter, which is in this case 
PmdErrorMessageReporter.
+            throw new RuntimeException(e);
+        }
+    }
+
+    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();

Review Comment:
   check return value



##########
src/main/java/org/apache/maven/plugins/pmd/exec/PmdErrorMessageReporter.java:
##########
@@ -0,0 +1,80 @@
+/*
+ * 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.util.ArrayList;
+import java.util.List;
+
+import net.sourceforge.pmd.util.log.PmdReporter;
+import org.apache.maven.reporting.MavenReportException;
+import org.checkerframework.checker.nullness.qual.Nullable;
+import org.slf4j.event.Level;
+
+class PmdErrorMessageReporter implements PmdReporter {
+    private final PmdReporter delegate;
+    private List<Throwable> errors = new ArrayList<>();
+
+    PmdErrorMessageReporter(PmdReporter delegate) {
+        this.delegate = delegate;
+    }
+
+    @Override
+    public boolean isLoggable(Level level) {
+        return delegate.isLoggable(level);
+    }
+
+    @Override
+    public void logEx(Level level, @Nullable String message, Object[] 
formatArgs, @Nullable Throwable error) {
+        if (error != null) {
+            if (error instanceof RuntimeException && error.getCause() 
instanceof MavenReportException) {
+                errors.add(error.getCause());
+            } else {
+                errors.add(error);
+            }
+        }
+
+        delegate.logEx(level, message, formatArgs, error);
+    }
+
+    @Override
+    public int numErrors() {
+        return delegate.numErrors();
+    }
+
+    public void rethrowErrorsAsMavenReportException() throws 
MavenReportException {
+        if (errors.isEmpty()) {
+            return;
+        }
+
+        final MavenReportException exception;
+
+        Throwable firstError = errors.get(0);

Review Comment:
   OK, I don't immediately see how that happens here, but if it does I would 
prefer not to do that. In general, log or throw, not both. 



##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdReportConsumer.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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 (MavenReportException e) {
+            // Exceptions happening during this consumer are logged by PMD
+            // through the CPDConfiguration's reporter, which is in this case 
PmdErrorMessageReporter.
+            throw new RuntimeException(e);
+        }
+    }
+
+    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("Error while copying cpd.xml 
report to site dir", e);
+            }
+        }
+    }
+
+    private void writeFormattedReport(CPDReport cpd) throws 
MavenReportException {
+        CPDReportRenderer r = CpdExecutor.createRenderer(request.getFormat(), 
request.getOutputEncoding());
+        writeReport(cpd, r, request.getFormat());
+    }
+
+    private File writeReport(CPDReport cpd, CPDReportRenderer r, String 
extension) throws MavenReportException {
+        if (r == null) {
+            return null;
+        }
+
+        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) {

Review Comment:
   I'm tempted to let this one bubble up and convert it higher up the stack if 
needed



##########
src/main/java/org/apache/maven/plugins/pmd/exec/CpdReportConsumer.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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 (MavenReportException e) {
+            // Exceptions happening during this consumer are logged by PMD
+            // through the CPDConfiguration's reporter, which is in this case 
PmdErrorMessageReporter.
+            throw new RuntimeException(e);
+        }
+    }
+
+    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) {

Review Comment:
   I'm tempted to let this one bubble up and convert it higher up the stack if 
needed





> Build doesn't fail for invalid CPD format
> -----------------------------------------
>
>                 Key: MPMD-395
>                 URL: https://issues.apache.org/jira/browse/MPMD-395
>             Project: Maven PMD Plugin
>          Issue Type: Bug
>          Components: CPD
>    Affects Versions: 3.22.0
>            Reporter: Andreas Dangel
>            Assignee: Andreas Dangel
>            Priority: Major
>             Fix For: next-version
>
>
> See 
> [https://github.com/apache/maven-pmd-plugin/pull/144#issuecomment-2094371392]
>  
> If an exception occurs while the CPD report is created, the exception is only 
> logged but doesn't fail the build. This means, that build appears to be 
> successful, although it didn't produce all expected build artifacts.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to