Compiler incorrectly reports missing required fields (#4123)

Summary: Resolves #4122 Hi, I stumbled across this error when I misremembered the an enum variant and got a weird error. Took me awhile to figure out the issue because the compiler error was wrong. I've added tests for both orderings & for the constant & variable cases. Let me know if there's any other tests you'd like updating 😄 Pull Request resolved: #4123 Reviewed By: davidmccabe Differential Revision: D41266571 Pulled By: alunyov fbshipit-source-id: 9a76211a8fa8b7a637693f65a76f58ba4eef5ab3
facebook · Nov 14, 2022 · ce5e1ed · ce5e1ed
1 parent fa0a3a6
commit ce5e1ed
Show file tree

Hide file tree

Showing 6 changed files with 130 additions and 7 deletions.
diff --git a/compiler/crates/graphql-ir/src/build.rs b/compiler/crates/graphql-ir/src/build.rs
@@ -1525,7 +1525,7 @@ impl<'schema, 'signatures, 'options> Builder<'schema, 'signatures, 'options> {
             .map(|x| x.name.0)
             .collect::<StringKeySet>();
 
-        let fields: DiagnosticsResult<Vec<Argument>> = object
+        let fields = object
             .items
             .iter()
             .map(
@@ -1577,9 +1577,9 @@ impl<'schema, 'signatures, 'options> Builder<'schema, 'signatures, 'options> {
                     )]),
                 },
             )
-            .collect();
+            .collect::<DiagnosticsResult<Vec<Argument>>>()?;
         if required_fields.is_empty() {
-            Ok(Value::Object(fields?))
+            Ok(Value::Object(fields))
         } else {
             let mut missing: Vec<StringKey> = required_fields.into_iter().collect();
             missing.sort();
@@ -1669,7 +1669,7 @@ impl<'schema, 'signatures, 'options> Builder<'schema, 'signatures, 'options> {
             .map(|x| x.name.0)
             .collect::<StringKeySet>();
 
-        let fields: DiagnosticsResult<Vec<ConstantArgument>> = object
+        let fields = object
             .items
             .iter()
             .map(
@@ -1721,9 +1721,9 @@ impl<'schema, 'signatures, 'options> Builder<'schema, 'signatures, 'options> {
                     )]),
                 },
             )
-            .collect();
+            .collect::<DiagnosticsResult<Vec<ConstantArgument>>>()?;
         if required_fields.is_empty() {
-            Ok(ConstantValue::Object(fields?))
+            Ok(ConstantValue::Object(fields))
         } else {
             let mut missing: Vec<StringKey> = required_fields.into_iter().collect();
             missing.sort();

diff --git a/...phql-ir/tests/parse/fixtures/complex-object-with-invalid-constant-fields.invalid.expected b/...phql-ir/tests/parse/fixtures/complex-object-with-invalid-constant-fields.invalid.expected
@@ -0,0 +1,36 @@
+==================================== INPUT ====================================
+# expected-to-throw
+mutation LikeMutation {
+  feedbackLikeStrict(input: {
+    feedbackId: false
+    userID: "some-user-id"
+  }) {
+    __typename
+  }
+}
+
+mutation LikeMutationReverse {
+  feedbackLikeStrict(input: {
+    userID: "some-user-id"
+    feedbackId: false
+  }) {
+    __typename
+  }
+}
+==================================== ERROR ====================================
+✖︎ Expected a value of type 'ID'
+
+  complex-object-with-invalid-constant-fields.invalid.graphql:4:17
+    3 │   feedbackLikeStrict(input: {
+    4 │     feedbackId: false
+      │                 ^^^^^
+    5 │     userID: "some-user-id"
+
+
+✖︎ Expected a value of type 'ID'
+
+  complex-object-with-invalid-constant-fields.invalid.graphql:14:17
+   13 │     userID: "some-user-id"
+   14 │     feedbackId: false
+      │                 ^^^^^
+   15 │   }) {
diff --git a/...aphql-ir/tests/parse/fixtures/complex-object-with-invalid-constant-fields.invalid.graphql b/...aphql-ir/tests/parse/fixtures/complex-object-with-invalid-constant-fields.invalid.graphql
@@ -0,0 +1,19 @@
+# expected-to-throw
+mutation LikeMutation {
+  feedbackLikeStrict(input: {
+    feedbackId: false
+    userID: "some-user-id"
+  }) {
+    __typename
+  }
+}
+
+mutation LikeMutationReverse {
+  feedbackLikeStrict(input: {
+    userID: "some-user-id"
+    feedbackId: false
+  }) {
+    __typename
+  }
+}
+
diff --git a/...rates/graphql-ir/tests/parse/fixtures/complex-object-with-invalid-fields.invalid.expected b/...rates/graphql-ir/tests/parse/fixtures/complex-object-with-invalid-fields.invalid.expected
@@ -0,0 +1,36 @@
+==================================== INPUT ====================================
+# expected-to-throw
+mutation LikeMutation($feedbackId: ID! $userID: String!) {
+  feedbackLikeStrict(input: {
+    userID: $userID
+    feedbackId: $feedbackId
+  }) {
+    __typename
+  }
+}
+
+mutation LikeMutationReverse($feedbackId: ID! $userID: String!) {
+  feedbackLikeStrict(input: {
+    feedbackId: $feedbackId
+    userID: $userID
+  }) {
+    __typename
+  }
+}
+==================================== ERROR ====================================
+✖︎ Variable was defined as type 'String!' but used where a variable of type 'ID!' is expected.
+
+  complex-object-with-invalid-fields.invalid.graphql:4:13
+    3 │   feedbackLikeStrict(input: {
+    4 │     userID: $userID
+      │             ^^^^^^^
+    5 │     feedbackId: $feedbackId
+
+
+✖︎ Variable was defined as type 'String!' but used where a variable of type 'ID!' is expected.
+
+  complex-object-with-invalid-fields.invalid.graphql:14:13
+   13 │     feedbackId: $feedbackId
+   14 │     userID: $userID
+      │             ^^^^^^^
+   15 │   }) {
diff --git a/...crates/graphql-ir/tests/parse/fixtures/complex-object-with-invalid-fields.invalid.graphql b/...crates/graphql-ir/tests/parse/fixtures/complex-object-with-invalid-fields.invalid.graphql
@@ -0,0 +1,18 @@
+# expected-to-throw
+mutation LikeMutation($feedbackId: ID! $userID: String!) {
+  feedbackLikeStrict(input: {
+    userID: $userID
+    feedbackId: $feedbackId
+  }) {
+    __typename
+  }
+}
+
+mutation LikeMutationReverse($feedbackId: ID! $userID: String!) {
+  feedbackLikeStrict(input: {
+    feedbackId: $feedbackId
+    userID: $userID
+  }) {
+    __typename
+  }
+}
diff --git a/compiler/crates/graphql-ir/tests/parse_test.rs b/compiler/crates/graphql-ir/tests/parse_test.rs
@@ -4,7 +4,7 @@
  * This source code is licensed under the MIT license found in the
  * LICENSE file in the root directory of this source tree.
  *
- * @generated SignedSource<<0ca1fdb35aff7435674d234f070c70fe>>
+ * @generated SignedSource<<a9ea4300cb2f740289d5767d3f58b914>>
  */
 
 mod parse;
@@ -61,6 +61,20 @@ fn argument_definitions_typo_invalid() {
     test_fixture(transform_fixture, "argument_definitions_typo.invalid.graphql", "parse/fixtures/argument_definitions_typo.invalid.expected", input, expected);
 }
 
+#[test]
+fn complex_object_with_invalid_constant_fields_invalid() {
+    let input = include_str!("parse/fixtures/complex-object-with-invalid-constant-fields.invalid.graphql");
+    let expected = include_str!("parse/fixtures/complex-object-with-invalid-constant-fields.invalid.expected");
+    test_fixture(transform_fixture, "complex-object-with-invalid-constant-fields.invalid.graphql", "parse/fixtures/complex-object-with-invalid-constant-fields.invalid.expected", input, expected);
+}
+
+#[test]
+fn complex_object_with_invalid_fields_invalid() {
+    let input = include_str!("parse/fixtures/complex-object-with-invalid-fields.invalid.graphql");
+    let expected = include_str!("parse/fixtures/complex-object-with-invalid-fields.invalid.expected");
+    test_fixture(transform_fixture, "complex-object-with-invalid-fields.invalid.graphql", "parse/fixtures/complex-object-with-invalid-fields.invalid.expected", input, expected);
+}
+
 #[test]
 fn complex_object_with_missing_fields_invalid() {
     let input = include_str!("parse/fixtures/complex-object-with-missing-fields.invalid.graphql");