Repository: apollographql/apollo-rs
Pull request #375 (2022-11-28 12:04:40)

Add coercion of hir::Value::Int to Float input value without losing precision
Previously the HIR stored an `i32` for values with integer syntax, restricting the supported range if the value ended up used as a `Float`.

Now it stores `f64` for all numeric literals (though still with `Int` and `Float` variants). Coercion to `Int` is a separate fallible method.


Patch:
diff --git a/crates/apollo-compiler/src/database/hir.rs b/crates/apollo-compiler/src/database/hir.rs
index 477daa3..a67e20e 100644
--- a/crates/apollo-compiler/src/database/hir.rs
+++ b/crates/apollo-compiler/src/database/hir.rs
@@ -882,7 +882,16 @@ pub type DefaultValue = Value;
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub enum Value {
     Variable(Variable),
-    Int(i32),
+
+    // A value of integer syntax may be coerced to a Float input value:
+    // https://spec.graphql.org/draft/#sec-Float.Input-Coercion
+    // Keep a f64 here instead of i32 in order to support
+    // the full range of f64 integer values for that case.
+    //
+    // All i32 values can be represented exactly in f64,
+    // so conversion to an Int input value is still exact:
+    // https://spec.graphql.org/draft/#sec-Int.Input-Coercion
+    Int(Float),
     Float(Float),
     String(String),
     Boolean(bool),
@@ -902,6 +911,82 @@ impl Value {
     }
 }
 
+/// Coerce to a `Float` input type (from either `Float` or `Int` syntax)
+///
+/// <https://spec.graphql.org/draft/#sec-Float.Input-Coercion>
+impl TryFrom<Value> for f64 {
+    type Error = FloatCoercionError;
+
+    #[inline]
+    fn try_from(value: Value) -> Result<Self, Self::Error> {
+        f64::try_from(&value)
+    }
+}
+
+/// Coerce to a `Float` input type (from either `Float` or `Int` syntax)
+///
+/// <https://spec.graphql.org/draft/#sec-Float.Input-Coercion>
+impl TryFrom<&'_ Value> for f64 {
+    type Error = FloatCoercionError;
+
+    fn try_from(value: &'_ Value) -> Result<Self, Self::Error> {
+        if let Value::Int(float) | Value::Float(float) = value {
+            // FIXME: what does "a value outside the available precision" mean?
+            // Should coercion fail when f64 does not have enough mantissa bits
+            // to represent the source token exactly?
+            Ok(float.inner.0)
+        } else {
+            Err(FloatCoercionError(()))
+        }
+    }
+}
+
+#[derive(thiserror::Error, Debug)]
+#[error("coercing a non-numeric value to a `Float` input value")]
+pub struct FloatCoercionError(());
+
+/// Coerce to an `Int` input type
+///
+/// <https://spec.graphql.org/draft/#sec-Int.Input-Coercion>
+impl TryFrom<Value> for i32 {
+    type Error = IntCoercionError;
+
+    #[inline]
+    fn try_from(value: Value) -> Result<Self, Self::Error> {
+        i32::try_from(&value)
+    }
+}
+
+/// Coerce to an `Int` input type
+///
+/// <https://spec.graphql.org/draft/#sec-Int.Input-Coercion>
+impl TryFrom<&'_ Value> for i32 {
+    type Error = IntCoercionError;
+
+    fn try_from(value: &'_ Value) -> Result<Self, Self::Error> {
+        if let Value::Int(float) = value {
+            let float = float.inner.0;
+            // The parser emitted an `ast::IntValue` instead of `ast::FloatValue`
+            // so we already know `float` does not have a frational part.
+            if float <= (i32::MAX as f64) && float >= (i32::MIN as f64) {
+                Ok(float as i32)
+            } else {
+                Err(IntCoercionError::RangeOverflow)
+            }
+        } else {
+            Err(IntCoercionError::NotAnInteger)
+        }
+    }
+}
+
+#[derive(thiserror::Error, Debug)]
+pub enum IntCoercionError {
+    #[error("coercing a non-integer value to an `Int` input value")]
+    NotAnInteger,
+    #[error("integer input value overflows the signed 32-bit range")]
+    RangeOverflow,
+}
+
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct Variable {
     pub(crate) name: String,
@@ -2238,3 +2323,40 @@ impl InputObjectTypeExtension {
         syntax_node_ptr.to_node(&rowan::SyntaxNode::new_root(db.document()))
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use crate::ApolloCompiler;
+    use crate::DocumentDatabase;
+
+    #[test]
+    fn huge_floats() {
+        let compiler = ApolloCompiler::new(
+            "input HugeFloats {
+                a: Float = 9876543210
+                b: Float = 9876543210.0
+                c: Float = 98765432109876543210
+                d: Float = 98765432109876543210.0
+            }",
+        );
+        let default_values: Vec<_> = compiler
+            .db
+            .find_input_object_by_name("HugeFloats".into())
+            .unwrap()
+            .input_fields_definition
+            .iter()
+            .map(|field| {
+                f64::try_from(field.default_value().unwrap())
+                    .unwrap()
+                    .to_string()
+            })
+            .collect();
+        // The exact value is preserved, even outside of the range of i32
+        assert_eq!(default_values[0], "9876543210");
+        assert_eq!(default_values[1], "9876543210");
+        // Beyond ~53 bits of mantissa we may lose precision,
+        // but this is approximation is still in the range of finite f64 values.
+        assert_eq!(default_values[2], "98765432109876540000");
+        assert_eq!(default_values[3], "98765432109876540000");
+    }
+}
diff --git a/crates/apollo-compiler/src/database/hir_db.rs b/crates/apollo-compiler/src/database/hir_db.rs
index 151e31b..2609f07 100644
--- a/crates/apollo-compiler/src/database/hir_db.rs
+++ b/crates/apollo-compiler/src/database/hir_db.rs
@@ -942,7 +942,7 @@ fn value(val: ast::Value) -> Value {
         ast::Value::StringValue(string_val) => Value::String(string_val.into()),
         // TODO(@goto-bus-stop) do not unwrap
         ast::Value::FloatValue(float) => Value::Float(Float::new(float.try_into().unwrap())),
-        ast::Value::IntValue(int) => Value::Int(int.try_into().unwrap()),
+        ast::Value::IntValue(int) => Value::Int(Float::new(f64::try_from(int).unwrap())),
         ast::Value::BooleanValue(bool) => Value::Boolean(bool.try_into().unwrap()),
         ast::Value::NullValue(_) => Value::Null,
         ast::Value::EnumValue(enum_) => Value::Enum(name(enum_.name())),


Test patch:
diff --git a/crates/apollo-compiler/test_data/ok/0014_float_values.graphql b/crates/apollo-compiler/test_data/ok/0014_float_values.graphql
new file mode 100644
index 0000000..c7291af
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0014_float_values.graphql
@@ -0,0 +1,6 @@
+input WithAllKindsOfFloats {
+  a_regular_float: Float = 1.2
+  an_integer_float: Float = 1234
+  a_float_that_doesnt_fit_an_int: Float = 9876543210
+  list_of_floats: [Float] = [4, 9876543210, 98765432109876543210]
+}
diff --git a/crates/apollo-compiler/test_data/ok/0014_float_values.txt b/crates/apollo-compiler/test_data/ok/0014_float_values.txt
new file mode 100644
index 0000000..00c8a02
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0014_float_values.txt
@@ -0,0 +1,87 @@
+- DOCUMENT@0..214
+    - INPUT_OBJECT_TYPE_DEFINITION@0..214
+        - input_KW@0..5 "input"
+        - WHITESPACE@5..6 " "
+        - NAME@6..27
+            - IDENT@6..26 "WithAllKindsOfFloats"
+            - WHITESPACE@26..27 " "
+        - INPUT_FIELDS_DEFINITION@27..214
+            - L_CURLY@27..28 "{"
+            - WHITESPACE@28..31 "\n  "
+            - INPUT_VALUE_DEFINITION@31..62
+                - NAME@31..46
+                    - IDENT@31..46 "a_regular_float"
+                - COLON@46..47 ":"
+                - WHITESPACE@47..48 " "
+                - NAMED_TYPE@48..53
+                    - NAME@48..53
+                        - IDENT@48..53 "Float"
+                - WHITESPACE@53..54 " "
+                - DEFAULT_VALUE@54..62
+                    - EQ@54..55 "="
+                    - WHITESPACE@55..56 " "
+                    - FLOAT_VALUE@56..62
+                        - FLOAT@56..59 "1.2"
+                        - WHITESPACE@59..62 "\n  "
+            - INPUT_VALUE_DEFINITION@62..95
+                - NAME@62..78
+                    - IDENT@62..78 "an_integer_float"
+                - COLON@78..79 ":"
+                - WHITESPACE@79..80 " "
+                - NAMED_TYPE@80..85
+                    - NAME@80..85
+                        - IDENT@80..85 "Float"
+                - WHITESPACE@85..86 " "
+                - DEFAULT_VALUE@86..95
+                    - EQ@86..87 "="
+                    - WHITESPACE@87..88 " "
+                    - INT_VALUE@88..95
+                        - INT@88..92 "1234"
+                        - WHITESPACE@92..95 "\n  "
+            - INPUT_VALUE_DEFINITION@95..148
+                - NAME@95..125
+                    - IDENT@95..125 "a_float_that_doesnt_fit_an_int"
+                - COLON@125..126 ":"
+                - WHITESPACE@126..127 " "
+                - NAMED_TYPE@127..132
+                    - NAME@127..132
+                        - IDENT@127..132 "Float"
+                - WHITESPACE@132..133 " "
+                - DEFAULT_VALUE@133..148
+                    - EQ@133..134 "="
+                    - WHITESPACE@134..135 " "
+                    - INT_VALUE@135..148
+                        - INT@135..145 "9876543210"
+                        - WHITESPACE@145..148 "\n  "
+            - INPUT_VALUE_DEFINITION@148..212
+                - NAME@148..162
+                    - IDENT@148..162 "list_of_floats"
+                - COLON@162..163 ":"
+                - WHITESPACE@163..164 " "
+                - LIST_TYPE@164..171
+                    - L_BRACK@164..165 "["
+                    - NAMED_TYPE@165..170
+                        - NAME@165..170
+                            - IDENT@165..170 "Float"
+                    - R_BRACK@170..171 "]"
+                - WHITESPACE@171..172 " "
+                - DEFAULT_VALUE@172..212
+                    - EQ@172..173 "="
+                    - WHITESPACE@173..174 " "
+                    - LIST_VALUE@174..212
+                        - L_BRACK@174..175 "["
+                        - INT_VALUE@175..178
+                            - INT@175..176 "4"
+                            - COMMA@176..177 ","
+                            - WHITESPACE@177..178 " "
+                        - INT_VALUE@178..190
+                            - INT@178..188 "9876543210"
+                            - COMMA@188..189 ","
+                            - WHITESPACE@189..190 " "
+                        - INT_VALUE@190..210
+                            - INT@190..210 "98765432109876543210"
+                        - R_BRACK@210..211 "]"
+                        - WHITESPACE@211..212 "\n"
+            - R_CURLY@212..213 "}"
+            - WHITESPACE@213..214 "\n"
+recursion limit: 4096, high: 0