Refactor Transform lifetimes

Reviewed By: captbaritone Differential Revision: D66908277 fbshipit-source-id: b0e62242b2ab323d7332604a612177101e24edd2
facebook · Dec 10, 2024 · 652ab3b · 652ab3b
1 parent 620e7a6
commit 652ab3b
Show file tree

Hide file tree

Showing 45 changed files with 141 additions and 120 deletions.
diff --git a/compiler/crates/graphql-ir/src/ir.rs b/compiler/crates/graphql-ir/src/ir.rs
@@ -11,6 +11,7 @@ use std::fmt;
 use std::fmt::Display;
 use std::fmt::Formatter;
 use std::hash::Hash;
+use std::hash::Hasher;
 use std::str::FromStr;
 use std::sync::Arc;
 
@@ -296,7 +297,7 @@ impl Named for VariableDefinition {
 // Selections
 
 /// A selection within an operation or fragment
-#[derive(Clone, Eq, PartialEq)]
+#[derive(Clone, Eq, PartialEq, Hash)]
 pub enum Selection {
     FragmentSpread(Arc<FragmentSpread>),
     InlineFragment(Arc<InlineFragment>),
@@ -401,7 +402,7 @@ impl fmt::Debug for Selection {
 }
 
 /// ... Name
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct FragmentSpread {
     pub fragment: WithLocation<FragmentDefinitionName>,
     pub arguments: Vec<Argument>,
@@ -437,6 +438,16 @@ pub struct InlineFragment {
     pub spread_location: Location,
 }
 
+impl Hash for InlineFragment {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.type_condition.hash(state);
+        self.directives.hash(state);
+        self.spread_location.hash(state);
+        // Avoid descending into selections, which leads to large recursion
+        self.selections.len().hash(state);
+    }
+}
+
 impl InlineFragment {
     /// Get the alias of this inline fragment from the optional `@alias` directive.
     /// If the `as` argument is not present, the type condition is used as the fallback.
@@ -495,6 +506,17 @@ pub struct LinkedField {
     pub selections: Vec<Selection>,
 }
 
+impl Hash for LinkedField {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.alias.hash(state);
+        self.definition.hash(state);
+        self.arguments.hash(state);
+        self.directives.hash(state);
+        // Avoid descending into selections, which leads to large recursion
+        self.selections.len().hash(state);
+    }
+}
+
 impl Field for LinkedField {
     fn alias(&self) -> Option<WithLocation<StringKey>> {
         self.alias
@@ -514,7 +536,7 @@ impl Field for LinkedField {
 }
 
 /// Name Arguments?
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct ScalarField {
     pub alias: Option<WithLocation<StringKey>>,
     pub definition: WithLocation<FieldID>,
@@ -549,6 +571,16 @@ pub struct Condition {
     pub location: Location,
 }
 
+impl Hash for Condition {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.value.hash(state);
+        self.passing_value.hash(state);
+        self.location.hash(state);
+        // Avoid descending into selections, which leads to large recursion
+        self.selections.len().hash(state);
+    }
+}
+
 impl Condition {
     pub fn directive_name(&self) -> &'static str {
         if self.passing_value {
@@ -702,7 +734,7 @@ impl ConstantValue {
     generate_unwrap_fn!(unwrap_object, self, &Vec<ConstantArgument>, ConstantValue::Object(o) => o);
 }
 
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub enum ConditionValue {
     Constant(bool),
     Variable(Variable),

diff --git a/compiler/crates/graphql-ir/src/signatures.rs b/compiler/crates/graphql-ir/src/signatures.rs
@@ -82,7 +82,7 @@ associated_data_impl!(ProvidedVariableMetadata);
 /// would depend on having checked its body! Since recursive fragments
 /// are allowed, we break the cycle by first computing signatures
 /// and using these to type check fragment spreads in selections.
-#[derive(Clone, Debug, Eq, PartialEq)]
+#[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct FragmentSignature {
     pub name: WithLocation<FragmentDefinitionName>,
     pub variable_definitions: Vec<VariableDefinition>,

diff --git a/compiler/crates/graphql-ir/src/transform.rs b/compiler/crates/graphql-ir/src/transform.rs
@@ -12,17 +12,17 @@ use common::WithLocation;
 use crate::ir::*;
 use crate::program::Program;
 
-pub trait Transformer {
+pub trait Transformer<'a> {
     const NAME: &'static str;
     const VISIT_ARGUMENTS: bool;
     const VISIT_DIRECTIVES: bool;
     const RETAIN_EMPTY_SELECTION_SETS: bool = false;
 
-    fn transform_program(&mut self, program: &Program) -> TransformedValue<Program> {
+    fn transform_program(&mut self, program: &'a Program) -> TransformedValue<Program> {
         self.default_transform_program(program)
     }
 
-    fn default_transform_program(&mut self, program: &Program) -> TransformedValue<Program> {
+    fn default_transform_program(&mut self, program: &'a Program) -> TransformedValue<Program> {
         let mut next_program = Program::new(Arc::clone(&program.schema));
         let mut has_changes = false;
         for operation in program.operations() {
@@ -55,14 +55,14 @@ pub trait Transformer {
     // Fragment Definition
     fn transform_fragment(
         &mut self,
-        fragment: &FragmentDefinition,
+        fragment: &'a FragmentDefinition,
     ) -> Transformed<FragmentDefinition> {
         self.default_transform_fragment(fragment)
     }
 
     fn default_transform_fragment(
         &mut self,
-        fragment: &FragmentDefinition,
+        fragment: &'a FragmentDefinition,
     ) -> Transformed<FragmentDefinition> {
         let selections = self.transform_selections(&fragment.selections);
         let directives = self.transform_directives(&fragment.directives);
@@ -95,14 +95,14 @@ pub trait Transformer {
     // Operation Definition
     fn transform_operation(
         &mut self,
-        operation: &OperationDefinition,
+        operation: &'a OperationDefinition,
     ) -> Transformed<OperationDefinition> {
         self.default_transform_operation(operation)
     }
 
     fn default_transform_operation(
         &mut self,
-        operation: &OperationDefinition,
+        operation: &'a OperationDefinition,
     ) -> Transformed<OperationDefinition> {
         let selections = self.transform_selections(&operation.selections);
         let directives = self.transform_directives(&operation.directives);
@@ -172,16 +172,16 @@ pub trait Transformer {
     // Selection
     fn transform_selections(
         &mut self,
-        selections: &[Selection],
+        selections: &'a [Selection],
     ) -> TransformedValue<Vec<Selection>> {
         transform_list(selections, |selection| self.transform_selection(selection))
     }
 
-    fn transform_selection(&mut self, selection: &Selection) -> Transformed<Selection> {
+    fn transform_selection(&mut self, selection: &'a Selection) -> Transformed<Selection> {
         self.default_transform_selection(selection)
     }
 
-    fn default_transform_selection(&mut self, selection: &Selection) -> Transformed<Selection> {
+    fn default_transform_selection(&mut self, selection: &'a Selection) -> Transformed<Selection> {
         match selection {
             Selection::FragmentSpread(selection) => self.transform_fragment_spread(selection),
             Selection::InlineFragment(selection) => self.transform_inline_fragment(selection),
@@ -209,11 +209,11 @@ pub trait Transformer {
         })))
     }
 
-    fn transform_linked_field(&mut self, field: &LinkedField) -> Transformed<Selection> {
+    fn transform_linked_field(&mut self, field: &'a LinkedField) -> Transformed<Selection> {
         self.default_transform_linked_field(field)
     }
 
-    fn default_transform_linked_field(&mut self, field: &LinkedField) -> Transformed<Selection> {
+    fn default_transform_linked_field(&mut self, field: &'a LinkedField) -> Transformed<Selection> {
         // Special-case for empty selections
         let selections = self.transform_selections(&field.selections);
         if let TransformedValue::Replace(selections) = &selections {
@@ -234,13 +234,16 @@ pub trait Transformer {
         })))
     }
 
-    fn transform_inline_fragment(&mut self, fragment: &InlineFragment) -> Transformed<Selection> {
+    fn transform_inline_fragment(
+        &mut self,
+        fragment: &'a InlineFragment,
+    ) -> Transformed<Selection> {
         self.default_transform_inline_fragment(fragment)
     }
 
     fn default_transform_inline_fragment(
         &mut self,
-        fragment: &InlineFragment,
+        fragment: &'a InlineFragment,
     ) -> Transformed<Selection> {
         // Special-case for empty selections
         let selections = self.transform_selections(&fragment.selections);
@@ -280,11 +283,11 @@ pub trait Transformer {
     }
 
     // Conditions
-    fn transform_condition(&mut self, condition: &Condition) -> Transformed<Selection> {
+    fn transform_condition(&mut self, condition: &'a Condition) -> Transformed<Selection> {
         self.default_transform_condition(condition)
     }
 
-    fn default_transform_condition(&mut self, condition: &Condition) -> Transformed<Selection> {
+    fn default_transform_condition(&mut self, condition: &'a Condition) -> Transformed<Selection> {
         // Special-case for empty selections
         let selections = self.transform_selections(&condition.selections);
         if let TransformedValue::Replace(selections) = &selections {
@@ -399,10 +402,10 @@ pub trait Transformer {
 }
 
 // Helpers
-pub fn transform_list<T, F, R>(list: &[T], mut transform: F) -> TransformedValue<Vec<T>>
+pub fn transform_list<'a, T, F, R>(list: &'a [T], mut transform: F) -> TransformedValue<Vec<T>>
 where
     T: Clone,
-    F: FnMut(&T) -> R,
+    F: FnMut(&'a T) -> R,
     R: Into<Transformed<T>>,
 {
     let mut result = Vec::new();
@@ -519,7 +522,7 @@ impl TransformProgramPipe {
 
     pub fn pipe<T>(self, transformer: T) -> Self
     where
-        T: Transformer,
+        T: for<'a> Transformer<'a>,
     {
         let mut transformer = transformer;
         let initial = self.initial;
@@ -539,7 +542,7 @@ impl TransformProgramPipe {
 
     pub fn pipe_option<X, T, F>(self, option: Option<X>, get_transformer: F) -> Self
     where
-        T: Transformer,
+        T: for<'a> Transformer<'a>,
         F: FnOnce(X) -> T,
     {
         if let Some(x) = option {

diff --git a/compiler/crates/relay-transforms/src/apply_fragment_arguments.rs b/compiler/crates/relay-transforms/src/apply_fragment_arguments.rs
@@ -169,7 +169,7 @@ struct ApplyFragmentArgumentsTransform<'flags, 'program, 'base_fragments> {
     split_operations: StringKeyMap<(Option<OperationDefinition>, ProvidedVariablesMap)>,
 }
 
-impl Transformer for ApplyFragmentArgumentsTransform<'_, '_, '_> {
+impl Transformer<'_> for ApplyFragmentArgumentsTransform<'_, '_, '_> {
     const NAME: &'static str = "ApplyFragmentArgumentsTransform";
     const VISIT_ARGUMENTS: bool = true;
     const VISIT_DIRECTIVES: bool = true;

diff --git a/...es/relay-transforms/src/assignable_fragment_spread/annotate_updatable_fragment_spreads.rs b/...es/relay-transforms/src/assignable_fragment_spread/annotate_updatable_fragment_spreads.rs
@@ -39,7 +39,7 @@ struct AnnotateUpdatableFragmentSpreads<'s> {
     program: &'s Program,
 }
 
-impl<'s> Transformer for AnnotateUpdatableFragmentSpreads<'s> {
+impl<'s> Transformer<'_> for AnnotateUpdatableFragmentSpreads<'s> {
     const NAME: &'static str = "AnnotateUpdatableFragmentSpreads";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;

diff --git a/...tes/relay-transforms/src/assignable_fragment_spread/replace_updatable_fragment_spreads.rs b/...tes/relay-transforms/src/assignable_fragment_spread/replace_updatable_fragment_spreads.rs
@@ -31,7 +31,7 @@ struct ReplaceAssignableFragmentSpreads<'s> {
     program: &'s Program,
 }
 
-impl<'s> Transformer for ReplaceAssignableFragmentSpreads<'s> {
+impl<'s> Transformer<'_> for ReplaceAssignableFragmentSpreads<'s> {
     const NAME: &'static str = "ReplaceAssignableFragmentSpreads";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;

diff --git a/...rc/assignable_fragment_spread/transform_assignable_fragment_spreads_in_regular_queries.rs b/...rc/assignable_fragment_spread/transform_assignable_fragment_spreads_in_regular_queries.rs
@@ -137,7 +137,7 @@ enum ValidGeneratedFlowType {
     Any,
 }
 
-impl<'s> Transformer for AssignableFragmentSpread<'s> {
+impl<'s> Transformer<'_> for AssignableFragmentSpread<'s> {
     const NAME: &'static str = "AssignableFragmentTransform";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;

diff --git a/.../assignable_fragment_spread/transform_assignable_fragment_spreads_in_updatable_queries.rs b/.../assignable_fragment_spread/transform_assignable_fragment_spreads_in_updatable_queries.rs
@@ -71,7 +71,7 @@ struct AssignableFragmentSpreadForUpdatable<'s> {
     program: &'s Program,
 }
 
-impl<'s> Transformer for AssignableFragmentSpreadForUpdatable<'s> {
+impl<'s> Transformer<'_> for AssignableFragmentSpreadForUpdatable<'s> {
     const NAME: &'static str = "AssignableFragmentTransformForUpdatable";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;

diff --git a/compiler/crates/relay-transforms/src/catch_directive.rs b/compiler/crates/relay-transforms/src/catch_directive.rs
@@ -137,7 +137,7 @@ impl<'program> CatchDirective<'program> {
     }
 }
 
-impl<'s> Transformer for CatchDirective<'s> {
+impl<'s> Transformer<'_> for CatchDirective<'s> {
     const NAME: &'static str = "CatchDirectiveTransform";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;

diff --git a/compiler/crates/relay-transforms/src/client_edges.rs b/compiler/crates/relay-transforms/src/client_edges.rs
@@ -103,7 +103,7 @@ associated_data_impl!(ClientEdgeGeneratedQueryMetadataDirective);
 
 pub struct ClientEdgeMetadata<'a> {
     /// The field which defines the graph relationship (currently always a Resolver)
-    pub backing_field: Selection,
+    pub backing_field: &'a Selection,
     /// Models the client edge field and its selections
     pub linked_field: &'a LinkedField,
     /// Additional metadata about the client edge
@@ -132,14 +132,10 @@ impl<'a> ClientEdgeMetadata<'a> {
                 fragment.selections.len() == 2,
                 "Expected Client Edge inline fragment to have exactly two selections. This is a bug in the Relay compiler."
             );
-            let mut backing_field = fragment
-                .selections.first()
-                .expect("Client Edge inline fragments have exactly two selections").clone();
 
-            let backing_field_directives = backing_field.directives().iter().filter(|directive|
-                directive.name.item != RequiredMetadataDirective::directive_name()
-            ).cloned().collect();
-            backing_field.set_directives(backing_field_directives);
+            let backing_field = fragment
+                .selections.first()
+                .expect("Client Edge inline fragments have exactly two selections");
 
             let linked_field = match fragment.selections.get(1) {
                 Some(Selection::LinkedField(linked_field)) => linked_field,
@@ -605,7 +601,20 @@ fn create_inline_fragment_for_client_edge(
     }
 
     let transformed_field = Arc::new(LinkedField {
+        selections: selections.clone(),
+        ..field.clone()
+    });
+
+    let backing_field_directives = field
+        .directives()
+        .iter()
+        .filter(|directive| directive.name.item != RequiredMetadataDirective::directive_name())
+        .cloned()
+        .collect();
+
+    let backing_field = Arc::new(LinkedField {
         selections,
+        directives: backing_field_directives,
         ..field.clone()
     });
 
@@ -614,14 +623,14 @@ fn create_inline_fragment_for_client_edge(
         directives: inline_fragment_directives,
         selections: vec![
             // NOTE: This creates 2^H selecitons where H is the depth of nested client edges
+            Selection::LinkedField(Arc::clone(&backing_field)),
             Selection::LinkedField(Arc::clone(&transformed_field)),
-            Selection::LinkedField(transformed_field),
         ],
         spread_location: Location::generated(),
     }
 }
 
-impl Transformer for ClientEdgesTransform<'_, '_> {
+impl Transformer<'_> for ClientEdgesTransform<'_, '_> {
     const NAME: &'static str = "ClientEdgesTransform";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;
@@ -715,7 +724,7 @@ pub fn remove_client_edge_selections(program: &Program) -> DiagnosticsResult<Pro
 #[derive(Default)]
 struct ClientEdgesCleanupTransform;
 
-impl Transformer for ClientEdgesCleanupTransform {
+impl Transformer<'_> for ClientEdgesCleanupTransform {
     const NAME: &'static str = "ClientEdgesCleanupTransform";
     const VISIT_ARGUMENTS: bool = false;
     const VISIT_DIRECTIVES: bool = false;
@@ -726,7 +735,7 @@ impl Transformer for ClientEdgesCleanupTransform {
                 let new_selection = metadata.backing_field;
 
                 Transformed::Replace(
-                    self.transform_selection(&new_selection)
+                    self.transform_selection(new_selection)
                         .unwrap_or_else(|| new_selection.clone()),
                 )
             }