Deprecate verbose resolver syntax with feature flag to enable

Reviewed By: alunyov

Differential Revision: D50513041

fbshipit-source-id: c04ce5736ee602b0335e18a8ab9017e6ec9f86a4

compiler/crates/common/src/feature_flags.rs

@@ -76,6 +76,10 @@ pub struct FeatureFlags {
     /// Enforce strict flavors for relay resolvers and disallow mixing flavors
     #[serde(default)]
     pub relay_resolvers_enable_strict_resolver_flavors: FeatureFlag,
+
+    /// Allow legacy verbose resolver syntax
+    #[serde(default)]
+    pub relay_resolvers_allow_legacy_verbose_syntax: FeatureFlag,
 }
 
 #[derive(Debug, Deserialize, Clone, Serialize)]

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

@@ -82,6 +82,9 @@ fn parse_source(
             enable_strict_resolver_flavors: &project_config
                 .feature_flags
                 .relay_resolvers_enable_strict_resolver_flavors,
+            allow_legacy_verbose_syntax: &project_config
+                .feature_flags
+                .relay_resolvers_allow_legacy_verbose_syntax,
         },
     )?;
     maybe_ir

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

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

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

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

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

@@ -55,9 +55,9 @@ use crate::ir::WeakObjectIr;
 use crate::untyped_representation::AllowedFieldName;
 use crate::untyped_representation::UntypedDocblockRepresentation;
 use crate::DocblockIr;
+use crate::LegacyVerboseResolverIr;
 use crate::On;
 use crate::ParseOptions;
-use crate::RelayResolverIr;
 
 pub(crate) fn parse_docblock_ir(
     project_name: ProjectName,
@@ -97,7 +97,7 @@ pub(crate) fn parse_docblock_ir(
     };
     let parsed_docblock_ir = match resolver_field {
         IrField::UnpopulatedIrField(unpopulated_ir_field) => {
-            DocblockIr::RelayResolver(parse_relay_resolver_ir(
+            let legacy_verbose_resolver = parse_relay_resolver_ir(
                 &mut fields,
                 definitions_in_file,
                 description,
@@ -106,7 +106,35 @@ pub(crate) fn parse_docblock_ir(
                 unpopulated_ir_field,
                 parse_options,
                 source_hash,
-            )?)
+            )?;
+
+            let field_name = format!(
+                "{}.{}",
+                legacy_verbose_resolver.on.type_name(),
+                legacy_verbose_resolver.field.name.value
+            )
+            .intern();
+
+            if !parse_options
+                .allow_legacy_verbose_syntax
+                .is_enabled_for(field_name)
+            {
+                match legacy_verbose_resolver.on {
+                    On::Type(field) => {
+                        return Err(vec![Diagnostic::error(
+                            IrParsingErrorMessages::UnexpectedOnType { field_name },
+                            field.key_location,
+                        )]);
+                    }
+                    On::Interface(field) => {
+                        return Err(vec![Diagnostic::error(
+                            IrParsingErrorMessages::UnexpectedOnInterface { field_name },
+                            field.key_location,
+                        )]);
+                    }
+                }
+            }
+            DocblockIr::LegacyVerboseResolver(legacy_verbose_resolver)
         }
         IrField::PopulatedIrField(populated_ir_field) => {
             if populated_ir_field.value.item.lookup().contains('.') {
@@ -165,7 +193,7 @@ fn parse_relay_resolver_ir(
     _resolver_field: UnpopulatedIrField,
     parse_options: &ParseOptions<'_>,
     source_hash: ResolverSourceHash,
-) -> DiagnosticsResult<RelayResolverIr> {
+) -> DiagnosticsResult<LegacyVerboseResolverIr> {
     let root_fragment =
         get_optional_populated_field_named(fields, AllowedFieldName::RootFragmentField)?;
     let field_name =
@@ -215,7 +243,7 @@ fn parse_relay_resolver_ir(
 
     validate_field_arguments(&field_definition_stub.arguments, location.source_location())?;
 
-    Ok(RelayResolverIr {
+    Ok(LegacyVerboseResolverIr {
         live: get_optional_unpopulated_field_named(fields, AllowedFieldName::LiveField)?,
         on,
         root_fragment: root_fragment
@@ -729,7 +757,7 @@ fn extract_fragment_arguments(
 
 fn get_resolver_field_path(docblock_ir: &DocblockIr) -> Option<String> {
     match docblock_ir {
-        DocblockIr::RelayResolver(resolver_ir) => {
+        DocblockIr::LegacyVerboseResolver(resolver_ir) => {
             let parent_type_name = match resolver_ir.on {
                 On::Type(field) => field.value.item,
                 On::Interface(field) => field.value.item,
@@ -757,7 +785,7 @@ fn validate_strict_resolver_flavors(
         output_type: Option<Location>,
     }
     let validation_info: Option<ResolverFlavorValidationInfo> = match docblock_ir {
-        DocblockIr::RelayResolver(resolver_ir) => {
+        DocblockIr::LegacyVerboseResolver(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();
@@ -806,14 +834,14 @@ fn validate_strict_resolver_flavors(
         let mut errs = vec![];
         if let Some(live_loc) = validation_info.live {
             errs.push(Diagnostic::error(
-                "@live is incompatible with @rootFragment",
+                IrParsingErrorMessages::IncompatibleLiveAndRootFragment,
                 live_loc,
             ));
         }
 
         if let Some(output_type_loc) = validation_info.output_type {
             errs.push(Diagnostic::error(
-                "@outputType is incompatible with @rootFragment",
+                IrParsingErrorMessages::IncompatibleOutputTypeAndRootFragment,
                 output_type_loc,
             ));
         }

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

@@ -122,6 +122,22 @@ pub enum IrParsingErrorMessages {
         "Unexpected `@outputType`. The deprecated `@outputType` option is not enabled for the field `{field_name}`."
     )]
     UnexpectedOutputType { field_name: StringKey },
+
+    #[error(
+        "Unexpected `@onType`. The deprecated `@onType` option is not enabled for the field `{field_name}`. Please use the new syntax: `@RelayResolver ParentType.field_name`."
+    )]
+    UnexpectedOnType { field_name: StringKey },
+
+    #[error(
+        "Unexpected `@onInterface`. The deprecated `@onType` option is not enabled for the field `{field_name}`. Please use the new syntax: `@RelayResolver ParentInterface.field_name`."
+    )]
+    UnexpectedOnInterface { field_name: StringKey },
+
+    #[error("@live is incompatible with @rootFragment")]
+    IncompatibleLiveAndRootFragment,
+
+    #[error("@outputType is incompatible with @rootFragment")]
+    IncompatibleOutputTypeAndRootFragment,
 }
 
 #[derive(Clone, Debug, Error, Eq, PartialEq, Ord, PartialOrd, Hash)]

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

@@ -89,7 +89,7 @@ lazy_static! {
 
 #[derive(Debug, Clone, PartialEq)]
 pub enum DocblockIr {
-    RelayResolver(RelayResolverIr),
+    LegacyVerboseResolver(LegacyVerboseResolverIr),
     TerseRelayResolver(TerseRelayResolverIr),
     StrongObjectResolver(StrongObjectIr),
     WeakObjectType(WeakObjectIr),
@@ -98,7 +98,7 @@ pub enum DocblockIr {
 impl DocblockIr {
     pub(crate) fn get_variant_name(&self) -> &'static str {
         match self {
-            DocblockIr::RelayResolver(_) => "legacy resolver declaration",
+            DocblockIr::LegacyVerboseResolver(_) => "legacy resolver declaration",
             DocblockIr::TerseRelayResolver(_) => "terse resolver declaration",
             DocblockIr::StrongObjectResolver(_) => "strong object type declaration",
             DocblockIr::WeakObjectType(_) => "weak object type declaration",
@@ -143,7 +143,7 @@ impl DocblockIr {
         };
 
         match self {
-            DocblockIr::RelayResolver(relay_resolver) => {
+            DocblockIr::LegacyVerboseResolver(relay_resolver) => {
                 relay_resolver.to_graphql_schema_ast(project_config)
             }
             DocblockIr::TerseRelayResolver(relay_resolver) => {
@@ -229,6 +229,15 @@ pub enum On {
     Interface(PopulatedIrField),
 }
 
+impl On {
+    pub fn type_name(&self) -> StringKey {
+        match self {
+            On::Type(field) => field.value.item,
+            On::Interface(field) => field.value.item,
+        }
+    }
+}
+
 #[derive(Debug, Clone, PartialEq)]
 pub struct Argument {
     pub name: Identifier,
@@ -745,7 +754,7 @@ impl ResolverTypeDefinitionIr for TerseRelayResolverIr {
 }
 
 #[derive(Debug, Clone, PartialEq)]
-pub struct RelayResolverIr {
+pub struct LegacyVerboseResolverIr {
     pub field: FieldDefinitionStub,
     pub on: On,
     pub root_fragment: Option<WithLocation<FragmentDefinitionName>>,
@@ -759,7 +768,7 @@ pub struct RelayResolverIr {
     pub source_hash: ResolverSourceHash,
 }
 
-impl ResolverIr for RelayResolverIr {
+impl ResolverIr for LegacyVerboseResolverIr {
     fn definitions(
         self,
         project_config: ResolverProjectConfig<'_, '_>,
@@ -897,7 +906,7 @@ impl ResolverIr for RelayResolverIr {
     }
 }
 
-impl ResolverTypeDefinitionIr for RelayResolverIr {
+impl ResolverTypeDefinitionIr for LegacyVerboseResolverIr {
     fn field_name(&self) -> &Identifier {
         &self.field.name
     }

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

@@ -31,15 +31,16 @@ use graphql_syntax::ExecutableDefinition;
 use graphql_syntax::TypeSystemDefinition;
 use intern::Lookup;
 pub use ir::DocblockIr;
+use ir::LegacyVerboseResolverIr;
 pub use ir::On;
-use ir::RelayResolverIr;
 use relay_config::ProjectName;
 use schema::SDLSchema;
 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 allow_legacy_verbose_syntax: &'a FeatureFlag,
 }
 
 pub fn parse_docblock_ast(

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

@@ -7,6 +7,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver
@@ -30,8 +31,8 @@ graphql`
 ==================================== ERROR ====================================
 ✖︎ Unexpected non-nullable item in list type given in `@edgeTo`.
 
-  /path/to/test/fixture/edge-to-non-null-plural-item.invalid.js:15:12
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo [Page!]
+  /path/to/test/fixture/edge-to-non-null-plural-item.invalid.js:16:12
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo [Page!]
       │            ^^^^^^^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment

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

@@ -6,6 +6,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver

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

@@ -7,6 +7,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver
@@ -30,8 +31,8 @@ graphql`
 ==================================== ERROR ====================================
 ✖︎ Unexpected non-nullable type given in `@edgeTo`.
 
-  /path/to/test/fixture/edge-to-non-null.invalid.js:15:12
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo Page!
+  /path/to/test/fixture/edge-to-non-null.invalid.js:16:12
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo Page!
       │            ^^^^^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment

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

@@ -6,6 +6,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver

compiler/crates/relay-docblock/tests/parse/fixtures/edge-to-not-identifier.invalid.expected

@@ -7,6 +7,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver
@@ -30,35 +31,35 @@ graphql`
 ==================================== ERROR ====================================
 ✖︎ Unsupported character
 
-  /path/to/test/fixture/edge-to-not-identifier.invalid.js:15:12
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo **LOL**
+  /path/to/test/fixture/edge-to-not-identifier.invalid.js:16:12
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo **LOL**
       │            ^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment
 
 
 ✖︎ Unsupported character
 
-  /path/to/test/fixture/edge-to-not-identifier.invalid.js:15:13
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo **LOL**
+  /path/to/test/fixture/edge-to-not-identifier.invalid.js:16:13
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo **LOL**
       │             ^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment
 
 
 ✖︎ Unsupported character
 
-  /path/to/test/fixture/edge-to-not-identifier.invalid.js:15:17
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo **LOL**
+  /path/to/test/fixture/edge-to-not-identifier.invalid.js:16:17
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo **LOL**
       │                 ^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment
 
 
 ✖︎ Unsupported character
 
-  /path/to/test/fixture/edge-to-not-identifier.invalid.js:15:18
-   14 │  * @fieldName favorite_page
-   15 │  * @edgeTo **LOL**
+  /path/to/test/fixture/edge-to-not-identifier.invalid.js:16:18
+   15 │  * @fieldName favorite_page
+   16 │  * @edgeTo **LOL**
       │                  ^
-   16 │  * @rootFragment myRootFragment
+   17 │  * @rootFragment myRootFragment

compiler/crates/relay-docblock/tests/parse/fixtures/edge-to-not-identifier.invalid.js

@@ -6,6 +6,7 @@
  */
 
 // expected-to-throw
+// relay:allow_legacy_verbose_syntax
 
 /**
  * @RelayResolver