[core][java] Expose Chars attributes in XPath

- taken from #4352 (avoid getImage())
- Exposes @LiteralText for ASTLiteral

Co-authored-by: Clément Fournier <[email protected]>

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

@@ -10,6 +10,8 @@
 import java.util.List;
 import java.util.Objects;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+import org.checkerframework.checker.nullness.qual.Nullable;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -24,55 +26,73 @@
  * <p>Two attributes are equal if they have the same name
  * and their parent nodes are equal.
  *
- * @author daniels
+ * <p>Note that attributes do not support just any type, but
+ * a restricted set of value types that can be mapped to XPath types.
+ * The exact supported types are not specified, but include at
+ * least Java primitives and String.
+ *
+ * @see Node#getXPathAttributesIterator()
  */
-public class Attribute {
+public final class Attribute {
  private static final Logger LOG = LoggerFactory.getLogger(Attribute.class);
 
- private final Node parent;
- private final String name;
+ private final @NonNull Node parent;
+ private final @NonNull String name;
 
- private final MethodHandle handle;
- private final Method method;
+ private final @Nullable MethodHandle handle;
+ private final @Nullable Method method;
+ /** If true, we won't invoke the method handle again. */
  private boolean invoked;
 
- private Object value;
+ /** May be null after invocation too. */
+ private @Nullable Object value;
+
+ /** Must be non-null after {@link #getStringValue()} has been invoked. */
  private String stringValue;
 
- /** Creates a new attribute belonging to the given node using its accessor. */
- public Attribute(Node parent, String name, MethodHandle handle, Method m) {
- this.parent = parent;
- this.name = name;
- this.handle = handle;
- this.method = m;
+ /**
+ * Creates a new attribute belonging to the given node using its accessor.
+ *
+ * @param handle A method handle, used to fetch the attribute.
+ * @param method The method corresponding to the method handle. This
+ * is used to perform reflective queries, eg to
+ * find annotations on the attribute getter, but only
+ * the method handle is ever invoked.
+ */
+ public Attribute(@NonNull Node parent, @NonNull String name, @NonNull MethodHandle handle, @NonNull Method method) {
+ this.parent = Objects.requireNonNull(parent);
+ this.name = Objects.requireNonNull(name);
+ this.handle = Objects.requireNonNull(handle);
+ this.method = Objects.requireNonNull(method);
  }
 
  /** Creates a new attribute belonging to the given node using its string value. */
- public Attribute(Node parent, String name, String value) {
- this.parent = parent;
- this.name = name;
+ public Attribute(@NonNull Node parent, @NonNull String name, @Nullable String value) {
+ this.parent = Objects.requireNonNull(parent);
+ this.name = Objects.requireNonNull(name);
  this.value = value;
  this.handle = null;
  this.method = null;
- this.stringValue = value;
+ this.stringValue = value == null ? "" : value;
  this.invoked = true;
  }
 
 
-
  /**
  * Gets the generic type of the value of this attribute.
  */
  public Type getType() {
  return method == null ? String.class : method.getGenericReturnType();
  }
 
- public String getName() {
+ /** Return the name of the attribute (without leading @ sign). */
+ public @NonNull String getName() {
  return name;
  }
 
 
- public Node getParent() {
+ /** Return the node that owns this attribute. */
+ public @NonNull Node getParent() {
  return parent;
  }
 
@@ -99,13 +119,23 @@ public String replacementIfDeprecated() {
  }
  }
 
+ /**
+ * Return whether this attribute was deprecated. This is the case if the getter
+ * has the annotation {@link Deprecated} or {@link DeprecatedAttribute}.
+ */
  public boolean isDeprecated() {
  return replacementIfDeprecated() != null;
  }
 
+ /**
+ * Return the value of the attribute. This may return null. The getter
+ * is invoked at most once.
+ */
  public Object getValue() {
  if (this.invoked) {
  return this.value;
+ } else if (handle == null) {
+ throw new NullPointerException("Cannot fetch value of attribute with null getter! " + this);
  }
 
  Object value;
@@ -121,13 +151,23 @@ public Object getValue() {
  return value;
  }
 
- public String getStringValue() {
+ /**
+ * Return the string value of the attribute. If the getter returned null,
+ * then return the empty string (which is a falsy value in XPath).
+ * Otherwise, return a string representation of the value (e.g. with
+ * {@link Object#toString()}, but this is not guaranteed).
+ */
+ public @NonNull String getStringValue() {
  if (stringValue != null) {
  return stringValue;
  }
  Object v = getValue();
 
- stringValue = v == null ? "" : String.valueOf(v);
+ if (v == null) {
+ stringValue = "";
+ } else {
+ stringValue = v.toString();
+ }
  return stringValue;
  }
 
@@ -148,11 +188,11 @@ public boolean equals(Object o) {
 
  @Override
  public int hashCode() {
- return Objects.hash(parent, name);
+ return parent.hashCode() * 31 + name.hashCode();
  }
 
  @Override
  public String toString() {
- return name + ':' + getValue() + ':' + parent.getXPathNodeName();
+ return parent.getXPathNodeName() + "/@" + name + " = " + getValue();
  }
 }

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

@@ -4,22 +4,28 @@
 
 package net.sourceforge.pmd.lang.rule.xpath.impl;
 
+import static net.sourceforge.pmd.util.CollectionUtil.emptyList;
+import static net.sourceforge.pmd.util.CollectionUtil.setOf;
+
 import java.lang.invoke.MethodHandle;
 import java.lang.invoke.MethodHandles;
 import java.lang.invoke.MethodHandles.Lookup;
 import java.lang.invoke.MethodType;
 import java.lang.reflect.Method;
+import java.lang.reflect.Modifier;
 import java.util.Arrays;
-import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
 import java.util.stream.Collectors;
 
+import org.checkerframework.checker.nullness.qual.NonNull;
+
 import net.sourceforge.pmd.lang.ast.Node;
 import net.sourceforge.pmd.lang.ast.impl.AbstractNode;
+import net.sourceforge.pmd.lang.document.Chars;
 import net.sourceforge.pmd.lang.rule.xpath.Attribute;
 import net.sourceforge.pmd.lang.rule.xpath.NoAttribute;
 import net.sourceforge.pmd.lang.rule.xpath.NoAttribute.NoAttrScope;
@@ -31,6 +37,8 @@
  * attributes. This is the default way the attributes of a node
  * are made accessible to XPath rules, and defines an important
  * piece of PMD's XPath support.
+ *
+ * @see Node#getXPathAttributesIterator()
  */
 public class AttributeAxisIterator implements Iterator<Attribute> {
 
@@ -39,25 +47,25 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
 
  /* Constants used to determine which methods are accessors */
  private static final Set<Class<?>> CONSIDERED_RETURN_TYPES
- = new HashSet<>(Arrays.<Class<?>>asList(Integer.TYPE, Boolean.TYPE, Double.TYPE, String.class,
- Long.TYPE, Character.TYPE, Float.TYPE));
+ = setOf(Integer.TYPE, Boolean.TYPE, Double.TYPE, String.class,
+ Long.TYPE, Character.TYPE, Float.TYPE, Chars.class);
 
  private static final Set<String> FILTERED_OUT_NAMES
- = new HashSet<>(Arrays.asList("toString",
-  "getNumChildren",
-  "getIndexInParent",
-  "getParent",
-  "getClass",
-  "getSourceCodeFile",
-  "isFindBoundary",
-  "getRuleIndex",
-  "getXPathNodeName",
-  "altNumber",
-  "toStringTree",
-  "getTypeNameNode",
-  "hashCode",
-  "getImportedNameNode",
-  "getScope"));
+ = setOf("toString",
+ "getNumChildren",
+ "getIndexInParent",
+ "getParent",
+ "getClass",
+ "getSourceCodeFile",
+ "isFindBoundary",
+ "getRuleIndex",
+ "getXPathNodeName",
+ "altNumber",
+ "toStringTree",
+ "getTypeNameNode",
+ "hashCode",
+ "getImportedNameNode",
+ "getScope");
 
  /* Iteration variables */
  private final Iterator<MethodWrapper> iterator;
@@ -69,7 +77,7 @@ public class AttributeAxisIterator implements Iterator<Attribute> {
  * Note: if you want to access the attributes of a node, don't use this directly,
  * use instead the overridable {@link Node#getXPathAttributesIterator()}.
  */
- public AttributeAxisIterator(Node contextNode) {
+ public AttributeAxisIterator(@NonNull Node contextNode) {
  this.node = contextNode;
  this.iterator = METHOD_CACHE.computeIfAbsent(contextNode.getClass(), this::getWrappersForClass).iterator();
  }
@@ -79,8 +87,8 @@ private List<MethodWrapper> getWrappersForClass(Class<?> nodeClass) {
  .filter(m -> isAttributeAccessor(nodeClass, m))
  .map(m -> {
  try {
- return new MethodWrapper(m);
- } catch (IllegalAccessException e) {
+ return new MethodWrapper(m, nodeClass);
+ } catch (ReflectiveOperationException e) {
  throw AssertionUtil.shouldNotReachHere("Method should be accessible " + e);
  }
  })
@@ -104,6 +112,8 @@ && isConsideredReturnType(method)
  // filter out methods declared in supertypes like the
  // Antlr ones, unless they're opted-in
  && Node.class.isAssignableFrom(method.getDeclaringClass())
+ // Methods of package-private classes are not accessible.
+ && Modifier.isPublic(method.getModifiers())
  && !isIgnored(nodeClass, method);
  }
 
@@ -171,15 +181,24 @@ public boolean hasNext() {
  private static class MethodWrapper {
  static final Lookup LOOKUP = MethodHandles.publicLookup();
  private static final MethodType GETTER_TYPE = MethodType.methodType(Object.class, Node.class);
- public MethodHandle methodHandle;
- public Method method;
- public String name;
+ public final MethodHandle methodHandle;
+ public final Method method;
+ public final String name;
 
 
- MethodWrapper(Method m) throws IllegalAccessException {
+ MethodWrapper(Method m, Class<?> nodeClass) throws IllegalAccessException, NoSuchMethodException {
  this.method = m;
- this.methodHandle = LOOKUP.unreflect(m).asType(GETTER_TYPE);
  this.name = truncateMethodName(m.getName());
+
+ if (!Modifier.isPublic(m.getDeclaringClass().getModifiers())) {
+ // This is a public method of a non-public class.
+ // To call it from reflection we need to call it via invokevirtual,
+ // whereas the default handle would use invokespecial.
+ MethodType methodType = MethodType.methodType(m.getReturnType(), emptyList());
+ this.methodHandle = MethodWrapper.LOOKUP.findVirtual(nodeClass, m.getName(), methodType).asType(GETTER_TYPE);
+ } else {
+ this.methodHandle = LOOKUP.unreflect(m).asType(GETTER_TYPE);
+ }
  }
 
 

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

@@ -133,8 +133,8 @@ public static AtomicValue getAtomicRepresentation(final Object value) {
  if (value == null) {
  return UntypedAtomicValue.ZERO_LENGTH_UNTYPED;
 
- } else if (value instanceof String) {
- return new StringValue((String) value);
+ } else if (value instanceof CharSequence) {
+ return new StringValue((CharSequence) value);
  } else if (value instanceof Boolean) {
  return BooleanValue.get((Boolean) value);
  } else if (value instanceof Integer) {

pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/AllJavaAstTreeDumpTest.java

@@ -10,6 +10,7 @@
 @Suite
 @SelectClasses({
  ParserCornersTest.class,
+ Java9TreeDumpTest.class,
  Java14TreeDumpTest.class,
  Java15TreeDumpTest.class,
  Java16TreeDumpTest.class,

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/Bug1429.txt

@@ -18,7 +18,7 @@
  +- InfixExpression[@CompileTimeConstant = false, @Operator = BinaryOp.EQ, @ParenthesisDepth = 0, @Parenthesized = false]
  | +- FieldAccess[@AccessType = AccessType.READ, @CompileTimeConstant = false, @Image = "attributes", @Name = "attributes", @ParenthesisDepth = 0, @Parenthesized = false]
  | | +- ThisExpression[@CompileTimeConstant = false, @ParenthesisDepth = 0, @Parenthesized = false]
- | +- NullLiteral[@CompileTimeConstant = false, @ParenthesisDepth = 0, @Parenthesized = false]
+ | +- NullLiteral[@CompileTimeConstant = false, @LiteralText = "null", @ParenthesisDepth = 0, @Parenthesized = false]
  +- MethodCall[@CompileTimeConstant = false, @Image = "emptySet", @MethodName = "emptySet", @ParenthesisDepth = 0, @Parenthesized = false]
  | +- AmbiguousName[@CompileTimeConstant = false, @Image = "Collections", @Name = "Collections", @ParenthesisDepth = 0, @Parenthesized = false]
  | +- TypeArguments[@Diamond = false, @Empty = false, @Size = 1]
@@ -91,7 +91,7 @@
  | +- MethodCall[@CompileTimeConstant = false, @Image = "concat", @MethodName = "concat", @ParenthesisDepth = 0, @Parenthesized = false]
  | | +- VariableAccess[@AccessType = AccessType.READ, @CompileTimeConstant = false, @Image = "result", @Name = "result", @ParenthesisDepth = 0, @Parenthesized = false]
  | | +- ArgumentList[@Empty = false, @Size = 1]
- | | +- StringLiteral[@CompileTimeConstant = true, @ConstValue = ":", @Empty = false, @Image = "\":\"", @Length = 1, @ParenthesisDepth = 0, @Parenthesized = false, @TextBlock = false]
+ | | +- StringLiteral[@CompileTimeConstant = true, @ConstValue = ":", @Empty = false, @Image = "\":\"", @Length = 1, @LiteralText = "\":\"", @ParenthesisDepth = 0, @Parenthesized = false, @TextBlock = false]
  | +- ArgumentList[@Empty = false, @Size = 1]
  | +- VariableAccess[@AccessType = AccessType.READ, @CompileTimeConstant = false, @Image = "value", @Name = "value", @ParenthesisDepth = 0, @Parenthesized = false]
  +- ReturnStatement[]

pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/ast/GitHubBug208.txt

@@ -23,4 +23,4 @@
  +- FormalParameters[@Empty = true, @Size = 0]
  +- Block[@Empty = false, @Size = 1, @containsComment = false]
  +- ReturnStatement[]
- +- NullLiteral[@CompileTimeConstant = false, @ParenthesisDepth = 0, @Parenthesized = false]
+ +- NullLiteral[@CompileTimeConstant = false, @LiteralText = "null", @ParenthesisDepth = 0, @Parenthesized = false]