Validate that resolver fields are always nullable

Reviewed By: josephsavona Differential Revision: D46498216 fbshipit-source-id: e0f430da9c65b66f9c76039ae87810fe76c0a47b
facebook · Jun 12, 2023 · feaba45 · feaba45
1 parent a2c2b64
commit feaba45
Show file tree

Hide file tree

Showing 11 changed files with 195 additions and 8 deletions.
diff --git a/compiler/crates/relay-docblock/src/docblock_ir.rs b/compiler/crates/relay-docblock/src/docblock_ir.rs
@@ -295,6 +295,13 @@ fn parse_terse_relay_resolver_ir(
         span_start + 1,
     )?;
 
+    if let TypeAnnotation::NonNull(non_null) = field.type_ {
+        return Err(vec![Diagnostic::error(
+            IrParsingErrorMessages::FieldWithNonNullType,
+            Location::new(type_str.location.source_location(), non_null.span),
+        )]);
+    }
+
     validate_field_arguments(&field.arguments, location.source_location())?;
 
     let (fragment_type_condition, fragment_arguments) = parse_fragment_definition(

diff --git a/compiler/crates/relay-docblock/src/errors.rs b/compiler/crates/relay-docblock/src/errors.rs
@@ -74,6 +74,11 @@ pub enum IrParsingErrorMessages {
     #[error("The `@RelayResolver` field `@{field_name}` requires data.")]
     FieldWithMissingData { field_name: AllowedFieldName },
 
+    #[error(
+        "Unexpected Relay Resolver field with non-nullable type. Relay expects all Resolver fields to be nullable since errors thrown by Resolvers are turned into `null` values."
+    )]
+    FieldWithNonNullType,
+
     #[error(
         "The compiler attempted to parse this `@RelayResolver` block as a {resolver_type}, but there were unexpected fields: {field_string}."
     )]

diff --git a/.../relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.expected b/.../relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.expected
@@ -0,0 +1,73 @@
+==================================== INPUT ====================================
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * @RelayResolver User.mandatory_greeting: [String!]
+ * Non-nullable
+ */
+==================================== OUTPUT ===================================
+TerseRelayResolver(
+    TerseRelayResolverIr {
+        field: FieldDefinition {
+            name: Identifier {
+                span: 25:43,
+                token: Token {
+                    span: 25:43,
+                    kind: Identifier,
+                },
+                value: "mandatory_greeting",
+            },
+            type_: List(
+                ListTypeAnnotation {
+                    span: 45:54,
+                    open: Token {
+                        span: 45:46,
+                        kind: OpenBracket,
+                    },
+                    type_: NonNull(
+                        NonNullTypeAnnotation {
+                            span: 46:53,
+                            type_: Named(
+                                NamedTypeAnnotation {
+                                    name: Identifier {
+                                        span: 46:52,
+                                        token: Token {
+                                            span: 46:52,
+                                            kind: Identifier,
+                                        },
+                                        value: "String",
+                                    },
+                                },
+                            ),
+                            exclamation: Token {
+                                span: 52:53,
+                                kind: Exclamation,
+                            },
+                        },
+                    ),
+                    close: Token {
+                        span: 53:54,
+                        kind: CloseBracket,
+                    },
+                },
+            ),
+            arguments: None,
+            directives: [],
+            description: None,
+        },
+        type_: WithLocation {
+            location: /path/to/test/fixture/terse-relay-resolver-non-nullable-list-item.js:20:24,
+            item: "User",
+        },
+        root_fragment: None,
+        deprecated: None,
+        live: None,
+        location: /path/to/test/fixture/terse-relay-resolver-non-nullable-list-item.js:0:71,
+        fragment_arguments: None,
+    },
+)
diff --git a/...crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.js b/...crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.js
@@ -0,0 +1,11 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+/**
+ * @RelayResolver User.mandatory_greeting: [String!]
+ * Non-nullable
+ */
diff --git a/...rates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list.expected b/...rates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list.expected
@@ -0,0 +1,22 @@
+==================================== INPUT ====================================
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// expected-to-throw
+
+/**
+ * @RelayResolver User.mandatory_greeting: [String]!
+ * Non-nullable 
+ */
+==================================== ERROR ====================================
+✖︎ Unexpected Relay Resolver field with non-nullable type. Relay expects all Resolver fields to be nullable since errors thrown by Resolvers are turned into `null` values.
+
+  /path/to/test/fixture/terse-relay-resolver-non-nullable-list.js:11:44
+   10 │ *
+   11 │  * @RelayResolver User.mandatory_greeting: [String]!
+      │                                            ^^^^^^^^^
+   12 │  * Non-nullable
diff --git a/...iler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list.js b/...iler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list.js
@@ -0,0 +1,13 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// expected-to-throw
+
+/**
+ * @RelayResolver User.mandatory_greeting: [String]!
+ * Non-nullable 
+ */
diff --git a/...ler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable.expected b/...ler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable.expected
@@ -0,0 +1,22 @@
+==================================== INPUT ====================================
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// expected-to-throw
+
+/**
+ * @RelayResolver User.mandatory_greeting: String!
+ * Non-nullable 
+ */
+==================================== ERROR ====================================
+✖︎ Unexpected Relay Resolver field with non-nullable type. Relay expects all Resolver fields to be nullable since errors thrown by Resolvers are turned into `null` values.
+
+  /path/to/test/fixture/terse-relay-resolver-non-nullable.js:11:44
+   10 │ *
+   11 │  * @RelayResolver User.mandatory_greeting: String!
+      │                                            ^^^^^^^
+   12 │  * Non-nullable
diff --git a/compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable.js b/compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable.js
@@ -0,0 +1,13 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+// expected-to-throw
+
+/**
+ * @RelayResolver User.mandatory_greeting: String!
+ * Non-nullable 
+ */
diff --git a/compiler/crates/relay-docblock/tests/parse_test.rs b/compiler/crates/relay-docblock/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<<808ccebf97208f983fbfd157e12804fe>>
+ * @generated SignedSource<<5d997f008adc06523bb95fb0fe4781a6>>
  */
 
 mod parse;
@@ -277,3 +277,24 @@ fn terse_relay_resolver_no_dot_invalid() {
     let expected = include_str!("parse/fixtures/terse-relay-resolver-no-dot.invalid.expected");
     test_fixture(transform_fixture, "terse-relay-resolver-no-dot.invalid.js", "parse/fixtures/terse-relay-resolver-no-dot.invalid.expected", input, expected);
 }
+
+#[test]
+fn terse_relay_resolver_non_nullable() {
+    let input = include_str!("parse/fixtures/terse-relay-resolver-non-nullable.js");
+    let expected = include_str!("parse/fixtures/terse-relay-resolver-non-nullable.expected");
+    test_fixture(transform_fixture, "terse-relay-resolver-non-nullable.js", "parse/fixtures/terse-relay-resolver-non-nullable.expected", input, expected);
+}
+
+#[test]
+fn terse_relay_resolver_non_nullable_list() {
+    let input = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list.js");
+    let expected = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list.expected");
+    test_fixture(transform_fixture, "terse-relay-resolver-non-nullable-list.js", "parse/fixtures/terse-relay-resolver-non-nullable-list.expected", input, expected);
+}
+
+#[test]
+fn terse_relay_resolver_non_nullable_list_item() {
+    let input = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list-item.js");
+    let expected = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list-item.expected");
+    test_fixture(transform_fixture, "terse-relay-resolver-non-nullable-list-item.js", "parse/fixtures/terse-relay-resolver-non-nullable-list-item.expected", input, expected);
+}
diff --git a/...es/react-relay/__tests__/__generated__/RelayResolverModelTestInterfaceFragment.graphql.js b/...es/react-relay/__tests__/__generated__/RelayResolverModelTestInterfaceFragment.graphql.js
diff --git a/packages/relay-runtime/store/__tests__/resolvers/TodoDescription.js b/packages/relay-runtime/store/__tests__/resolvers/TodoDescription.js
@@ -59,7 +59,7 @@ function color(instance: TodoDescription): string {
 }
 
 /**
- * @RelayResolver TodoDescription.some_interface: ClientInterface!
+ * @RelayResolver TodoDescription.some_interface: ClientInterface
  */
 function some_interface(
   instance: TodoDescription,
@@ -71,7 +71,7 @@ function some_interface(
 }
 
 /**
- * @RelayResolver TodoDescription.some_client_type_with_interface: ClientTypeWithNestedClientInterface!
+ * @RelayResolver TodoDescription.some_client_type_with_interface: ClientTypeWithNestedClientInterface
  */
 function some_client_type_with_interface(
   instance: TodoDescription,