Repository: apollographql/apollo-rs
Pull request #164 (2022-02-08 14:07:20)

fix(apollo-parser): fix a bad parsing on fragment detected by apollo-smith


Patch:
diff --git a/crates/apollo-parser/src/parser/grammar/selection.rs b/crates/apollo-parser/src/parser/grammar/selection.rs
index 8980788..e9054e5 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 5caffaf..dcb2337 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<TokenKind> {
         self.tokens


Test patch:
diff --git a/crates/apollo-parser/src/tests.rs b/crates/apollo-parser/src/tests.rs
index fc0a649..047af0a 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::<Vec<&str>>()
+                .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 d26a698..bd39311 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 049588b..a370379 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 86a36b3..3c2cbdd 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 18a22a1..7fa9e91 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"