Fix number typing used in maps (#7676)

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.

Author: jpbempel

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/PrettyPrintVisitor.java

@@ -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;
@@ -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";

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/expressions/ComparisonOperator.java

@@ -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;
         }
@@ -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;
       }

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/values/ListValue.java

@@ -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;

dd-java-agent/agent-debugger/debugger-el/src/main/java/com/datadog/debugger/el/values/NumericValue.java

@@ -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) {
@@ -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 + '}';

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/ValueScriptTest.java

@@ -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,

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ComparisonExpressionTest.java

@@ -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"),

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/LenExpressionTest.java

@@ -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));
   }
 }

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/expressions/ValueRefExpressionTest.java

@@ -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 =

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/values/ListValueTest.java

@@ -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);
       }
@@ -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));

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/values/MapValueTest.java

@@ -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));
+  }
 }

dd-java-agent/agent-debugger/debugger-el/src/test/java/com/datadog/debugger/el/values/NumericValueTest.java

@@ -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));
   }
 
@@ -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));
   }
 
@@ -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));
   }
 
@@ -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));
   }
 

dd-java-agent/agent-debugger/src/main/java/com/datadog/debugger/instrumentation/MetricInstrumentor.java

@@ -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());

dd-smoke-tests/debugger-integration-tests/src/test/java/datadog/smoketest/MoshiConfigTestHelper.java

@@ -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());