T116895311 [bootcamp] Add location information to conditions

Reviewed By: rbalicki2

Differential Revision: D38034991

fbshipit-source-id: 007d1f0fc9de788902c9b01064592337d039b51f

compiler/crates/graphql-ir/src/build.rs

@@ -1924,6 +1924,7 @@ fn wrap_selection_with_condition(selection: &Selection, condition: &Directive) -
         },
         passing_value: condition.name.item.lookup() == "include",
         selections: vec![selection.clone()],
+        location: condition.name.location,
     }))
 }
 

compiler/crates/graphql-ir/src/ir.rs

@@ -157,14 +157,13 @@ impl Selection {
     /// A quick method to get the location of the selection. This may
     /// be helpful for error reporting. Please note, this implementation
     /// prefers the location of the alias for scalar and linked field selections.
-    /// It also returns `None` for conditional nodes and inline fragments.
-    pub fn location(&self) -> Option<Location> {
+    pub fn location(&self) -> Location {
         match self {
-            Selection::Condition(_) => None,
-            Selection::FragmentSpread(node) => Some(node.fragment.location),
-            Selection::InlineFragment(_) => None,
-            Selection::LinkedField(node) => Some(node.alias_or_name_location()),
-            Selection::ScalarField(node) => Some(node.alias_or_name_location()),
+            Selection::Condition(node) => node.location,
+            Selection::FragmentSpread(node) => node.fragment.location,
+            Selection::InlineFragment(node) => node.spread_location,
+            Selection::LinkedField(node) => node.alias_or_name_location(),
+            Selection::ScalarField(node) => node.alias_or_name_location(),
         }
     }
 
@@ -317,6 +316,7 @@ pub struct Condition {
     pub selections: Vec<Selection>,
     pub value: ConditionValue,
     pub passing_value: bool,
+    pub location: Location,
 }
 
 impl Condition {

compiler/crates/graphql-ir/tests/parse/fixtures/directive-include.expected

@@ -63,6 +63,7 @@ fragment Foo on User {
                         },
                     ),
                     passing_value: true,
+                    location: directive-include.graphql:37:45,
                 },
                 Condition {
                     selections: [
@@ -99,6 +100,7 @@ fragment Foo on User {
                         },
                     ),
                     passing_value: true,
+                    location: directive-include.graphql:71:79,
                 },
                 Condition {
                     selections: [
@@ -125,6 +127,7 @@ fragment Foo on User {
                         },
                     ),
                     passing_value: true,
+                    location: directive-include.graphql:120:128,
                 },
             ],
         },

compiler/crates/graphql-ir/tests/parse_with_provider/fixtures/use_fragment_spread_with_provider.expected

@@ -192,6 +192,7 @@ fragment ChildFragment2 on User
                                 },
                             ),
                             passing_value: true,
+                            location: use_fragment_spread_with_provider.graphql:331:339,
                         },
                     ],
                 },
@@ -272,6 +273,7 @@ fragment ChildFragment2 on User
                         },
                     ),
                     passing_value: true,
+                    location: use_fragment_spread_with_provider.graphql:533:541,
                 },
             ],
         },

compiler/crates/relay-transforms/src/assignable_fragment_spread/ensure_discriminated_union_is_created.rs

@@ -65,10 +65,13 @@ pub(super) fn ensure_discriminated_union_is_created(
                     errors.extend(e.into_iter());
                 }
             }
-            _ => errors.push(Diagnostic::error(
-                ValidationMessage::EnsureDiscriminatedUnionNonInlineFragment { reason_message },
-                linked_field.definition.location,
-            )),
+            _ => errors.push(
+                Diagnostic::error(
+                    ValidationMessage::EnsureDiscriminatedUnionNonInlineFragment { reason_message },
+                    selection.location(),
+                )
+                .annotate("enclosing linked field", linked_field.definition.location),
+            ),
         }
     }
 

compiler/crates/relay-transforms/src/assignable_fragment_spread/errors.rs

@@ -58,14 +58,8 @@ pub enum ValidationMessage {
         outer_type_plural: &'static str,
     },
 
-    // Note: conditions do not have a location, hence this awkward phrasing
-    #[error(
-        "Within updatable {outer_type_plural}, the directives @include and @skip are not allowed. The directive was found in {operation_or_fragment_name}."
-    )]
-    UpdatableNoConditions {
-        outer_type_plural: &'static str,
-        operation_or_fragment_name: StringKey,
-    },
+    #[error("The directives @include and @skip are not allowed within {outer_type_plural}.")]
+    UpdatableNoConditions { outer_type_plural: &'static str },
 
     #[error(
         "Within updatable {outer_type_plural}, if a linked field contains an inline fragment spread, it must contain only inline fragment spreads."
@@ -149,7 +143,7 @@ pub enum ValidationMessage {
     },
 
     #[error(
-        "Because {reason_message}, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @if."
+        "Because {reason_message}, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include."
     )]
     EnsureDiscriminatedUnionNonInlineFragment { reason_message: &'static str },
 

compiler/crates/relay-transforms/src/assignable_fragment_spread/validate_updatable_directive.rs

@@ -354,13 +354,12 @@ impl<'a> Validator for UpdatableDirective<'a> {
         )
     }
 
-    fn validate_condition(&mut self, _condition: &Condition) -> DiagnosticsResult<()> {
+    fn validate_condition(&mut self, condition: &Condition) -> DiagnosticsResult<()> {
         Err(vec![Diagnostic::error(
             ValidationMessage::UpdatableNoConditions {
                 outer_type_plural: self.executable_definition_info.unwrap().type_plural,
-                operation_or_fragment_name: self.executable_definition_info.unwrap().name,
             },
-            self.executable_definition_info.unwrap().location,
+            condition.location,
         )])
     }
 }

compiler/crates/relay-transforms/src/flatten.rs

@@ -285,6 +285,7 @@ impl FlattenTransform {
                         value: node.value.clone(),
                         passing_value: node.passing_value,
                         selections: next_selections,
+                        location: node.location,
                     }))
                 }),
             Selection::FragmentSpread(_) | Selection::ScalarField(_) => TransformedValue::Keep,

compiler/crates/relay-transforms/src/relay_actor_change.rs

@@ -99,9 +99,7 @@ impl<'program, 'feature> Transformer for ActorChangeTransform<'program, 'feature
                 selection => {
                     self.errors.push(Diagnostic::error(
                         ValidationMessage::ActorChangeInvalidSelection,
-                        selection
-                            .location()
-                            .unwrap_or_else(|| field.alias_or_name_location()),
+                        selection.location(),
                     ));
                     return Transformed::Keep;
                 }

compiler/crates/relay-transforms/tests/assignable_fragment_spread/fixtures/assignable-fragment-spread-within-skipped-inline-fragment.invalid.expected

@@ -12,7 +12,15 @@ fragment Assignable_user on User @assignable {
   __typename
 }
 ==================================== ERROR ====================================
-✖︎ Because an assignable fragment was spread in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @if.
+✖︎ Because an assignable fragment was spread in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include.
+
+  assignable-fragment-spread-within-skipped-inline-fragment.invalid.graphql:4:9
+    3 │   node(id: "4") {
+    4 │     ... @skip(if: $skip) {
+      │         ^^^^^
+    5 │       ...Assignable_user
+
+  ℹ︎ enclosing linked field
 
   assignable-fragment-spread-within-skipped-inline-fragment.invalid.graphql:3:3
     2 │ query TestQuery($skip: Boolean!) {

compiler/crates/relay-transforms/tests/updatable_directive/fixtures/include.invalid.expected

@@ -6,10 +6,10 @@ query TestQuery @updatable {
   }
 }
 ==================================== ERROR ====================================
-✖︎ Within updatable operations, the directives @include and @skip are not allowed. The directive was found in TestQuery.
+✖︎ The directives @include and @skip are not allowed within operations.
 
-  include.invalid.graphql:2:7
-    1 │ # expected-to-throw
+  include.invalid.graphql:3:15
     2 │ query TestQuery @updatable {
-      │       ^^^^^^^^^
     3 │   node(id: 4) @include(if: true) {
+      │               ^^^^^^^^
+    4 │     id

compiler/crates/relay-transforms/tests/updatable_directive/fixtures/skip.invalid.expected

@@ -6,10 +6,10 @@ query TestQuery @updatable {
   }
 }
 ==================================== ERROR ====================================
-✖︎ Within updatable operations, the directives @include and @skip are not allowed. The directive was found in TestQuery.
+✖︎ The directives @include and @skip are not allowed within operations.
 
-  skip.invalid.graphql:2:7
-    1 │ # expected-to-throw
+  skip.invalid.graphql:3:15
     2 │ query TestQuery @updatable {
-      │       ^^^^^^^^^
     3 │   node(id: 4) @skip(if: true) {
+      │               ^^^^^
+    4 │     id

compiler/crates/relay-transforms/tests/updatable_fragment_spread/fixtures/updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_2.expected

@@ -14,7 +14,15 @@ fragment Updatable_user on User @updatable {
   __typename
 }
 ==================================== ERROR ====================================
-✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @if.
+✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include.
+
+  updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_2.graphql:8:8
+    7 │     }
+    8 │     id @skip(if: true)
+      │        ^^^^^
+    9 │   }
+
+  ℹ︎ enclosing linked field
 
   updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_2.graphql:3:3
     2 │ fragment Foo on Query {

compiler/crates/relay-transforms/tests/updatable_fragment_spread/fixtures/updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_3.expected

@@ -14,7 +14,15 @@ fragment Updatable_user on User @updatable {
   __typename
 }
 ==================================== ERROR ====================================
-✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @if.
+✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include.
+
+  updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_3.graphql:8:5
+    7 │     }
+    8 │     id
+      │     ^^
+    9 │   }
+
+  ℹ︎ enclosing linked field
 
   updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_3.graphql:3:3
     2 │ fragment Foo on Query {

compiler/crates/relay-transforms/tests/updatable_fragment_spread/fixtures/updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_4.expected

@@ -19,7 +19,15 @@ fragment Foo_node on Node {
   __id
 }
 ==================================== ERROR ====================================
-✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @if.
+✖︎ Because an updatable fragment was spread in an inline fragment in this linked field, this linked field can only contain inline fragments, and any inline fragments cannot have @skip or @include.
+
+  updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_4.graphql:8:8
+    7 │     }
+    8 │     ...Foo_node
+      │        ^^^^^^^^
+    9 │   }
+
+  ℹ︎ enclosing linked field
 
   updatable_fragment_spread_in_inline_fragment_other_selections_wrong_type.invalid_4.graphql:3:3
     2 │ fragment Foo on Query {