diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index aca33a5526..19f58e0947 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,6 +17,8 @@ This is a {{ site.pmd.release_type }} release. ### 🐛 Fixed Issues * apex * [#5053](https://github.com/pmd/pmd/issues/5053): \[apex] CPD fails to parse string literals with escaped characters +* java + * [#4885](https://github.com/pmd/pmd/issues/4885): \[java] AssertionError: Method should be accessible * java-bestpractices * [#5047](https://github.com/pmd/pmd/issues/5047): \[java] UnusedPrivateMethod FP for Generics & Overloads * plsql diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AttributeAxisIterator.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AttributeAxisIterator.java index 001e32572c..3ca70f0686 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AttributeAxisIterator.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AttributeAxisIterator.java @@ -4,7 +4,6 @@ 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; @@ -91,7 +90,7 @@ public class AttributeAxisIterator implements Iterator { .filter(m -> isAttributeAccessor(nodeClass, m)) .map(m -> { try { - return new MethodWrapper(m, nodeClass); + return new MethodWrapper(m); } catch (ReflectiveOperationException e) { throw AssertionUtil.shouldNotReachHere("Method '" + m + "' should be accessible, but: " + e, e); } @@ -210,19 +209,13 @@ public class AttributeAxisIterator implements Iterator { public final String name; - MethodWrapper(Method m, Class nodeClass) throws IllegalAccessException, NoSuchMethodException { + MethodWrapper(Method m) throws IllegalAccessException { this.method = m; 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); - } + // Note: We only support public methods on public types. If the method being called is implemented + // in a package-private class, this won't work. + // See git history here and https://github.com/pmd/pmd/issues/4885 + this.methodHandle = LOOKUP.unreflect(m).asType(GETTER_TYPE); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/AbstractNode.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/AbstractNode.java index 7f1d4a366e..7ca1219450 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/AbstractNode.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/AbstractNode.java @@ -8,16 +8,16 @@ import net.sourceforge.pmd.lang.ast.DummyNode; import net.sourceforge.pmd.lang.document.Chars; // This class is package private -// and provides the implementation for getValue(). This -// class is the DeclaringClass for that method. -class AbstractNode extends DummyNode implements ValueNode { +// and provides the implementation for getValue(). +// This method is not accessible from outside this package, +// it is made available in the subclass ConcreteNode. +class AbstractNode extends DummyNode { AbstractNode() { } - @Override - public final Chars getValue() { + Chars getValue() { return Chars.wrap("actual_value"); } } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/ConcreteNode.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/ConcreteNode.java index bd2b214529..ebc4c3d634 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/ConcreteNode.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/impl/dummyast/ConcreteNode.java @@ -4,5 +4,11 @@ package net.sourceforge.pmd.lang.rule.xpath.impl.dummyast; +import net.sourceforge.pmd.lang.document.Chars; + public final class ConcreteNode extends AbstractNode implements ValueNode { + @Override + public Chars getValue() { + return super.getValue(); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBooleanLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBooleanLiteral.java index 694ae630fb..619cca36f4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBooleanLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTBooleanLiteral.java @@ -6,10 +6,12 @@ package net.sourceforge.pmd.lang.java.ast; import org.checkerframework.checker.nullness.qual.NonNull; +import net.sourceforge.pmd.lang.document.Chars; + /** * The boolean literal, either "true" or "false". */ -public final class ASTBooleanLiteral extends AbstractLiteral { +public final class ASTBooleanLiteral extends AbstractLiteral implements ASTLiteral { private boolean isTrue; @@ -32,6 +34,11 @@ public final class ASTBooleanLiteral extends AbstractLiteral { return isTrue; } + @Override + public Chars getLiteralText() { + return super.getLiteralText(); + } + @Override protected R acceptVisitor(JavaVisitor visitor, P data) { return visitor.visit(this, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCharLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCharLiteral.java index 5cc8e60b6f..aa0e25d324 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCharLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCharLiteral.java @@ -15,7 +15,7 @@ import net.sourceforge.pmd.lang.document.Chars; * retrieve the actual runtime value. Use {@link #getLiteralText()} to * retrieve the text. */ -public final class ASTCharLiteral extends AbstractLiteral { +public final class ASTCharLiteral extends AbstractLiteral implements ASTLiteral { ASTCharLiteral(int id) { @@ -39,4 +39,8 @@ public final class ASTCharLiteral extends AbstractLiteral { return StringEscapeUtils.UNESCAPE_JAVA.translate(woDelims).charAt(0); } + @Override + public Chars getLiteralText() { + return super.getLiteralText(); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNullLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNullLiteral.java index 05a1cb1b24..da168327c2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNullLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNullLiteral.java @@ -6,6 +6,8 @@ package net.sourceforge.pmd.lang.java.ast; import org.checkerframework.checker.nullness.qual.Nullable; +import net.sourceforge.pmd.lang.document.Chars; + /** * The null literal. * @@ -15,7 +17,7 @@ import org.checkerframework.checker.nullness.qual.Nullable; * * */ -public final class ASTNullLiteral extends AbstractLiteral { +public final class ASTNullLiteral extends AbstractLiteral implements ASTLiteral { ASTNullLiteral(int id) { super(id); } @@ -35,4 +37,9 @@ public final class ASTNullLiteral extends AbstractLiteral { public @Nullable Object getConstValue() { return null; } + + @Override + public Chars getLiteralText() { + return super.getLiteralText(); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNumericLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNumericLiteral.java index c0e4d21383..c8f711fdb5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNumericLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTNumericLiteral.java @@ -14,7 +14,7 @@ import net.sourceforge.pmd.lang.java.types.JPrimitiveType; /** * A numeric literal of any type (double, int, long, float, etc). */ -public final class ASTNumericLiteral extends AbstractLiteral { +public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiteral { /** * True if this is an integral literal, ie int OR long, @@ -36,6 +36,11 @@ public final class ASTNumericLiteral extends AbstractLiteral { return visitor.visit(this, data); } + @Override + public Chars getLiteralText() { + return super.getLiteralText(); + } + @Override public @NonNull Number getConstValue() { return (Number) super.getConstValue(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java index 928138bb5f..ab3684fb36 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTStringLiteral.java @@ -18,7 +18,7 @@ import net.sourceforge.pmd.util.StringUtil; * in the source ({@link #getLiteralText()}). {@link #getConstValue()} allows to recover * the actual runtime value, by processing escapes. */ -public final class ASTStringLiteral extends AbstractLiteral { +public final class ASTStringLiteral extends AbstractLiteral implements ASTLiteral { private static final String TEXTBLOCK_DELIMITER = "\"\"\""; @@ -36,6 +36,11 @@ public final class ASTStringLiteral extends AbstractLiteral { return getText().toString(); } + @Override + public Chars getLiteralText() { + return super.getLiteralText(); + } + void setTextBlock() { this.isTextBlock = true; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractLiteral.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractLiteral.java index 7c48889275..677204c967 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractLiteral.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/AbstractLiteral.java @@ -10,8 +10,11 @@ import net.sourceforge.pmd.lang.rule.xpath.NoAttribute; /** * @author Clément Fournier + * @see ASTLiteral#getLiteralText() + * @see #getLiteralText() */ -abstract class AbstractLiteral extends AbstractJavaExpr implements ASTLiteral { +// Note: This class must not implement ASTLiteral, see comment on #getLiteralText() +abstract class AbstractLiteral extends AbstractJavaExpr { private JavaccToken literalToken; @@ -41,13 +44,16 @@ abstract class AbstractLiteral extends AbstractJavaExpr implements ASTLiteral { return firstToken.getImageCs(); } - @Override - public final Chars getLiteralText() { + // This method represents ASTLiteral#getLiteralText(). + // However, since this class is package private, this method is not reliably accessible + // via reflection/method handles (see https://github.com/pmd/pmd/issues/4885). + // Subclasses of this class need to implement ASTLiteral and override this method + // as public. + Chars getLiteralText() { assert literalToken.getImageCs() != null; return literalToken.getImageCs(); } - @Override public boolean isCompileTimeConstant() { return true; // note: NullLiteral overrides this to false