Improve NumberNode

smithy-model/src/main/java/software/amazon/smithy/model/node/NumberNode.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License").
  * You may not use this file except in compliance with the License.
@@ -31,17 +31,45 @@
  */
 public final class NumberNode extends Node {
 
-    private final Number value;
+    private final BigDecimal value;
+    private final Number originalValue;
     private final String stringCache;
-    private volatile BigDecimal equality;
+    private boolean isNaN;
+    private boolean isPositiveInfinity;
+    private boolean isNegativeInfinity;
 
     public NumberNode(Number value, SourceLocation sourceLocation) {
         super(sourceLocation);
-        this.value = Objects.requireNonNull(value);
+        originalValue = value;
         stringCache = value.toString();
+        this.value = toBigDecimal(originalValue);
+    }
 
+    private BigDecimal toBigDecimal(Number value) {
         if (value instanceof BigDecimal) {
-            equality = (BigDecimal) value;
+            return (BigDecimal) value;
+        } else if (value instanceof Integer || value instanceof Long || value instanceof Short
+                   || value instanceof Byte) {
+            return BigDecimal.valueOf(value.longValue());
+        } else if (value instanceof Float || value instanceof Double) {
+            double d = value.doubleValue();
+            if (Double.isNaN(d)) {
+                isNaN = true;
+                return null;
+            } else if (Double.isInfinite(d)) {
+                if (stringCache.startsWith("-")) {
+                    isNegativeInfinity = true;
+                } else {
+                    isPositiveInfinity = true;
+                }
+                return null;
+            } else {
+                return BigDecimal.valueOf(d);
+            }
+        } else if (value instanceof BigInteger) {
+            return new BigDecimal((BigInteger) value);
+        } else {
+            return new BigDecimal(stringCache);
         }
     }
 
@@ -51,28 +79,61 @@ public NumberNode(Number value, SourceLocation sourceLocation) {
      * @return Returns a number.
      */
     public Number getValue() {
-        return value;
+        return originalValue;
     }
 
     /**
-     * Returns true if the node contains a natural number.
+     * Gets the number value as a BigDecimal if possible.
      *
-     * @return Returns true if the node contains a natural number.
+     * <p>NaN and infinite numbers will return an empty Optional.
+     *
+     * @return Returns the BigDecimal value of the wrapped number.
      */
+    public Optional<BigDecimal> asBigDecimal() {
+        return Optional.ofNullable(value);
+    }
+
+    @Deprecated
     public boolean isNaturalNumber() {
         return !isFloatingPointNumber();
     }
 
+    /**
+     * Check the value is negative, including negative infinity.
+     *
+     * <p>Any number >= 0, +Infinity, and NaN return false.
+     *
+     * @return Return true if negative.
+     */
+    public boolean isNegative() {
+        return isNegativeInfinity || (value != null && value.compareTo(BigDecimal.ZERO) < 0);
+    }
+
     /**
      * Returns true if the node contains a floating point number.
      *
      * @return Returns true if the node contains a floating point number.
      */
     public boolean isFloatingPointNumber() {
-        return value instanceof Float
-                || value instanceof Double
-                || value instanceof BigDecimal
-                || stringCache.contains(".");
+        return value == null || value.scale() > 0 || toString().contains(".");
+    }
+
+    /**
+     * Returns true if the number is a floating point NaN.
+     *
+     * @return Return true if NaN.
+     */
+    public boolean isNaN() {
+        return isNaN;
+    }
+
+    /**
+     * Returns true if the number is infinite.
+     *
+     * @return Return true if infinite.
+     */
+    public boolean isInfinite() {
+        return isPositiveInfinity || isNegativeInfinity;
     }
 
     @Override
@@ -117,19 +178,7 @@ public Optional<NumberNode> asNumberNode() {
      * @return Returns true if set to zero.
      */
     public boolean isZero() {
-        // Do a cheap test based on the serialized value of the number first.
-        // This test covers byte, short, integer, and long.
-        if (toString().equals("0") || toString().equals("0.0")) {
-            return true;
-        } else if (value instanceof BigDecimal) {
-            return value.equals(BigDecimal.ZERO);
-        } else if (value instanceof BigInteger) {
-            return value.equals(BigInteger.ZERO);
-        } else if (value instanceof Float) {
-            return value.floatValue() == 0f;
-        } else {
-            return value.doubleValue() == 0d;
-        }
+        return value != null && value.compareTo(BigDecimal.ZERO) == 0;
     }
 
     @Override
@@ -139,42 +188,12 @@ public boolean equals(Object other) {
         } else if (other == this) {
             return true;
         } else {
-            NumberNode otherNode = (NumberNode) other;
-
-            // This only works if both values are the same type.
-            if (value.equals(otherNode.value)) {
-                return true;
-            }
-
-            // Attempt a cheap check based on the string cache.
-            if (stringCache.equals(otherNode.stringCache)) {
-                return true;
-            }
-
-            // Convert both to BigDecimal and compare equality.
-            return getEquality().equals(otherNode.getEquality());
-        }
-    }
-
-    private BigDecimal getEquality() {
-        BigDecimal e = equality;
-
-        if (e == null) {
-            if (value instanceof Integer) {
-                e = BigDecimal.valueOf(value.intValue());
-            } else if (value instanceof Short) {
-                e = BigDecimal.valueOf(value.shortValue());
-            } else if (value instanceof Byte) {
-                e = BigDecimal.valueOf(value.byteValue());
-            } else if (value instanceof Long) {
-                e = BigDecimal.valueOf(value.longValue());
-            } else {
-                e = new BigDecimal(stringCache);
-            }
-            equality = e;
+            NumberNode o = (NumberNode) other;
+            return isNaN == o.isNaN
+                   && isPositiveInfinity == o.isPositiveInfinity
+                   && isNegativeInfinity == o.isNegativeInfinity
+                   && Objects.equals(value, o.value);
         }
-
-        return e;
     }
 
     @Override

smithy-model/src/main/java/software/amazon/smithy/model/validation/NodeValidationVisitor.java

@@ -218,7 +218,7 @@ public List<ValidationEvent> bigIntegerShape(BigIntegerShape shape) {
     private List<ValidationEvent> validateNaturalNumber(Shape shape, Long min, Long max) {
         return value.asNumberNode()
                 .map(number -> {
-                    if (!number.isNaturalNumber()) {
+                    if (number.isFloatingPointNumber()) {
                         return ListUtils.of(event(String.format(
                                 "%s shapes must not have floating point values, but found `%s` provided for `%s`",
                                 shape.getType(), number.getValue(), shape.getId())));

smithy-model/src/test/java/software/amazon/smithy/model/node/NumberNodeTest.java

@@ -17,6 +17,7 @@
 
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.not;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -199,6 +200,11 @@ public float floatValue() {
             public double doubleValue() {
                 return 0;
             }
+
+            @Override
+            public String toString() {
+                return "0.000";
+            }
         }, true);
         cases.put(new Number() {
             @Override
@@ -283,4 +289,14 @@ public void compareDoubleAndFloat() {
 
         assertThat(left, equalTo(right));
     }
+
+    @Test
+    public void detectsNegativeValues() {
+        assertThat(NumberNode.from(Double.NEGATIVE_INFINITY).isNegative(), is(true));
+        assertThat(NumberNode.from(Double.POSITIVE_INFINITY).isNegative(), is(false));
+        assertThat(NumberNode.from(Double.NaN).isNegative(), is(false));
+        assertThat(NumberNode.from(0).isNegative(), is(false));
+        assertThat(NumberNode.from(1).isNegative(), is(false));
+        assertThat(NumberNode.from(-1).isNegative(), is(true));
+    }
 }