Skip to content

Commit

Permalink
detect cyclic @inline fragments
Browse files Browse the repository at this point in the history
Reviewed By: josephsavona

Differential Revision: D35046903

fbshipit-source-id: e768787582330ce7fe3981241b7da209fc4d711f
  • Loading branch information
kassens authored and facebook-github-bot committed Mar 22, 2022
1 parent 5955505 commit 928728a
Show file tree
Hide file tree
Showing 4 changed files with 139 additions and 5 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
==================================== INPUT ====================================
# expected-to-throw

fragment circularInlineFragmentF1 on User @inline {
...circularInlineFragmentF2
}

fragment circularInlineFragmentF2 on User @inline {
...circularInlineFragmentF3
}

fragment circularInlineFragmentF3 on User @inline {
...circularInlineFragmentF1
}
==================================== ERROR ====================================
✖︎ Found a circular reference from fragment 'circularInlineFragmentF1'.

circular-inline-fragment.invalid.graphql:12:6
11 │ fragment circularInlineFragmentF3 on User @inline {
12 │ ...circularInlineFragmentF1
│ ^^^^^^^^^^^^^^^^^^^^^^^^
13 │ }

ℹ︎ spreading circularInlineFragmentF2

circular-inline-fragment.invalid.graphql:4:6
3 │ fragment circularInlineFragmentF1 on User @inline {
4 │ ...circularInlineFragmentF2
│ ^^^^^^^^^^^^^^^^^^^^^^^^
5 │ }

ℹ︎ spreading circularInlineFragmentF3

circular-inline-fragment.invalid.graphql:8:6
7 │ fragment circularInlineFragmentF2 on User @inline {
8 │ ...circularInlineFragmentF3
│ ^^^^^^^^^^^^^^^^^^^^^^^^
9 │ }


✖︎ Found a circular reference from fragment 'circularInlineFragmentF2'.

circular-inline-fragment.invalid.graphql:4:6
3 │ fragment circularInlineFragmentF1 on User @inline {
4 │ ...circularInlineFragmentF2
│ ^^^^^^^^^^^^^^^^^^^^^^^^
5 │ }

ℹ︎ spreading circularInlineFragmentF3

circular-inline-fragment.invalid.graphql:8:6
7 │ fragment circularInlineFragmentF2 on User @inline {
8 │ ...circularInlineFragmentF3
│ ^^^^^^^^^^^^^^^^^^^^^^^^
9 │ }

ℹ︎ spreading circularInlineFragmentF1

circular-inline-fragment.invalid.graphql:12:6
11 │ fragment circularInlineFragmentF3 on User @inline {
12 │ ...circularInlineFragmentF1
│ ^^^^^^^^^^^^^^^^^^^^^^^^
13 │ }


✖︎ Found a circular reference from fragment 'circularInlineFragmentF3'.

circular-inline-fragment.invalid.graphql:8:6
7 │ fragment circularInlineFragmentF2 on User @inline {
8 │ ...circularInlineFragmentF3
│ ^^^^^^^^^^^^^^^^^^^^^^^^
9 │ }

ℹ︎ spreading circularInlineFragmentF1

circular-inline-fragment.invalid.graphql:12:6
11 │ fragment circularInlineFragmentF3 on User @inline {
12 │ ...circularInlineFragmentF1
│ ^^^^^^^^^^^^^^^^^^^^^^^^
13 │ }

ℹ︎ spreading circularInlineFragmentF2

circular-inline-fragment.invalid.graphql:4:6
3 │ fragment circularInlineFragmentF1 on User @inline {
4 │ ...circularInlineFragmentF2
│ ^^^^^^^^^^^^^^^^^^^^^^^^
5 │ }
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# expected-to-throw

fragment circularInlineFragmentF1 on User @inline {
...circularInlineFragmentF2
}

fragment circularInlineFragmentF2 on User @inline {
...circularInlineFragmentF3
}

fragment circularInlineFragmentF3 on User @inline {
...circularInlineFragmentF1
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @generated SignedSource<<1d6f350f3c9c690369bd4c7ee6c434df>>
* @generated SignedSource<<e41886c3abe3d2ccd18f08212a954a32>>
*/

mod compile_relay_artifacts;
Expand Down Expand Up @@ -152,6 +152,13 @@ fn auto_filled_argument_on_match() {
test_fixture(transform_fixture, "auto-filled-argument-on-match.graphql", "compile_relay_artifacts/fixtures/auto-filled-argument-on-match.expected", input, expected);
}

#[test]
fn circular_inline_fragment_invalid() {
let input = include_str!("compile_relay_artifacts/fixtures/circular-inline-fragment.invalid.graphql");
let expected = include_str!("compile_relay_artifacts/fixtures/circular-inline-fragment.invalid.expected");
test_fixture(transform_fixture, "circular-inline-fragment.invalid.graphql", "compile_relay_artifacts/fixtures/circular-inline-fragment.invalid.expected", input, expected);
}

#[test]
fn circular_no_inline_fragment() {
let input = include_str!("compile_relay_artifacts/fixtures/circular-no-inline-fragment.graphql");
Expand Down
34 changes: 30 additions & 4 deletions compiler/crates/relay-transforms/src/inline_data_fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

use common::{Diagnostic, DiagnosticsResult, NamedItem};
use common::{Diagnostic, DiagnosticsResult, NamedItem, WithLocation};
use graphql_ir::{
associated_data_impl, FragmentSpread, InlineFragment, Program, Selection, Transformed,
Transformer,
Expand Down Expand Up @@ -34,13 +34,15 @@ pub const INLINE_DIRECTIVE_NAME: Lazy<StringKey> = Lazy::new(|| "inline".intern(
struct InlineDataFragmentsTransform<'s> {
program: &'s Program,
errors: Vec<Diagnostic>,
parent_inline_fragments: Vec<WithLocation<StringKey>>,
}

impl<'s> InlineDataFragmentsTransform<'s> {
fn new(program: &'s Program) -> Self {
Self {
program,
errors: vec![],
errors: Vec::new(),
parent_inline_fragments: Vec::new(),
}
}
}
Expand All @@ -63,8 +65,7 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {
.fragment(spread.fragment.item)
.unwrap_or_else(|| panic!("was expecting to find fragment `{}`", spread.fragment.item));

let inline_directive = fragment.directives.named(*INLINE_DIRECTIVE_NAME);
if inline_directive.is_none() {
if fragment.directives.named(*INLINE_DIRECTIVE_NAME).is_none() {
next_fragment_spread
} else {
if !fragment.variable_definitions.is_empty()
Expand Down Expand Up @@ -107,7 +108,29 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {
}
};

if self
.parent_inline_fragments
.iter()
.any(|name| name.item == fragment.name.item)
{
let mut cyclic_fragments = self.parent_inline_fragments.iter();
let first = cyclic_fragments.next().unwrap();
let mut diagnostic = Diagnostic::error(
ValidationMessage::CircularFragmentReference {
fragment_name: first.item,
},
first.location,
);
for spread in cyclic_fragments {
diagnostic =
diagnostic.annotate(format!("spreading {}", spread.item), spread.location);
}
self.errors.push(diagnostic);
return Transformed::Keep;
}
self.parent_inline_fragments.push(spread.fragment);
let transformed_fragment = self.default_transform_fragment(fragment);
self.parent_inline_fragments.pop();

let (name, selections) = match transformed_fragment {
Transformed::Keep => (fragment.name.item, fragment.selections.clone()),
Expand Down Expand Up @@ -143,6 +166,9 @@ impl<'s> Transformer for InlineDataFragmentsTransform<'s> {

#[derive(Error, Debug)]
enum ValidationMessage {
#[error("Found a circular reference from fragment '{fragment_name}'.")]
CircularFragmentReference { fragment_name: StringKey },

#[error("Variables are not yet supported inside @inline fragments.")]
InlineDataFragmentArgumentsNotSupported,

Expand Down

0 comments on commit 928728a

Please sign in to comment.