From e4facf3c9100ff91ebafc200e4671fcdf165a6ee Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Tue, 15 Oct 2024 16:25:45 -0700 Subject: [PATCH] Allow granular FeatureFlag gating of `@alias` enforcement Reviewed By: gordyf Differential Revision: D64411904 fbshipit-source-id: 52c6f320436496aa23056fa6ef391f0e3195ba36 --- .../relay-codegen/tests/aliased_fragments.rs | 3 ++- compiler/crates/relay-codemod/Cargo.toml | 1 + compiler/crates/relay-codemod/src/codemod.rs | 4 +++- .../relay-transforms/src/apply_transforms.rs | 10 ++++------ .../src/fragment_alias_directive.rs | 19 ++++++++++++++----- .../relay-transforms/tests/catch_directive.rs | 3 ++- .../tests/fragment_alias_directive.rs | 5 ++++- .../relay-transforms/tests/relay_resolvers.rs | 3 ++- .../tests/required_directive.rs | 3 ++- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/compiler/crates/relay-codegen/tests/aliased_fragments.rs b/compiler/crates/relay-codegen/tests/aliased_fragments.rs index 4a46f196e2155..7f49c09c40bcd 100644 --- a/compiler/crates/relay-codegen/tests/aliased_fragments.rs +++ b/compiler/crates/relay-codegen/tests/aliased_fragments.rs @@ -7,6 +7,7 @@ use std::sync::Arc; +use common::FeatureFlag; use common::SourceLocationKey; use fixture_tests::Fixture; use graphql_ir::build; @@ -29,7 +30,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result .map_err(|diagnostics| diagnostics_to_sorted_string(fixture.content, &diagnostics))?; let program = Program::from_definitions(Arc::clone(&schema), ir); - fragment_alias_directive(&program, true) + fragment_alias_directive(&program, &FeatureFlag::Enabled) .map(|next_program| { next_program .fragments() diff --git a/compiler/crates/relay-codemod/Cargo.toml b/compiler/crates/relay-codemod/Cargo.toml index d01546f8435ba..3f4372fb73a12 100644 --- a/compiler/crates/relay-codemod/Cargo.toml +++ b/compiler/crates/relay-codemod/Cargo.toml @@ -10,6 +10,7 @@ license = "MIT" [dependencies] clap = { version = "4.5.20", features = ["derive", "env", "string", "unicode", "wrap_help"] } +common = { path = "../common" } log = { version = "0.4.22", features = ["kv_unstable"] } lsp-types = "0.94.1" relay-compiler = { path = "../relay-compiler" } diff --git a/compiler/crates/relay-codemod/src/codemod.rs b/compiler/crates/relay-codemod/src/codemod.rs index c9305f60b19c0..a701bff3f177c 100644 --- a/compiler/crates/relay-codemod/src/codemod.rs +++ b/compiler/crates/relay-codemod/src/codemod.rs @@ -9,6 +9,7 @@ use std::fs; use std::sync::Arc; use clap::ValueEnum; +use common::FeatureFlag; use log::info; use lsp_types::CodeActionOrCommand; use lsp_types::TextEdit; @@ -32,7 +33,8 @@ pub async fn run_codemod( .iter() .flat_map(|programs| match &codemod { AvailableCodemod::MarkDangerousConditionalFragmentSpreads => { - match fragment_alias_directive(&programs.source, true) { + // TODO: Figure out how to accept FeatureFlag as an optional CLI argument + match fragment_alias_directive(&programs.source, &FeatureFlag::Enabled) { Ok(_) => vec![], Err(e) => e, } diff --git a/compiler/crates/relay-transforms/src/apply_transforms.rs b/compiler/crates/relay-transforms/src/apply_transforms.rs index 44b4834e58bfc..25ab8c6af5d8c 100644 --- a/compiler/crates/relay-transforms/src/apply_transforms.rs +++ b/compiler/crates/relay-transforms/src/apply_transforms.rs @@ -160,10 +160,9 @@ fn apply_common_transforms( program = log_event.time("fragment_alias_directive", || { fragment_alias_directive( &program, - project_config + &project_config .feature_flags - .enforce_fragment_alias_where_ambiguous - .is_fully_enabled(), + .enforce_fragment_alias_where_ambiguous, ) })?; @@ -658,10 +657,9 @@ fn apply_typegen_transforms( program = log_event.time("fragment_alias_directive", || { fragment_alias_directive( &program, - project_config + &project_config .feature_flags - .enforce_fragment_alias_where_ambiguous - .is_fully_enabled(), + .enforce_fragment_alias_where_ambiguous, ) })?; diff --git a/compiler/crates/relay-transforms/src/fragment_alias_directive.rs b/compiler/crates/relay-transforms/src/fragment_alias_directive.rs index 6a0ac3f80ee8f..4a3234986044d 100644 --- a/compiler/crates/relay-transforms/src/fragment_alias_directive.rs +++ b/compiler/crates/relay-transforms/src/fragment_alias_directive.rs @@ -11,6 +11,7 @@ use common::ArgumentName; use common::Diagnostic; use common::DiagnosticsResult; use common::DirectiveName; +use common::FeatureFlag; use common::NamedItem; use common::WithLocation; use graphql_ir::associated_data_impl; @@ -56,7 +57,7 @@ associated_data_impl!(FragmentAliasMetadata); pub fn fragment_alias_directive( program: &Program, - is_enforced: bool, + is_enforced: &FeatureFlag, ) -> DiagnosticsResult { let mut transform = FragmentAliasTransform::new(program, is_enforced); let next_program = transform @@ -119,21 +120,23 @@ impl Transformer for AliasedInlineFragmentRemovalTransform { struct FragmentAliasTransform<'program> { program: &'program Program, - is_enforced: bool, + is_enforced: &'program FeatureFlag, document_name: Option, parent_type: Option, + parent_name: Option, within_inline_fragment_type_condition: bool, maybe_condition: Option, errors: Vec, } impl<'program> FragmentAliasTransform<'program> { - fn new(program: &'program Program, enforced: bool) -> Self { + fn new(program: &'program Program, enforced: &'program FeatureFlag) -> Self { Self { program, is_enforced: enforced, document_name: None, parent_type: None, + parent_name: None, within_inline_fragment_type_condition: false, maybe_condition: None, errors: Vec::new(), @@ -163,8 +166,10 @@ impl<'program> FragmentAliasTransform<'program> { type_condition: Option, spread: &FragmentSpread, ) { - if !self.is_enforced { - return; + if let Some(parent_name) = self.parent_name { + if !self.is_enforced.is_enabled_for(parent_name) { + return; + } } if spread .directives @@ -238,9 +243,11 @@ impl Transformer for FragmentAliasTransform<'_> { fragment: &FragmentDefinition, ) -> Transformed { self.document_name = Some(fragment.name.item.0); + self.parent_name = Some(fragment.name.item.0); self.parent_type = Some(fragment.type_condition); let transformed = self.default_transform_fragment(fragment); self.parent_type = None; + self.parent_name = None; self.document_name = None; transformed } @@ -251,8 +258,10 @@ impl Transformer for FragmentAliasTransform<'_> { ) -> Transformed { self.document_name = Some(operation.name.item.0); self.parent_type = Some(operation.type_); + self.parent_name = Some(operation.name.item.0); let transformed = self.default_transform_operation(operation); self.parent_type = None; + self.parent_name = None; self.document_name = None; transformed } diff --git a/compiler/crates/relay-transforms/tests/catch_directive.rs b/compiler/crates/relay-transforms/tests/catch_directive.rs index 3bb76b24431d4..c5e37b9e129d3 100644 --- a/compiler/crates/relay-transforms/tests/catch_directive.rs +++ b/compiler/crates/relay-transforms/tests/catch_directive.rs @@ -6,6 +6,7 @@ */ use common::DiagnosticsResult; +use common::FeatureFlag; use fixture_tests::Fixture; use graphql_ir::Program; use graphql_test_helpers::apply_transform_for_test; @@ -13,7 +14,7 @@ use relay_transforms::catch_directive; use relay_transforms::fragment_alias_directive; fn transform(program: &Program) -> DiagnosticsResult { - catch_directive(&fragment_alias_directive(program, true)?) + catch_directive(&fragment_alias_directive(program, &FeatureFlag::Enabled)?) } pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result { diff --git a/compiler/crates/relay-transforms/tests/fragment_alias_directive.rs b/compiler/crates/relay-transforms/tests/fragment_alias_directive.rs index ca85416eb3a61..251e550086ad5 100644 --- a/compiler/crates/relay-transforms/tests/fragment_alias_directive.rs +++ b/compiler/crates/relay-transforms/tests/fragment_alias_directive.rs @@ -5,10 +5,13 @@ * LICENSE file in the root directory of this source tree. */ +use common::FeatureFlag; use fixture_tests::Fixture; use graphql_test_helpers::apply_transform_for_test; use relay_transforms::fragment_alias_directive; pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result { - apply_transform_for_test(fixture, |program| fragment_alias_directive(program, true)) + apply_transform_for_test(fixture, |program| { + fragment_alias_directive(program, &FeatureFlag::Enabled) + }) } diff --git a/compiler/crates/relay-transforms/tests/relay_resolvers.rs b/compiler/crates/relay-transforms/tests/relay_resolvers.rs index 2f72741924a43..4b4d73b22857b 100644 --- a/compiler/crates/relay-transforms/tests/relay_resolvers.rs +++ b/compiler/crates/relay-transforms/tests/relay_resolvers.rs @@ -8,6 +8,7 @@ use std::sync::Arc; use common::Diagnostic; +use common::FeatureFlag; use common::SourceLocationKey; use common::TextSource; use fixture_tests::Fixture; @@ -40,7 +41,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result // Run `fragment_alias_directive` first because we want to ensure we // correctly generate paths for named inline fragment spreads. - let next_program = fragment_alias_directive(&program, true) + let next_program = fragment_alias_directive(&program, &FeatureFlag::Enabled) .and_then(|program| relay_resolvers(ProjectName::default(), &program)) .map_err(|diagnostics| diagnostics_to_sorted_string(base, extensions, &diagnostics))?; diff --git a/compiler/crates/relay-transforms/tests/required_directive.rs b/compiler/crates/relay-transforms/tests/required_directive.rs index 23ef4530b70ac..5272cf93b9061 100644 --- a/compiler/crates/relay-transforms/tests/required_directive.rs +++ b/compiler/crates/relay-transforms/tests/required_directive.rs @@ -6,6 +6,7 @@ */ use common::DiagnosticsResult; +use common::FeatureFlag; use fixture_tests::Fixture; use graphql_ir::Program; use graphql_test_helpers::apply_transform_for_test; @@ -13,7 +14,7 @@ use relay_transforms::fragment_alias_directive; use relay_transforms::required_directive; fn transform(program: &Program) -> DiagnosticsResult { - required_directive(&fragment_alias_directive(program, false)?) + required_directive(&fragment_alias_directive(program, &FeatureFlag::Enabled)?) } pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result {