Make module name prefix validation opt-in for non-Haste projects (#5200)

Summary:
The compiler previously enforced that all GraphQL operation/fragment names are prefixed with the file name. This restriction only makes sense for Haste module resolution where file names are globally unique. Now the validation runs automatically for Haste projects, and non-Haste projects can opt in via the enforce_module_name_prefix_for_non_haste feature flag.

Pull Request resolved: #5200

Reviewed By: josephsavona

Differential Revision: D95463492

Pulled By: captbaritone

fbshipit-source-id: 8a6cab182374c6f59abd189e4eb5f4ada405e020

Author: captbaritone

compiler/Cargo.lock

@@ -203,6 +203,12 @@ pub struct FeatureFlags {
     /// alias for `@relayType` / `@relayField`.
     #[serde(default)]
     pub allow_legacy_relay_resolver_tag: FeatureFlag,
+
+    /// Enforce that GraphQL operation and fragment names start with the file
+    /// name. Haste projects have this enforcement automatically; this flag
+    /// is only needed for non-Haste projects that want the same validation.
+    #[serde(default)]
+    pub enforce_module_name_prefix_for_non_haste: bool,
 }
 
 impl Default for FeatureFlags {
@@ -238,6 +244,7 @@ impl Default for FeatureFlags {
             enable_shadow_resolvers: Default::default(),
             new_flow_casting_syntax: Default::default(),
             allow_legacy_relay_resolver_tag: Default::default(),
+            enforce_module_name_prefix_for_non_haste: Default::default(),
 
             // enabled-by-default
             enforce_fragment_alias_where_ambiguous: FeatureFlag::Enabled,

compiler/crates/common/src/feature_flags.rs

@@ -682,6 +682,11 @@
             "kind": "enabled"
           }
         },
+        "enforce_module_name_prefix_for_non_haste": {
+          "description": "Enforce that GraphQL operation and fragment names start with the file\nname. Haste projects have this enforcement automatically; this flag\nis only needed for non-Haste projects that want the same validation.",
+          "type": "boolean",
+          "default": false
+        },
         "legacy_include_path_in_required_reader_nodes": {
           "description": "The `path` field in `@required` Reader AST nodes is no longer used. But\nremoving them in one diff is too large of a change to ship at once.\n\nThis flag will allow us to use the rollout FeatureFlag to remove them\nacross a number of diffs.",
           "$ref": "#/$defs/FeatureFlag",
@@ -947,6 +952,7 @@
             "enforce_fragment_alias_where_ambiguous": {
               "kind": "enabled"
             },
+            "enforce_module_name_prefix_for_non_haste": false,
             "legacy_include_path_in_required_reader_nodes": {
               "kind": "disabled"
             },

compiler/crates/relay-compiler/relay-compiler-config-schema.json

@@ -12,6 +12,7 @@ use common::WithDiagnostics;
 use common::escalate_and_check;
 use errors::try_all;
 use graphql_ir::Program;
+use relay_config::JsModuleFormat;
 use relay_config::ProjectConfig;
 use relay_transforms::ValidateVariablesOptions;
 use relay_transforms::disallow_circular_no_inline_fragments;
@@ -76,7 +77,15 @@ pub fn validate(
         validate_connections(program, &project_config.schema_config.connection_interface),
         validate_relay_directives(program),
         validate_global_variable_names(program),
-        validate_module_names(program),
+        if matches!(project_config.js_module_format, JsModuleFormat::Haste)
+            || project_config
+                .feature_flags
+                .enforce_module_name_prefix_for_non_haste
+        {
+            validate_module_names(program)
+        } else {
+            Ok(())
+        },
         validate_client_schema_extensions_use_catch(program),
         validate_no_inline_fragments_with_raw_response_type(program),
         disallow_typename_on_root(program),

compiler/crates/relay-compiler/src/build_project/validate.rs

@@ -0,0 +1,27 @@
+==================================== INPUT ====================================
+//- foo.js
+graphql`
+  fragment notMatchingModuleName on User {
+     name
+  }`;
+
+//- relay.config.json
+{
+   "language": "typescript",
+   "schema": "./schema.graphql",
+   "featureFlags": {
+       "enforce_module_name_prefix_for_non_haste": true
+   }
+}
+
+//- schema.graphql
+type Query { me: User }
+type User { name: String }
+==================================== OUTPUT ===================================
+✖︎ Fragments in graphql tags must start with the module name ('foo'). Got 'notMatchingModuleName' instead.
+
+  foo.js:2:12
+    1 │ 
+    2 │   fragment notMatchingModuleName on User {
+      │            ^^^^^^^^^^^^^^^^^^^^^
+    3 │      name

compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/module_name_validation_enforced_with_flag.invalid.expected

@@ -0,0 +1,18 @@
+//- foo.js
+graphql`
+  fragment notMatchingModuleName on User {
+     name
+  }`;
+
+//- relay.config.json
+{
+   "language": "typescript",
+   "schema": "./schema.graphql",
+   "featureFlags": {
+       "enforce_module_name_prefix_for_non_haste": true
+   }
+}
+
+//- schema.graphql
+type Query { me: User }
+type User { name: String }

compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/module_name_validation_enforced_with_flag.invalid.input

@@ -0,0 +1,68 @@
+==================================== INPUT ====================================
+//- foo.js
+graphql`
+  fragment notMatchingModuleName on User {
+     name
+  }`;
+
+//- relay.config.json
+{
+   "language": "typescript",
+   "schema": "./schema.graphql"
+}
+
+//- schema.graphql
+type Query { me: User }
+type User { name: String }
+==================================== OUTPUT ===================================
+//-++ __generated__/notMatchingModuleName.graphql.ts
+/**
+ * <auto-generated> SignedSource<<a726d4c8346058ffbabeb81fc4927256>>
+ * @lightSyntaxTransform
+ * @nogrep
+ */
+
+/* tslint:disable */
+/* eslint-disable */
+// @ts-nocheck
+
+import { ReaderFragment } from 'relay-runtime';
+import { FragmentRefs } from "relay-runtime";
+export type notMatchingModuleName$data = {
+  readonly name: string | null | undefined;
+  readonly " $fragmentType": "notMatchingModuleName";
+};
+export type notMatchingModuleName$key = {
+  readonly " $data"?: notMatchingModuleName$data;
+  readonly " $fragmentSpreads": FragmentRefs<"notMatchingModuleName">;
+};
+
+const node: ReaderFragment = {
+  "argumentDefinitions": [],
+  "kind": "Fragment",
+  "metadata": null,
+  "name": "notMatchingModuleName",
+  "selections": [
+    {
+      "alias": null,
+      "args": null,
+      "kind": "ScalarField",
+      "name": "name",
+      "storageKey": null
+    }
+  ],
+  "type": "User",
+  "abstractKey": null
+};
+
+(node as any).hash = "e153e2a2bafa3b8ef4a86c54db559336";
+
+export default node;
+
+
+
+Artifact Map:
+Project: default
+  Type: Mapping
+  - Source: ExecutableDefinition: notMatchingModuleName
+    Path: __generated__/notMatchingModuleName.graphql.ts

compiler/crates/relay-compiler/tests/relay_compiler_integration/fixtures/module_name_validation_skipped_for_non_haste.expected

@@ -0,0 +1,15 @@
+//- foo.js
+graphql`
+  fragment notMatchingModuleName on User {
+     name
+  }`;
+
+//- relay.config.json
+{
+   "language": "typescript",
+   "schema": "./schema.graphql"
+}
+
+//- schema.graphql
+type Query { me: User }
+type User { name: String }