diff --git a/README.md b/README.md index fa1852e804..f9e7e67a02 100644 --- a/README.md +++ b/README.md @@ -3,6 +3,7 @@ [![Join the chat at https://gitter.im/pmd/pmd](https://badges.gitter.im/pmd/pmd.svg)](https://gitter.im/pmd/pmd?utm_source=badge&utm_medium=badge&utm_campaign=pr-badge&utm_content=badge) [![Build Status](https://travis-ci.org/pmd/pmd.svg?branch=master)](https://travis-ci.org/pmd/pmd) [![Maven Central](https://maven-badges.herokuapp.com/maven-central/net.sourceforge.pmd/pmd/badge.svg)](https://maven-badges.herokuapp.com/maven-central/net.sourceforge.pmd/pmd) +[![Reproducible Builds](https://img.shields.io/badge/Reproducible_Builds-ok-green?labelColor=blue)](https://github.com/jvm-repo-rebuild/reproducible-central#net.sourceforge.pmd:pmd) [![Coverage Status](https://coveralls.io/repos/github/pmd/pmd/badge.svg)](https://coveralls.io/github/pmd/pmd) [![Codacy Badge](https://api.codacy.com/project/badge/Grade/a674ee8642ed44c6ba7633626ee95967)](https://www.codacy.com/app/pmd/pmd?utm_source=github.com&utm_medium=referral&utm_content=pmd/pmd&utm_campaign=Badge_Grade) [![Contributor Covenant](https://img.shields.io/badge/Contributor%20Covenant-v2.0%20adopted-ff69b4.svg)](code_of_conduct.md) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3c1423c57f..7d4e8ccf24 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -29,14 +29,19 @@ AbstractTokenizer and the custom tokenizers of Fortran, Perl and Ruby are deprec * [#2758](https://github.com/pmd/pmd/pull/2758): \[cpd] Improve AnyTokenizer * [#2760](https://github.com/pmd/pmd/issues/2760): \[cpd] AnyTokenizer doesn't count columns correctly -* pmd-java - * [#2708](https://github.com/pmd/pmd/issues/2708): \[java] False positive FinalFieldCouldBeStatic when using lombok Builder.Default +* apex-security + * [#2774](https://github.com/pmd/pmd/issues/2774): \[apex] ApexSharingViolations does not correlate sharing settings with class that contains data access + +* java * [#2738](https://github.com/pmd/pmd/issues/2738): \[java] Custom rule with @ExhaustiveEnumSwitch throws NPE * [#2755](https://github.com/pmd/pmd/issues/2755): \[java] \[6.27.0] Exception applying rule CloseResource on file ... java.lang.NullPointerException * [#2756](https://github.com/pmd/pmd/issues/2756): \[java] TypeTestUtil fails with NPE for anonymous class - * [#2759](https://github.com/pmd/pmd/issues/2759): \[java] False positive in UnusedAssignment * [#2767](https://github.com/pmd/pmd/issues/2767): \[java] IndexOutOfBoundsException when parsing an initializer BlockStatement * [#2783](https://github.com/pmd/pmd/issues/2783): \[java] Error while parsing with lambda of custom interface +* java-bestpractices + * [#2759](https://github.com/pmd/pmd/issues/2759): \[java] False positive in UnusedAssignment +* java-design + * [#2708](https://github.com/pmd/pmd/issues/2708): \[java] False positive FinalFieldCouldBeStatic when using lombok Builder.Default ### API Changes @@ -69,6 +74,8 @@ AbstractTokenizer and the custom tokenizers of Fortran, Perl and Ruby are deprec * [#2735](https://github.com/pmd/pmd/pull/2735): \[ci] Add github actions for a fast view of pr succeed/not - [XenoAmess](https://github.com/XenoAmess) * [#2747](https://github.com/pmd/pmd/pull/2747): \[java] Don't trigger FinalFieldCouldBeStatic when field is annotated with lombok @Builder.Default - [Ollie Abbey](https://github.com/ollieabbey) * [#2773](https://github.com/pmd/pmd/pull/2773): \[java] issue-2738: Adding null check to avoid npe when switch case is default - [Nimit Patel](https://github.com/nimit-patel) +* [#2789](https://github.com/pmd/pmd/pull/2789): Add badge for reproducible build - [Dan Rollo](https://github.com/bhamail) +* [#2791](https://github.com/pmd/pmd/pull/2791): \[apex] Analyze inner classes for sharing violations - [Jeff Bartolotta](https://github.com/jbartolotta-sfdc) {% endtocmaker %} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java index 2e7e69ef82..cf6e8323f8 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsRule.java @@ -4,11 +4,20 @@ package net.sourceforge.pmd.lang.apex.rule.security; -import java.util.List; +import java.util.Optional; import java.util.WeakHashMap; +import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlDeleteStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlInsertStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlMergeStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUndeleteStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpdateStatement; +import net.sourceforge.pmd.lang.apex.ast.ASTDmlUpsertStatement; import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.ast.ASTModifierNode; +import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTSoslExpression; import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; import net.sourceforge.pmd.lang.apex.ast.ApexNode; import net.sourceforge.pmd.lang.apex.rule.AbstractApexRule; @@ -21,45 +30,103 @@ import net.sourceforge.pmd.lang.apex.rule.internal.Helper; */ public class ApexSharingViolationsRule extends AbstractApexRule { + /** + * Keep track of previously reported violations to avoid duplicates. + */ private WeakHashMap, Object> localCacheOfReportedNodes = new WeakHashMap<>(); public ApexSharingViolationsRule() { setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); + addRuleChainVisit(ASTDmlDeleteStatement.class); + addRuleChainVisit(ASTDmlInsertStatement.class); + addRuleChainVisit(ASTDmlMergeStatement.class); + addRuleChainVisit(ASTDmlUndeleteStatement.class); + addRuleChainVisit(ASTDmlUpdateStatement.class); + addRuleChainVisit(ASTDmlUpsertStatement.class); + addRuleChainVisit(ASTMethodCallExpression.class); + addRuleChainVisit(ASTSoqlExpression.class); + addRuleChainVisit(ASTSoslExpression.class); } @Override - public Object visit(ASTUserClass node, Object data) { - - if (Helper.isTestMethodOrClass(node) || Helper.isSystemLevelClass(node)) { - return data; // stops all the rules - } - - if (!Helper.isTestMethodOrClass(node)) { - boolean sharingFound = isSharingPresent(node); - checkForSharingDeclaration(node, data, sharingFound); - checkForDatabaseMethods(node, data, sharingFound); - } - + public void start(RuleContext ctx) { + super.start(ctx); localCacheOfReportedNodes.clear(); + } + + @Override + public Object visit(ASTSoqlExpression node, Object data) { + checkForViolation(node, data); return data; } - /** - * Check if class contains any Database.query / Database.insert [ Database.* - * ] methods - * - * @param node - * @param data - */ - private void checkForDatabaseMethods(ASTUserClass node, Object data, boolean sharingFound) { - List calls = node.findDescendantsOfType(ASTMethodCallExpression.class); - for (ASTMethodCallExpression call : calls) { - if (Helper.isMethodName(call, "Database", Helper.ANY_METHOD)) { - if (!sharingFound) { - reportViolation(node, data); - } + @Override + public Object visit(ASTSoslExpression node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlUpsertStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlUpdateStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlUndeleteStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlMergeStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlInsertStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTDmlDeleteStatement node, Object data) { + checkForViolation(node, data); + return data; + } + + @Override + public Object visit(ASTMethodCallExpression node, Object data) { + if (Helper.isMethodName(node, "Database", Helper.ANY_METHOD)) { + checkForViolation(node, data); + } + + return data; + } + + private void checkForViolation(ApexNode node, Object data) { + // The closest ASTUserClass class in the tree hierarchy is the node that requires the sharing declaration + ASTUserClass sharingDeclarationClass = node.getFirstParentOfType(ASTUserClass.class); + + // This is null in the case of triggers + if (sharingDeclarationClass != null) { + // Apex allows a single level of class nesting. Check to see if sharingDeclarationClass has an outer class + ASTUserClass outerClass = sharingDeclarationClass.getFirstParentOfType(ASTUserClass.class); + // The test annotation needs to be on the outermost class + ASTUserClass testAnnotationClass = Optional.ofNullable(outerClass).orElse(sharingDeclarationClass); + + if (!Helper.isTestMethodOrClass(testAnnotationClass) && !Helper.isSystemLevelClass(sharingDeclarationClass) && !isSharingPresent(sharingDeclarationClass)) { + // The violation is reported on the class, not the node that performs data access + reportViolation(sharingDeclarationClass, data); } } } @@ -77,19 +144,6 @@ public class ApexSharingViolationsRule extends AbstractApexRule { } } - /** - * Check if class has no sharing declared - * - * @param node - * @param data - */ - private void checkForSharingDeclaration(ApexNode node, Object data, boolean sharingFound) { - final boolean foundAnyDMLorSOQL = Helper.foundAnyDML(node) || Helper.foundAnySOQLorSOSL(node); - if (!sharingFound && !Helper.isTestMethodOrClass(node) && foundAnyDMLorSOQL) { - reportViolation(node, data); - } - } - /** * Does class have sharing keyword declared? * diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsNestedClassTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsNestedClassTest.java new file mode 100644 index 0000000000..64ac540a72 --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSharingViolationsNestedClassTest.java @@ -0,0 +1,194 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule.security; + +import static org.junit.Assert.assertEquals; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.stream.Collectors; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.Rule; +import net.sourceforge.pmd.RuleViolation; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.apex.ApexLanguageModule; +import net.sourceforge.pmd.testframework.RuleTst; + +/** + *

Sharing settings are not inherited by inner classes. Sharing settings need to be declared on the class that + * contains the Database method, DML, SOQL, or SOSL.

+ * + *

This test runs against Apex code that has an Outer class and and Inner class. Different Apex code is generated + * based on the boolean permutations. Any classes that includes data access cod, but doesn't declare a sharing setting + * should trigger a violation.

+ */ +@RunWith(Parameterized.class) +public class ApexSharingViolationsNestedClassTest extends RuleTst { + /** + * Type of operation that may require a sharing declaration. + */ + private enum Operation { + NONE(null), + DML_DELETE("Contact c = new Contact(); delete c;"), + DML_INSERT("Contact c = new Contact(); insert c;"), + DML_MERGE("Contact c1 = new Contact(); Contact c2 = new Contact(); merge c1 c2;"), + DML_UNDELETE("Contact c = new Contact(); undelete c;"), + DML_UPDATE("Contact c = new Contact(); update c;"), + DML_UPSERT("Contact c = new Contact(); upsert c;"), + METHOD_DATABASE("Database.query('Select Id from Contact LIMIT 100');"), + SOQL("[SELECT Name FROM Contact];"), + SOSL("[FIND 'Foo' IN ALL FIELDS RETURNING Account(Name)];"); + + final boolean requiresSharingDeclaration; + final String codeSnippet; + + Operation(String codeSnippet) { + this.requiresSharingDeclaration = codeSnippet != null; + this.codeSnippet = codeSnippet; + } + } + + /** + * The permutations used for class generation. See {@link #generateClass(boolean, Operation, boolean, Operation)} + */ + private final boolean outerSharingDeclared; + private final Operation outerOperation; + private final boolean innerSharingDeclared; + private final Operation innerOperation; + + /** + * Track the expected violations based on the permutations. + */ + private final int expectedViolations; + private final List expectedLineNumbers; + + public ApexSharingViolationsNestedClassTest(boolean outerSharingDeclared, Operation outerOperation, + boolean innerSharingDeclared, Operation innerOperation, + int expectedViolations, List expectedLineNumbers) { + this.outerSharingDeclared = outerSharingDeclared; + this.outerOperation = outerOperation; + this.innerSharingDeclared = innerSharingDeclared; + this.innerOperation = innerOperation; + this.expectedViolations = expectedViolations; + this.expectedLineNumbers = expectedLineNumbers; + } + + @Test + public void testSharingPermutation() { + String apexClass = generateClass(outerSharingDeclared, outerOperation, innerSharingDeclared, innerOperation); + Report rpt = new Report(); + runTestFromString(apexClass, new ApexSharingViolationsRule(), rpt, + LanguageRegistry.getLanguage(ApexLanguageModule.NAME).getDefaultVersion()); + List violations = rpt.getViolations(); + assertEquals("Unexpected Violation Size\n" + apexClass, expectedViolations, violations.size()); + List lineNumbers = violations.stream().map(v -> v.getBeginLine()).collect(Collectors.toList()); + assertEquals("Unexpected Line Numbers\n" + apexClass, expectedLineNumbers, lineNumbers); + } + + @Override + protected List getRules() { + Rule rule = findRule("category/apex/security.xml", "ApexSharingViolations"); + return Collections.singletonList(rule); + } + + /** + * Parameter provider that covers are all permutations + */ + @Parameterized.Parameters + public static Collection data() { + List data = new ArrayList<>(); + + boolean[] boolPermutations = {false, true}; + + for (boolean outerSharingDeclared : boolPermutations) { + for (Operation outerOperation : Operation.values()) { + for (boolean innerSharingDeclared : boolPermutations) { + for (Operation innerOperation : Operation.values()) { + int expectedViolations = 0; + List expectedLineNumbers = new ArrayList<>(); + if (outerOperation.requiresSharingDeclaration && !outerSharingDeclared) { + // The outer class contains SOQL but doesn't declare sharing + expectedViolations++; + expectedLineNumbers.add(1); + } + + if (innerOperation.requiresSharingDeclaration && !innerSharingDeclared) { + // The inner class contains SOQL but doesn't declare sharing + expectedViolations++; + // The location of the inner class declaration depends upon the content of the outer class + expectedLineNumbers.add(outerOperation.requiresSharingDeclaration ? 3 : 2); + } + data.add(new Object[]{outerSharingDeclared, outerOperation, innerSharingDeclared, innerOperation, + expectedViolations, expectedLineNumbers}); + } + } + } + } + + return data; + } + + /** + *

Generate an Apex class with various Sharing/Database/DML/SOQL/SOSL permutations. An example of the class + * returned when invoked with generateClass(true, SOQL, true, SOQL).

+ * + * 
+     * public with sharing class Outer {
+     *    public void outerSOQL() {[SELECT Name FROM Contact];}
+     *    public with sharing class Inner {
+     *       public void innerSOQL() {[SELECT Name FROM Contact];}
+     *    }
+     * }
+     *
+ * + * @param outerSharing Add 'with sharing' to Outer class definition + * @param outerOperation Add a method to Outer class that performs the given operation + * @param innerSharing Add 'with sharing' to Inner class definition + * @param innerOperation Add a method to Inner class that performs the given operation + * @return String that represents Apex code + */ + private static String generateClass(boolean outerSharing, Operation outerOperation, boolean innerSharing, + Operation innerOperation) { + StringBuilder sb = new StringBuilder(); + + sb.append("public "); + if (outerSharing) { + sb.append("with sharing "); + } + sb.append("class Outer {\n"); + switch (outerOperation) { + case NONE: + // Do nothing + break; + default: + sb.append(String.format("\t\tpublic void outer%s(){ %s }\n", outerOperation.name(), outerOperation.codeSnippet)); + break; + } + sb.append("\tpublic "); + if (innerSharing) { + sb.append("with sharing "); + } + sb.append("class Inner {\n"); + switch (innerOperation) { + case NONE: + // DO Nothing + break; + default: + sb.append(String.format("\t\tpublic void inner%s(){ %s }\n", innerOperation.name(), innerOperation.codeSnippet)); + break; + } + sb.append("\t}\n"); // Closes class Inner + sb.append("}\n"); // Closes class Outer + + return sb.toString(); + } +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSharingViolations.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSharingViolations.xml index 5ff9aebef9..7cd3d83052 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSharingViolations.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSharingViolations.xml @@ -99,6 +99,87 @@ public inherited sharing class MyClass { public List getAllTheSecrets() { return [SELECT Name FROM Contact]; } +} + ]]> + + + + Apex test classes do not need a sharing declaration + 0 + getAllTheSecrets() { + return [SELECT Name FROM Contact]; + } +} + ]]> + + + + + Apex inner test classes do not need a sharing declaration + 0 + getAllTheOuterSecrets() { + return [SELECT Name FROM Contact]; + } + public class Inner { + public List getAllTheInnerSecrets() { + return [SELECT Name FROM Contact]; + } + } +} + ]]> + + + + Nested method calls are detected + 1 + > recordsMap = new Map>(); + + public List getRecords(String query) { + if (!recordsMap.containsKey(query)) { + recordsMap.put(query, Database.query(query)); + } + return recordsMap.get(query); + } +} + ]]> + + + + Trigger does not require sharing + 0 + + + + + Issue #2774(false positive). Sharing correctly declared on inner class. + 0 + getAllInnerSoqlSecrets() { return [SELECT Name FROM Contact]; } + } +} + ]]> + + + + Issue #2774(false negative). Sharing incorrectly declared on outer class. + 1 + getAllInnerSoqlSecrets() { return [SELECT Name FROM Contact]; } + } } ]]> diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java index a2d187c631..b5a8e559bf 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java @@ -4,10 +4,9 @@ package net.sourceforge.pmd.lang.java.types; +import java.lang.reflect.Modifier; import java.util.List; -import org.objectweb.asm.Opcodes; - import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; @@ -42,10 +41,10 @@ public final class TypeTestUtil { * if the type of the node is parameterized. Examples: * * 
{@code
-     * isA(()>, List.class)      = true
-     * isA(()>, ArrayList.class) = true
-     * isA(, int[].class)                  = true
-     * isA(, Object[].class)            = true
+     * isA(List.class, ()>)      = true
+     * isA(ArrayList.class, ()>) = true
+     * isA(int[].class, )                  = true
+     * isA(Object[].class, )            = true
      * isA(_, null) = false
      * isA(null, _) = NullPointerException
      * }
@@ -65,8 +64,11 @@ public final class TypeTestUtil { return true; } - return canBeExtended(clazz) ? isA(clazz.getName(), node) - : isExactlyA(clazz, node); + if (hasNoSubtypes(clazz)) { + return isExactlyA(clazz, node); + } + String canoName = clazz.getCanonicalName(); + return canoName != null && isA(canoName, node); } @@ -76,10 +78,10 @@ public final class TypeTestUtil { * if the type of the node is parameterized. Examples: * * 
{@code
-     * isA(()>, "java.util.List")      = true
-     * isA(()>, "java.util.ArrayList") = true
-     * isA(, "int[]")                            = true
-     * isA(, "java.lang.Object[]")            = true
+     * isA("java.util.List", ()>)      = true
+     * isA("java.util.ArrayList", ()>) = true
+     * isA("int[]", )                            = true
+     * isA("java.lang.Object[]", )            = true
      * isA(_, null) = false
      * isA(null, _) = NullPointerException
      * }
@@ -156,10 +158,10 @@ public final class TypeTestUtil { * if the type of the node is parameterized. * * 
{@code
-     * isExactlyA(()>, List.class)      = false
-     * isExactlyA(()>, ArrayList.class) = true
-     * isExactlyA(, int[].class)                  = true
-     * isExactlyA(, Object[].class)            = false
+     * isExactlyA(List.class, ()>)      = false
+     * isExactlyA(ArrayList.class, ()>) = true
+     * isExactlyA(int[].class, )                  = true
+     * isExactlyA(Object[].class, )            = false
      * isExactlyA(_, null) = false
      * isExactlyA(null, _) = NullPointerException
      * }
@@ -198,9 +200,12 @@ public final class TypeTestUtil { } } - private static boolean canBeExtended(Class clazz) { + + private static boolean hasNoSubtypes(Class clazz) { // Neither final nor an annotation. Enums & records have ACC_FINAL - return (clazz.getModifiers() & (Opcodes.ACC_ANNOTATION | Opcodes.ACC_FINAL)) == 0; + // Note: arrays have ACC_FINAL, but have subtypes by covariance + // Note: annotations may be implemented by classes + return Modifier.isFinal(clazz.getModifiers()) && !clazz.isArray(); } // those fallbacks can be removed with the newer symbol resolution, diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypeTestUtilTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypeTestUtilTest.java index d672afc702..b1c7c660c0 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypeTestUtilTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/types/TypeTestUtilTest.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.types; +import java.io.ObjectStreamField; import java.io.Serializable; import java.lang.annotation.Annotation; import java.util.concurrent.Callable; @@ -20,6 +21,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMarkerAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTType; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.symboltable.BaseNonParserTest; import net.sourceforge.pmd.lang.java.types.testdata.SomeClassWithAnon; @@ -56,12 +58,60 @@ public class TypeTestUtilTest extends BaseNonParserTest { Assert.assertNull(klass.getType()); Assert.assertTrue(TypeTestUtil.isA("org.FooBar", klass)); - assertIsA(klass, Iterable.class); - assertIsA(klass, Enum.class); - assertIsA(klass, Serializable.class); - assertIsA(klass, Object.class); + assertIsStrictSubtype(klass, Iterable.class); + assertIsStrictSubtype(klass, Enum.class); + assertIsStrictSubtype(klass, Serializable.class); + assertIsStrictSubtype(klass, Object.class); } + + @Test + public void testIsAnArrayClass() { + + ASTType arrayT = + java.parse("import java.io.ObjectStreamField; " + + "class Foo { private static final ObjectStreamField[] serialPersistentFields; }") + .getFirstDescendantOfType(ASTType.class); + + + assertIsExactlyA(arrayT, ObjectStreamField[].class); + assertIsStrictSubtype(arrayT, Object[].class); + assertIsStrictSubtype(arrayT, Serializable.class); + assertIsNot(arrayT, Serializable[].class); + assertIsStrictSubtype(arrayT, Object.class); + } + + @Test + public void testIsAnAnnotationClass() { + + ASTType arrayT = + java.parse("class Foo { org.junit.Test field; }") + .getFirstDescendantOfType(ASTType.class); + + + assertIsExactlyA(arrayT, Test.class); + assertIsStrictSubtype(arrayT, Annotation.class); + assertIsStrictSubtype(arrayT, Object.class); + } + + @Test + public void testIsAPrimitiveArrayClass() { + + ASTType arrayT = + java.parse("import java.io.ObjectStreamField; " + + "class Foo { private static final int[] serialPersistentFields; }") + .getFirstDescendantOfType(ASTType.class); + + + assertIsExactlyA(arrayT, int[].class); + assertIsNot(arrayT, long[].class); + assertIsNot(arrayT, Object[].class); + + assertIsStrictSubtype(arrayT, Serializable.class); + assertIsStrictSubtype(arrayT, Object.class); + } + + @Test public void testIsAFallbackAnnotation() { @@ -161,10 +211,38 @@ public class TypeTestUtilTest extends BaseNonParserTest { } private void assertIsA(TypeNode node, Class type) { - Assert.assertTrue("TypeTestUtil::isA with class arg: " + type.getCanonicalName(), - TypeTestUtil.isA(type, node)); - Assert.assertTrue("TypeTestUtil::isA with string arg: " + type.getCanonicalName(), - TypeTestUtil.isA(type.getCanonicalName(), node)); + assertIsA(node, type, false, true); } + private void assertIsExactlyA(TypeNode node, Class type) { + assertIsA(node, type, true, true); + assertIsA(node, type, false, true); + } + + private void assertIsNot(TypeNode node, Class type) { + assertIsA(node, type, true, false); + assertIsA(node, type, false, false); + } + + private void assertIsNotExactly(TypeNode node, Class type) { + assertIsA(node, type, true, false); + } + + private void assertIsStrictSubtype(TypeNode node, Class type) { + assertIsNotExactly(node, type); + assertIsA(node, type); + } + + private void assertIsA(TypeNode node, Class type, boolean exactly, boolean expectTrue) { + Assert.assertEquals("TypeTestUtil::isA with class arg: " + type.getCanonicalName(), + expectTrue, + exactly ? TypeTestUtil.isExactlyA(type, node) + : TypeTestUtil.isA(type, node)); + Assert.assertEquals("TypeTestUtil::isA with string arg: " + type.getCanonicalName(), + expectTrue, + exactly ? TypeTestUtil.isExactlyA(type.getCanonicalName(), node) + : TypeTestUtil.isA(type.getCanonicalName(), node)); + } + + } diff --git a/pom.xml b/pom.xml index 186208bfcb..2b01158a5a 100644 --- a/pom.xml +++ b/pom.xml @@ -95,7 +95,7 @@ 8.30 3.1.1 3.13.0 - 1.10.1 + 1.10.8 3.2.0 4.7