From b85d71f632dbcd14d4824ea6e0c60cb41800f022 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 27 Aug 2019 11:00:33 -0300 Subject: [PATCH 1/3] [core] Resolve cached rule instances by more than class name - Fixes #1990 --- .../pmd/cache/CachedRuleMapper.java | 19 ++++++++++++++----- .../pmd/cache/CachedRuleViolation.java | 19 ++++++++++++++----- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleMapper.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleMapper.java index b29bb1a80c..6a647f3d73 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleMapper.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleMapper.java @@ -9,21 +9,26 @@ import java.util.Map; import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.RuleSets; +import net.sourceforge.pmd.annotation.InternalApi; /** * A mapper from rule class names to rule instances for cached rules. */ +@Deprecated +@InternalApi public class CachedRuleMapper { - private final Map ruleByClassName = new HashMap<>(); + private final Map cachedRulesInstances = new HashMap<>(); /** - * Finds a rule instance for the given rule class name + * Finds a rule instance for the given rule class name, name and target language * @param className The name of the rule class that generated the cache entry + * @param ruleName The name of the rule that generated the cache entry + * @param languageName The terse name of the language for which the rule applies * @return The requested rule */ - public Rule getRuleForClass(final String className) { - return ruleByClassName.get(className); + public Rule getRuleForClass(final String className, final String ruleName, final String languageName) { + return cachedRulesInstances.get(getRuleKey(className, ruleName, languageName)); } /** @@ -32,7 +37,11 @@ public class CachedRuleMapper { */ public void initialize(final RuleSets rs) { for (final Rule r : rs.getAllRules()) { - ruleByClassName.put(r.getRuleClass(), r); + cachedRulesInstances.put(getRuleKey(r.getRuleClass(), r.getName(), r.getLanguage().getTerseName()), r); } } + + private String getRuleKey(final String className, final String ruleName, final String languageName) { + return className + "$$" + ruleName + "$$" + languageName; + } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleViolation.java b/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleViolation.java index e366eef46a..cff85a5b8e 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleViolation.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cache/CachedRuleViolation.java @@ -21,6 +21,8 @@ public final class CachedRuleViolation implements RuleViolation { private final String description; private final String fileName; private final String ruleClassName; + private final String ruleName; + private final String ruleTargetLanguage; private final int beginLine; private final int beginColumn; private final int endLine; @@ -31,13 +33,16 @@ public final class CachedRuleViolation implements RuleViolation { private final String variableName; private CachedRuleViolation(final CachedRuleMapper mapper, final String description, - final String fileName, final String ruleClassName, final int beginLine, - final int beginColumn, final int endLine, final int endColumn, final String packageName, + final String fileName, final String ruleClassName, final String ruleName, + final String ruleTargetLanguage, final int beginLine, final int beginColumn, + final int endLine, final int endColumn, final String packageName, final String className, final String methodName, final String variableName) { this.mapper = mapper; this.description = description; this.fileName = fileName; this.ruleClassName = ruleClassName; + this.ruleName = ruleName; + this.ruleTargetLanguage = ruleTargetLanguage; this.beginLine = beginLine; this.beginColumn = beginColumn; this.endLine = endLine; @@ -51,7 +56,7 @@ public final class CachedRuleViolation implements RuleViolation { @Override public Rule getRule() { // The mapper may be initialized after cache is loaded, so use it lazily - return mapper.getRuleForClass(ruleClassName); + return mapper.getRuleForClass(ruleClassName, ruleName, ruleTargetLanguage); } @Override @@ -122,6 +127,8 @@ public final class CachedRuleViolation implements RuleViolation { final String fileName, final CachedRuleMapper mapper) throws IOException { final String description = stream.readUTF(); final String ruleClassName = stream.readUTF(); + final String ruleName = stream.readUTF(); + final String ruleTargetLanguage = stream.readUTF(); final int beginLine = stream.readInt(); final int beginColumn = stream.readInt(); final int endLine = stream.readInt(); @@ -131,8 +138,8 @@ public final class CachedRuleViolation implements RuleViolation { final String methodName = stream.readUTF(); final String variableName = stream.readUTF(); - return new CachedRuleViolation(mapper, description, fileName, ruleClassName, beginLine, beginColumn, - endLine, endColumn, packageName, className, methodName, variableName); + return new CachedRuleViolation(mapper, description, fileName, ruleClassName, ruleName, ruleTargetLanguage, + beginLine, beginColumn, endLine, endColumn, packageName, className, methodName, variableName); } /** @@ -147,6 +154,8 @@ public final class CachedRuleViolation implements RuleViolation { final RuleViolation violation) throws IOException { stream.writeUTF(getValueOrEmpty(violation.getDescription())); stream.writeUTF(getValueOrEmpty(violation.getRule().getRuleClass())); + stream.writeUTF(getValueOrEmpty(violation.getRule().getName())); + stream.writeUTF(getValueOrEmpty(violation.getRule().getLanguage().getTerseName())); stream.writeInt(violation.getBeginLine()); stream.writeInt(violation.getBeginColumn()); stream.writeInt(violation.getEndLine()); From bf2ad56799a7e48162cadb2aa13511586f799931 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Tue, 27 Aug 2019 11:03:37 -0300 Subject: [PATCH 2/3] Update changelog, refs #1990 --- 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 8f48750d25..5cd8ce4193 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -40,6 +40,7 @@ about the usage and features of the rule designer. * all * [#1983](https://github.com/pmd/pmd/pull/1983): \[core] Avoid crashes with analysis cache when classpath references non-existing directories + * [#1990](https://github.com/pmd/pmd/pull/1990): \[core] Incremental analysis mixes XPath rule violations * java-bestpractices * [#1862](https://github.com/pmd/pmd/issues/1862): \[java] New rule for MessageDigest.getInstance * java-codestyle From 29921f03e5002ee328e2761d008991f86ca8afab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Mart=C3=ADn=20Sotuyo=20Dodero?= Date: Thu, 29 Aug 2019 12:53:22 -0300 Subject: [PATCH 3/3] Fix breaking tests --- .../java/net/sourceforge/pmd/cache/FileAnalysisCacheTest.java | 4 ++++ 1 file changed, 4 insertions(+) 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 7583ca6297..80e7095403 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 @@ -32,6 +32,7 @@ import org.mockito.Mockito; import net.sourceforge.pmd.RuleSets; import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.lang.Language; public class FileAnalysisCacheTest { @@ -100,6 +101,7 @@ public class FileAnalysisCacheTest { final RuleViolation rv = mock(RuleViolation.class); when(rv.getFilename()).thenReturn(sourceFile.getPath()); final net.sourceforge.pmd.Rule rule = mock(net.sourceforge.pmd.Rule.class, Mockito.RETURNS_SMART_NULLS); + when(rule.getLanguage()).thenReturn(mock(Language.class)); when(rv.getRule()).thenReturn(rule); cache.ruleViolationAdded(rv); @@ -187,6 +189,7 @@ public class FileAnalysisCacheTest { final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); when(r.isDfa()).thenReturn(true); + when(r.getLanguage()).thenReturn(mock(Language.class)); when(rs.getAllRules()).thenReturn(Collections.singleton(r)); reloadedCache.checkValidity(rs, cl); assertFalse("Cache believes unmodified file is up to date after auxclasspath changed", @@ -203,6 +206,7 @@ public class FileAnalysisCacheTest { final net.sourceforge.pmd.Rule r = mock(net.sourceforge.pmd.Rule.class); when(r.isDfa()).thenReturn(true); + when(r.getLanguage()).thenReturn(mock(Language.class)); when(rs.getAllRules()).thenReturn(Collections.singleton(r)); setupCacheWithFiles(newCacheFile, rs, cl, sourceFile);