[ 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)