Merge branch 'pr/4969'

Ref pmd#4969

docs/pages/pmd/userdocs/extending/writing_xpath_rules.md

@@ -49,23 +49,21 @@ To represent attributes, we must map Java values to [XPath Data Model (XDM)](htt
 values. In the following table we refer to the type conversion function as `conv`, a function from Java types
 to XDM types.
 
-| Java type `T` | XSD type `conv(T)`                    |
-|---------------|---------------------------------------|
-| `int`         | `xs:integer`                          |
-| `long`        | `xs:integer`                          |
-| `double`      | `xs:decimal`                          |
-| `float`       | `xs:decimal`                          |
-| `boolean`     | `xs:boolean`                          |
-| `String`      | `xs:string`                           |
-| `Character`   | `xs:string`                           |
-| `Enum<E>`     | `xs:string` (uses `Object::toString`) |
-| `List<E>`     | `conv(E)*` (a sequence type)          |
+| Java type `T`     | XSD type `conv(T)`                    |
+|-------------------|---------------------------------------|
+| `int`             | `xs:integer`                          |
+| `long`            | `xs:integer`                          |
+| `double`          | `xs:decimal`                          |
+| `float`           | `xs:decimal`                          |
+| `boolean`         | `xs:boolean`                          |
+| `String`          | `xs:string`                           |
+| `Character`       | `xs:string`                           |
+| `Enum<E>`         | `xs:string` (uses `Object::toString`) |
+| `Collection<E>`   | `conv(E)*` (a sequence type)          |
 
 The same `conv` function is used to translate rule property values to XDM values.
 
-{% include warning.html content="Lists are only supported for rule properties, not attributes." %}
-
-
+Additionaly, PMD's own `net.sourceforge.pmd.lang.document.Chars` is also translated to a `xs:string`
 
 
 ## Rule properties

docs/pages/release_notes.md

@@ -14,9 +14,19 @@ This is a {{ site.pmd.release_type }} release.
 
 ### 🚀 New and noteworthy
 
-### 🐛 Fixed Issues
+#### Collections exposed as XPath attributes
+
+Up to now, all AST node getters would be exposed to XPath, as long as the return type was a primitive (boxed or unboxed), String or Enum. That meant that collections, even of these basic types, were not exposed, so for instance accessing Apex's `ASTUserClass.getInterfaceNames()` to list the interfaces implemented by a class was impossible from XPath, and would require writing a Java rule to check it.
+
+Since this release, PMD will also expose any getter returning a collection of any supported type as a sequence through an XPath attribute. They would require to use apropriate XQuery functions to manipulate the sequence. So for instance, to detect any given `ASTUserClass` in Apex that implements `Queueable`, it is now possible to write:
 
+```xml
+/UserClass[@InterfaceNames = 'Queueable']
+```
+
+### 🐛 Fixed Issues
 * core
+  * [#4467](https://github.com/pmd/pmd/issues/4467): \[core] Expose collections from getters as XPath sequence attributes
   * [#4978](https://github.com/pmd/pmd/issues/4978): \[core] Referenced Rulesets do not emit details on validation errors
   * [#4983](https://github.com/pmd/pmd/pull/4983): \[cpd] Fix CPD crashes about unicode escapes
 * java

pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTReferenceExpression.java

@@ -7,6 +7,8 @@
 import java.util.List;
 import java.util.stream.Collectors;
 
+import net.sourceforge.pmd.lang.rule.xpath.NoAttribute;
+
 import com.google.summit.ast.Identifier;
 
 public final class ASTReferenceExpression extends AbstractApexNode.Many<Identifier> {
@@ -38,6 +40,7 @@ public String getImage() {
         return "";
     }
 
+    @NoAttribute
     public List<String> getNames() {
         return nodes.stream().map(Identifier::getString).collect(Collectors.toList());
     }

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/InnerClassLocations.txt

@@ -1,7 +1,7 @@
 +- ApexFile[@DefiningType = "InnerClassLocations", @RealLoc = true]
-   +- UserClass[@DefiningType = "InnerClassLocations", @Image = "InnerClassLocations", @RealLoc = true, @SimpleName = "InnerClassLocations", @SuperClassName = ""]
+   +- UserClass[@DefiningType = "InnerClassLocations", @Image = "InnerClassLocations", @InterfaceNames = (), @RealLoc = true, @SimpleName = "InnerClassLocations", @SuperClassName = ""]
       +- ModifierNode[@Abstract = false, @DefiningType = "InnerClassLocations", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
-      +- UserClass[@DefiningType = "InnerClassLocations.bar1", @Image = "bar1", @RealLoc = true, @SimpleName = "bar1", @SuperClassName = ""]
+      +- UserClass[@DefiningType = "InnerClassLocations.bar1", @Image = "bar1", @InterfaceNames = (), @RealLoc = true, @SimpleName = "bar1", @SuperClassName = ""]
       |  +- ModifierNode[@Abstract = false, @DefiningType = "InnerClassLocations.bar1", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       |  +- Method[@Arity = 0, @CanonicalName = "m", @Constructor = false, @DefiningType = "InnerClassLocations.bar1", @Image = "m", @RealLoc = true, @ReturnType = "void", @StaticInitializer = false]
       |     +- ModifierNode[@Abstract = false, @DefiningType = "InnerClassLocations.bar1", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
@@ -14,7 +14,7 @@
       |           +- MethodCallExpression[@DefiningType = "InnerClassLocations.bar1", @FullMethodName = "System.out.println", @InputParametersSize = 1, @MethodName = "println", @RealLoc = true]
       |              +- ReferenceExpression[@DefiningType = "InnerClassLocations.bar1", @Image = "System", @RealLoc = true, @ReferenceType = ReferenceType.METHOD, @SObjectType = false, @SafeNav = false]
       |              +- LiteralExpression[@Boolean = false, @Decimal = false, @DefiningType = "InnerClassLocations.bar1", @Double = false, @Image = "foo", @Integer = false, @LiteralType = LiteralType.STRING, @Long = false, @Name = null, @Null = false, @RealLoc = true, @String = true]
-      +- UserClass[@DefiningType = "InnerClassLocations.bar2", @Image = "bar2", @RealLoc = true, @SimpleName = "bar2", @SuperClassName = ""]
+      +- UserClass[@DefiningType = "InnerClassLocations.bar2", @Image = "bar2", @InterfaceNames = (), @RealLoc = true, @SimpleName = "bar2", @SuperClassName = ""]
          +- ModifierNode[@Abstract = false, @DefiningType = "InnerClassLocations.bar2", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
          +- Method[@Arity = 0, @CanonicalName = "m", @Constructor = false, @DefiningType = "InnerClassLocations.bar2", @Image = "m", @RealLoc = true, @ReturnType = "void", @StaticInitializer = false]
             +- ModifierNode[@Abstract = false, @DefiningType = "InnerClassLocations.bar2", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/NullCoalescingOperator.txt

@@ -1,5 +1,5 @@
 +- ApexFile[@DefiningType = "NullCoalescingOperator", @RealLoc = true]
-   +- UserClass[@DefiningType = "NullCoalescingOperator", @Image = "NullCoalescingOperator", @RealLoc = true, @SimpleName = "NullCoalescingOperator", @SuperClassName = ""]
+   +- UserClass[@DefiningType = "NullCoalescingOperator", @Image = "NullCoalescingOperator", @InterfaceNames = (), @RealLoc = true, @SimpleName = "NullCoalescingOperator", @SuperClassName = ""]
       +- ModifierNode[@Abstract = false, @DefiningType = "NullCoalescingOperator", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       +- Method[@Arity = 2, @CanonicalName = "leftOrRight", @Constructor = false, @DefiningType = "NullCoalescingOperator", @Image = "leftOrRight", @RealLoc = true, @ReturnType = "String", @StaticInitializer = false]
          +- ModifierNode[@Abstract = false, @DefiningType = "NullCoalescingOperator", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]

pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt

@@ -1,11 +1,11 @@
 +- ApexFile[@DefiningType = "Foo", @RealLoc = true]
-   +- UserClass[@DefiningType = "Foo", @Image = "Foo", @RealLoc = true, @SimpleName = "Foo", @SuperClassName = ""]
+   +- UserClass[@DefiningType = "Foo", @Image = "Foo", @InterfaceNames = (), @RealLoc = true, @SimpleName = "Foo", @SuperClassName = ""]
       +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 1, @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       +- Field[@DefiningType = "Foo", @Image = "x", @Name = "x", @RealLoc = true, @Type = "Integer", @Value = null]
       |  +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       +- Field[@DefiningType = "Foo", @Image = "profileUrl", @Name = "profileUrl", @RealLoc = true, @Type = "String", @Value = null]
       |  +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
-      +- FieldDeclarationStatements[@DefiningType = "Foo", @RealLoc = true, @TypeName = "Integer"]
+      +- FieldDeclarationStatements[@DefiningType = "Foo", @RealLoc = true, @TypeArguments = (), @TypeName = "Integer"]
       |  +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       |  +- FieldDeclaration[@DefiningType = "Foo", @Image = "x", @Name = "x", @RealLoc = true]
       |     +- VariableExpression[@DefiningType = "Foo", @Image = "anIntegerField", @RealLoc = true]
@@ -14,7 +14,7 @@
       |     |        +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false]
       |     +- VariableExpression[@DefiningType = "Foo", @Image = "x", @RealLoc = true]
       |        +- EmptyReferenceExpression[@DefiningType = null, @RealLoc = false]
-      +- FieldDeclarationStatements[@DefiningType = "Foo", @RealLoc = true, @TypeName = "String"]
+      +- FieldDeclarationStatements[@DefiningType = "Foo", @RealLoc = true, @TypeArguments = (), @TypeName = "String"]
       |  +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Modifiers = 0, @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @Virtual = false, @WebService = false, @WithSharing = false, @WithoutSharing = false]
       |  +- FieldDeclaration[@DefiningType = "Foo", @Image = "profileUrl", @Name = "profileUrl", @RealLoc = true]
       |     +- MethodCallExpression[@DefiningType = "Foo", @FullMethodName = "toExternalForm", @InputParametersSize = 0, @MethodName = "toExternalForm", @RealLoc = true]

pmd-cli/src/test/java/net/sourceforge/pmd/cli/TreeExportCliTest.java

@@ -31,9 +31,9 @@ void testReadStandardInput() throws Exception {
             final CliExecutionResult output = runCliSuccessfully("-i", "-f", "xml", "-PlineSeparator=LF");
 
             output.checkStdOut(equalTo("<?xml version='1.0' encoding='UTF-8' ?>\n"
-                                           + "<dummyRootNode Image=''>\n"
-                                           + "    <dummyNode Image='a'>\n"
-                                           + "        <dummyNode Image='b' />\n"
+                                           + "<dummyRootNode Image='' Lines='[, , ]'>\n"
+                                           + "    <dummyNode Image='a' Lines='[a, a, a]'>\n"
+                                           + "        <dummyNode Image='b' Lines='[b, b, b]' />\n"
                                            + "    </dummyNode>\n"
                                            + "</dummyRootNode>\n"));
         });
@@ -44,9 +44,9 @@ void testReadFile() throws Exception {
         File file = newFileWithContents("(a(b))");
         final CliExecutionResult result = runCliSuccessfully("--file", file.getAbsolutePath(), "-f", "xml", "-PlineSeparator=LF");
         result.checkStdOut(equalTo("<?xml version='1.0' encoding='UTF-8' ?>\n"
-                                       + "<dummyRootNode Image=''>\n"
-                                       + "    <dummyNode Image='a'>\n"
-                                       + "        <dummyNode Image='b' />\n"
+                                       + "<dummyRootNode Image='' Lines='[, , ]'>\n"
+                                       + "    <dummyNode Image='a' Lines='[a, a, a]'>\n"
+                                       + "        <dummyNode Image='b' Lines='[b, b, b]' />\n"
                                        + "    </dummyNode>\n"
                                        + "</dummyRootNode>\n"));
     }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/Attribute.java

@@ -7,7 +7,6 @@
 import java.lang.invoke.MethodHandle;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
-import java.util.List;
 import java.util.Objects;
 
 import org.checkerframework.checker.nullness.qual.NonNull;
@@ -106,16 +105,11 @@ String replacementIfDeprecated() {
             return null;
         } else {
             DeprecatedAttribute annot = method.getAnnotation(DeprecatedAttribute.class);
-            String result = annot != null
+            return annot != null
                    ? annot.replaceWith()
                    : method.isAnnotationPresent(Deprecated.class)
                      ? DeprecatedAttribute.NO_REPLACEMENT
                      : null;
-            if (result == null && List.class.isAssignableFrom(method.getReturnType())) {
-                // Lists are generally deprecated, see #2451
-                result = DeprecatedAttribute.NO_REPLACEMENT;
-            }
-            return result;
         }
     }
 

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AttributeAxisIterator.java

@@ -13,7 +13,11 @@
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.lang.reflect.ParameterizedType;
+import java.lang.reflect.Type;
+import java.lang.reflect.TypeVariable;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
@@ -119,7 +123,27 @@ && isConsideredReturnType(method)
 
     private boolean isConsideredReturnType(Method method) {
         Class<?> klass = method.getReturnType();
-        return CONSIDERED_RETURN_TYPES.contains(klass) || klass.isEnum();
+        if (CONSIDERED_RETURN_TYPES.contains(klass) || klass.isEnum()) {
+            return true;
+        }
+
+        if (Collection.class.isAssignableFrom(klass)) {
+            Type t = method.getGenericReturnType();
+            if (t instanceof ParameterizedType) {
+                try {
+                    // ignore type variables, such as List<N>… we could check all bounds, but probably it's overkill
+                    Type actualTypeArgument = ((ParameterizedType) t).getActualTypeArguments()[0];
+                    if (!TypeVariable.class.isAssignableFrom(actualTypeArgument.getClass())) {
+                        Class<?> elementKlass = Class.forName(actualTypeArgument.getTypeName());
+                        return CONSIDERED_RETURN_TYPES.contains(elementKlass) || elementKlass.isEnum();
+                    }
+                } catch (ClassNotFoundException e) {
+                    throw AssertionUtil.shouldNotReachHere("Method '" + method + "' should return a known type, but: " + e, e);
+                }
+            }
+        }
+
+        return false;
     }
 
     private boolean isIgnored(Class<?> nodeClass, Method method) {

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/AstAttributeNode.java

@@ -68,6 +68,7 @@ protected AxisIterator iterateSiblings(NodeTest nodeTest, boolean forwards) {
 
     @Override
     public AtomicSequence atomize() {
+        getTreeInfo().getLogger().recordUsageOf(attribute);
         if (value == null) {
             value = DomainConversion.convert(attribute.getValue());
         }

pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonXPathRuleQuery.java

@@ -258,13 +258,14 @@ public static NamePool getNamePool() {
         return NAME_POOL;
     }
 
-
     final class StaticContextWithProperties extends IndependentContext {
 
         private final Map<StructuredQName, PropertyDescriptor<?>> propertiesByName = new HashMap<>();
 
         StaticContextWithProperties(Configuration config) {
             super(config);
+            // This statement is necessary for Saxon to support sequence-valued attributes
+            getPackageData().setSchemaAware(true);
         }
 
         public void declareProperty(PropertyDescriptor<?> prop) {

pmd-core/src/test/java/net/sourceforge/pmd/lang/ast/DummyNode.java

@@ -5,6 +5,7 @@
 package net.sourceforge.pmd.lang.ast;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
@@ -125,6 +126,11 @@ public Iterator<Attribute> getXPathAttributesIterator() {
         return attributes.iterator();
     }
 
+    // phony attribute that repeats the image 3 times
+    public List<String> getLines() {
+        return Arrays.asList(getImage(), getImage(), getImage());
+    }
+
     public static class DummyRootNode extends DummyNode implements RootNode, GenericNode<DummyNode> {
 
         // FIXME remove this