diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8dafa2df93..0a764950e6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,12 +17,13 @@ This is a major release. * [Java Type Resolution](#java-type-resolution) * [Metrics Framework](#metrics-framework) * [Error Reporting](#error-reporting) - * [Java Symbol Table](#java-symbol-table) - * [Apex Parser Update](#apex-parser-update) * [New Rules](#new-rules) * [Modified Rules](#modified-rules) * [Deprecated Rules](#deprecated-rules) * [Removed Rules](#removed-rules) + * [Java Symbol Table](#java-symbol-table) + * [Apex Parser Update](#apex-parser-update) + * [Incremental Analysis](#incremental-analysis) * [Fixed Issues](#fixed-issues) * [API Changes](#api-changes) * [External Contributions](#external-contributions) @@ -155,6 +156,21 @@ of the latest improvements from Salesforce, but introduces some breaking changes All existing rules have been updated to reflect these changes. If you have custom rules, be sure to update them. +#### Incremental Analysis + +The incremental analysis feature first introduced in PMD 5.6.0 has been enhanced. A few minor issues have been fixed, +and several improvements have been performed to make it more accurate. + +The cache will now detect changes to the JARs referenced in the `auxclasspath` instead of simply looking at their paths +and order. This means that if you are referencing a JAR you are overwriting in some way, the incremental analysis can +now detect it and invalidate it's cache to avoid false reports. + +We have also improved logging on the analysis code, allowing better insight into how the cache is performing, +under debug / verbose builds you can even see individual hits / misses to the cache (and the reason for any miss!) + +Finally, as this feature keeps maturing, we are gently pushing this forward. If not using incremental analysis, +a warning will now be produced suggesting users to adopt it for better performance. + ### Fixed Issues * all diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java index 537d27b5c5..fbe2196aab 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/AbstractAnalysisCache.java @@ -5,14 +5,19 @@ package net.sourceforge.pmd.cache; import java.io.File; +import java.io.IOException; +import java.net.URL; import java.net.URLClassLoader; -import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.logging.Level; import java.util.logging.Logger; +import java.util.zip.Adler32; +import java.util.zip.CheckedInputStream; + +import org.apache.commons.io.IOUtils; import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.Rule; @@ -96,7 +101,7 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { final long classLoaderChecksum; if (classLoader instanceof URLClassLoader) { final URLClassLoader urlClassLoader = (URLClassLoader) classLoader; - classLoaderChecksum = Arrays.hashCode(urlClassLoader.getURLs()); + classLoaderChecksum = computeClassLoaderHash(urlClassLoader); if (cacheIsValid && classLoaderChecksum != classpathChecksum) { // Do we even care? @@ -123,6 +128,23 @@ public abstract class AbstractAnalysisCache implements AnalysisCache { ruleMapper.initialize(ruleSets); } + private long computeClassLoaderHash(final URLClassLoader classLoader) { + final Adler32 adler32 = new Adler32(); + for (final URL url : classLoader.getURLs()) { + try (CheckedInputStream inputStream = new CheckedInputStream(url.openStream(), adler32)) { + // Just read it, the CheckedInputStream will update the checksum on it's own + while (IOUtils.skip(inputStream, Long.MAX_VALUE) == Long.MAX_VALUE) { + // just loop + } + } catch (final IOException e) { + // Can this even happen? + LOG.log(Level.SEVERE, "Incremental analysis can't check auxclasspath contents", e); + throw new RuntimeException(e); + } + } + return adler32.getValue(); + } + @Override public void ruleViolationAdded(final RuleViolation ruleViolation) { final AnalysisResult analysisResult = updatedResultsCache.get(ruleViolation.getFilename()); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java index 973d227663..8c21fd2106 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java @@ -110,7 +110,7 @@ public class FileAnalysisCacheTest { @Test public void testCacheValidityWithNoChanges() { final RuleSets rs = mock(RuleSets.class); - final URLClassLoader cl = mock(URLClassLoader.class); + final ClassLoader cl = mock(ClassLoader.class); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -123,7 +123,7 @@ public class FileAnalysisCacheTest { @Test public void testRulesetChangeInvalidatesCache() { final RuleSets rs = mock(RuleSets.class); - final URLClassLoader cl = mock(URLClassLoader.class); + final ClassLoader cl = mock(ClassLoader.class); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -138,6 +138,7 @@ public class FileAnalysisCacheTest { public void testClasspathChangeWithoutDFAorTypeResolutionDoesNotInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); + when(cl.getURLs()).thenReturn(new URL[] { }); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); @@ -152,11 +153,17 @@ public class FileAnalysisCacheTest { public void testClasspathChangeInvalidatesCache() throws MalformedURLException, IOException { final RuleSets rs = mock(RuleSets.class); final URLClassLoader cl = mock(URLClassLoader.class); + when(cl.getURLs()).thenReturn(new URL[] { }); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); - when(cl.getURLs()).thenReturn(new URL[] { tempFolder.newFile().toURI().toURL(), }); + final File classpathFile = tempFolder.newFile(); + when(cl.getURLs()).thenReturn(new URL[] { classpathFile.toURI().toURL(), }); + + // Make sure the classpath file is not empty + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); when(r.usesDFA()).thenReturn(true); when(rs.getAllRules()).thenReturn(Collections.singleton(r)); @@ -164,6 +171,29 @@ public class FileAnalysisCacheTest { assertFalse("Cache believes unmodified file is up to date after classpath changed", reloadedCache.isUpToDate(sourceFile)); } + + @Test + public void testClasspathJarContentsChangeInvalidatesCache() throws MalformedURLException, IOException { + final RuleSets rs = mock(RuleSets.class); + final URLClassLoader cl = mock(URLClassLoader.class); + + final File classpathFile = tempFolder.newFile(); + when(cl.getURLs()).thenReturn(new URL[] { classpathFile.toURI().toURL(), }); + + final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); + when(r.usesDFA()).thenReturn(true); + when(rs.getAllRules()).thenReturn(Collections.singleton(r)); + + setupCacheWithFiles(newCacheFile, rs, cl, sourceFile); + + // Edit the classpath referenced file + Files.write(Paths.get(classpathFile.getAbsolutePath()), "some text".getBytes()); + + final FileAnalysisCache reloadedCache = new FileAnalysisCache(newCacheFile); + reloadedCache.checkValidity(rs, cl); + assertFalse("Cache believes cache is up to date when a classpath file changed", + reloadedCache.isUpToDate(sourceFile)); + } @Test public void testUnknownFileIsNotUpToDate() throws IOException {