From e60ff2900429473df6d774d10d5f1fe10c625820 Mon Sep 17 00:00:00 2001 From: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> Date: Tue, 8 Feb 2022 15:06:33 +0100 Subject: [PATCH] fix(apollo-parser): fix a bad parsing on fragment detected by apollo-smith Signed-off-by: Benjamin Coenen <5719034+bnjjj@users.noreply.github.com> --- .../src/parser/grammar/selection.rs | 24 +++++--- crates/apollo-parser/src/parser/mod.rs | 9 +++ crates/apollo-parser/src/tests.rs | 15 +++-- ...on_definition_with_empty_selection_set.txt | 2 +- ..._operation_definition_with_description.txt | 2 +- ...006_selection_with_fragment_spread.graphql | 9 ++- .../0006_selection_with_fragment_spread.txt | 59 ++++++++++++++----- 7 files changed, 87 insertions(+), 33 deletions(-) diff --git a/crates/apollo-parser/src/parser/grammar/selection.rs b/crates/apollo-parser/src/parser/grammar/selection.rs index 89807886f..e9054e514 100644 --- a/crates/apollo-parser/src/parser/grammar/selection.rs +++ b/crates/apollo-parser/src/parser/grammar/selection.rs @@ -28,14 +28,22 @@ pub(crate) fn selection(p: &mut Parser) { while let Some(node) = p.peek() { match node { T![...] => { - if let Some(node) = p.peek_data_n(2) { - match node.as_str() { - "on" | "{" => fragment::inline_fragment(p), - _ => fragment::fragment_spread(p), + let next_token = p.peek_token_n(2); + match next_token { + Some(next_token) => { + if next_token.kind() == TokenKind::Name && next_token.data() != "on" { + fragment::fragment_spread(p); + } else if matches!( + next_token.kind(), + TokenKind::At | TokenKind::Name | TokenKind::LCurly + ) { + fragment::inline_fragment(p); + } else { + p.err("expected an Inline Fragment or a Fragment Spread"); + } + has_selection = true; } - has_selection = true; - } else { - p.err("expected an Inline Fragment or a Fragment Spread"); + None => p.err("expected an Inline Fragment or a Fragment Spread"), } } T!['{'] => { @@ -47,7 +55,7 @@ pub(crate) fn selection(p: &mut Parser) { } _ => { if !has_selection { - p.err("exepcted at least one Selection in Selection Set"); + p.err("expected at least one Selection in Selection Set"); } break; } diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs index 5caffafc9..dcb2337b8 100644 --- a/crates/apollo-parser/src/parser/mod.rs +++ b/crates/apollo-parser/src/parser/mod.rs @@ -244,6 +244,15 @@ impl Parser { self.tokens.last() } + /// Peek Token `n` and return it. + pub(crate) fn peek_token_n(&self, n: usize) -> Option<&Token> { + self.tokens + .iter() + .rev() + .filter(|token| !matches!(token.kind(), TokenKind::Whitespace | TokenKind::Comment)) + .nth(n - 1) + } + /// Peek Token `n` and return its TokenKind. pub(crate) fn peek_n(&self, n: usize) -> Option { self.tokens diff --git a/crates/apollo-parser/src/tests.rs b/crates/apollo-parser/src/tests.rs index fc0a6491e..047af0a1e 100644 --- a/crates/apollo-parser/src/tests.rs +++ b/crates/apollo-parser/src/tests.rs @@ -62,11 +62,16 @@ fn assert_errors_are_present(errors: Iter<'_, Error>, path: &Path) { } fn assert_errors_are_absent(errors: Iter<'_, Error>, path: &Path) { - assert!( - errors.len() == 0, - "There should be no errors in the file {:?}", - path.display(), - ); + if errors.len() > 0 { + println!( + "errors: {}", + errors + .map(|e| e.message()) + .collect::>() + .join("\n") + ); + panic!("There should be no errors in the file {:?}", path.display(),); + } } /// Concatenate tokens and errors. diff --git a/crates/apollo-parser/test_data/parser/err/0029_operation_definition_with_empty_selection_set.txt b/crates/apollo-parser/test_data/parser/err/0029_operation_definition_with_empty_selection_set.txt index d26a6983b..bd39311fa 100644 --- a/crates/apollo-parser/test_data/parser/err/0029_operation_definition_with_empty_selection_set.txt +++ b/crates/apollo-parser/test_data/parser/err/0029_operation_definition_with_empty_selection_set.txt @@ -6,4 +6,4 @@ - SELECTION_SET@6..8 - L_CURLY@6..7 "{" - R_CURLY@7..8 "}" -- ERROR@7:8 "exepcted at least one Selection in Selection Set" } +- ERROR@7:8 "expected at least one Selection in Selection Set" } diff --git a/crates/apollo-parser/test_data/parser/err/0030_operation_definition_with_description.txt b/crates/apollo-parser/test_data/parser/err/0030_operation_definition_with_description.txt index 049588bf3..a3703799a 100644 --- a/crates/apollo-parser/test_data/parser/err/0030_operation_definition_with_description.txt +++ b/crates/apollo-parser/test_data/parser/err/0030_operation_definition_with_description.txt @@ -11,4 +11,4 @@ - L_CURLY@13..14 "{" - R_CURLY@14..15 "}" - ERROR@0:93 "expected an Operation Type or a Selection Set" "after this PR this should not be an issue: https://github.com/graphql/graphql-spec/pull/892" -- ERROR@107:108 "exepcted at least one Selection in Selection Set" } +- ERROR@107:108 "expected at least one Selection in Selection Set" } diff --git a/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.graphql b/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.graphql index 86a36b35b..3c2cbdd77 100644 --- a/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.graphql +++ b/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.graphql @@ -1,4 +1,7 @@ { - pet - ...snackSelection -} \ No newline at end of file + pet + ...snackSelection + ... @J(N: 0) { + a + } +} diff --git a/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.txt b/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.txt index 18a22a162..7fa9e9106 100644 --- a/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.txt +++ b/crates/apollo-parser/test_data/parser/ok/0006_selection_with_fragment_spread.txt @@ -1,16 +1,45 @@ -- DOCUMENT@0..33 - - OPERATION_DEFINITION@0..33 - - SELECTION_SET@0..33 +- DOCUMENT@0..57 + - OPERATION_DEFINITION@0..57 + - SELECTION_SET@0..57 - L_CURLY@0..1 "{" - - WHITESPACE@1..6 "\n " - - FIELD@6..14 - - NAME@6..14 - - IDENT@6..9 "pet" - - WHITESPACE@9..14 "\n " - - FRAGMENT_SPREAD@14..32 - - SPREAD@14..17 "..." - - FRAGMENT_NAME@17..32 - - NAME@17..32 - - IDENT@17..31 "snackSelection" - - WHITESPACE@31..32 "\n" - - R_CURLY@32..33 "}" + - WHITESPACE@1..4 "\n " + - FIELD@4..10 + - NAME@4..10 + - IDENT@4..7 "pet" + - WHITESPACE@7..10 "\n " + - FRAGMENT_SPREAD@10..30 + - SPREAD@10..13 "..." + - FRAGMENT_NAME@13..30 + - NAME@13..30 + - IDENT@13..27 "snackSelection" + - WHITESPACE@27..30 "\n " + - INLINE_FRAGMENT@30..55 + - SPREAD@30..33 "..." + - WHITESPACE@33..34 " " + - DIRECTIVES@34..43 + - DIRECTIVE@34..43 + - AT@34..35 "@" + - NAME@35..36 + - IDENT@35..36 "J" + - ARGUMENTS@36..43 + - L_PAREN@36..37 "(" + - ARGUMENT@37..41 + - NAME@37..38 + - IDENT@37..38 "N" + - COLON@38..39 ":" + - WHITESPACE@39..40 " " + - INT_VALUE@40..41 + - INT@40..41 "0" + - R_PAREN@41..42 ")" + - WHITESPACE@42..43 " " + - SELECTION_SET@43..55 + - L_CURLY@43..44 "{" + - WHITESPACE@44..49 "\n " + - FIELD@49..53 + - NAME@49..53 + - IDENT@49..50 "a" + - WHITESPACE@50..53 "\n " + - R_CURLY@53..54 "}" + - WHITESPACE@54..55 "\n" + - R_CURLY@55..56 "}" + - WHITESPACE@56..57 "\n"