Skip to content

Commit

Permalink
Allow granular FeatureFlag gating of @alias enforcement
Browse files Browse the repository at this point in the history
Reviewed By: gordyf

Differential Revision: D64411904

fbshipit-source-id: 52c6f320436496aa23056fa6ef391f0e3195ba36
  • Loading branch information
captbaritone authored and facebook-github-bot committed Oct 15, 2024
1 parent d116444 commit e4facf3
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 17 deletions.
3 changes: 2 additions & 1 deletion compiler/crates/relay-codegen/tests/aliased_fragments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

use std::sync::Arc;

use common::FeatureFlag;
use common::SourceLocationKey;
use fixture_tests::Fixture;
use graphql_ir::build;
Expand All @@ -29,7 +30,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>
.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()
Expand Down
1 change: 1 addition & 0 deletions compiler/crates/relay-codemod/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
4 changes: 3 additions & 1 deletion compiler/crates/relay-codemod/src/codemod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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,
}
Expand Down
10 changes: 4 additions & 6 deletions compiler/crates/relay-transforms/src/apply_transforms.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
})?;

Expand Down Expand Up @@ -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,
)
})?;

Expand Down
19 changes: 14 additions & 5 deletions compiler/crates/relay-transforms/src/fragment_alias_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -56,7 +57,7 @@ associated_data_impl!(FragmentAliasMetadata);

pub fn fragment_alias_directive(
program: &Program,
is_enforced: bool,
is_enforced: &FeatureFlag,
) -> DiagnosticsResult<Program> {
let mut transform = FragmentAliasTransform::new(program, is_enforced);
let next_program = transform
Expand Down Expand Up @@ -119,21 +120,23 @@ impl Transformer for AliasedInlineFragmentRemovalTransform {

struct FragmentAliasTransform<'program> {
program: &'program Program,
is_enforced: bool,
is_enforced: &'program FeatureFlag,
document_name: Option<StringKey>,
parent_type: Option<Type>,
parent_name: Option<StringKey>,
within_inline_fragment_type_condition: bool,
maybe_condition: Option<Condition>,
errors: Vec<Diagnostic>,
}

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(),
Expand Down Expand Up @@ -163,8 +166,10 @@ impl<'program> FragmentAliasTransform<'program> {
type_condition: Option<Type>,
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
Expand Down Expand Up @@ -238,9 +243,11 @@ impl Transformer for FragmentAliasTransform<'_> {
fragment: &FragmentDefinition,
) -> Transformed<FragmentDefinition> {
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
}
Expand All @@ -251,8 +258,10 @@ impl Transformer for FragmentAliasTransform<'_> {
) -> Transformed<OperationDefinition> {
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
}
Expand Down
3 changes: 2 additions & 1 deletion compiler/crates/relay-transforms/tests/catch_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
*/

use common::DiagnosticsResult;
use common::FeatureFlag;
use fixture_tests::Fixture;
use graphql_ir::Program;
use graphql_test_helpers::apply_transform_for_test;
use relay_transforms::catch_directive;
use relay_transforms::fragment_alias_directive;

fn transform(program: &Program) -> DiagnosticsResult<Program> {
catch_directive(&fragment_alias_directive(program, true)?)
catch_directive(&fragment_alias_directive(program, &FeatureFlag::Enabled)?)
}

pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String> {
apply_transform_for_test(fixture, |program| fragment_alias_directive(program, true))
apply_transform_for_test(fixture, |program| {
fragment_alias_directive(program, &FeatureFlag::Enabled)
})
}
3 changes: 2 additions & 1 deletion compiler/crates/relay-transforms/tests/relay_resolvers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use std::sync::Arc;

use common::Diagnostic;
use common::FeatureFlag;
use common::SourceLocationKey;
use common::TextSource;
use fixture_tests::Fixture;
Expand Down Expand Up @@ -40,7 +41,7 @@ pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String>

// 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))?;

Expand Down
3 changes: 2 additions & 1 deletion compiler/crates/relay-transforms/tests/required_directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
*/

use common::DiagnosticsResult;
use common::FeatureFlag;
use fixture_tests::Fixture;
use graphql_ir::Program;
use graphql_test_helpers::apply_transform_for_test;
use relay_transforms::fragment_alias_directive;
use relay_transforms::required_directive;

fn transform(program: &Program) -> DiagnosticsResult<Program> {
required_directive(&fragment_alias_directive(program, false)?)
required_directive(&fragment_alias_directive(program, &FeatureFlag::Enabled)?)
}

pub async fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String> {
Expand Down

0 comments on commit e4facf3

Please sign in to comment.