fix:[core] Wrong deprecation warnings for unused XPath attributes

* Improve integration tests in pmd-dist to detect
  warnings about deprecated attributes.
* Wrap the attribute value in a singleton list, to be able to distinguish
  between no value (null in the list) and value not determined yet
  (list is null).
* Add integration test for apex.
* Updated release notes

fixes #2020
This commit is contained in:
Andreas Dangel
2019-09-29 14:59:55 +02:00
parent dff6f4d7ab
commit 0ff97e5959
10 changed files with 594 additions and 41 deletions

View File

@ -32,6 +32,7 @@ This is a {{ site.pmd.release_type }} release.
* core
* [#2014](https://github.com/pmd/pmd/issues/2014): \[core] Making add(SourceCode sourceCode) public for alternative file systems
* [#2020](https://github.com/pmd/pmd/issues/2020): \[core] Wrong deprecation warnings for unused XPath attributes
* [#2036](https://github.com/pmd/pmd/issues/2036): \[core] Wrong include/exclude patterns are silently ignored
* java
* [#2042](https://github.com/pmd/pmd/issues/2042): \[java] PMD crashes with ClassFormatError: Absent Code attribute...

View File

@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.ast.xpath;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
@ -36,7 +38,7 @@ public class Attribute {
private final Node parent;
private final String name;
private Method method;
private Object value;
private List<?> value;
private String stringValue;
/** Creates a new attribute belonging to the given node using its accessor. */
@ -50,7 +52,7 @@ public class Attribute {
public Attribute(Node parent, String name, String value) {
this.parent = parent;
this.name = name;
this.value = value;
this.value = Collections.singletonList(value);
this.stringValue = value;
}
@ -71,20 +73,20 @@ public class Attribute {
}
public Object getValue() {
if (value != null) { // TODO if the method returned null we'll call it again...
return value;
if (value != null) {
return value.get(0);
}
if (method.isAnnotationPresent(Deprecated.class) && LOG.isLoggable(Level.WARNING)
&& DETECTED_DEPRECATED_ATTRIBUTES.putIfAbsent(getLoggableAttributeName(), Boolean.TRUE) == null) {
// this message needs to be kept in sync with PMDCoverageTest
// this message needs to be kept in sync with PMDCoverageTest / BinaryDistributionIT
LOG.warning("Use of deprecated attribute '" + getLoggableAttributeName() + "' in XPath query");
}
// this lazy loading reduces calls to Method.invoke() by about 90%
try {
value = method.invoke(parent, EMPTY_OBJ_ARRAY);
return value;
value = Collections.singletonList(method.invoke(parent, EMPTY_OBJ_ARRAY));
return value.get(0);
} catch (IllegalAccessException | InvocationTargetException iae) {
iae.printStackTrace();
}
@ -95,10 +97,8 @@ public class Attribute {
if (stringValue != null) {
return stringValue;
}
Object v = this.value;
if (this.value == null) {
v = getValue();
}
Object v = getValue();
stringValue = v == null ? "" : String.valueOf(v);
return stringValue;
}

View File

@ -90,7 +90,7 @@ public class BinaryDistributionIT {
@Test
public void runPMD() throws Exception {
String srcDir = new File(".", "src/test/resources/sample-source/").getAbsolutePath();
String srcDir = new File(".", "src/test/resources/sample-source/java/").getAbsolutePath();
ExecutionResult result;
@ -98,12 +98,38 @@ public class BinaryDistributionIT {
result.assertExecutionResult(0, "apex, ecmascript, java, jsp, plsql, pom, scala, vf, vm, wsdl, xml, xsl");
result = PMDExecutor.runPMDRules(tempDir, srcDir, "src/test/resources/rulesets/sample-ruleset.xml");
result.assertExecutionResult(4, "JumbledIncrementer.java:8:");
result.assertExecutionResult(4, "", "JumbledIncrementer.java:8:");
result = PMDExecutor.runPMDRules(tempDir, srcDir, "rulesets/java/quickstart.xml");
result.assertExecutionResult(4, "");
}
@Test
public void testAllJavaRules() throws Exception {
String srcDir = new File(".", "src/test/resources/sample-source/java/").getAbsolutePath();
ExecutionResult result = PMDExecutor.runPMDRules(tempDir, srcDir, "src/test/resources/rulesets/all-java.xml");
assertDefaultExecutionResult(result);
}
@Test
public void testAllApexRules() throws Exception {
String srcDir = new File(".", "src/test/resources/sample-source/apex/").getAbsolutePath();
ExecutionResult result = PMDExecutor.runPMDRules(tempDir, srcDir, "src/test/resources/rulesets/all-apex.xml");
assertDefaultExecutionResult(result);
}
private static void assertDefaultExecutionResult(ExecutionResult result) {
result.assertExecutionResult(4, "");
result.assertNoError("Exception applying rule");
result.assertNoError("Ruleset not found");
result.assertNoError("Use of deprecated attribute");
result.assertNoErrorInReport("Error while processing");
result.assertNoErrorInReport("Error while parsing");
}
@Test
public void runCPD() throws Exception {
String srcDir = new File(".", "src/test/resources/sample-source-cpd/").getAbsolutePath();

View File

@ -34,7 +34,7 @@ public class CpdExecutor {
String output = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8);
int result = process.waitFor();
return new ExecutionResult(result, output);
return new ExecutionResult(result, output, null, null);
}
private static ExecutionResult runCpdWindows(Path tempDir, String ... arguments) throws Exception {
@ -46,7 +46,7 @@ public class CpdExecutor {
String output = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8);
int result = process.waitFor();
return new ExecutionResult(result, output);
return new ExecutionResult(result, output, null, null);
}
/**

View File

@ -5,7 +5,9 @@
package net.sourceforge.pmd.it;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import net.sourceforge.pmd.PMD;
@ -18,10 +20,14 @@ import net.sourceforge.pmd.PMD;
public class ExecutionResult {
private final int exitCode;
private final String output;
private final String errorOutput;
private final String report;
ExecutionResult(int theExitCode, String theOutput) {
ExecutionResult(int theExitCode, String theOutput, String theErrorOutput, String theReport) {
this.exitCode = theExitCode;
this.output = theOutput;
this.errorOutput = theErrorOutput;
this.report = theReport;
}
@Override
@ -30,7 +36,9 @@ public class ExecutionResult {
sb.append("ExecutionResult:")
.append(PMD.EOL)
.append(" exit code: ").append(exitCode).append(PMD.EOL)
.append(" output:").append(PMD.EOL).append(output).append(PMD.EOL);
.append(" output:").append(PMD.EOL).append(output).append(PMD.EOL)
.append(" errorOutput:").append(PMD.EOL).append(errorOutput).append(PMD.EOL)
.append(" report:").append(PMD.EOL).append(report).append(PMD.EOL);
return sb.toString();
}
@ -42,10 +50,80 @@ public class ExecutionResult {
* @param expectedOutput the output to search for
*/
public void assertExecutionResult(int expectedExitCode, String expectedOutput) {
assertEquals("Command exited with wrong code", expectedExitCode, exitCode);
assertExecutionResult(expectedExitCode, expectedOutput, null);
}
/**
* Asserts that the command exited with the expected exit code and that the given expected
* output is contained in the actual command 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 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) {
assertEquals("Command exited with wrong code.\nComplete result:\n\n" + this, expectedExitCode, exitCode);
assertNotNull("No output found", output);
if (!output.contains(expectedOutput)) {
fail("Expected output '" + expectedOutput + "' not present.\nComplete output:\n\n" + output);
if (expectedOutput != null && !expectedOutput.isEmpty()) {
if (!output.contains(expectedOutput)) {
fail("Expected output '" + expectedOutput + "' not present.\nComplete result:\n\n" + this);
}
} else {
assertTrue("The output should have been empty.\nComplete result:\n\n" + this, output.isEmpty());
}
if (expectedReport != null && !expectedReport.isEmpty()) {
assertTrue("Expected report '" + expectedReport + "'.\nComplete result:\n\n" + this,
report.contains(expectedReport));
}
}
/**
* Asserts that the given error message is not in the error output.
* @param errorMessage the error message to search for
*/
public void assertNoError(String errorMessage) {
assertFalse("Found error message: " + errorMessage + ".\nComplete result:\n\n" + this,
errorOutput.contains(errorMessage));
}
/**
* Asserts that the given error message is not in the report.
* @param errorMessage the error message to search for
*/
public void assertNoErrorInReport(String errorMessage) {
assertFalse("Found error message in report: " + errorMessage + ".\nComplete result:\n\n" + this,
report.contains(errorMessage));
}
static class Builder {
private int exitCode;
private String output;
private String errorOutput;
private String report;
Builder withExitCode(int exitCode) {
this.exitCode = exitCode;
return this;
}
Builder withOutput(String output) {
this.output = output;
return this;
}
Builder withErrorOutput(String errorOutput) {
this.errorOutput = errorOutput;
return this;
}
Builder withReport(String report) {
this.report = report;
return this;
}
ExecutionResult build() {
return new ExecutionResult(exitCode, output, errorOutput, report);
}
}
}

View File

@ -4,9 +4,13 @@
package net.sourceforge.pmd.it;
import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.TimeUnit;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.SystemUtils;
@ -24,33 +28,65 @@ public class PMDExecutor {
private static final String RULESET_FLAG = "-R";
private static final String FORMAT_FLAG = "-f";
private static final String FORMATTER = "text";
private static final String REPORTFILE_FLAG = "-r";
private PMDExecutor() {
// this is a helper class only
}
private static ExecutionResult runPMDUnix(Path tempDir, String ... arguments) throws Exception {
private static ExecutionResult runPMDUnix(Path tempDir, Path reportFile, String ... arguments) throws Exception {
String cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/run.sh").toAbsolutePath().toString();
ProcessBuilder pb = new ProcessBuilder(cmd, "pmd");
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);
return runPMD(cmd, Arrays.asList(arguments), reportFile);
}
private static ExecutionResult runPMDWindows(Path tempDir, String ... arguments) throws Exception {
private static ExecutionResult runPMDWindows(Path tempDir, Path reportFile, String ... arguments) throws Exception {
String cmd = tempDir.resolve(PMD_BIN_PREFIX + PMDVersion.VERSION + "/bin/pmd.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);
return runPMD(cmd, Arrays.asList(arguments), reportFile);
}
int result = process.waitFor();
return new ExecutionResult(result, output);
private static ExecutionResult runPMD(String cmd, List<String> arguments, Path reportFile) throws Exception {
ProcessBuilder pb = new ProcessBuilder(cmd, "pmd");
pb.command().addAll(arguments);
pb.redirectErrorStream(false);
final Process process = pb.start();
final ExecutionResult.Builder result = new ExecutionResult.Builder();
Thread outputReader = new Thread(new Runnable() {
@Override
public void run() {
String output;
try {
output = IOUtils.toString(process.getInputStream(), StandardCharsets.UTF_8);
result.withOutput(output);
} catch (IOException e) {
result.withOutput("Exception occurred: " + e.toString());
}
}
});
outputReader.start();
Thread errorReader = new Thread(new Runnable() {
@Override
public void run() {
String error;
try {
error = IOUtils.toString(process.getErrorStream(), StandardCharsets.UTF_8);
result.withErrorOutput(error);
} catch (IOException e) {
result.withErrorOutput("Exception occurred: " + e.toString());
}
}
});
errorReader.start();
int exitCode = process.waitFor();
outputReader.join(TimeUnit.MINUTES.toMillis(5));
errorReader.join(TimeUnit.MINUTES.toMillis(5));
String report = null;
if (reportFile != null) {
report = IOUtils.toString(reportFile.toUri(), StandardCharsets.UTF_8);
}
return result.withExitCode(exitCode).withReport(report).build();
}
/**
@ -63,10 +99,15 @@ public class PMDExecutor {
* @throws Exception if the execution fails for any reason (executable not found, ...)
*/
public static ExecutionResult runPMDRules(Path tempDir, String sourceDirectory, String ruleset) throws Exception {
Path reportFile = Files.createTempFile("pmd-it-report", "txt");
reportFile.toFile().deleteOnExit();
if (SystemUtils.IS_OS_WINDOWS) {
return runPMDWindows(tempDir, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset, FORMAT_FLAG, FORMATTER);
return runPMDWindows(tempDir, reportFile, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset,
FORMAT_FLAG, FORMATTER, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString());
} else {
return runPMDUnix(tempDir, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset, FORMAT_FLAG, FORMATTER);
return runPMDUnix(tempDir, reportFile, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset,
FORMAT_FLAG, FORMATTER, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString());
}
}
@ -79,9 +120,9 @@ public class PMDExecutor {
*/
public static ExecutionResult runPMD(Path tempDir, String ... arguments) throws Exception {
if (SystemUtils.IS_OS_WINDOWS) {
return runPMDWindows(tempDir, arguments);
return runPMDWindows(tempDir, null, arguments);
} else {
return runPMDUnix(tempDir, arguments);
return runPMDUnix(tempDir, null, arguments);
}
}
}

View File

@ -0,0 +1,18 @@
<?xml version="1.0"?>
<ruleset name="All Apex Rules"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>Every Apex Rule in PMD</description>
<rule ref="category/apex/bestpractices.xml" />
<rule ref="category/apex/codestyle.xml" />
<rule ref="category/apex/design.xml" />
<rule ref="category/apex/documentation.xml" />
<rule ref="category/apex/errorprone.xml" />
<rule ref="category/apex/multithreading.xml" />
<rule ref="category/apex/performance.xml" />
<rule ref="category/apex/security.xml" />
</ruleset>

View File

@ -0,0 +1,18 @@
<?xml version="1.0"?>
<ruleset name="All Java Rules"
xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 https://pmd.sourceforge.io/ruleset_2_0_0.xsd">
<description>Every Java Rule in PMD</description>
<rule ref="category/java/bestpractices.xml" />
<rule ref="category/java/codestyle.xml" />
<rule ref="category/java/design.xml" />
<rule ref="category/java/documentation.xml" />
<rule ref="category/java/errorprone.xml" />
<rule ref="category/java/multithreading.xml" />
<rule ref="category/java/performance.xml" />
<rule ref="category/java/security.xml" />
</ruleset>

File diff suppressed because it is too large Load Diff