Add flag and validation for strict resolver flavors

Reviewed By: captbaritone

Differential Revision: D48844413

fbshipit-source-id: 99810d110c01e0eb72933d21e93e3dcf82d134e0

compiler/crates/common/src/feature_flags.rs

@@ -72,6 +72,10 @@ pub struct FeatureFlags {
     /// Fully build the schema resolvers artifact
     #[serde(default)]
     pub enable_schema_resolvers: bool,
+
+    /// Enforce strict flavors for relay resolvers and disallow mixing flavors
+    #[serde(default)]
+    pub relay_resolvers_enable_strict_resolver_flavors: FeatureFlag,
 }
 
 #[derive(Debug, Deserialize, Clone, Serialize)]

compiler/crates/relay-compiler/src/docblocks.rs

@@ -79,6 +79,9 @@ fn parse_source(
             enable_output_type: &project_config
                 .feature_flags
                 .relay_resolver_enable_output_type,
+            enable_strict_resolver_flavors: &project_config
+                .feature_flags
+                .relay_resolvers_enable_strict_resolver_flavors,
         },
     )?;
     maybe_ir

compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs

@@ -119,6 +119,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
             .content
             .contains("# enable_resolver_normalization_ast"),
         enable_schema_resolvers: false,
+        relay_resolvers_enable_strict_resolver_flavors: FeatureFlag::Disabled,
     };
 
     let default_project_config = ProjectConfig {

compiler/crates/relay-compiler/tests/compile_relay_artifacts_with_custom_id/mod.rs

@@ -97,6 +97,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
         relay_resolver_enable_interface_output_type: FeatureFlag::Disabled,
         enable_resolver_normalization_ast: false,
         enable_schema_resolvers: false,
+        relay_resolvers_enable_strict_resolver_flavors: FeatureFlag::Disabled,
     };
 
     let default_schema_config = SchemaConfig::default();

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

@@ -19,6 +19,7 @@ use docblock_shared::ARGUMENT_DEFINITIONS;
 use docblock_shared::ARGUMENT_TYPE;
 use docblock_shared::DEFAULT_VALUE;
 use docblock_shared::KEY_RESOLVER_ID_FIELD;
+use docblock_shared::OUTPUT_TYPE_FIELD;
 use docblock_shared::PROVIDER_ARG_NAME;
 use graphql_ir::reexport::StringKey;
 use graphql_ir::FragmentDefinitionName;
@@ -144,6 +145,8 @@ pub(crate) fn parse_docblock_ir(
         }
     };
 
+    validate_strict_resolver_flavors(parse_options, &parsed_docblock_ir)?;
+
     assert_all_fields_removed(
         fields,
         resolver_field.key_location(),
@@ -724,6 +727,103 @@ fn extract_fragment_arguments(
         })
 }
 
+fn get_resolver_field_path(docblock_ir: &DocblockIr) -> Option<String> {
+    match docblock_ir {
+        DocblockIr::RelayResolver(resolver_ir) => {
+            let parent_type_name = match resolver_ir.on {
+                On::Type(field) => field.value.item,
+                On::Interface(field) => field.value.item,
+            };
+
+            Some(format!("{}.{}", parent_type_name, resolver_ir.field.name))
+        }
+        DocblockIr::TerseRelayResolver(terse_ir) => {
+            Some(format!("{}.{}", terse_ir.type_.item, terse_ir.field.name))
+        }
+        DocblockIr::StrongObjectResolver(_) => None,
+        DocblockIr::WeakObjectType(_) => None,
+    }
+}
+
+// Flavorings help the compiler understand what execution strategies are viable for the resolver.
+// We perform validation here (if enabled) to ensure that the resolver conforms to a specific flavor.
+fn validate_strict_resolver_flavors(
+    parse_options: &ParseOptions<'_>,
+    docblock_ir: &DocblockIr,
+) -> DiagnosticsResult<()> {
+    struct ResolverFlavorValidationInfo {
+        live: Option<Location>,
+        root_fragment: Option<Location>,
+        output_type: Option<Location>,
+    }
+    let validation_info: Option<ResolverFlavorValidationInfo> = match docblock_ir {
+        DocblockIr::RelayResolver(resolver_ir) => {
+            let output_type_location = resolver_ir.output_type.as_ref().map(|ot| {
+                let type_loc = ot.inner().location;
+                let (type_start, _) = type_loc.span().as_usize();
+                let output_type_end = type_start.saturating_sub(1); // -1 for space
+                let output_type_start =
+                    output_type_end.saturating_sub(OUTPUT_TYPE_FIELD.lookup().len());
+
+                type_loc.with_span(Span::from_usize(output_type_start, output_type_end))
+            });
+            Some(ResolverFlavorValidationInfo {
+                live: resolver_ir.live.map(|l| l.key_location),
+                root_fragment: resolver_ir.root_fragment.map(|rf| rf.location),
+                output_type: output_type_location,
+            })
+        }
+        DocblockIr::TerseRelayResolver(terse_ir) => Some(ResolverFlavorValidationInfo {
+            live: terse_ir.live.map(|l| l.key_location),
+            root_fragment: terse_ir.root_fragment.map(|rf| rf.location),
+            output_type: None,
+        }),
+        DocblockIr::StrongObjectResolver(_) => None,
+        DocblockIr::WeakObjectType(_) => None,
+    };
+
+    let validation_info = if let Some(validation_info) = validation_info {
+        validation_info
+    } else {
+        return Ok(());
+    };
+
+    // TODO(T161157239): also check if this is a model resolver, which is not compatible with root fragment
+
+    if validation_info.root_fragment.is_some()
+        && (validation_info.live.is_some() || validation_info.output_type.is_some())
+    {
+        let resolver_field_path = get_resolver_field_path(docblock_ir)
+            .expect("Should have a resolver path for RelayResolver or TerseRelayResolver");
+
+        if !parse_options
+            .enable_strict_resolver_flavors
+            .is_enabled_for(resolver_field_path.intern())
+        {
+            return Ok(());
+        }
+
+        let mut errs = vec![];
+        if let Some(live_loc) = validation_info.live {
+            errs.push(Diagnostic::error(
+                "@live is incompatible with @rootFragment",
+                live_loc,
+            ));
+        }
+
+        if let Some(output_type_loc) = validation_info.output_type {
+            errs.push(Diagnostic::error(
+                "@outputType is incompatible with @rootFragment",
+                output_type_loc,
+            ));
+        }
+
+        Err(errs)
+    } else {
+        Ok(())
+    }
+}
+
 fn validate_field_arguments(
     arguments: &Option<List<InputValueDefinition>>,
     source_location: SourceLocationKey,

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

@@ -39,6 +39,7 @@ use untyped_representation::parse_untyped_docblock_representation;
 
 pub struct ParseOptions<'a> {
     pub enable_output_type: &'a FeatureFlag,
+    pub enable_strict_resolver_flavors: &'a FeatureFlag,
 }
 
 pub fn parse_docblock_ast(

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-live-resolver-with-root-fragment.invalid.expected

@@ -0,0 +1,34 @@
+==================================== 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
+// relay:enable_strict_resolver_flavors
+
+/**
+ * @RelayResolver User.favorite_page: Page
+ * @rootFragment myRootFragment
+ * @live
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`
+==================================== ERROR ====================================
+✖︎ @live is incompatible with @rootFragment
+
+  /path/to/test/fixture/strict-flavors-live-resolver-with-root-fragment.invalid.js:14:5
+   13 │  * @rootFragment myRootFragment
+   14 │  * @live
+      │     ^^^^
+   15 │  *

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-live-resolver-with-root-fragment.invalid.js

@@ -0,0 +1,25 @@
+/**
+ * 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
+// relay:enable_strict_resolver_flavors
+
+/**
+ * @RelayResolver User.favorite_page: Page
+ * @rootFragment myRootFragment
+ * @live
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-multiple-errors.invalid.expected

@@ -0,0 +1,47 @@
+==================================== 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
+// relay:enable_strict_resolver_flavors
+// relay:enable_output_type
+
+/**
+ * @RelayResolver
+ * @fieldName favorite_page
+ * @onType User
+ * @rootFragment myRootFragment
+ * @outputType SomeType
+ * @live
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`
+==================================== ERROR ====================================
+✖︎ @live is incompatible with @rootFragment
+
+  /path/to/test/fixture/strict-flavors-multiple-errors.invalid.js:18:5
+   17 │  * @outputType SomeType
+   18 │  * @live
+      │     ^^^^
+   19 │  *
+
+
+✖︎ @outputType is incompatible with @rootFragment
+
+  /path/to/test/fixture/strict-flavors-multiple-errors.invalid.js:17:5
+   16 │  * @rootFragment myRootFragment
+   17 │  * @outputType SomeType
+      │     ^^^^^^^^^^
+   18 │  * @live

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-multiple-errors.invalid.js

@@ -0,0 +1,29 @@
+/**
+ * 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
+// relay:enable_strict_resolver_flavors
+// relay:enable_output_type
+
+/**
+ * @RelayResolver
+ * @fieldName favorite_page
+ * @onType User
+ * @rootFragment myRootFragment
+ * @outputType SomeType
+ * @live
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-output-type-with-root-fragment.invalid.expected

@@ -0,0 +1,37 @@
+==================================== 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
+// relay:enable_strict_resolver_flavors
+// relay:enable_output_type
+
+/**
+ * @RelayResolver
+ * @onType User
+ * @fieldName favorite_page
+ * @rootFragment myRootFragment
+ * @outputType SomeType
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`
+==================================== ERROR ====================================
+✖︎ @outputType is incompatible with @rootFragment
+
+  /path/to/test/fixture/strict-flavors-output-type-with-root-fragment.invalid.js:17:5
+   16 │  * @rootFragment myRootFragment
+   17 │  * @outputType SomeType
+      │     ^^^^^^^^^^
+   18 │  *

compiler/crates/relay-docblock/tests/parse/fixtures/strict-flavors-output-type-with-root-fragment.invalid.js

@@ -0,0 +1,28 @@
+/**
+ * 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
+// relay:enable_strict_resolver_flavors
+// relay:enable_output_type
+
+/**
+ * @RelayResolver
+ * @onType User
+ * @fieldName favorite_page
+ * @rootFragment myRootFragment
+ * @outputType SomeType
+ *
+ * The user's favorite page! They probably clicked something in the UI
+ * to tell us that it was their favorite page and then we put that in a
+ * database or something. Then we got that info out again and put it out
+ * again. Anyway, I'm rambling now. Its a page that the user likes. A lot.
+ */
+ graphql`
+ fragment myRootFragment on User {
+   name
+ }
+`

compiler/crates/relay-docblock/tests/parse/mod.rs

@@ -71,6 +71,14 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
                             } else {
                                 &FeatureFlag::Disabled
                             },
+                            enable_strict_resolver_flavors: if fixture
+                                .content
+                                .contains("// relay:enable_strict_resolver_flavors")
+                            {
+                                &FeatureFlag::Enabled
+                            } else {
+                                &FeatureFlag::Disabled
+                            },
                         },
                     )
                 })