Quote suggested values, unify rest of error messages

Cito · Cito · commit 0ef225113255 · 2019-12-14T18:37:06.000+01:00
Replicates graphql/graphql-js@0d6d9b7 and graphql/graphql-js@10b8e95 Also adds Oxford comma to suggested values as in original.
diff --git a/src/graphql/execution/execute.py b/src/graphql/execution/execute.py
@@ -797,7 +797,7 @@ def complete_list_value(
         if not isinstance(result, Iterable) or isinstance(result, str):
             raise GraphQLError(
                 "Expected Iterable, but did not find one for field"
-                f" {info.parent_type.name}.{info.field_name}."
+                f" '{info.parent_type.name}.{info.field_name}'."
             )
 
         # This is specified as a simple map, however we're optimizing the path where
@@ -916,13 +916,13 @@ def ensure_valid_runtime_type(
 
         if not is_object_type(runtime_type):
             raise GraphQLError(
-                f"Abstract type {return_type.name} must resolve"
+                f"Abstract type '{return_type.name}' must resolve"
                 " to an Object type at runtime"
-                f" for field {info.parent_type.name}.{info.field_name}"
+                f" for field '{info.parent_type.name}.{info.field_name}'"
                 f" with value {inspect(result)}, received '{inspect(runtime_type)}'."
-                f" Either the {return_type.name} type should provide"
-                ' a "resolve_type" function or each possible type should'
-                ' provide an "is_type_of" function.',
+                f" Either the '{return_type.name}' type should provide"
+                " a 'resolve_type' function or each possible type should"
+                " provide an 'is_type_of' function.",
                 field_nodes,
             )
         runtime_type = cast(GraphQLObjectType, runtime_type)
diff --git a/src/graphql/pyutils/did_you_mean.py b/src/graphql/pyutils/did_you_mean.py
@@ -1,4 +1,4 @@
-from typing import Sequence, List
+from typing import Sequence
 
 __all__ = ["did_you_mean"]
 
@@ -7,15 +7,22 @@
 
 def did_you_mean(suggestions: Sequence[str], sub_message: str = None) -> str:
     """Given [ A, B, C ] return ' Did you mean A, B, or C?'"""
-    parts: List[str] = []
-    if suggestions:
-        append = parts.append
-        append(" Did you mean")
-        if sub_message:
-            append(sub_message)
-        suggestions = suggestions[:MAX_LENGTH]
-        if len(suggestions) > 1:
-            append(", ".join(suggestions[:-1]))
-            append("or")
-        append(suggestions[-1] + "?")
-    return " ".join(parts)
+    if not suggestions:
+        return ""
+    parts = [" Did you mean "]
+    if sub_message:
+        parts.extend([sub_message, " "])
+    suggestions = suggestions[:MAX_LENGTH]
+    n = len(suggestions)
+    if n == 1:
+        parts.append(f"'{suggestions[0]}'?")
+    elif n == 2:
+        parts.append(f"'{suggestions[0]}' or '{suggestions[1]}'?")
+    else:
+        parts.extend(
+            [
+                ", ".join(f"'{s}'" for s in suggestions[:-1]),
+                f", or '{suggestions[-1]}'?",
+            ]
+        )
+    return "".join(parts)
diff --git a/src/graphql/utilities/coerce_input_value.py b/src/graphql/utilities/coerce_input_value.py
@@ -48,7 +48,7 @@ def coerce_input_value(
             path.as_list() if path else [],
             input_value,
             GraphQLError(
-                f"Expected non-nullable type {inspect(type_)} not to be None."
+                f"Expected non-nullable type '{inspect(type_)}' not to be None."
             ),
         )
         return INVALID
@@ -79,7 +79,7 @@ def coerce_input_value(
             on_error(
                 path.as_list() if path else [],
                 input_value,
-                GraphQLError(f"Expected type {type_.name} to be a dict."),
+                GraphQLError(f"Expected type '{type_.name}' to be a dict."),
             )
             return INVALID
 
@@ -99,7 +99,7 @@ def coerce_input_value(
                         path.as_list() if path else [],
                         input_value,
                         GraphQLError(
-                            f"Field {field_name} of required type {type_str}"
+                            f"Field '{field_name}' of required type '{type_str}'"
                             " was not provided."
                         ),
                     )
@@ -117,7 +117,7 @@ def coerce_input_value(
                     path.as_list() if path else [],
                     input_value,
                     GraphQLError(
-                        f"Field '{field_name}' is not defined by type {type_.name}."
+                        f"Field '{field_name}' is not defined by type '{type_.name}'."
                         + did_you_mean(suggestions)
                     ),
                 )
@@ -134,15 +134,15 @@ def coerce_input_value(
                 path.as_list() if path else [],
                 input_value,
                 GraphQLError(
-                    f"Expected type {type_.name}. {error}", original_error=error
+                    f"Expected type '{type_.name}'. {error}", original_error=error
                 ),
             )
             return INVALID
         if parse_result is INVALID:
             on_error(
                 path.as_list() if path else [],
                 input_value,
-                GraphQLError(f"Expected type {type_.name}."),
+                GraphQLError(f"Expected type '{type_.name}'."),
             )
         return parse_result
 
@@ -157,7 +157,10 @@ def coerce_input_value(
         on_error(
             path.as_list() if path else [],
             input_value,
-            GraphQLError(f"Expected type {type_.name}." + did_you_mean(suggestions)),
+            GraphQLError(
+                f"Expected type '{type_.name}'."
+                + did_you_mean(suggestions, "the enum value")
+            ),
         )
         return INVALID
 
diff --git a/src/graphql/utilities/find_deprecated_usages.py b/src/graphql/utilities/find_deprecated_usages.py
@@ -40,7 +40,7 @@ def enter_field(self, node, *_args):
                 reason = field_def.deprecation_reason
                 self.errors.append(
                     GraphQLError(
-                        f"The field {parent_type.name}.{field_name}"
+                        f"The field '{parent_type.name}.{field_name}'"
                         " is deprecated." + (f" {reason}" if reason else ""),
                         node,
                     )
@@ -55,7 +55,7 @@ def enter_enum_value(self, node, *_args):
                 reason = enum_val.deprecation_reason
                 self.errors.append(
                     GraphQLError(
-                        f"The enum value {type_.name}.{enum_val_name}"
+                        f"The enum value '{type_.name}.{enum_val_name}'"
                         " is deprecated." + (f" {reason}" if reason else ""),
                         node,
                     )
diff --git a/src/graphql/validation/rules/fields_on_correct_type.py b/src/graphql/validation/rules/fields_on_correct_type.py
@@ -42,14 +42,12 @@ def enter_field(self, node: FieldNode, *_args):
         )
 
         # Report an error, including helpful suggestions.
-        quoted_type_names = [f"'{s}'" for s in suggested_type_names]
-        quoted_field_names = [f"'{s}'" for s in suggested_field_names]
         self.report_error(
             GraphQLError(
                 f"Cannot query field '{field_name}' on type '{type_}'."
                 + (
-                    did_you_mean(quoted_type_names, "to use an inline fragment on")
-                    or did_you_mean(quoted_field_names)
+                    did_you_mean(suggested_type_names, "to use an inline fragment on")
+                    or did_you_mean(suggested_field_names)
                 ),
                 node,
             )
diff --git a/src/graphql/validation/rules/known_argument_names.py b/src/graphql/validation/rules/known_argument_names.py
@@ -47,7 +47,7 @@ def enter_directive(self, directive_node: DirectiveNode, *_args):
                         GraphQLError(
                             f"Unknown argument '{arg_name}'"
                             f" on directive '@{directive_name}'."
-                            + did_you_mean([f"'{s}'" for s in suggestions]),
+                            + did_you_mean(suggestions),
                             arg_node,
                         )
                     )
@@ -79,7 +79,7 @@ def enter_argument(self, arg_node: ArgumentNode, *args):
                 GraphQLError(
                     f"Unknown argument '{arg_name}'"
                     f" on field '{parent_type.name}.{field_name}'."
-                    + did_you_mean([f"'{s}'" for s in suggestions]),
+                    + did_you_mean(suggestions),
                     arg_node,
                 )
             )
diff --git a/src/graphql/validation/rules/known_type_names.py b/src/graphql/validation/rules/known_type_names.py
@@ -61,8 +61,7 @@ def enter_named_type(
             )
             self.report_error(
                 GraphQLError(
-                    f"Unknown type '{type_name}'."
-                    + did_you_mean([f"'{s}'" for s in suggested_types]),
+                    f"Unknown type '{type_name}'." + did_you_mean(suggested_types),
                     node,
                 )
             )
diff --git a/src/graphql/validation/rules/possible_type_extensions.py b/src/graphql/validation/rules/possible_type_extensions.py
@@ -64,7 +64,7 @@ def check_extension(self, node: TypeExtensionNode, *_args):
             self.report_error(
                 GraphQLError(
                     f"Cannot extend type '{type_name}' because it is not defined."
-                    + did_you_mean([f"'{s}'" for s in suggested_types]),
+                    + did_you_mean(suggested_types),
                     node.name,
                 )
             )
diff --git a/src/graphql/validation/rules/values_of_correct_type.py b/src/graphql/validation/rules/values_of_correct_type.py
@@ -74,7 +74,7 @@ def enter_object_field(self, node: ObjectFieldNode, *_args):
                 GraphQLError(
                     f"Field '{node.name.value}'"
                     f" is not defined by type '{parent_type.name}'."
-                    + did_you_mean([f"'{name}'" for name in suggestions]),
+                    + did_you_mean(suggestions),
                     node,
                 )
             )
@@ -119,9 +119,7 @@ def is_valid_value_node(self, node: ValueNode) -> None:
         if is_enum_type(type_):
             if not isinstance(node, EnumValueNode) or node.value not in type_.values:
                 all_names = list(type_.values)
-                suggested_values = [
-                    f"'{name}'" for name in suggestion_list(print_ast(node), all_names)
-                ]
+                suggested_values = suggestion_list(print_ast(node), all_names)
                 self.report_error(
                     GraphQLError(
                         f"Expected value of type '{type_.name}',"
diff --git a/tests/execution/test_abstract.py b/tests/execution/test_abstract.py
@@ -361,11 +361,11 @@ def returning_invalid_value_from_resolve_type_yields_useful_error():
             {"foo": None},
             [
                 {
-                    "message": "Abstract type FooInterface must resolve to an"
-                    " Object type at runtime for field Query.foo with value 'dummy',"
-                    " received '[]'. Either the FooInterface type should provide"
-                    ' a "resolve_type" function or each possible type'
-                    ' should provide an "is_type_of" function.',
+                    "message": "Abstract type 'FooInterface' must resolve to an"
+                    " Object type at runtime for field 'Query.foo' with value 'dummy',"
+                    " received '[]'. Either the 'FooInterface' type should provide"
+                    " a 'resolve_type' function or each possible type"
+                    " should provide an 'is_type_of' function.",
                     "locations": [(1, 3)],
                     "path": ["foo"],
                 }
diff --git a/tests/execution/test_lists.py b/tests/execution/test_lists.py
@@ -106,7 +106,7 @@ def does_not_accept_iterable_string_literal_as_a_list_value():
                 [
                     {
                         "message": "Expected Iterable,"
-                        " but did not find one for field DataType.test.",
+                        " but did not find one for field 'DataType.test'.",
                         "locations": [(1, 10)],
                         "path": ["nest", "test"],
                     }
diff --git a/tests/execution/test_variables.py b/tests/execution/test_variables.py
@@ -365,7 +365,7 @@ def errors_on_null_for_nested_non_null():
                         {
                             "message": "Variable '$input' got invalid value"
                             " None at 'input.c';"
-                            " Expected non-nullable type String! not to be None.",
+                            " Expected non-nullable type 'String!' not to be None.",
                             "locations": [(2, 24)],
                         }
                     ],
@@ -379,7 +379,7 @@ def errors_on_incorrect_type():
                     [
                         {
                             "message": "Variable '$input' got invalid value 'foo bar';"
-                            " Expected type TestInputObject to be a dict.",
+                            " Expected type 'TestInputObject' to be a dict.",
                             "locations": [(2, 24)],
                             "path": None,
                         }
@@ -395,7 +395,7 @@ def errors_on_omission_of_nested_non_null():
                         {
                             "message": "Variable '$input' got invalid value"
                             " {'a': 'foo', 'b': 'bar'};"
-                            " Field c of required type String! was not provided.",
+                            " Field 'c' of required type 'String!' was not provided.",
                             "locations": [(2, 24)],
                         }
                     ],
@@ -415,13 +415,13 @@ def errors_on_deep_nested_errors_and_with_many_errors():
                         {
                             "message": "Variable '$input' got invalid value"
                             " {'a': 'foo'} at 'input.na';"
-                            " Field c of required type String! was not provided.",
+                            " Field 'c' of required type 'String!' was not provided.",
                             "locations": [(2, 28)],
                         },
                         {
                             "message": "Variable '$input' got invalid value"
                             " {'na': {'a': 'foo'}};"
-                            " Field nb of required type String! was not provided.",
+                            " Field 'nb' of required type 'String!' was not provided.",
                             "locations": [(2, 28)],
                         },
                     ],
@@ -437,7 +437,7 @@ def errors_on_addition_of_unknown_input_field():
                         {
                             "message": "Variable '$input' got invalid value {'a':"
                             " 'foo', 'b': 'bar', 'c': 'baz', 'extra': 'dog'}; Field"
-                            " 'extra' is not defined by type TestInputObject.",
+                            " 'extra' is not defined by type 'TestInputObject'.",
                             "locations": [(2, 24)],
                         }
                     ],
@@ -658,7 +658,7 @@ def reports_error_for_array_passed_into_string_input():
                 [
                     {
                         "message": "Variable '$value' got invalid value [1, 2, 3];"
-                        " Expected type String. String cannot represent"
+                        " Expected type 'String'. String cannot represent"
                         " a non string value: [1, 2, 3]",
                         "locations": [(2, 24)],
                         "path": None,
@@ -807,7 +807,7 @@ def does_not_allow_lists_of_non_nulls_to_contain_null():
                     {
                         "message": "Variable '$input' got invalid value None"
                         " at 'input[1]';"
-                        " Expected non-nullable type String! not to be None.",
+                        " Expected non-nullable type 'String!' not to be None.",
                         "locations": [(2, 24)],
                     }
                 ],
@@ -856,7 +856,7 @@ def does_not_allow_non_null_lists_of_non_nulls_to_contain_null():
                     {
                         "message": "Variable '$input' got invalid value None"
                         " at 'input[1]';"
-                        " Expected non-nullable type String! not to be None.",
+                        " Expected non-nullable type 'String!' not to be None.",
                         "locations": [(2, 24)],
                         "path": None,
                     }
@@ -974,7 +974,7 @@ def _invalid_value_error(value, index):
             return {
                 "message": "Variable '$input' got invalid value"
                 f" {value} at 'input[{index}]';"
-                " Expected type String."
+                " Expected type 'String'."
                 f" String cannot represent a non string value: {value}",
                 "locations": [(2, 20)],
             }
diff --git a/tests/pyutils/test_did_you_mean.py b/tests/pyutils/test_did_you_mean.py
@@ -6,19 +6,19 @@ def does_accept_an_empty_list():
         assert did_you_mean([]) == ""
 
     def handles_single_suggestion():
-        assert did_you_mean(["A"]) == " Did you mean A?"
+        assert did_you_mean(["A"]) == " Did you mean 'A'?"
 
     def handles_two_suggestions():
-        assert did_you_mean(["A", "B"]) == " Did you mean A or B?"
+        assert did_you_mean(["A", "B"]) == " Did you mean 'A' or 'B'?"
 
     def handles_multiple_suggestions():
-        assert did_you_mean(["A", "B", "C"]) == " Did you mean A, B or C?"
+        assert did_you_mean(["A", "B", "C"]) == " Did you mean 'A', 'B', or 'C'?"
 
     def limits_to_five_suggestions():
         assert (
             did_you_mean(["A", "B", "C", "D", "E", "F"])
-            == " Did you mean A, B, C, D or E?"
+            == " Did you mean 'A', 'B', 'C', 'D', or 'E'?"
         )
 
     def adds_sub_message():
-        assert did_you_mean(["A"], "the letter") == " Did you mean the letter A?"
+        assert did_you_mean(["A"], "the letter") == " Did you mean the letter 'A'?"
diff --git a/tests/subscription/test_subscribe.py b/tests/subscription/test_subscribe.py
@@ -472,7 +472,7 @@ async def resolves_to_an_error_if_variables_were_wrong_type():
             [
                 {
                     "message": "Variable '$priority' got invalid value 'meow'; Expected"
-                    " type Int. Int cannot represent non-integer value: 'meow'",
+                    " type 'Int'. Int cannot represent non-integer value: 'meow'",
                     "locations": [(2, 27)],
                 }
             ],
diff --git a/tests/type/test_enum.py b/tests/type/test_enum.py
@@ -249,7 +249,7 @@ def does_not_accept_internal_value_as_enum_variable():
             [
                 {
                     "message": "Variable '$color' got invalid value 2;"
-                    " Expected type Color.",
+                    " Expected type 'Color'.",
                     "locations": [(1, 8)],
                 }
             ],
diff --git a/tests/utilities/test_coerce_input_value.py b/tests/utilities/test_coerce_input_value.py
diff --git a/tests/utilities/test_find_deprecated_usages.py b/tests/utilities/test_find_deprecated_usages.py
diff --git a/tests/validation/test_fields_on_correct_type.py b/tests/validation/test_fields_on_correct_type.py
diff --git a/tests/validation/test_known_type_names.py b/tests/validation/test_known_type_names.py
diff --git a/tests/validation/test_values_of_correct_type.py b/tests/validation/test_values_of_correct_type.py

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ def enter_directive(self, directive_node: DirectiveNode, *_args):`
`47`	`47`	`GraphQLError(`
`48`	`48`	`f"Unknown argument '{arg_name}'"`
`49`	`49`	`f" on directive '@{directive_name}'."`
`50`		`- + did_you_mean([f"'{s}'" for s in suggestions]),`
	`50`	`+ + did_you_mean(suggestions),`
`51`	`51`	`arg_node,`
`52`	`52`	`)`
`53`	`53`	`)`
`@@ -79,7 +79,7 @@ def enter_argument(self, arg_node: ArgumentNode, *args):`
`79`	`79`	`GraphQLError(`
`80`	`80`	`f"Unknown argument '{arg_name}'"`
`81`	`81`	`f" on field '{parent_type.name}.{field_name}'."`
`82`		`- + did_you_mean([f"'{s}'" for s in suggestions]),`
	`82`	`+ + did_you_mean(suggestions),`
`83`	`83`	`arg_node,`
`84`	`84`	`)`
`85`	`85`	`)`
Original file line number	Diff line number	Diff line change
`@@ -61,8 +61,7 @@ def enter_named_type(`
`61`	`61`	`)`
`62`	`62`	`self.report_error(`
`63`	`63`	`GraphQLError(`
`64`		`- f"Unknown type '{type_name}'."`
`65`		`- + did_you_mean([f"'{s}'" for s in suggested_types]),`
	`64`	`+ f"Unknown type '{type_name}'." + did_you_mean(suggested_types),`
`66`	`65`	`node,`
`67`	`66`	`)`
`68`	`67`	`)`
Original file line number	Diff line number	Diff line change
`@@ -64,7 +64,7 @@ def check_extension(self, node: TypeExtensionNode, *_args):`
`64`	`64`	`self.report_error(`
`65`	`65`	`GraphQLError(`
`66`	`66`	`f"Cannot extend type '{type_name}' because it is not defined."`
`67`		`- + did_you_mean([f"'{s}'" for s in suggested_types]),`
	`67`	`+ + did_you_mean(suggested_types),`
`68`	`68`	`node.name,`
`69`	`69`	`)`
`70`	`70`	`)`
Original file line number	Diff line number	Diff line change
`@@ -361,11 +361,11 @@ def returning_invalid_value_from_resolve_type_yields_useful_error():`
`361`	`361`	`{"foo": None},`
`362`	`362`	`[`
`363`	`363`	`{`
`364`		`- "message": "Abstract type FooInterface must resolve to an"`
`365`		`- " Object type at runtime for field Query.foo with value 'dummy',"`
`366`		`- " received '[]'. Either the FooInterface type should provide"`
`367`		`- ' a "resolve_type" function or each possible type'`
`368`		`- ' should provide an "is_type_of" function.',`
	`364`	`+ "message": "Abstract type 'FooInterface' must resolve to an"`
	`365`	`+ " Object type at runtime for field 'Query.foo' with value 'dummy',"`
	`366`	`+ " received '[]'. Either the 'FooInterface' type should provide"`
	`367`	`+ " a 'resolve_type' function or each possible type"`
	`368`	`+ " should provide an 'is_type_of' function.",`
`369`	`369`	`"locations": [(1, 3)],`
`370`	`370`	`"path": ["foo"],`
`371`	`371`	`}`