fix(apollo-parser): add ignored tokens to TYPE nodes in correct place

This commit re-writes the parsing logic for TYPE nodes to have an easier time accounting for ignored tokens and their correct place in the AST. An intermediate representation of the TYPE and it's nullability and list status is created and then processed to put the correct tokens in their spots.

Before this commit this sort of query
```graphql
mutation MyMutation($custId: Int!, $b: String) {
  myMutation(custId: $custId)
}
```
would result the `ast.document.to_string()` to have this output:
```
mutation MyMutation($custId: , Int!$b:  String) {
    myMutation(custId: $custId)
}
```
which is incorrect. The `to_string()` now results in the exact same output, as the AST created is correct.

crates/apollo-parser/src/parser/grammar/selection.rs

@@ -173,7 +173,6 @@ query GraphQuery($graph_id: ID!, $variant: String) {
                             .flatten()
                             .filter_map(|v| {
                                 if let ast::Value::Variable(var) = v.value()? {
-                                    dbg!(&var.name()?.text());
                                     return Some(var.name()?.text());
                                 }
                                 None

crates/apollo-parser/src/parser/grammar/ty.rs

@@ -1,5 +1,3 @@
-use std::collections::VecDeque;
-
 use crate::{parser::grammar::name, Parser, SyntaxKind, Token, TokenKind, S, T};
 
 /// See: https://spec.graphql.org/October2021/#InputValueDefinition
@@ -21,93 +19,153 @@ use crate::{parser::grammar::name, Parser, SyntaxKind, Token, TokenKind, S, T};
 // unwrap them once the last possible child has been parsed. Nodes are then
 // created in the processing stage of this parsing rule.
 pub(crate) fn ty(p: &mut Parser) {
-    let mut types = parse(p);
-
-    process(&mut types, p);
-
-    return;
-
-    fn parse(p: &mut Parser) -> VecDeque<(SyntaxKind, Token)> {
-        let token = p.pop();
-        let mut types = match token.kind() {
-            T!['['] => {
-                let mut types = parse(p);
-                types.push_front((S!['['], token));
-                if let Some(T![']']) = p.peek() {
-                    types.push_back((S![']'], p.pop()));
-                }
+    let ty = parse(p);
+    process(ty, p);
+}
 
-                types
-            }
-            TokenKind::Name => {
-                let mut types = VecDeque::new();
-                types.push_back((SyntaxKind::NAMED_TYPE, token));
+#[derive(Debug)]
+enum TokenTy {
+    List {
+        nullable: Option<Token>,
+        open_token: Token,
+        close_token: Option<Token>,
+        inner: Box<TokenTy>,
+        comma: Option<Token>,
+        trailing_ws: Option<Token>,
+    },
+    Named {
+        nullable: Option<Token>,
+        token: Token,
+        comma: Option<Token>,
+        trailing_ws: Option<Token>,
+    },
+}
 
-                types
+fn parse(p: &mut Parser) -> TokenTy {
+    let token = p.pop();
+    let mut types = match token.kind() {
+        T!['['] => {
+            let inner = parse(p);
+            let close_token = if let Some(T![']']) = p.peek() {
+                Some(p.pop())
+            } else {
+                None
+            };
+
+            TokenTy::List {
+                inner: Box::new(inner),
+                open_token: token,
+                close_token,
+                nullable: None,
+                comma: None,
+                trailing_ws: None,
             }
-            // TODO(@lrlna): this should not panic
-            token => panic!("unexpected token, {:?}", token),
-        };
-
-        if let Some(T![!]) = p.peek() {
-            types.push_front((SyntaxKind::NON_NULL_TYPE, p.pop()));
         }
+        TokenKind::Name => TokenTy::Named {
+            token,
+            nullable: None,
+            comma: None,
+            trailing_ws: None,
+        },
+        // TODO(@lrlna): this should not panic
+        token => panic!("unexpected token, {:?}", token),
+    };
+
+    // Deal with nullable types
+    if let Some(T![!]) = p.peek() {
+        match &mut types {
+            TokenTy::List { nullable, .. } => nullable.replace(p.pop()),
+            TokenTy::Named { nullable, .. } => nullable.replace(p.pop()),
+        };
+    }
 
-        // deal with ignored tokens
-        if let Some(TokenKind::Whitespace) = p.peek() {
-            types.push_back((SyntaxKind::WHITESPACE, p.pop()));
-        }
+    // deal with ignored tokens
+    if let Some(T![,]) = p.peek() {
+        match &mut types {
+            TokenTy::List { comma, .. } => comma.replace(p.pop()),
+            TokenTy::Named { comma, .. } => comma.replace(p.pop()),
+        };
+    }
 
-        types
+    if let Some(TokenKind::Whitespace) = p.peek() {
+        match &mut types {
+            TokenTy::List { trailing_ws, .. } => trailing_ws.replace(p.pop()),
+            TokenTy::Named { trailing_ws, .. } => trailing_ws.replace(p.pop()),
+        };
     }
 
-    fn process(types: &mut VecDeque<(SyntaxKind, Token)>, p: &mut Parser) {
-        dbg!(&types);
-        match types.pop_front() {
-            Some((kind @ S!['['], token)) => {
-                let _list_g = p.start_node(SyntaxKind::LIST_TYPE);
-                p.push_ast(kind, token);
-                process(types, p);
-                while let Some((_kind @ S![']'], _t)) | Some((_kind @ SyntaxKind::WHITESPACE, _t)) =
-                    peek(types)
-                {
-                    process(types, p);
-                }
+    types
+}
+
+fn process(ty: TokenTy, p: &mut Parser) {
+    match ty {
+        TokenTy::List {
+            nullable,
+            open_token,
+            close_token,
+            inner,
+            comma,
+            trailing_ws,
+        } => match nullable {
+            Some(nullable_token) => {
+                let _non_null_g = p.start_node(SyntaxKind::NON_NULL_TYPE);
+                process_list(p, open_token, *inner, close_token);
+                p.push_ast(S![!], nullable_token);
+                process_ignored_tokens(comma, p, trailing_ws);
             }
-            Some((kind @ SyntaxKind::NON_NULL_TYPE, token)) => {
-                let _non_null_g = p.start_node(kind);
-                process(types, p);
-                p.push_ast(S![!], token);
-                while let Some((_kind @ SyntaxKind::WHITESPACE, _token)) = peek(types) {
-                    process(types, p);
-                }
+            None => {
+                process_list(p, open_token, *inner, close_token);
+                process_ignored_tokens(comma, p, trailing_ws);
             }
-            // Cannot use `name::name` or `named_type` function here as we
-            // cannot bump from this function. Instead, the process function has
-            // already popped Tokens off the token vec, and we are simply adding
-            // to the AST.
-            Some((SyntaxKind::NAMED_TYPE, token)) => {
-                let named_g = p.start_node(SyntaxKind::NAMED_TYPE);
-                let name_g = p.start_node(SyntaxKind::NAME);
-                name::validate_name(token.data().to_string(), p);
-                p.push_ast(SyntaxKind::IDENT, token);
-
-                while let Some((_kind @ SyntaxKind::WHITESPACE, _token)) = peek(types) {
-                    process(types, p);
-                }
-
-                name_g.finish_node();
-                named_g.finish_node();
+        },
+        TokenTy::Named {
+            nullable,
+            token,
+            comma,
+            trailing_ws,
+        } => match nullable {
+            Some(nullable_token) => {
+                let _non_null_g = p.start_node(SyntaxKind::NON_NULL_TYPE);
+                process_named(p, token);
+
+                p.push_ast(S![!], nullable_token);
+                process_ignored_tokens(comma, p, trailing_ws);
             }
-            Some((SyntaxKind::WHITESPACE, token)) => p.push_ast(SyntaxKind::WHITESPACE, token),
-            Some((kind @ S![']'], token)) => {
-                p.push_ast(kind, token);
+            None => {
+                process_named(p, token);
+                process_ignored_tokens(comma, p, trailing_ws);
             }
-            _ => p.err("Internal apollo-parser error: unexpected when creating a Type"),
-        }
+        },
     }
 }
 
+fn process_ignored_tokens(comma: Option<Token>, p: &mut Parser, whitespace: Option<Token>) {
+    if let Some(comma_token) = comma {
+        p.push_ast(SyntaxKind::COMMA, comma_token);
+    }
+    if let Some(ws_token) = whitespace {
+        p.push_ast(SyntaxKind::WHITESPACE, ws_token);
+    }
+}
+
+fn process_list(p: &mut Parser, open_token: Token, inner: TokenTy, close_token: Option<Token>) {
+    let _list_g = p.start_node(SyntaxKind::LIST_TYPE);
+    p.push_ast(S!['['], open_token);
+    process(inner, p);
+    if let Some(close_token) = close_token {
+        p.push_ast(S![']'], close_token);
+    }
+}
+
+fn process_named(p: &mut Parser, token: Token) {
+    let named_g = p.start_node(SyntaxKind::NAMED_TYPE);
+    let name_g = p.start_node(SyntaxKind::NAME);
+    name::validate_name(token.data().to_string(), p);
+    p.push_ast(SyntaxKind::IDENT, token);
+    name_g.finish_node();
+    named_g.finish_node();
+}
+
 /// See: https://spec.graphql.org/October2021/#NamedType
 ///
 /// *NamedType*:
@@ -117,28 +175,22 @@ pub(crate) fn named_type(p: &mut Parser) {
     name::name(p);
 }
 
-fn peek<T>(target: &VecDeque<T>) -> Option<&T> {
-    match target.len() {
-        0 => None,
-        len => target.get(len - 1),
-    }
-}
-
 #[cfg(test)]
 mod test {
     use crate::{ast, Parser};
 
     #[test]
     fn it_parses_nested_wrapped_types_in_op_def_and_returns_matching_stringified_doc() {
         let mutation = r#"
-mutation MyMutation($custId: [Int!]!) {
+mutation MyMutation($custId: [Int!]!, $b: String) {
   myMutation(custId: $custId)
 }"#;
         let parser = Parser::new(mutation);
         let ast = parser.parse();
         assert!(ast.errors.is_empty());
 
         let doc = ast.document();
+        dbg!(&doc);
         assert_eq!(&mutation, &doc.to_string());
 
         for definition in doc.definitions() {
@@ -176,9 +228,15 @@ mutation MyMutation($a: Int $b: [Int] $c: String! $d: [Int!]!
     }
 
     #[test]
-    fn stringified_ast_matches_input_with_nested_wrapped_types() {
+    fn stringified_ast_matches_input_with_deeply_nested_wrapped_types_with_commas() {
         let mutation = r#"
-mutation MyMutation($a: String! ) {
+mutation MyMutation($a: Int, $b: [Int], $c: String!, $d: [Int!]!,
+
+    $e: String,
+    $f: [String],
+    $g: String!,
+    $h: [String!]!,
+) {
   myMutation(custId: $a)
 }"#;
         let parser = Parser::new(mutation);

crates/apollo-parser/test_data/parser/err/0010_input_definition_with_missing_name.txt

@@ -10,20 +10,20 @@
                     - IDENT@12..13 "a"
                 - COLON@13..14 ":"
                 - WHITESPACE@14..15 " "
-                - NAMED_TYPE@15..26
-                    - WHITESPACE@15..20 "\n    "
-                    - NAME@20..26
-                        - IDENT@20..26 "String"
+                - NAMED_TYPE@15..21
+                    - NAME@15..21
+                        - IDENT@15..21 "String"
+                - WHITESPACE@21..26 "\n    "
             - INPUT_VALUE_DEFINITION@26..34
                 - NAME@26..27
                     - IDENT@26..27 "b"
                 - COLON@27..28 ":"
                 - WHITESPACE@28..29 " "
                 - NON_NULL_TYPE@29..34
-                    - WHITESPACE@29..30 "\n"
-                    - NAMED_TYPE@30..33
-                        - NAME@30..33
-                            - IDENT@30..33 "Int"
-                    - BANG@33..34 "!"
+                    - NAMED_TYPE@29..32
+                        - NAME@29..32
+                            - IDENT@29..32 "Int"
+                    - BANG@32..33 "!"
+                    - WHITESPACE@33..34 "\n"
             - R_CURLY@34..35 "}"
 - ERROR@6:7 "expected a Name" {

crates/apollo-parser/test_data/parser/err/0012_input_extension_with_missing_name.txt

@@ -12,9 +12,9 @@
                     - IDENT@19..20 "a"
                 - COLON@20..21 ":"
                 - WHITESPACE@21..22 " "
-                - NAMED_TYPE@22..29
-                    - WHITESPACE@22..23 "\n"
-                    - NAME@23..29
-                        - IDENT@23..29 "String"
+                - NAMED_TYPE@22..28
+                    - NAME@22..28
+                        - IDENT@22..28 "String"
+                - WHITESPACE@28..29 "\n"
             - R_CURLY@29..30 "}"
 - ERROR@13:14 "expected a Name" {

crates/apollo-parser/test_data/parser/err/0014_interface_extension_with_missing_name.txt

@@ -12,9 +12,9 @@
                     - IDENT@23..28 "value"
                 - COLON@28..29 ":"
                 - WHITESPACE@29..30 " "
-                - NAMED_TYPE@30..34
-                    - WHITESPACE@30..31 "\n"
-                    - NAME@31..34
-                        - IDENT@31..34 "Int"
+                - NAMED_TYPE@30..33
+                    - NAME@30..33
+                        - IDENT@30..33 "Int"
+                - WHITESPACE@33..34 "\n"
             - R_CURLY@34..35 "}"
 - ERROR@17:18 "expected a Name" {

crates/apollo-parser/test_data/parser/err/0016_object_type_extension_with_missing_name.txt

@@ -12,27 +12,27 @@
                     - IDENT@18..22 "name"
                 - COLON@22..23 ":"
                 - WHITESPACE@23..24 " "
-                - NAMED_TYPE@24..35
-                    - WHITESPACE@24..29 "\n    "
-                    - NAME@29..35
-                        - IDENT@29..35 "String"
+                - NAMED_TYPE@24..30
+                    - NAME@24..30
+                        - IDENT@24..30 "String"
+                - WHITESPACE@30..35 "\n    "
             - FIELD_DEFINITION@35..48
                 - NAME@35..38
                     - IDENT@35..38 "age"
                 - COLON@38..39 ":"
                 - WHITESPACE@39..40 " "
-                - NAMED_TYPE@40..48
-                    - WHITESPACE@40..45 "\n    "
-                    - NAME@45..48
-                        - IDENT@45..48 "Int"
+                - NAMED_TYPE@40..43
+                    - NAME@40..43
+                        - IDENT@40..43 "Int"
+                - WHITESPACE@43..48 "\n    "
             - FIELD_DEFINITION@48..61
                 - NAME@48..55
                     - IDENT@48..55 "picture"
                 - COLON@55..56 ":"
                 - WHITESPACE@56..57 " "
-                - NAMED_TYPE@57..61
-                    - WHITESPACE@57..58 "\n"
-                    - NAME@58..61
-                        - IDENT@58..61 "Url"
+                - NAMED_TYPE@57..60
+                    - NAME@57..60
+                        - IDENT@57..60 "Url"
+                - WHITESPACE@60..61 "\n"
             - R_CURLY@61..62 "}"
 - ERROR@12:13 "expected a Name" {