From 4cf9f5904ad76f13aecc2210a758168974657eb1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 1 Aug 2020 17:53:45 +0200 Subject: [PATCH 1/3] [core] Fix java7 compatibility * Scala also requires java8 and it failed indirectly via a ServiceConfigurationError while discovering the language. Scala's dependencies require java8. * Downgrade JCommander for java7 compatibility * Downgrade commons-text for java7 compatibility * Add integration test --- .travis/before_install.sh | 18 ++++++++ .travis/build-deploy.sh | 6 ++- .../sourceforge/pmd/cpd/LanguageFactory.java | 3 +- .../pmd/lang/LanguageRegistry.java | 3 +- .../pmd/util/treeexport/TreeExportCli.java | 5 +-- pmd-dist/pom.xml | 43 ++++++++++++++++--- .../net/sourceforge/pmd/it/AllRulesIT.java | 5 +++ .../pmd/it/BinaryDistributionIT.java | 38 +++++++++++++--- .../sourceforge/pmd/it/ExecutionResult.java | 11 ++++- .../net/sourceforge/pmd/it/PMDExecutor.java | 13 +++++- pom.xml | 17 +++++--- 11 files changed, 136 insertions(+), 26 deletions(-) diff --git a/.travis/before_install.sh b/.travis/before_install.sh index a75a4d597b..4b0182f2cf 100644 --- a/.travis/before_install.sh +++ b/.travis/before_install.sh @@ -18,6 +18,24 @@ if travis_isLinux; then bundle config set --local path vendor/bundle bundle config set --local with release_notes_preprocessing bundle install + + # install jdk7 for integration test + LOCAL_DIR=${HOME}/.cache/jdk7 + TARGET_DIR=${HOME}/oraclejdk7 + JDK7_ARCHIVE=jdk-7u80-linux-x64.tar.gz + DOWNLOAD_URL=https://pmd-code.org/oraclejdk/${JDK7_ARCHIVE} + mkdir -p ${LOCAL_DIR} + mkdir -p ${TARGET_DIR} + if [ ! -e ${LOCAL_DIR}/${JDK7_ARCHIVE} ]; then + log_info "Downloading from ${DOWNLOAD_URL} to ${LOCAL_DIR}" + wget --directory-prefix ${LOCAL_DIR} --timestamping --continue ${DOWNLOAD_URL} + else + log_info "Skipped download, file ${LOCAL_DIR}/${JDK7_ARCHIVE} already exists" + fi + log_info "Extracting to ${TARGET_DIR}" + tar --extract --file ${LOCAL_DIR}/${JDK7_ARCHIVE} -C ${TARGET_DIR} --strip-components=1 + log_info "OracleJDK7 can be used via -Djava7.home=${TARGET_DIR}" + else log_info "Not setting up ruby for ${TRAVIS_OS_NAME}." exit 0 diff --git a/.travis/build-deploy.sh b/.travis/build-deploy.sh index f4bbddc2d5..e9250ad7c4 100755 --- a/.travis/build-deploy.sh +++ b/.travis/build-deploy.sh @@ -10,7 +10,11 @@ source .travis/regression-tester.sh VERSION=$(get_pom_version) log_info "Building PMD ${VERSION} on branch ${TRAVIS_BRANCH}" -MVN_BUILD_FLAGS="-B -V" +if travis_isLinux; then + MVN_BUILD_FLAGS="-B -V -Djava7.home=${HOME}/oraclejdk7" +else + MVN_BUILD_FLAGS="-B -V" +fi if travis_isOSX; then diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java index a032ae7663..d7f2cda927 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/LanguageFactory.java @@ -14,6 +14,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Properties; +import java.util.ServiceConfigurationError; import java.util.ServiceLoader; public final class LanguageFactory { @@ -40,7 +41,7 @@ public final class LanguageFactory { try { Language language = iterator.next(); languagesList.add(language); - } catch (UnsupportedClassVersionError e) { + } catch (ServiceConfigurationError | UnsupportedClassVersionError e) { // Some languages require java8 and are therefore only available // if java8 or later is used as runtime. System.err.println("Ignoring language for CPD: " + e.toString()); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageRegistry.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageRegistry.java index 518231c756..26628ef457 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageRegistry.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageRegistry.java @@ -12,6 +12,7 @@ import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.ServiceConfigurationError; import java.util.ServiceLoader; import org.apache.commons.lang3.StringUtils; @@ -34,7 +35,7 @@ public final class LanguageRegistry { try { Language language = iterator.next(); languagesList.add(language); - } catch (UnsupportedClassVersionError e) { + } catch (ServiceConfigurationError | UnsupportedClassVersionError e) { // Some languages require java8 and are therefore only available // if java8 or later is used as runtime. System.err.println("Ignoring language for PMD: " + e.toString()); diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/treeexport/TreeExportCli.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/treeexport/TreeExportCli.java index a3b94bf9eb..25cd5397e6 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/treeexport/TreeExportCli.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/treeexport/TreeExportCli.java @@ -59,9 +59,8 @@ public class TreeExportCli { public static void main(String[] args) throws IOException { TreeExportCli cli = new TreeExportCli(); - JCommander jcommander = JCommander.newBuilder() - .addObject(cli) - .build(); + JCommander jcommander = new JCommander(cli); + try { jcommander.parse(args); } catch (ParameterException e) { diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index c7974d49bf..22e9c7719c 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -80,9 +80,9 @@ org.apache.maven.plugins maven-failsafe-plugin - 2.19.1 + failsafe-default integration-test verify @@ -198,11 +198,6 @@ pmd-ruby ${project.version} - - net.sourceforge.pmd - pmd-scala_2.13 - ${project.version} - net.sourceforge.pmd pmd-swift @@ -259,6 +254,11 @@ pmd-apex ${project.version} + + net.sourceforge.pmd + pmd-scala_2.13 + ${project.version} + net.sourceforge.pmd pmd-ui @@ -266,5 +266,36 @@ + + jdk7-compat-it + + + java7.home + + + + + + org.apache.maven.plugins + maven-failsafe-plugin + + + jdk7-compat-it + + integration-test + verify + + + + ${java7.home} + ${java7.home}/bin:${env.PATH} + + + + + + + + diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java index 823e304a04..9249c3ca3b 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java @@ -21,6 +21,11 @@ public class AllRulesIT extends AbstractBinaryDistributionTest { @Parameters public static Iterable languagesToTest() { + if (PMDExecutor.isJava7Test()) { + // note: apex and scala require java8 + return Arrays.asList("java", "javascript", "jsp", "modelica", + "plsql", "pom", "visualforce", "velocitytemplate", "xml", "xsl"); + } // note: scala and wsdl have no rules return Arrays.asList("java", "apex", "javascript", "jsp", "modelica", "plsql", "pom", "visualforce", "velocitytemplate", "xml", "xsl"); diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java index c88f5a8c32..3910ef4a5c 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -21,7 +21,19 @@ import net.sourceforge.pmd.PMDVersion; public class BinaryDistributionIT extends AbstractBinaryDistributionTest { - private static final String SUPPORTED_LANGUAGES = "Supported languages: [apex, cpp, cs, dart, ecmascript, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, scala, swift, vf, xml]"; + private static final String SUPPORTED_LANGUAGES_CPD; + private static final String SUPPORTED_LANGUAGES_PMD; + + static { + // note: apex and scala require java8 + if (PMDExecutor.isJava7Test()) { + SUPPORTED_LANGUAGES_CPD = "Supported languages: [cpp, cs, dart, ecmascript, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, swift, vf, xml]"; + SUPPORTED_LANGUAGES_PMD = "ecmascript, java, jsp, modelica, plsql, pom, vf, vm, wsdl, xml, xsl"; + } else { + SUPPORTED_LANGUAGES_CPD = "Supported languages: [apex, cpp, cs, dart, ecmascript, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, scala, swift, vf, xml]"; + SUPPORTED_LANGUAGES_PMD = "apex, ecmascript, java, jsp, modelica, plsql, pom, scala, vf, vm, wsdl, xml, xsl"; + } + } @Test public void testFileExistence() { @@ -66,12 +78,20 @@ public class BinaryDistributionIT extends AbstractBinaryDistributionTest { ExecutionResult result; + result = PMDExecutor.runPMD(tempDir); // without any argument, display usage help and error + result.assertExecutionResult(1, SUPPORTED_LANGUAGES_PMD); + result = PMDExecutor.runPMD(tempDir, "-h"); - result.assertExecutionResult(0, "apex, ecmascript, java, jsp, modelica, plsql, pom, scala, vf, vm, wsdl, xml, xsl"); + result.assertExecutionResult(0, SUPPORTED_LANGUAGES_PMD); result = PMDExecutor.runPMDRules(tempDir, srcDir, "src/test/resources/rulesets/sample-ruleset.xml"); result.assertExecutionResult(4, "", "JumbledIncrementer.java:8:"); + // also test XML format + result = PMDExecutor.runPMDRules(tempDir, srcDir, "src/test/resources/rulesets/sample-ruleset.xml", "xml"); + result.assertExecutionResult(4, "", "JumbledIncrementer.java\">"); + result.assertExecutionResult(4, "", ""); + result.assertExecutionResult(4, "Class1.java\"/>"); + result.assertExecutionResult(4, "Class2.java\"/>"); + result = CpdExecutor.runCpd(tempDir, "--minimum-tokens", "1000", "--format", "text", "--files", srcDir); - result.assertExecutionResult(0, ""); + result.assertExecutionResult(0); } } diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/ExecutionResult.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/ExecutionResult.java index 04efb3971b..1196d99179 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/ExecutionResult.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/ExecutionResult.java @@ -42,6 +42,15 @@ public class ExecutionResult { return sb.toString(); } + /** + * Asserts that the command exited with the expected exit code. Any output is ignored. + * + * @param expectedExitCode the exit code, e.g. 0 if no rule violations are expected, or 4 if violations are found + */ + public void assertExecutionResult(int expectedExitCode) { + assertExecutionResult(expectedExitCode, null); + } + /** * Asserts that the command exited with the expected exit code and that the given expected * output is contained in the actual command output. @@ -69,7 +78,7 @@ public class ExecutionResult { if (!output.contains(expectedOutput)) { fail("Expected output '" + expectedOutput + "' not present.\nComplete result:\n\n" + this); } - } else { + } else if (expectedOutput != null && expectedOutput.isEmpty()) { assertTrue("The output should have been empty.\nComplete result:\n\n" + this, output.isEmpty()); } if (expectedReport != null && !expectedReport.isEmpty()) { diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/PMDExecutor.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/PMDExecutor.java index 2ea7115844..fab4350ea6 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/PMDExecutor.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/PMDExecutor.java @@ -14,6 +14,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; import org.apache.commons.io.IOUtils; +import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.SystemUtils; import net.sourceforge.pmd.PMDVersion; @@ -103,15 +104,19 @@ 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 { + return runPMDRules(tempDir, sourceDirectory, ruleset, FORMATTER); + } + + public static ExecutionResult runPMDRules(Path tempDir, String sourceDirectory, String ruleset, String formatter) throws Exception { Path reportFile = Files.createTempFile("pmd-it-report", "txt"); reportFile.toFile().deleteOnExit(); if (SystemUtils.IS_OS_WINDOWS) { return runPMDWindows(tempDir, reportFile, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset, - FORMAT_FLAG, FORMATTER, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString()); + FORMAT_FLAG, formatter, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString()); } else { return runPMDUnix(tempDir, reportFile, SOURCE_DIRECTORY_FLAG, sourceDirectory, RULESET_FLAG, ruleset, - FORMAT_FLAG, FORMATTER, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString()); + FORMAT_FLAG, formatter, REPORTFILE_FLAG, reportFile.toAbsolutePath().toString()); } } @@ -129,4 +134,8 @@ public class PMDExecutor { return runPMDUnix(tempDir, null, arguments); } } + + public static boolean isJava7Test() { + return StringUtils.equals(System.getenv("JAVA_HOME"), System.getProperty("java7.home")); + } } diff --git a/pom.xml b/pom.xml index 1846f3823b..265fad30dc 100644 --- a/pom.xml +++ b/pom.xml @@ -356,6 +356,11 @@ maven-enforcer-plugin 3.0.0-M2 + + org.apache.maven.plugins + maven-failsafe-plugin + 2.19.1 + org.apache.maven.plugins maven-pmd-plugin @@ -652,7 +657,7 @@ com.beust jcommander - 1.72 + 1.48 org.ow2.asm @@ -699,7 +704,7 @@ org.apache.commons commons-text - 1.6 + 1.3 org.slf4j @@ -1064,10 +1069,6 @@ pmd-plsql pmd-python pmd-ruby - pmd-scala - pmd-scala-modules/pmd-scala-common - pmd-scala-modules/pmd-scala_2.13 - pmd-scala-modules/pmd-scala_2.12 pmd-swift pmd-test pmd-visualforce @@ -1080,5 +1081,9 @@ pmd-java8 pmd-doc pmd-lang-test + pmd-scala + pmd-scala-modules/pmd-scala-common + pmd-scala-modules/pmd-scala_2.13 + pmd-scala-modules/pmd-scala_2.12 From 60244845390faa90df8a2ac67cadf0920e9e95cf Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 1 Aug 2020 18:17:43 +0200 Subject: [PATCH 2/3] [doc] Update release notes, refs #2690 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 088a843d37..68461f9b0d 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -25,6 +25,7 @@ This is a {{ site.pmd.release_type }} release. * core * [#724](https://github.com/pmd/pmd/issues/724): \[core] Avoid parsing rulesets multiple times * [#2653](https://github.com/pmd/pmd/issues/2653): \[lang-test] Upgrade kotlintest to Kotest + * [#2690](https://github.com/pmd/pmd/pull/2690): \[core] Fix java7 compatibility * java-bestpractices * [#2471](https://github.com/pmd/pmd/issues/2471): \[java] New Rule: AvoidReassigningCatchVariables * [#2668](https://github.com/pmd/pmd/issues/2668): \[java] UnusedAssignment false positives From 54637c60ecfc6e8897a1e78165930b448ecf834f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sun, 2 Aug 2020 11:53:53 +0200 Subject: [PATCH 3/3] [core] Remove dependency to commons text, it is actually not needed Escaping in CDATA section is not necessary. --- pmd-core/pom.xml | 4 ---- .../main/java/net/sourceforge/pmd/cpd/XMLRenderer.java | 6 ++++-- .../main/java/net/sourceforge/pmd/util/StringUtil.java | 5 +++-- .../java/net/sourceforge/pmd/cpd/XMLRendererTest.java | 9 ++++++--- .../sourceforge/pmd/cpd/CPDCommandLineInterfaceTest.java | 2 +- 5 files changed, 14 insertions(+), 12 deletions(-) diff --git a/pmd-core/pom.xml b/pmd-core/pom.xml index 2cbb7c9407..b98b8c7c73 100644 --- a/pmd-core/pom.xml +++ b/pmd-core/pom.xml @@ -137,10 +137,6 @@ org.apache.commons commons-lang3 - - org.apache.commons - commons-text - org.ow2.asm asm diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/XMLRenderer.java b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/XMLRenderer.java index 5d8ee511d7..23bece7739 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cpd/XMLRenderer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cpd/XMLRenderer.java @@ -18,7 +18,6 @@ import javax.xml.transform.TransformerFactory; import javax.xml.transform.dom.DOMSource; import javax.xml.transform.stream.StreamResult; -import org.apache.commons.text.StringEscapeUtils; import org.w3c.dom.Document; import org.w3c.dom.Element; @@ -145,7 +144,10 @@ public final class XMLRenderer implements Renderer, CPDRenderer { // the code snippet has normalized line endings String platformSpecific = codeSnippet.replace("\n", System.lineSeparator()); Element codefragment = doc.createElement("codefragment"); - codefragment.appendChild(doc.createCDATASection(StringEscapeUtils.escapeXml10(platformSpecific))); + // only remove invalid characters, escaping is not necessary in CDATA. + // if the string contains the end marker of a CDATA section, then the DOM impl will + // create two cdata sections automatically. + codefragment.appendChild(doc.createCDATASection(StringUtil.removedInvalidXml10Characters(platformSpecific))); duplication.appendChild(codefragment); } return duplication; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java index c4f5fa1422..2cb2ddab43 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java @@ -13,7 +13,6 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import org.apache.commons.lang3.StringUtils; -import org.apache.commons.text.StringEscapeUtils; import net.sourceforge.pmd.annotation.InternalApi; @@ -220,7 +219,9 @@ public final class StringUtil { * @param src * @param supportUTF8 override the default setting, whether special characters should be replaced with entities ( * false) or should be included as is ( true). - * @deprecated for removal. Use {@link StringEscapeUtils#escapeXml10(String)} instead. + * @deprecated for removal. Use Java's XML implementations, that do the escaping, + * use {@link #removedInvalidXml10Characters(String)} for fixing invalid characters in XML 1.0 + * documents or use {@code StringEscapeUtils#escapeXml10(String)} from apache commons-text instead. */ @Deprecated public static void appendXmlEscaped(StringBuilder buf, String src, boolean supportUTF8) { diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java index d5340d2c67..406c8183aa 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cpd/XMLRendererTest.java @@ -204,11 +204,11 @@ public class XMLRendererTest { @Test public void testRendererXMLEscaping() throws IOException { - String codefragment = "code fragment" + FORM_FEED + "\nline2\nline3"; + String codefragment = "code fragment" + FORM_FEED + "\nline2\nline3\nno & escaping necessary in CDATA\nx=\"]]>\";"; CPDRenderer renderer = new XMLRenderer(); List list = new ArrayList<>(); - Mark mark1 = createMark("public", "file1", 1, 2, codefragment); - Mark mark2 = createMark("public", "file2", 5, 2, codefragment); + Mark mark1 = createMark("public", "file1", 1, 5, codefragment); + Mark mark2 = createMark("public", "file2", 5, 5, codefragment); Match match1 = new Match(75, mark1, mark2); list.add(match1); @@ -217,6 +217,9 @@ public class XMLRendererTest { String report = sw.toString(); assertFalse(report.contains(FORM_FEED)); assertFalse(report.contains(FORM_FEED_ENTITY)); + assertTrue(report.contains("no & escaping necessary in CDATA")); + assertFalse(report.contains("x=\"]]>\";")); // must be escaped + assertTrue(report.contains("x=\"]]]]>\";")); } private Mark createMark(String image, String tokenSrcID, int beginLine, int lineCount, String code) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/cpd/CPDCommandLineInterfaceTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/cpd/CPDCommandLineInterfaceTest.java index dad635c5d5..6150151d61 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/cpd/CPDCommandLineInterfaceTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/cpd/CPDCommandLineInterfaceTest.java @@ -77,7 +77,7 @@ public class CPDCommandLineInterfaceTest extends BaseCPDCLITest { String out = getOutput(); Assert.assertTrue(out.startsWith("")); - Assert.assertTrue(Pattern.compile("System\\.out\\.println\\([ij] \\+ "ä"\\);").matcher(out).find()); + Assert.assertTrue(Pattern.compile("System\\.out\\.println\\([ij] \\+ \"ä\"\\);").matcher(out).find()); Assert.assertEquals(4, Integer.parseInt(System.getProperty(CPDCommandLineInterface.STATUS_CODE_PROPERTY))); }