Enforce that terse resolvers don't expose list of non-nullable types

Reviewed By: alunyov

Differential Revision: D50517685

fbshipit-source-id: c69a581c1824d756a6ca5a8f6e23abdaf8443291

Author: captbaritone

compiler/crates/relay-docblock/src/docblock_ir.rs

@@ -366,12 +366,9 @@ fn parse_terse_relay_resolver_ir(
         value: description.item,
     });
 
-    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),
-        )]);
-    }
+    let type_name_location = type_str.location.with_span(field.type_.span());
+
+    validate_type_annotation(&field.type_, type_name_location)?;
 
     validate_field_arguments(&field.arguments, location.source_location())?;
 
@@ -534,25 +531,29 @@ fn parse_type_annotation(
         value.location.span().start,
     )?;
 
-    let valid_type_annotation = match &type_annotation {
-        TypeAnnotation::Named(_) => type_annotation,
+    validate_type_annotation(&type_annotation, value.location)?;
+
+    Ok(WithLocation::new(value.location, type_annotation))
+}
+
+fn validate_type_annotation(
+    type_annotation: &TypeAnnotation,
+    location: Location,
+) -> DiagnosticsResult<()> {
+    match type_annotation {
+        TypeAnnotation::Named(_) => Ok(()),
         TypeAnnotation::List(item_type) => match &item_type.type_ {
-            TypeAnnotation::NonNull(_) => {
-                return Err(vec![Diagnostic::error(
-                    IrParsingErrorMessages::UnexpectedNonNullableItemInListEdgeTo {},
-                    value.location,
-                )]);
-            }
-            _ => type_annotation,
+            TypeAnnotation::NonNull(_) => Err(vec![Diagnostic::error(
+                IrParsingErrorMessages::FieldWithNonNullableListItems {},
+                location,
+            )]),
+            _ => Ok(()),
         },
-        TypeAnnotation::NonNull(_) => {
-            return Err(vec![Diagnostic::error(
-                IrParsingErrorMessages::UnexpectedNonNullableEdgeTo {},
-                value.location,
-            )]);
-        }
-    };
-    Ok(WithLocation::new(value.location, valid_type_annotation))
+        TypeAnnotation::NonNull(_) => Err(vec![Diagnostic::error(
+            IrParsingErrorMessages::FieldWithNonNullType {},
+            location,
+        )]),
+    }
 }
 
 fn get_optional_unpopulated_field_named(

compiler/crates/relay-docblock/src/errors.rs

@@ -79,6 +79,11 @@ pub enum IrParsingErrorMessages {
     )]
     FieldWithNonNullType,
 
+    #[error(
+        "Unexpected Relay Resolver field returning list with non-nullable items. Relay expects all plural Resolver fields to have nullable items since errors thrown by Resolvers are turned into `null` values."
+    )]
+    FieldWithNonNullableListItems,
+
     #[error(
         "The compiler attempted to parse this `@RelayResolver` block as a {resolver_type}, but there were unexpected fields: {field_string}."
     )]
@@ -90,12 +95,6 @@ pub enum IrParsingErrorMessages {
     #[error("Defining arguments with default values for resolver fields is not supported, yet.")]
     ArgumentDefaultValuesNoSupported,
 
-    #[error("Unexpected non-nullable type given in `@edgeTo`.")]
-    UnexpectedNonNullableEdgeTo,
-
-    #[error("Unexpected non-nullable item in list type given in `@edgeTo`.")]
-    UnexpectedNonNullableItemInListEdgeTo,
-
     #[error(
         "The type specified in the fragment (`{fragment_type_condition}`) and the type specified in `@{on_field_name}` (`{on_field_value}`) are different. Please make sure these are exactly the same."
     )]

compiler/crates/relay-docblock/tests/parse/fixtures/edge-to-non-null-plural-item.invalid.expected

@@ -7,14 +7,9 @@
  */
 
 // expected-to-throw
-// relay:allow_legacy_verbose_syntax
 
 /**
- * @RelayResolver
- *
- * @onType User
- * @fieldName favorite_page
- * @edgeTo [Page!]
+ * @RelayResolver User.favorite_page: [Page!]
  * @rootFragment myRootFragment
  *
  * The user's favorite page! They probably clicked something in the UI
@@ -29,10 +24,10 @@ graphql`
  }
 `
 ==================================== ERROR ====================================
-✖︎ Unexpected non-nullable item in list type given in `@edgeTo`.
+✖︎ Unexpected Relay Resolver field returning list with non-nullable items. Relay expects all plural Resolver fields to have nullable items since errors thrown by Resolvers are turned into `null` values.
 
-  /path/to/test/fixture/edge-to-non-null-plural-item.invalid.js:16:12
-   15 │  * @fieldName favorite_page
-   16 │  * @edgeTo [Page!]
-      │            ^^^^^^^
-   17 │  * @rootFragment myRootFragment
+  /path/to/test/fixture/edge-to-non-null-plural-item.invalid.js:11:39
+   10 │ *
+   11 │  * @RelayResolver User.favorite_page: [Page!]
+      │                                       ^^^^^^^
+   12 │  * @rootFragment myRootFragment

compiler/crates/relay-docblock/tests/parse/fixtures/edge-to-non-null-plural-item.invalid.js

@@ -6,14 +6,9 @@
  */
 
 // expected-to-throw
-// relay:allow_legacy_verbose_syntax
 
 /**
- * @RelayResolver
- *
- * @onType User
- * @fieldName favorite_page
- * @edgeTo [Page!]
+ * @RelayResolver User.favorite_page: [Page!]
  * @rootFragment myRootFragment
  *
  * The user's favorite page! They probably clicked something in the UI

compiler/crates/relay-docblock/tests/parse/fixtures/edge-to-non-null.invalid.expected

@@ -29,7 +29,7 @@ graphql`
  }
 `
 ==================================== ERROR ====================================
-✖︎ Unexpected non-nullable type given in `@edgeTo`.
+✖︎ 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/edge-to-non-null.invalid.js:16:12
    15 │  * @fieldName favorite_page

compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.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 returning list with non-nullable items. Relay expects all plural Resolver fields to have nullable items since errors thrown by Resolvers are turned into `null` values.
+
+  /path/to/test/fixture/terse-relay-resolver-non-nullable-list-item.invalid.js:11:44
+   10 │ *
+   11 │  * @RelayResolver User.mandatory_greeting: [String!]
+      │                                            ^^^^^^^^^
+   12 │  * Non-nullable

compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.expected

@@ -5,6 +5,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
+// expected-to-throw
+
 /**
  * @RelayResolver User.mandatory_greeting: [String!]
  * Non-nullable

…relay-resolver-non-nullable-list-item.js …solver-non-nullable-list-item.invalid.jscompiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.js renamed to compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.js compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.js renamed to compiler/crates/relay-docblock/tests/parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.js

@@ -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<<c8739ad0b60736cf314b0c9beeecd93a>>
+ * @generated SignedSource<<fcaf5f3ed475c0aa56f387b6cd82691e>>
  */
 
 mod parse;
@@ -328,8 +328,8 @@ async fn terse_relay_resolver_non_nullable_list() {
 }
 
 #[tokio::test]
-async 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).await;
+async fn terse_relay_resolver_non_nullable_list_item_invalid() {
+    let input = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.js");
+    let expected = include_str!("parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.expected");
+    test_fixture(transform_fixture, "terse-relay-resolver-non-nullable-list-item.invalid.js", "parse/fixtures/terse-relay-resolver-non-nullable-list-item.invalid.expected", input, expected).await;
 }

compiler/crates/relay-docblock/tests/parse_test.rs

@@ -24,7 +24,7 @@ graphql`
   }
 `
 ==================================== ERROR ====================================
-✖︎ Unexpected non-nullable type given in `@edgeTo`.
+✖︎ 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/client-edge-to-non-null-plural-server-object-relay-resolver.invalid.js:6:12
     5 │  * @fieldName favorite_pages