Skip to content

Fix number typing used in maps #7676

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,6 @@
import com.datadog.debugger.el.values.SetValue;
import com.datadog.debugger.el.values.StringValue;
import datadog.trace.bootstrap.debugger.el.Values;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -226,13 +224,10 @@ public String visit(ObjectValue objectValue) {
@Override
public String visit(NumericValue numericValue) {
Number value = numericValue.value;
if (value instanceof Double) {
return String.valueOf(value.doubleValue());
}
if (value instanceof Long) {
return String.valueOf(value.longValue());
}
if (value instanceof BigDecimal || value instanceof BigInteger) {
if (value != null) {
if (value instanceof Double || value instanceof Float) {
return String.valueOf(value.doubleValue());
}
return value.toString();
}
return "null";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ public enum ComparisonOperator {
@Override
public Boolean apply(Value<?> left, Value<?> right) {
if (left instanceof NumericValue && right instanceof NumericValue) {
Number leftNumber = (Number) left.getValue();
Number rightNumber = (Number) right.getValue();
Number leftNumber = ((NumericValue) left).getWidenValue();
Number rightNumber = ((NumericValue) right).getWidenValue();
if (isNan(leftNumber, rightNumber)) {
return Boolean.FALSE;
}
Expand Down Expand Up @@ -152,8 +152,8 @@ protected static boolean isNan(Number... numbers) {

protected static Integer compare(Value<?> left, Value<?> right) {
if (left instanceof NumericValue && right instanceof NumericValue) {
Number leftNumber = (Number) left.getValue();
Number rightNumber = (Number) right.getValue();
Number leftNumber = ((NumericValue) left).getWidenValue();
Number rightNumber = ((NumericValue) right).getWidenValue();
if (isNan(leftNumber, rightNumber)) {
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ public boolean contains(Value<?> val) {
}
}
} else if (arrayType == long.class) {
long longValue = (Long) val.getValue();
long longValue = ((Number) val.getValue()).longValue();
for (int i = 0; i < count; i++) {
if (Array.getLong(arrayHolder, i) == longValue) {
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
/** A numeric {@linkplain com.datadog.debugger.el.Value} */
public final class NumericValue extends Literal<Number> {
public NumericValue(Number value) {
super(widen(value));
super(value);
}

private static Number widen(Number value) {
Expand All @@ -19,6 +19,10 @@ private static Number widen(Number value) {
return value;
}

public Number getWidenValue() {
return widen(getValue());
}

@Override
public String toString() {
return "NumericLiteral{" + "value=" + value + '}';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ public void topLevelPrimitives() {
new Object[] {
"hello",
"hello",
10L,
10,
100_000_000_000L,
2.5D,
2.5F,
3.14D,
"a",
"b",
5L,
3L,
5,
3,
"el",
Values.NULL_OBJECT,
Boolean.TRUE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,10 @@ private static Stream<Arguments> expressions() {
"null instanceof \"java.lang.String\""),
Arguments.of(
new NumericValue(1),
new StringValue("java.lang.Long"),
new StringValue("java.lang.Integer"),
INSTANCEOF,
true,
"1 instanceof \"java.lang.Long\""),
"1 instanceof \"java.lang.Integer\""),
Arguments.of(
new NumericValue(1.0),
new StringValue("java.lang.Double"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,24 @@ void undefinedExpression() {
@Test
void stringExpression() {
LenExpression expression = new LenExpression(DSL.value("a"));
assertEquals(1L, expression.evaluate(resolver).getValue());
assertEquals(1, expression.evaluate(resolver).getValue());
assertEquals("len(\"a\")", print(expression));
}

@Test
void collectionExpression() {
LenExpression expression = new LenExpression(DSL.value(Arrays.asList("a", "b")));
assertEquals(2L, expression.evaluate(resolver).getValue());
assertEquals(2, expression.evaluate(resolver).getValue());
assertEquals("len(List)", print(expression));
expression = new LenExpression(DSL.value(new HashSet<>(Arrays.asList("a", "b"))));
assertEquals(2L, expression.evaluate(resolver).getValue());
assertEquals(2, expression.evaluate(resolver).getValue());
assertEquals("len(Set)", print(expression));
}

@Test
void mapExpression() {
LenExpression expression = new LenExpression(DSL.value(Collections.singletonMap("a", "b")));
assertEquals(1L, expression.evaluate(resolver).getValue());
assertEquals(1, expression.evaluate(resolver).getValue());
assertEquals("len(Map)", print(expression));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ class Obj {
assertEquals("Hello there", expression.evaluate(resolver).getValue());
assertEquals("msg", print(expression));
expression = DSL.ref("i");
assertEquals(
(long) 6, expression.evaluate(resolver).getValue()); // int value is widened to long
assertEquals(6, expression.evaluate(resolver).getValue()); // int value is widened to long
assertEquals("i", print(expression));
ValueRefExpression invalidExpression = ref(ValueReferences.synthetic("invalid"));
RuntimeException runtimeException =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,7 @@ void testFromArray() {
Value<?> v = listValue.get(i);
assertNotNull(v);
if (expected != null) {
assertEquals(
((Integer) array[i]).longValue(), v.getValue()); // int is automatically widened to long
assertEquals(array[i], v.getValue());
} else {
assertEquals(Value.nullValue(), v);
}
Expand All @@ -78,7 +77,7 @@ void testFromArrayOfArrays() {
ListValue collection = (ListValue) v;
for (int j = 0; j < collection.count(); j++) {
Value<?> v1 = collection.get(j);
assertEquals((long) intArray[i][j], v1.getValue()); // int is automatically widened to long
assertEquals(intArray[i][j], v1.getValue()); // int is automatically widened to long
}
}
assertThrows(IllegalArgumentException.class, () -> listValue.get(-1));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,17 @@ void get() {
assertEquals(Value.undefinedValue(), instance.get(Values.UNDEFINED_OBJECT));
assertEquals(Value.undefinedValue(), instance.get(Value.undefinedValue()));
}

@Test
void intMap() {
Map<Integer, Integer> map = new HashMap<>();
map.put(1, 1);
map.put(2, 2);
instance = new MapValue(map);
assertEquals(2, instance.count());
assertEquals(Value.of(1), instance.get(1));
assertEquals(Value.of(2), instance.get(2));
Value<?> key = Value.of(1);
assertEquals(Value.of(1), instance.get(key));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,7 @@ void testByteLiteral() {
NumericValue instance = new NumericValue(expected);
assertFalse(instance.isNull());
assertFalse(instance.isUndefined());
assertNotEquals(expected, instance.getValue());
assertEquals((long) expected, instance.getValue());
assertEquals(expected, instance.getValue());
assertEquals("1", print(instance));
}

Expand All @@ -34,8 +33,7 @@ void testShortLiteral() {
NumericValue instance = new NumericValue(expected);
assertFalse(instance.isNull());
assertFalse(instance.isUndefined());
assertNotEquals(expected, instance.getValue());
assertEquals((long) expected, instance.getValue());
assertEquals(expected, instance.getValue());
assertEquals("1", print(instance));
}

Expand All @@ -45,8 +43,7 @@ void testIntLiteral() {
NumericValue instance = new NumericValue(expected);
assertFalse(instance.isNull());
assertFalse(instance.isUndefined());
assertNotEquals(expected, instance.getValue());
assertEquals((long) expected, instance.getValue());
assertEquals(expected, instance.getValue());
assertEquals("1", print(instance));
}

Expand All @@ -66,7 +63,7 @@ void testFloatLiteral() {
NumericValue instance = new NumericValue(expected);
assertFalse(instance.isNull());
assertFalse(instance.isUndefined());
assertEquals((double) expected, instance.getValue());
assertEquals(expected, instance.getValue());
assertEquals("1.0", print(instance));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ public VisitorResult visit(StringValue stringValue) {

@Override
public VisitorResult visit(NumericValue numericValue) {
Number number = numericValue.getValue();
Number number = numericValue.getWidenValue();
InsnList insnList = new InsnList();
if (number instanceof Long) {
ldc(insnList, number.longValue());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -296,10 +296,11 @@ public Void visit(StringValue stringValue) {
@Override
public Void visit(NumericValue numericValue) {
try {
if (numericValue.getValue() instanceof Long) {
jsonWriter.value(numericValue.getValue().longValue());
} else if (numericValue.getValue() instanceof Double) {
jsonWriter.value(numericValue.getValue().doubleValue());
Number widenValue = numericValue.getWidenValue();
if (widenValue instanceof Long) {
jsonWriter.value(widenValue.longValue());
} else if (widenValue instanceof Double) {
jsonWriter.value(widenValue.doubleValue());
} else {
throw new UnsupportedOperationException(
"numeric value unsupported:" + numericValue.getValue().getClass());
Expand Down