Improve NumberNode

This change now makes the BigDecimal get created eagerly, and accounts
for NaN and Infinity. The previous implementation would have failed
in these cases. NodeValidationVisitor now uses the computed BigDecimal
rather than create a one-off each time. It also doesn't recreate the
already available BigDecimcal of the range trait.

This change deprecates isNaturalNumber since that method was poorly
named. A natural number is an integer >= 1, but we were including 0
(which is likely why some callers assumed it asserts >= 0).

This also adds an isNegative method since multiple callers need to
assert that a NumberNode contains a non-negative value.

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