[core] Expose collections from getters as XPath sequence attributes

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