[core][java] Integrate improvements from #4352 (avoid getImage())

- This improves ASTLiteral implementation
- Adds ASTLiteral#getLiteralText() - not yet exposed as XPath attribute

Co-authored-by: Clément Fournier <clement.fournier76@gmail.com>
This commit is contained in:
Andreas Dangel 2023-12-14 12:10:00 +01:00
parent 7a0f7316b7
commit 73fcf6e38c
No known key found for this signature in database
GPG Key ID: 93450DF2DF9A3FA3
17 changed files with 232 additions and 110 deletions

View File

@ -455,6 +455,8 @@ public interface Node extends Reportable {
* from XPath for this node.
*
* @return An attribute iterator for this node
*
* @see AttributeAxisIterator
*/
default Iterator<Attribute> getXPathAttributesIterator() {
return new AttributeAxisIterator(this);

View File

@ -198,6 +198,13 @@ public final class Chars implements CharSequence {
return NOT_FOUND;
}
/**
* See {@link String#indexOf(int)}
*/
public int indexOf(int ch) {
return indexOf(ch, 0);
}
/**
* See {@link String#indexOf(int, int)}.
*/

View File

@ -16,6 +16,9 @@ import java.lang.annotation.Target;
* to mark the attribute as deprecated for XPath. Unlike {@link Deprecated}, this
* annotation does not deprecate the method for java usage.
*
* <p>When used in combination with {@link Deprecated}, this attribute allows specifying
* a replacement for the XPath attribute.
*
* @since 6.21.0
*/
@Documented
@ -23,12 +26,16 @@ import java.lang.annotation.Target;
@Target(ElementType.METHOD)
public @interface DeprecatedAttribute {
/** Sentinel expressing that the attribute is deprecated without replacement. */
String NO_REPLACEMENT = "";
/**
* The simple name of the attribute to use for replacement (with '@' prefix).
* An XPath expression to suggest as a replacement for use of the
* deprecated attribute.
* If empty, then the attribute is deprecated for removal.
* Example values: {@code @NewerAttribute}, {@code NodeType/@SomeAttribute},
* {@code some-function()}.
*/
String replaceWith() default NO_REPLACEMENT;
}

View File

@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.rule.xpath;
import java.lang.annotation.Documented;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
@ -17,6 +18,7 @@ import net.sourceforge.pmd.lang.ast.impl.AbstractNode;
*
* @author Clément Fournier
*/
@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target({ElementType.METHOD, ElementType.TYPE})
public @interface NoAttribute {

View File

@ -7,11 +7,13 @@ package net.sourceforge.pmd.lang.java.ast;
import org.apache.commons.lang3.StringEscapeUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.document.Chars;
/**
* Represents a character literal. The image of this node can be the literal as it appeared
* in the source, but JavaCC performs its own unescaping and some escapes may be lost. At the
* very least it has delimiters. {@link #getConstValue()} allows to recover the actual runtime value.
* Represents a character literal. {@link #getConstValue()} allows to
* retrieve the actual runtime value. Use {@link #getLiteralText()} to
* retrieve the text.
*/
public final class ASTCharLiteral extends AbstractLiteral implements ASTLiteral {
@ -32,9 +34,9 @@ public final class ASTCharLiteral extends AbstractLiteral implements ASTLiteral
*/
@Override
public @NonNull Character getConstValue() {
String image = getImage();
String woDelims = image.substring(1, image.length() - 1);
return StringEscapeUtils.unescapeJava(woDelims).charAt(0);
Chars image = getLiteralText();
Chars woDelims = image.subSequence(1, image.length() - 1);
return StringEscapeUtils.UNESCAPE_JAVA.translate(woDelims).charAt(0);
}
}

View File

@ -4,6 +4,8 @@
package net.sourceforge.pmd.lang.java.ast;
import net.sourceforge.pmd.lang.document.Chars;
/**
* A lexical literal. This is an expression that is represented by exactly
* one token. This interface is implemented by several nodes.
@ -103,4 +105,9 @@ public interface ASTLiteral extends ASTPrimaryExpression {
return false;
}
/**
* Return the text of the literal in the source file. Note that
* {@link #getText()} may include parentheses.
*/
Chars getLiteralText();
}

View File

@ -4,11 +4,10 @@
package net.sourceforge.pmd.lang.java.ast;
import java.math.BigInteger;
import java.util.Locale;
import org.apache.commons.lang3.StringUtils;
import org.checkerframework.checker.nullness.qual.NonNull;
import net.sourceforge.pmd.lang.document.Chars;
import net.sourceforge.pmd.lang.java.types.JPrimitiveType;
@ -22,6 +21,9 @@ public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiter
* false if this is a floating-point literal, ie float OR double.
*/
private boolean isIntegral;
private boolean is64bits;
private long longValue;
private double doubleValue;
ASTNumericLiteral(int id) {
@ -36,6 +38,11 @@ public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiter
@Override
public @NonNull Number getConstValue() {
return (Number) super.getConstValue();
}
@Override
protected @NonNull Number buildConstValue() {
// don't use ternaries, the compiler messes up autoboxing.
if (isIntegral()) {
if (isIntLiteral()) {
@ -65,73 +72,44 @@ public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiter
}
@Override
public boolean isIntLiteral() {
return isIntegral && !isLongLiteral();
public void jjtClose() {
super.jjtClose();
Chars image = getLiteralText();
char lastChar = image.charAt(image.length() - 1);
if (isIntegral) {
is64bits = lastChar == 'l' || lastChar == 'L';
longValue = parseIntegralValue(image);
doubleValue = (double) longValue;
} else {
is64bits = !(lastChar == 'f' || lastChar == 'F');
doubleValue = Double.parseDouble(StringUtils.remove(image.toString(), '_'));
longValue = (long) doubleValue;
}
}
// TODO all of this can be done once in jjtCloseNodeScope
@Override
public boolean isIntLiteral() {
return isIntegral && !is64bits;
}
@Override
public boolean isLongLiteral() {
if (isIntegral) {
String image = getImage();
char lastChar = image.charAt(image.length() - 1);
return lastChar == 'l' || lastChar == 'L';
}
return false;
return isIntegral && is64bits;
}
@Override
public boolean isFloatLiteral() {
if (!isIntegral) {
String image = getImage();
char lastChar = image.charAt(image.length() - 1);
return lastChar == 'f' || lastChar == 'F';
}
return false;
return !isIntegral && !is64bits;
}
@Override
public boolean isDoubleLiteral() {
return !isIntegral && !isFloatLiteral();
return !isIntegral && is64bits;
}
private String stripIntValue() {
String image = getImage().toLowerCase(Locale.ROOT).replaceAll("_++", "");
// literals never have a sign.
char last = image.charAt(image.length() - 1);
// This method is only called if this is an int,
// in which case the 'd' and 'f' suffixes can only
// be hex digits, which we must not remove
if (last == 'l') {
image = image.substring(0, image.length() - 1);
}
// ignore base prefix if any
if (image.charAt(0) == '0' && image.length() > 1) {
if (image.charAt(1) == 'x' || image.charAt(1) == 'b') {
image = image.substring(2);
} else {
image = image.substring(1);
}
}
return image;
}
private String stripFloatValue() {
// This method is only called if this is a floating point literal.
// there can't be any 'l' suffix that the double parser doesn't support,
// so it's enough to just remove underscores
return getImage().replaceAll("_++", "");
}
/**
* Returns true if this is an integral literal, ie either a long or
* an integer literal. Otherwise, this is a floating point literal.
@ -146,7 +124,10 @@ public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiter
* for the literal {@code 0} (which can really be any base).
*/
public int getBase() {
final String image = getImage();
return getBase(getLiteralText());
}
static int getBase(Chars image) {
if (image.length() > 1 && image.charAt(0) == '0') {
switch (image.charAt(1)) {
case 'x':
@ -169,33 +150,51 @@ public final class ASTNumericLiteral extends AbstractLiteral implements ASTLiter
// In 6.0.x, eg getValueAsInt was giving up when this was a double.
public int getValueAsInt() {
if (isIntegral) {
// the downcast allows to parse 0x80000000+ numbers as negative instead of a NumberFormatException
return (int) getValueAsLong();
} else {
return (int) getValueAsDouble();
}
return (int) longValue;
}
public long getValueAsLong() {
if (isIntegral) {
// Using BigInteger to allow parsing 0x8000000000000000+ numbers as negative instead of a NumberFormatException
BigInteger bigInt = new BigInteger(stripIntValue(), getBase());
return bigInt.longValue();
} else {
return (long) getValueAsDouble();
}
return longValue;
}
public float getValueAsFloat() {
return isIntegral ? (float) getValueAsLong() : (float) getValueAsDouble();
return (float) doubleValue;
}
public double getValueAsDouble() {
return isIntegral ? (double) getValueAsLong() : Double.parseDouble(stripFloatValue());
return doubleValue;
}
/**
* Parse an int or long literal into a long. This avoids creating
* and discarding a BigInteger, and avoids exceptions if the literal
* is malformed.
*
* <p>Invalid literals or overflows result in {@code 0L}.
*/
static long parseIntegralValue(Chars image) {
final int base = getBase(image);
if (base == 8) {
image = image.subSequence(1); // 0
} else if (base != 10) {
image = image.subSequence(2); // 0x / 0b
}
int length = image.length();
char lastChar = image.charAt(length - 1);
if (lastChar == 'l' || lastChar == 'L') {
length--;
}
try {
String literalImage = image.substring(0, length).replaceAll("_", "");
return Long.parseUnsignedLong(literalImage, base);
} catch (NumberFormatException e) {
// invalid literal or overflow
return 0L;
}
}
}

View File

@ -15,7 +15,7 @@ import net.sourceforge.pmd.util.StringUtil;
/**
* Represents a string literal. The image of this node is the literal as it appeared
* in the source ({@link #getText()}). {@link #getConstValue()} allows to recover
* in the source ({@link #getLiteralText()}). {@link #getConstValue()} allows to recover
* the actual runtime value, by processing escapes.
*/
public final class ASTStringLiteral extends AbstractLiteral implements ASTLiteral {
@ -29,7 +29,7 @@ public final class ASTStringLiteral extends AbstractLiteral implements ASTLitera
}
// todo deprecate this
// TODO deprecate / remove this
// it's ambiguous whether it returns getOriginalText or getTranslatedText
@Override
public String getImage() {
@ -50,7 +50,7 @@ public final class ASTStringLiteral extends AbstractLiteral implements ASTLitera
if (isTextBlock) {
return getConstValue().isEmpty(); // could be a bunch of ignorable indents?
} else {
return getImage().length() == 2; // ""
return getLiteralText().length() == 2; // ""
}
}
@ -85,9 +85,9 @@ public final class ASTStringLiteral extends AbstractLiteral implements ASTLitera
@Override
protected @NonNull String buildConstValue() {
if (isTextBlock()) {
return determineTextBlockContent(getText());
return determineTextBlockContent(getLiteralText());
} else {
return determineStringContent(getText());
return determineStringContent(getLiteralText());
}
}

View File

@ -4,15 +4,50 @@
package net.sourceforge.pmd.lang.java.ast;
import net.sourceforge.pmd.lang.ast.impl.javacc.JavaccToken;
import net.sourceforge.pmd.lang.document.Chars;
import net.sourceforge.pmd.lang.rule.xpath.NoAttribute;
/**
* @author Clément Fournier
*/
abstract class AbstractLiteral extends AbstractJavaExpr implements ASTLiteral {
private JavaccToken literalToken;
AbstractLiteral(int i) {
super(i);
}
@Override
public void jjtClose() {
super.jjtClose();
// Note that in this method, if the literal is parenthesized,
// its parentheses have not yet been set yet so the text is
// just the literal.
assert getParenthesisDepth() == 0;
assert getFirstToken() == getLastToken(); // NOPMD
literalToken = getFirstToken();
}
@Override
@NoAttribute
public final Chars getText() {
JavaccToken firstToken = getFirstToken();
// this literal has parentheses, the text includes them
if (firstToken.kind == JavaTokenKinds.LPAREN) {
return super.getText();
}
return firstToken.getImageCs();
}
@Override
public final Chars getLiteralText() {
assert literalToken.getImageCs() != null;
return literalToken.getImageCs();
}
@Override
public boolean isCompileTimeConstant() {
return true; // note: NullLiteral overrides this to false

View File

@ -427,7 +427,7 @@ public class LanguageLevelChecker<T> {
@Override
public Void visit(ASTStringLiteral node, T data) {
if (SPACE_ESCAPE_PATTERN.matcher(node.getImage()).find()) {
if (!node.isTextBlock() && SPACE_ESCAPE_PATTERN.matcher(node.getLiteralText()).find()) {
check(node, RegularLanguageFeature.SPACE_STRING_ESCAPES, data);
}
if (node.isTextBlock()) {
@ -521,7 +521,7 @@ public class LanguageLevelChecker<T> {
@Override
public Void visit(ASTEnumDeclaration node, T data) {
check(node, RegularLanguageFeature.ENUMS, data);
visitTypeDecl((ASTTypeDeclaration) node, data);
visitTypeDecl(node, data);
return null;
}
@ -532,7 +532,7 @@ public class LanguageLevelChecker<T> {
check(node, RegularLanguageFeature.HEXADECIMAL_FLOATING_POINT_LITERALS, data);
} else if (base == 2) {
check(node, RegularLanguageFeature.BINARY_NUMERIC_LITERALS, data);
} else if (node.getImage().indexOf('_') >= 0) {
} else if (node.getLiteralText().indexOf('_') >= 0) {
check(node, RegularLanguageFeature.UNDERSCORES_IN_NUMERIC_LITERALS, data);
}
return null;

View File

@ -182,7 +182,7 @@ public final class PrettyPrintingUtil {
} else if (node instanceof ASTVariableId) {
return ((ASTVariableId) node).getName();
} else {
return node.getImage(); // todo get rid of this
throw new IllegalArgumentException("Node " + node + " has no defined name");
}
}

View File

@ -0,0 +1,41 @@
/*
* BSD-style license; for more info see http://pmd.sourceforge.net/license.html
*/
package net.sourceforge.pmd.lang.java.ast;
import static org.junit.jupiter.api.Assertions.assertEquals;
import org.junit.jupiter.api.Test;
import net.sourceforge.pmd.lang.document.Chars;
class ASTNumericLiteralTest {
@Test
void testParseLongBinary() {
long literalLong = 0b0000000000000000000000000000000000000000100010001000010000010000L;
long parsedLong = ASTNumericLiteral.parseIntegralValue(Chars.wrap("0b0000000000000000000000000000000000000000100010001000010000010000L"));
assertEquals(literalLong, parsedLong); // in decimal: 8946704
}
@Test
void testParseSmallInt() {
int literalInt = 0x81;
long parsedInt = ASTNumericLiteral.parseIntegralValue(Chars.wrap("0x81"));
assertEquals((long) literalInt, parsedInt); // in decimal: 129
}
@Test
void testParseLongBigNegativeBinary() {
long literalLong = 0b1000_0000_0000_0000_0000_0000_0000_0000_0000_0000_1000_1000_1000_0100_0001_0000L;
long parsedLong = ASTNumericLiteral.parseIntegralValue(Chars.wrap("0b1000_0000_0000_0000_0000_0000_0000_0000_0000_0000_1000_1000_1000_0100_0001_0000L"));
assertEquals(literalLong, parsedLong); // in decimal: -9223372036845829104L
}
@Test
void malformedLiteral() {
assertEquals(0L, ASTNumericLiteral.parseIntegralValue(Chars.wrap("0x1g")));
assertEquals(0L, ASTNumericLiteral.parseIntegralValue(Chars.wrap("0x1_0000_0000_0000_0000L"))); // too big, 65bits
}
}

View File

@ -38,7 +38,7 @@ class XPathRuleTest {
}
@Test
void testPluginname() {
void testImageIsAccessibleAsFormatArgument() {
XPathRule rule = makeXPath("//VariableId[string-length(@Name) < 3]");
rule.setMessage("{0}");
Report report = getReportForTestString(rule, TEST1);

View File

@ -5,8 +5,7 @@
package net.sourceforge.pmd.lang.java.ast
import io.kotest.matchers.shouldBe
import net.sourceforge.pmd.lang.ast.test.NodeSpec
import net.sourceforge.pmd.lang.ast.test.ValuedNodeSpec
import net.sourceforge.pmd.lang.ast.test.*
import net.sourceforge.pmd.lang.ast.test.shouldBe
import net.sourceforge.pmd.lang.java.ast.JavaVersion.*
import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Earliest
@ -29,12 +28,14 @@ class ASTLiteralTest : ParserTestSpec({
"\"\"" should parseAs {
stringLit("\"\"") {
it::getConstValue shouldBe ""
it shouldHaveText "\"\""
}
}
"\"foo\"" should parseAs {
stringLit("\"foo\"") {
it::getConstValue shouldBe "foo"
it shouldHaveText "\"foo\""
}
}
@ -43,6 +44,14 @@ class ASTLiteralTest : ParserTestSpec({
it::getConstValue shouldBe "foo\t"
}
}
"(\"foo\\t\")" should parseAs {
stringLit("\"foo\\t\"") {
it::getConstValue shouldBe "foo\t"
it shouldHaveText "(\"foo\\t\")"
it::getParenthesisDepth shouldBe 1
}
}
}
}
@ -58,7 +67,7 @@ class ASTLiteralTest : ParserTestSpec({
this should parseAs {
textBlock {
it::getImage shouldBe this@testTextBlock.trim()
it.literalText.toString() shouldBe this@testTextBlock.trim()
contents()
}
}
@ -92,7 +101,7 @@ $delim
"""
$delim
Hi, "Bob"
Hi, "Alice"
$delim
""".testTextBlock()
@ -217,6 +226,12 @@ $delim
}
}
"('c')" should parseAs {
charLit("'c'") {
it::getConstValue shouldBe 'c'
}
}
"'\t'" should parseAs {
charLit("'\t'") {
it::getConstValue shouldBe '\t'
@ -261,29 +276,34 @@ $delim
it::getValueAsLong shouldBe 12L
it::getValueAsFloat shouldBe 12.0f
it::getValueAsDouble shouldBe 12.0
it::getImage shouldBe "12"
it.literalText.toString() shouldBe "12"
}
}
"1___234" should parseAs {
number(INT) {
it::getValueAsInt shouldBe 1234
it::getImage shouldBe "1___234"
it.literalText.toString() shouldBe "1___234"
}
}
"0b0000_0010" should parseAs {
number(INT) {
it::getValueAsInt shouldBe 2
it::getImage shouldBe "0b0000_0010"
it.literalText.toString() shouldBe "0b0000_0010"
}
}
"1234_5678_9012_3456L" should parseAs {
number(LONG) {
it::getValueAsLong shouldBe 1234_5678_9012_3456L
}
}
"-0X0000_000f" should parseAs { // this is not a float, it's hex
unaryExpr(UNARY_MINUS) {
number(INT) {
it::getImage shouldBe "0X0000_000f"
it.literalText.toString() shouldBe "0X0000_000f"
it::getValueAsInt shouldBe 15
it::getValueAsFloat shouldBe 15f
it::getValueAsDouble shouldBe 15.0
@ -297,7 +317,7 @@ $delim
it::getValueAsLong shouldBe 12L
it::getValueAsFloat shouldBe 12.0f
it::getValueAsDouble shouldBe 12.0
it::getImage shouldBe "12l"
it.literalText.toString() shouldBe "12l"
}
}
@ -305,8 +325,7 @@ $delim
number(LONG) {
it::getValueAsInt shouldBe 12
it::getValueAsLong shouldBe 12L
it::getImage shouldBe "12L"
it.literalText.toString() shouldBe "12L"
}
}
@ -315,7 +334,7 @@ $delim
it::getValueAsInt shouldBe 12
it::getValueAsFloat shouldBe 12.0f
it::getValueAsDouble shouldBe 12.0
it::getImage shouldBe "12d"
it.literalText.toString() shouldBe "12d"
}
}
@ -324,18 +343,17 @@ $delim
it::getValueAsInt shouldBe 12
it::getValueAsFloat shouldBe 12.0f
it::getValueAsDouble shouldBe 12.0
it::getImage shouldBe "12f"
it.literalText.toString() shouldBe "12f"
}
}
"-3_456.123_456" should parseAs {
"-3_456.12_3" should parseAs {
unaryExpr(UNARY_MINUS) {
number(DOUBLE) {
it::getValueAsInt shouldBe 3456
it::getValueAsFloat shouldBe 3456.123456f
it::getValueAsDouble shouldBe 3456.123456
it::getImage shouldBe "3_456.123_456"
it::getValueAsFloat shouldBe 3456.123f
it::getValueAsDouble shouldBe 3456.123
it.literalText.toString() shouldBe "3_456.12_3"
}
}
}

View File

@ -4,6 +4,7 @@
package net.sourceforge.pmd.lang.java.ast
import io.kotest.matchers.shouldBe
import net.sourceforge.pmd.lang.ast.test.shouldBe
class Java15KotlinTest : ParserTestSpec({
@ -29,7 +30,7 @@ class Java15KotlinTest : ParserTestSpec({
" </body>\n" +
"</html>\n"
it::getImage shouldBe tblock
it.literalText.toString() shouldBe tblock
}
}
}

View File

@ -437,7 +437,7 @@ fun TreeNodeWrapper<Node, *>.castExpr(contents: NodeSpec<ASTCastExpression>) =
fun TreeNodeWrapper<Node, *>.stringLit(image: String, contents: NodeSpec<ASTStringLiteral> = EmptyAssertions) =
child<ASTStringLiteral> {
it::getImage shouldBe image
it.literalText.toString() shouldBe image
it::isTextBlock shouldBe false
it::isEmpty shouldBe it.constValue.isEmpty()
contents()
@ -446,7 +446,7 @@ fun TreeNodeWrapper<Node, *>.stringLit(image: String, contents: NodeSpec<ASTStri
fun TreeNodeWrapper<Node, *>.charLit(image: String, contents: NodeSpec<ASTCharLiteral> = EmptyAssertions) =
child<ASTCharLiteral> {
it::getImage shouldBe image
it.literalText.toString() shouldBe image
contents()
}
@ -701,7 +701,7 @@ fun TreeNodeWrapper<Node, *>.annotationMethod(contents: NodeSpec<ASTMethodDeclar
fun TreeNodeWrapper<Node, *>.classDecl(simpleName: String, assertions: NodeSpec<ASTClassDeclaration> = EmptyAssertions) =
child<ASTClassDeclaration>(ignoreChildren = assertions == EmptyAssertions) {
it::getImage shouldBe simpleName
it::getSimpleName shouldBe simpleName
assertions()
}

View File

@ -5,6 +5,7 @@
package net.sourceforge.pmd.lang.ast.test
import net.sourceforge.pmd.lang.ast.Node
import net.sourceforge.pmd.lang.document.Chars
import net.sourceforge.pmd.lang.rule.xpath.Attribute
import net.sourceforge.pmd.util.StringUtil
import net.sourceforge.pmd.util.treeexport.TextTreeRenderer
@ -87,7 +88,7 @@ open class BaseNodeAttributePrinter : TextTreeRenderer(true, -1) {
protected open fun valueToString(value: Any?): String? {
return when (value) {
is String -> "\"" + StringUtil.escapeJava(value) + "\""
is String, is Chars -> "\"" + StringUtil.escapeJava(value.toString()) + "\""
is Char -> '\''.toString() + value.toString().replace("'".toRegex(), "\\'") + '\''.toString()
is Enum<*> -> value.enumDeclaringClass.simpleName + "." + value.name
is Class<*> -> value.canonicalName?.let { "$it.class" }