Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MPMD-379] Upgrade to use PMD 7.0.0 by default #144

Merged
merged 13 commits into from Apr 18, 2024
2 changes: 1 addition & 1 deletion pom.xml
Expand Up @@ -83,7 +83,7 @@ under the License.
<properties>
<mavenVersion>3.2.5</mavenVersion>
<javaVersion>8</javaVersion>
<pmdVersion>6.55.0</pmdVersion>
<pmdVersion>7.0.0-rc4</pmdVersion>
mkolesnikov marked this conversation as resolved.
Show resolved Hide resolved
<slf4jVersion>1.7.36</slf4jVersion>
<aetherVersion>1.0.0.v20140518</aetherVersion>
<doxiaVersion>1.12.0</doxiaVersion>
Expand Down
2 changes: 1 addition & 1 deletion src/it/MPMD-219-pmd-processing-error/verify.groovy
Expand Up @@ -20,5 +20,5 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
assert buildLog.text.contains( "PMD processing errors" )
assert buildLog.text.contains( "Error while parsing" )
assert buildLog.text.contains( "Parse exception in file" )
assert buildLog.text.contains( "BrokenFile.java" )
7 changes: 4 additions & 3 deletions src/it/MPMD-244-logging/verify.groovy
Expand Up @@ -20,12 +20,13 @@
File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
assert buildLog.text.contains( "PMD processing errors" )
assert buildLog.text.contains( "Error while parsing" )
assert buildLog.text.contains( "Parse exception in file" )

String disabledPath = new File( basedir, 'logging-disabled/src/main/java/BrokenFile.java' ).getCanonicalPath()
String enabledPath = new File( basedir, 'logging-enabled/src/main/java/BrokenFile.java' ).getCanonicalPath()

// logging disabled: the pmd exception is only output through the processing error reporting (since MPMD-246)
assert 1 == buildLog.text.count( "net.sourceforge.pmd.PMDException: Error while parsing ${disabledPath}" )
assert 1 == buildLog.text.count( "net.sourceforge.pmd.lang.ast.ParseException: Parse exception in file \'${disabledPath}\'" )
// logging enabled: the pmd exception is output twice: through the processing error reporting (since MPMD-246) and through PMD's own logging
assert 2 == buildLog.text.count( "net.sourceforge.pmd.PMDException: Error while parsing ${enabledPath}" )
// not true anymore, logged always once
assert 1 == buildLog.text.count( "net.sourceforge.pmd.lang.ast.ParseException: Parse exception in file \'${enabledPath}\'" )
adangel marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions src/it/MPMD-258-multiple-executions/invoker.properties
Expand Up @@ -15,5 +15,6 @@
# specific language governing permissions and limitations
# under the License.

invoker.debug = true
mkolesnikov marked this conversation as resolved.
Show resolved Hide resolved
invoker.goals = clean compile pmd:pmd
invoker.maven.version = 3.1.0+
2 changes: 1 addition & 1 deletion src/it/MPMD-268-deprecated-rules/verify.groovy
Expand Up @@ -19,4 +19,4 @@

File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()
assert buildLog.text.contains( "[WARNING] Use Rule name category/java/codestyle.xml/ControlStatementBraces instead of the deprecated" )
assert buildLog.text.contains( "Use Rule name category/java/codestyle.xml/ControlStatementBraces instead of the deprecated Rule name" )
6 changes: 3 additions & 3 deletions src/it/mpmd-138/verify.groovy
Expand Up @@ -22,12 +22,12 @@ File buildLog = new File( basedir, 'build.log' )
assert buildLog.exists()

// Module 1
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:27 Rule:UnnecessarySemicolon Priority:3 Unnecessary semicolon')
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:28 Rule:UnnecessaryReturn Priority:3 Avoid unnecessary return statements')
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:27 Rule:UnnecessarySemicolon Priority:3 Unnecessary semicolon.')
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:28 Rule:UnnecessaryReturn Priority:3 Unnecessary return statement.')
assert 1 == buildLog.getText().count('[INFO] You have 2 PMD violations. For more details see:')

// Module 2
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:24 Rule:UnusedPrivateField Priority:3 Avoid unused private fields such as \'x\'')
assert 1 == buildLog.getText().count('[INFO] PMD Failure: test.MyClass:24 Rule:UnusedPrivateField Priority:3 Avoid unused private fields such as \'x\'..')
assert 1 == buildLog.getText().count('[INFO] You have 1 PMD violation. For more details see:')

// Module 3
Expand Down
11 changes: 5 additions & 6 deletions src/main/java/org/apache/maven/plugins/pmd/CpdReport.java
Expand Up @@ -23,8 +23,7 @@
import java.util.Locale;
import java.util.Properties;

import net.sourceforge.pmd.cpd.JavaTokenizer;
import net.sourceforge.pmd.cpd.renderer.CPDRenderer;
import net.sourceforge.pmd.cpd.CPDReportRenderer;
import org.apache.maven.plugins.annotations.Component;
import org.apache.maven.plugins.annotations.Mojo;
import org.apache.maven.plugins.annotations.Parameter;
Expand Down Expand Up @@ -179,13 +178,13 @@ private void executeCpd() throws MavenReportException {

Properties languageProperties = new Properties();
if (ignoreLiterals) {
languageProperties.setProperty(JavaTokenizer.IGNORE_LITERALS, "true");
languageProperties.setProperty("ignore_literals", "true");
}
if (ignoreIdentifiers) {
languageProperties.setProperty(JavaTokenizer.IGNORE_IDENTIFIERS, "true");
languageProperties.setProperty("ignore_identifiers", "true");
}
if (ignoreAnnotations) {
languageProperties.setProperty(JavaTokenizer.IGNORE_ANNOTATIONS, "true");
languageProperties.setProperty("ignore_annotations", "true");
}
try {
filesToProcess = getFilesToProcess();
Expand Down Expand Up @@ -238,7 +237,7 @@ public String getOutputName() {
* @deprecated Use {@link CpdExecutor#createRenderer(String, String)} instead.
*/
@Deprecated
public CPDRenderer createRenderer() throws MavenReportException {
public CPDReportRenderer createRenderer() throws MavenReportException {
adangel marked this conversation as resolved.
Show resolved Hide resolved
return CpdExecutor.createRenderer(format, getOutputEncoding());
}
}
Expand Up @@ -64,7 +64,7 @@ public boolean isExcludedFromFailure(final Duplication errorDetail) {
public boolean isExcludedFromFailure(final Match errorDetail) {
final Set<String> uniquePaths = new HashSet<>();
for (Mark mark : errorDetail.getMarkSet()) {
uniquePaths.add(mark.getFilename());
uniquePaths.add(mark.getLocation().getFileId().getAbsolutePath());
}
return isExcludedFromFailure(uniquePaths);
}
Expand Down
Expand Up @@ -84,8 +84,10 @@ public boolean isExcludedFromFailure(final Violation errorDetail) {
* @return <code>true</code> if the violation should be excluded, <code>false</code> otherwise.
*/
public boolean isExcludedFromFailure(final RuleViolation errorDetail) {
final String className =
extractClassName(errorDetail.getPackageName(), errorDetail.getClassName(), errorDetail.getFilename());
final String className = extractClassName(
errorDetail.getPackageName(),
errorDetail.getClassName(),
errorDetail.getFileId().getAbsolutePath());
mkolesnikov marked this conversation as resolved.
Show resolved Hide resolved
return isExcludedFromFailure(className, errorDetail.getRule().getName());
}

Expand Down
98 changes: 47 additions & 51 deletions src/main/java/org/apache/maven/plugins/pmd/exec/CpdExecutor.java
Expand Up @@ -26,23 +26,22 @@
import java.io.ObjectOutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.nio.charset.Charset;
import java.util.Objects;
import java.util.function.Predicate;

import net.sourceforge.pmd.cpd.CPD;
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.EcmascriptLanguage;
import net.sourceforge.pmd.cpd.JSPLanguage;
import net.sourceforge.pmd.cpd.JavaLanguage;
import net.sourceforge.pmd.cpd.Language;
import net.sourceforge.pmd.cpd.LanguageFactory;
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.cpd.renderer.CPDRenderer;
import net.sourceforge.pmd.lang.Language;
import net.sourceforge.pmd.lang.ecmascript.EcmascriptLanguageModule;
import net.sourceforge.pmd.lang.java.JavaLanguageModule;
import net.sourceforge.pmd.lang.jsp.JspLanguageModule;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.pmd.ExcludeDuplicationsFromFile;
import org.apache.maven.reporting.MavenReportException;
Expand Down Expand Up @@ -158,43 +157,44 @@ private CpdResult run() throws MavenReportException {

Language cpdLanguage;
if ("java".equals(request.getLanguage()) || null == request.getLanguage()) {
adangel marked this conversation as resolved.
Show resolved Hide resolved
cpdLanguage = new JavaLanguage(request.getLanguageProperties());
mkolesnikov marked this conversation as resolved.
Show resolved Hide resolved
cpdLanguage = new JavaLanguageModule();
} else if ("javascript".equals(request.getLanguage())) {
cpdLanguage = new EcmascriptLanguage();
cpdLanguage = new EcmascriptLanguageModule();
} else if ("jsp".equals(request.getLanguage())) {
cpdLanguage = new JSPLanguage();
cpdLanguage = new JspLanguageModule();
} else {
cpdLanguage = LanguageFactory.createLanguage(request.getLanguage(), request.getLanguageProperties());
cpdLanguage = cpdConfiguration.getLanguageRegistry().getLanguageById(request.getLanguage());
}

cpdConfiguration.setLanguage(cpdLanguage);
cpdConfiguration.setSourceEncoding(request.getSourceEncoding());
cpdConfiguration.setOnlyRecognizeLanguage(cpdLanguage);
cpdConfiguration.setSourceEncoding(Charset.forName(request.getSourceEncoding()));

CPD cpd = new CPD(cpdConfiguration);
try {
cpd.add(request.getFiles());
} catch (IOException e) {
throw new MavenReportException(e.getMessage(), e);
}
request.getFiles().forEach(f -> cpdConfiguration.addInputPath(f.toPath()));

LOG.debug("Executing CPD...");
cpd.go();
LOG.debug("CPD finished.");

// 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
writeXmlReport(cpd);
CpdAnalysis cpd = CpdAnalysis.create(cpdConfiguration);
mkolesnikov marked this conversation as resolved.
Show resolved Hide resolved
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(cpd);
}
// 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(e.getMessage(), e);
}
});
LOG.debug("CPD finished.");

return new CpdResult(new File(request.getTargetDirectory(), "cpd.xml"), request.getOutputEncoding());
}

private void writeXmlReport(CPD cpd) throws MavenReportException {
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());
Expand All @@ -207,7 +207,7 @@ private void writeXmlReport(CPD cpd) throws MavenReportException {
}
}

private File writeReport(CPD cpd, CPDRenderer r, String extension) throws MavenReportException {
private File writeReport(CPDReport cpd, CPDReportRenderer r, String extension) throws MavenReportException {
if (r == null) {
return null;
}
Expand All @@ -216,16 +216,16 @@ private File writeReport(CPD cpd, CPDRenderer r, String extension) throws MavenR
targetDir.mkdirs();
File targetFile = new File(targetDir, "cpd." + extension);
try (Writer writer = new OutputStreamWriter(new FileOutputStream(targetFile), request.getOutputEncoding())) {
r.render(filterMatches(cpd.getMatches()), writer);
r.render(cpd.filterMatches(filterMatches()), writer);
writer.flush();
} catch (IOException ioe) {
throw new MavenReportException(ioe.getMessage(), ioe);
}
return targetFile;
}

private void writeFormattedReport(CPD cpd) throws MavenReportException {
CPDRenderer r = createRenderer(request.getFormat(), request.getOutputEncoding());
private void writeFormattedReport(CPDReport cpd) throws MavenReportException {
CPDReportRenderer r = createRenderer(request.getFormat(), request.getOutputEncoding());
writeReport(cpd, r, request.getFormat());
}

Expand All @@ -235,8 +235,8 @@ private void writeFormattedReport(CPD cpd) throws MavenReportException {
* @return the renderer based on the configured output
* @throws org.apache.maven.reporting.MavenReportException if no renderer found for the output type
*/
public static CPDRenderer createRenderer(String format, String outputEncoding) throws MavenReportException {
CPDRenderer renderer = null;
public static CPDReportRenderer createRenderer(String format, String outputEncoding) throws MavenReportException {
CPDReportRenderer renderer = null;
if ("xml".equals(format)) {
renderer = new XMLRenderer(outputEncoding);
} else if ("csv".equals(format)) {
Expand All @@ -245,7 +245,8 @@ public static CPDRenderer createRenderer(String format, String outputEncoding) t
renderer = new SimpleRenderer();
} else if (!"".equals(format) && !"none".equals(format)) {
try {
renderer = (CPDRenderer) Class.forName(format).getConstructor().newInstance();
renderer = (CPDReportRenderer)
Class.forName(format).getConstructor().newInstance();
} catch (Exception e) {
throw new MavenReportException(
"Can't find CPD custom format " + format + ": "
Expand All @@ -257,22 +258,17 @@ public static CPDRenderer createRenderer(String format, String outputEncoding) t
return renderer;
}

private Iterator<Match> filterMatches(Iterator<Match> matches) {
LOG.debug("Filtering duplications. Using " + excludeDuplicationsFromFile.countExclusions()
+ " configured exclusions.");
private Predicate<Match> filterMatches() {
return (Match match) -> {
LOG.debug("Filtering duplications. Using " + excludeDuplicationsFromFile.countExclusions()
+ " configured exclusions.");

List<Match> filteredMatches = new ArrayList<>();
int excludedDuplications = 0;
while (matches.hasNext()) {
Match match = matches.next();
if (excludeDuplicationsFromFile.isExcludedFromFailure(match)) {
excludedDuplications++;
LOG.debug("Excluded " + match + " duplications.");
return false;
} else {
filteredMatches.add(match);
return true;
}
}

LOG.debug("Excluded " + excludedDuplications + " duplications.");
return filteredMatches.iterator();
};
}
}
17 changes: 6 additions & 11 deletions src/main/java/org/apache/maven/plugins/pmd/exec/PmdExecutor.java
Expand Up @@ -27,6 +27,7 @@
import java.io.ObjectOutputStream;
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
Expand All @@ -37,7 +38,6 @@
import net.sourceforge.pmd.RulePriority;
import net.sourceforge.pmd.RuleSetLoadException;
import net.sourceforge.pmd.RuleSetLoader;
import net.sourceforge.pmd.RuleViolation;
import net.sourceforge.pmd.benchmark.TextTimingReportRenderer;
import net.sourceforge.pmd.benchmark.TimeTracker;
import net.sourceforge.pmd.benchmark.TimingReport;
Expand All @@ -50,7 +50,6 @@
import net.sourceforge.pmd.renderers.Renderer;
import net.sourceforge.pmd.renderers.TextRenderer;
import net.sourceforge.pmd.renderers.XMLRenderer;
import net.sourceforge.pmd.util.Predicate;
import org.apache.maven.plugin.MojoExecutionException;
import org.apache.maven.plugins.pmd.ExcludeViolationsFromFile;
import org.apache.maven.reporting.MavenReportException;
Expand Down Expand Up @@ -172,7 +171,7 @@ private PmdResult run() throws MavenReportException {
configuration.setDefaultLanguageVersion(languageVersion);

if (request.getSourceEncoding() != null) {
configuration.setSourceEncoding(request.getSourceEncoding());
configuration.setSourceEncoding(Charset.forName(request.getSourceEncoding()));
}

configuration.prependAuxClasspath(request.getAuxClasspath());
Expand All @@ -190,7 +189,7 @@ private PmdResult run() throws MavenReportException {
configuration.setRuleSets(request.getRulesets());
configuration.setMinimumPriority(RulePriority.valueOf(request.getMinimumPriority()));
if (request.getBenchmarkOutputLocation() != null) {
configuration.setBenchmark(true);
TimeTracker.startGlobalTracking();
adangel marked this conversation as resolved.
Show resolved Hide resolved
}
List<File> files = request.getFiles();

Expand Down Expand Up @@ -262,7 +261,7 @@ private PmdResult run() throws MavenReportException {
private String getErrorsAsString(List<Report.ProcessingError> errors, boolean withDetails) {
List<String> errorsAsString = new ArrayList<>(errors.size());
for (Report.ProcessingError error : errors) {
errorsAsString.add(error.getFile() + ": " + error.getMsg());
errorsAsString.add(error.getFileId().getAbsolutePath() + ": " + error.getMsg());
if (withDetails) {
errorsAsString.add(error.getDetail());
}
Expand Down Expand Up @@ -413,12 +412,8 @@ private Report removeExcludedViolations(Report report) throws MavenReportExcepti
LOG.debug("Removing excluded violations. Using {} configured exclusions.", excludeFromFile.countExclusions());
int violationsBefore = report.getViolations().size();

Report filtered = report.filterViolations(new Predicate<RuleViolation>() {
@Override
public boolean test(RuleViolation ruleViolation) {
return !excludeFromFile.isExcludedFromFailure(ruleViolation);
}
});
Report filtered =
report.filterViolations(ruleViolation -> !excludeFromFile.isExcludedFromFailure(ruleViolation));

int numberOfExcludedViolations =
violationsBefore - filtered.getViolations().size();
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/apache/maven/plugins/pmd/CpdReportTest.java
Expand Up @@ -62,7 +62,7 @@ public void testDefaultConfiguration() throws Exception {
assertTrue(lowerCaseContains(str, "AppSample.java"));
assertTrue(lowerCaseContains(str, "App.java"));
assertTrue(lowerCaseContains(str, "public String dup( String str )"));
assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, i + 1);"));
assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, (i + 1));"));

// the version should be logged
String output = CapturingPrintStream.getOutput();
Expand Down Expand Up @@ -149,7 +149,7 @@ public void testWriteNonHtml() throws Exception {
assertTrue(lowerCaseContains(str, "AppSample.java"));
assertTrue(lowerCaseContains(str, "App.java"));
assertTrue(lowerCaseContains(str, "public String dup( String str )"));
assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, i + 1);"));
assertTrue(lowerCaseContains(str, "tmp = tmp + str.substring( i, (i + 1));"));
}

/**
Expand Down