NaN values should be checked inside scalar's serialize method

Cito · Cito · commit 3ddb4b635eb2 · 2020-03-03T17:29:14.000+01:00
The is_invalid and is_nullish functions now are not needed any more and have been removed from the pyutils package. Replicates graphql/graphql-js@c9850c1
diff --git a/docs/modules/pyutils.rst b/docs/modules/pyutils.rst
@@ -20,8 +20,6 @@ PyUtils
 .. autofunction:: inspect
 .. autofunction:: is_finite
 .. autofunction:: is_integer
-.. autofunction:: is_invalid
-.. autofunction:: is_nullish
 .. autoclass:: AwaitableOrValue
    :members:
 .. autofunction:: suggestion_list
diff --git a/src/graphql/execution/execute.py b/src/graphql/execution/execute.py
@@ -28,8 +28,6 @@
 )
 from ..pyutils import (
     inspect,
-    is_invalid,
-    is_nullish,
     AwaitableOrValue,
     FrozenList,
     Path,
@@ -756,8 +754,8 @@ def complete_value(
                 )
             return completed
 
-        # If result value is null-ish (null, Undefined, or NaN) then return null.
-        if is_nullish(result):
+        # If result value is null or undefined then return null.
+        if result is None or result is Undefined:
             return None
 
         # If field type is List, complete each item in the list with inner type
@@ -852,7 +850,7 @@ def complete_leaf_value(return_type: GraphQLLeafType, result: Any) -> Any:
         serialization is not possible.
         """
         serialized_result = return_type.serialize(result)
-        if is_invalid(serialized_result):
+        if serialized_result is Undefined:
             raise TypeError(
                 f"Expected a value of type '{inspect(return_type)}'"
                 f" but received: {inspect(result)}"
diff --git a/src/graphql/pyutils/__init__.py b/src/graphql/pyutils/__init__.py
@@ -24,8 +24,6 @@
 from .is_collection import is_collection
 from .is_finite import is_finite
 from .is_integer import is_integer
-from .is_invalid import is_invalid
-from .is_nullish import is_nullish
 from .awaitable_or_value import AwaitableOrValue
 from .suggestion_list import suggestion_list
 from .frozen_error import FrozenError
@@ -52,8 +50,6 @@
     "is_collection",
     "is_finite",
     "is_integer",
-    "is_invalid",
-    "is_nullish",
     "AwaitableOrValue",
     "suggestion_list",
     "FrozenError",
diff --git a/src/graphql/pyutils/is_invalid.py b/src/graphql/pyutils/is_invalid.py
diff --git a/src/graphql/pyutils/is_nullish.py b/src/graphql/pyutils/is_nullish.py
diff --git a/src/graphql/utilities/ast_from_value.py b/src/graphql/utilities/ast_from_value.py
@@ -1,5 +1,5 @@
-from math import isfinite
 import re
+from math import isfinite
 from typing import Any, Iterable, Mapping, Optional, cast
 
 from ..language import (
@@ -15,7 +15,7 @@
     StringValueNode,
     ValueNode,
 )
-from ..pyutils import FrozenList, inspect, is_nullish, is_invalid
+from ..pyutils import inspect, FrozenList, Undefined
 from ..type import (
     GraphQLID,
     GraphQLInputType,
@@ -67,8 +67,8 @@ def ast_from_value(value: Any, type_: GraphQLInputType) -> Optional[ValueNode]:
     if value is None:
         return NullValueNode()
 
-    # Undefined or NaN
-    if is_invalid(value):
+    # undefined
+    if value is Undefined:
         return None
 
     # Convert Python list to GraphQL list. If the GraphQLType is a list, but the value
@@ -104,7 +104,7 @@ def ast_from_value(value: Any, type_: GraphQLInputType) -> Optional[ValueNode]:
         # Since value is an internally represented value, it must be serialized to an
         # externally represented value before converting into an AST.
         serialized = type_.serialize(value)  # type: ignore
-        if is_nullish(serialized):
+        if serialized is None or serialized is Undefined:
             return None
 
         # Others serialize based on their corresponding Python scalar types.
diff --git a/src/graphql/utilities/value_from_ast.py b/src/graphql/utilities/value_from_ast.py
@@ -7,7 +7,7 @@
     ValueNode,
     VariableNode,
 )
-from ..pyutils import inspect, is_invalid, Undefined
+from ..pyutils import inspect, Undefined
 from ..type import (
     GraphQLInputObjectType,
     GraphQLInputType,
@@ -88,12 +88,12 @@ def value_from_ast(
                     append_value(None)
                 else:
                     item_value = value_from_ast(item_node, item_type, variables)
-                    if is_invalid(item_value):
+                    if item_value is Undefined:
                         return Undefined
                     append_value(item_value)
             return coerced_values
         coerced_value = value_from_ast(value_node, item_type, variables)
-        if is_invalid(coerced_value):
+        if coerced_value is Undefined:
             return Undefined
         return [coerced_value]
 
@@ -114,7 +114,7 @@ def value_from_ast(
                     return Undefined
                 continue
             field_value = value_from_ast(field_node.value, field.type, variables)
-            if is_invalid(field_value):
+            if field_value is Undefined:
                 return Undefined
             coerced_obj[field.out_name or field_name] = field_value
 
@@ -143,5 +143,5 @@ def is_missing_variable(
 ) -> bool:
     """Check if `value_node` is a variable not defined in the `variables` dict."""
     return isinstance(value_node, VariableNode) and (
-        not variables or is_invalid(variables.get(value_node.name.value, Undefined))
+        not variables or variables.get(value_node.name.value, Undefined) is Undefined
     )
diff --git a/src/graphql/validation/rules/values_of_correct_type.py b/src/graphql/validation/rules/values_of_correct_type.py
@@ -14,7 +14,7 @@
     ValueNode,
     print_ast,
 )
-from ...pyutils import did_you_mean, is_invalid, suggestion_list
+from ...pyutils import did_you_mean, suggestion_list, Undefined
 from ...type import (
     GraphQLScalarType,
     get_named_type,
@@ -130,7 +130,7 @@ def is_valid_value_node(self, node: ValueNode) -> None:
         type_ = cast(GraphQLScalarType, type_)
         try:
             parse_result = type_.parse_literal(node)
-            if is_invalid(parse_result):
+            if parse_result is Undefined:
                 self.report_error(
                     GraphQLError(
                         f"Expected value of type '{location_type}',"
diff --git a/tests/pyutils/test_is_invalid.py b/tests/pyutils/test_is_invalid.py
diff --git a/tests/pyutils/test_is_nullish.py b/tests/pyutils/test_is_nullish.py
diff --git a/tests/utilities/test_ast_from_value.py b/tests/utilities/test_ast_from_value.py
@@ -40,8 +40,6 @@ def converts_boolean_values_to_asts():
 
         assert ast_from_value(Undefined, GraphQLBoolean) is None
 
-        assert ast_from_value(nan, GraphQLInt) is None
-
         assert ast_from_value(None, GraphQLBoolean) == NullValueNode()
 
         assert ast_from_value(0, GraphQLBoolean) == BooleanValueNode(value=False)
@@ -71,6 +69,11 @@ def converts_int_values_to_int_asts():
         msg = str(exc_info.value)
         assert msg == "Int cannot represent non 32-bit signed integer value: 1e+40"
 
+        with raises(GraphQLError) as exc_info:
+            ast_from_value(nan, GraphQLInt)
+        msg = str(exc_info.value)
+        assert msg == "Int cannot represent non-integer value: nan"
+
     def converts_float_values_to_float_asts():
         # luckily in Python we can discern between float and int
         assert ast_from_value(-1, GraphQLFloat) == FloatValueNode(value="-1")
@@ -134,7 +137,10 @@ def converts_using_serialize_from_a_custom_scalar_type():
             value="value"
         )
 
-        assert ast_from_value(nan, pass_through_scalar) is None
+        with raises(TypeError) as exc_info:
+            assert ast_from_value(nan, pass_through_scalar)
+        assert str(exc_info.value) == "Cannot convert value to AST: nan."
+
         with raises(TypeError) as exc_info:
             ast_from_value(inf, pass_through_scalar)
         assert str(exc_info.value) == "Cannot convert value to AST: inf."
diff --git a/tests/utilities/test_value_from_ast_untyped.py b/tests/utilities/test_value_from_ast_untyped.py
@@ -1,18 +1,28 @@
-from math import isnan
+from math import nan
 
 from graphql.language import parse_value, FloatValueNode, IntValueNode
 from graphql.pyutils import Undefined
 from graphql.utilities import value_from_ast_untyped
 
 
 def describe_value_from_ast_untyped():
+    def _compare_value(value, expected):
+        if expected is None:
+            assert value is None
+        elif expected is Undefined:
+            assert value is Undefined
+        elif expected is nan:
+            assert value is nan
+        else:
+            assert value == expected
+
     def _test_case(value_text, expected):
         value_node = parse_value(value_text)
-        assert value_from_ast_untyped(value_node) == expected
+        _compare_value(value_from_ast_untyped(value_node), expected)
 
     def _test_case_with_vars(value_text, variables, expected):
         value_node = parse_value(value_text)
-        assert value_from_ast_untyped(value_node, variables) == expected
+        _compare_value(value_from_ast_untyped(value_node, variables), expected)
 
     def parses_simple_values():
         _test_case("null", None)
@@ -43,10 +53,12 @@ def parses_variables():
             "{a:[$testVariable]}", {"testVariable": "foo"}, {"a": ["foo"]}
         )
         _test_case_with_vars("$testVariable", {"testVariable": None}, None)
+        _test_case_with_vars("$testVariable", {"testVariable": nan}, nan)
         _test_case_with_vars("$testVariable", {}, Undefined)
+        _test_case_with_vars("$testVariable", None, Undefined)
 
     def parse_invalid_int_as_nan():
-        assert isnan(value_from_ast_untyped(IntValueNode(value="invalid")))
+        assert value_from_ast_untyped(IntValueNode(value="invalid")) is nan
 
     def parse_invalid_float_as_nan():
-        assert isnan(value_from_ast_untyped(FloatValueNode(value="invalid")))
+        assert value_from_ast_untyped(FloatValueNode(value="invalid")) is nan
