Skip to content

Commit

Permalink
Fix number typing used in maps
Browse files Browse the repository at this point in the history
We are automatically widening numbers from Integer to Long and Float
to Double to be able to compare them. But used as key for maps it does
 not work because the type needs to be exact.
For fixing this we are moving to widening then numbers only at
comparison time. Otherwise we keep the original typing.
  • Loading branch information
jpbempel committed Sep 24, 2024
1 parent 67342bb commit 30a4b6c
Show file tree
Hide file tree
Showing 13 changed files with 50 additions and 42 deletions.
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

0 comments on commit 30a4b6c

Please sign in to comment.