Repository: apollographql/apollo-rs
Pull request #845 (2024-03-26 22:01:06)

Bug fix - implement missing parser validation for interface and object having missing fields
## Description

The parser is a bit lenient around letting types being defined without fields.

The example below should fail parsing but doesn't today:
```
type User {
}
```

This PR implements the missing validation to bail parsing for object types and interfaces when defined as empty (within curly braces).

The parser already implemented this validation for enum and input types. 



Patch:
diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md
index b887b51..4145bc7 100644
--- a/crates/apollo-compiler/CHANGELOG.md
+++ b/crates/apollo-compiler/CHANGELOG.md
@@ -17,6 +17,19 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ## Maintenance
 ## Documentation-->
 
+# [x.x.x] (unreleased) - 2024-mm-dd
+
+## Features
+
+- **Serialize with initial indentation - [SimonSapin], [pull/848]**
+  This helps use indentation in something that is partly but not entirely GraphQL syntax,
+  such as the debugging representation of an Apollo Router query plan.
+  Example: `document.serialize().initial_indent_level(2).to_string()`
+
+[SimonSapin]: https://github.com/SimonSapin
+[pull/838]: https://github.com/apollographql/apollo-rs/pull/848
+
+
 # [1.0.0-beta.14](https://crates.io/crates/apollo-compiler/1.0.0-beta.14) - 2024-03-13
 
 ## BREAKING
diff --git a/crates/apollo-compiler/src/ast/serialize.rs b/crates/apollo-compiler/src/ast/serialize.rs
index 04169f5..4350ad3 100644
--- a/crates/apollo-compiler/src/ast/serialize.rs
+++ b/crates/apollo-compiler/src/ast/serialize.rs
@@ -13,6 +13,7 @@ pub struct Serialize<'a, T> {
 #[derive(Debug, Clone)]
 pub(crate) struct Config<'a> {
     indent_prefix: Option<&'a str>,
+    initial_indent_level: usize,
 }
 
 pub(crate) struct State<'config, 'fmt, 'fmt2> {
@@ -38,12 +39,18 @@ impl<'a, T> Serialize<'a, T> {
         self.config.indent_prefix = None;
         self
     }
+
+    pub fn initial_indent_level(mut self, initial_indent_level: usize) -> Self {
+        self.config.initial_indent_level = initial_indent_level;
+        self
+    }
 }
 
 impl Default for Config<'_> {
     fn default() -> Self {
         Self {
             indent_prefix: Some("  "),
+            initial_indent_level: 0,
         }
     }
 }
@@ -979,12 +986,20 @@ macro_rules! impl_display {
             /// Serialize to GraphQL syntax
             impl Display for Serialize<'_, $ty> {
                 fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-                    self.node.serialize_impl(&mut State {
+                    let mut state = State {
                         config: self.config.clone(),
-                        indent_level: 0,
+                        indent_level: self.config.initial_indent_level,
                         output: f,
                         output_empty: true,
-                    })
+                    };
+                    // Indent the first line.
+                    // Subsequent lines will be indented when writing a line break.
+                    if let Some(prefix) = state.config.indent_prefix {
+                        for _ in 0..state.indent_level {
+                            state.write(prefix)?;
+                        }
+                    }
+                    self.node.serialize_impl(&mut state)
                 }
             }
         )+
diff --git a/crates/apollo-parser/src/parser/grammar/field.rs b/crates/apollo-parser/src/parser/grammar/field.rs
index a391c9d..777416f 100644
--- a/crates/apollo-parser/src/parser/grammar/field.rs
+++ b/crates/apollo-parser/src/parser/grammar/field.rs
@@ -41,6 +41,12 @@ pub(crate) fn field(p: &mut Parser) {
 pub(crate) fn fields_definition(p: &mut Parser) {
     let _g = p.start_node(SyntaxKind::FIELDS_DEFINITION);
     p.bump(S!['{']);
+
+    match p.peek() {
+        Some(TokenKind::Name | TokenKind::StringValue) => field_definition(p),
+        _ => p.err("expected Field Definition"),
+    }
+
     p.peek_while(|p, kind| match kind {
         TokenKind::Name | TokenKind::StringValue => {
             field_definition(p);


Test patch:
diff --git a/crates/apollo-compiler/tests/executable.rs b/crates/apollo-compiler/tests/executable.rs
index 9f18317..2188ae8 100644
--- a/crates/apollo-compiler/tests/executable.rs
+++ b/crates/apollo-compiler/tests/executable.rs
@@ -35,7 +35,7 @@ fn get_operations() {
 #[test]
 fn is_introspection_operation() {
     let query_input = r#"
-        type Query {}
+        type Query { foo: String }
         query TypeIntrospect {
           __type(name: "User") {
             name
@@ -184,7 +184,7 @@ fn is_introspection_deep() {
 #[test]
 fn is_introspection_repeated_fragment() {
     let query_input_indirect = r#"
-      type Query {}
+      type Query { foo: String }
 
       query IntrospectRepeatedIndirectFragment {
         ...A
@@ -204,7 +204,7 @@ fn is_introspection_repeated_fragment() {
     "#;
 
     let query_input_direct = r#"
-      type Query {}
+      type Query { foo: String }
 
       query IntrospectRepeatedDirectFragment {
         ...C
diff --git a/crates/apollo-compiler/tests/misc.rs b/crates/apollo-compiler/tests/misc.rs
index 1210a50..92ab509 100644
--- a/crates/apollo-compiler/tests/misc.rs
+++ b/crates/apollo-compiler/tests/misc.rs
@@ -1,3 +1,4 @@
+use apollo_compiler::ast::Document;
 use apollo_compiler::executable::Selection;
 use apollo_compiler::name;
 use apollo_compiler::parse_mixed_validate;
@@ -825,3 +826,25 @@ scalar URL
         assert_eq!(result.unwrap().as_deref(), Some("URL"));
     }
 }
+
+#[test]
+fn initial_indent() {
+    let ast = Document::parse("query Op { field ... @defer { otherField }}", "").unwrap();
+    let formatted = format!(
+        "Request:\n  operationName: {}\n  query:\n{}",
+        "Op",
+        ast.serialize().initial_indent_level(2)
+    );
+    let expected = expect_test::expect![[r#"
+        Request:
+          operationName: Op
+          query:
+            query Op {
+              field
+              ... @defer {
+                otherField
+              }
+            }
+    "#]];
+    expected.assert_eq(&formatted);
+}
diff --git a/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.graphql b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.graphql
index 03ac94f..7f2799b 100644
--- a/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.graphql
+++ b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.graphql
@@ -1,7 +1,7 @@
 cats
 # https://github.com/apollographql/apollo-rs/issues/325
 
-interface X {}
+interface X
 
 cats
 type Query {
diff --git a/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.txt b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.txt
index 7d0334f..b4d393d 100644
--- a/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.txt
+++ b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.txt
@@ -1,40 +1,36 @@
-- DOCUMENT@0..115
+- DOCUMENT@0..112
     - ERROR@0..4 "cats"
     - WHITESPACE@4..5 "\n"
     - COMMENT@5..60 "# https://github.com/apollographql/apollo-rs/issues/325"
     - WHITESPACE@60..62 "\n\n"
-    - INTERFACE_TYPE_DEFINITION@62..76
+    - INTERFACE_TYPE_DEFINITION@62..73
         - interface_KW@62..71 "interface"
         - WHITESPACE@71..72 " "
         - NAME@72..73
             - IDENT@72..73 "X"
-        - WHITESPACE@73..74 " "
-        - FIELDS_DEFINITION@74..76
-            - L_CURLY@74..75 "{"
-            - R_CURLY@75..76 "}"
-    - WHITESPACE@76..78 "\n\n"
-    - ERROR@78..82 "cats"
-    - WHITESPACE@82..83 "\n"
-    - OBJECT_TYPE_DEFINITION@83..114
-        - type_KW@83..87 "type"
-        - WHITESPACE@87..88 " "
-        - NAME@88..93
-            - IDENT@88..93 "Query"
-        - WHITESPACE@93..94 " "
-        - FIELDS_DEFINITION@94..114
-            - L_CURLY@94..95 "{"
-            - WHITESPACE@95..100 "\n    "
-            - FIELD_DEFINITION@100..112
-                - NAME@100..104
-                    - IDENT@100..104 "name"
-                - COLON@104..105 ":"
-                - WHITESPACE@105..106 " "
-                - NAMED_TYPE@106..112
-                    - NAME@106..112
-                        - IDENT@106..112 "String"
-            - WHITESPACE@112..113 "\n"
-            - R_CURLY@113..114 "}"
-    - WHITESPACE@114..115 "\n"
+    - WHITESPACE@73..75 "\n\n"
+    - ERROR@75..79 "cats"
+    - WHITESPACE@79..80 "\n"
+    - OBJECT_TYPE_DEFINITION@80..111
+        - type_KW@80..84 "type"
+        - WHITESPACE@84..85 " "
+        - NAME@85..90
+            - IDENT@85..90 "Query"
+        - WHITESPACE@90..91 " "
+        - FIELDS_DEFINITION@91..111
+            - L_CURLY@91..92 "{"
+            - WHITESPACE@92..97 "\n    "
+            - FIELD_DEFINITION@97..109
+                - NAME@97..101
+                    - IDENT@97..101 "name"
+                - COLON@101..102 ":"
+                - WHITESPACE@102..103 " "
+                - NAMED_TYPE@103..109
+                    - NAME@103..109
+                        - IDENT@103..109 "String"
+            - WHITESPACE@109..110 "\n"
+            - R_CURLY@110..111 "}"
+    - WHITESPACE@111..112 "\n"
 - ERROR@0:4 "expected definition" cats
-- ERROR@78:82 "expected definition" cats
+- ERROR@75:79 "expected definition" cats
 recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.graphql b/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.graphql
new file mode 100644
index 0000000..38f16d5
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.graphql
@@ -0,0 +1,3 @@
+type  {
+  id: String
+}
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.txt b/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.txt
new file mode 100644
index 0000000..9fa99dc
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0055_object_definition_with_missing_name.txt
@@ -0,0 +1,19 @@
+- DOCUMENT@0..22
+    - OBJECT_TYPE_DEFINITION@0..22
+        - type_KW@0..4 "type"
+        - WHITESPACE@4..6 "  "
+        - FIELDS_DEFINITION@6..22
+            - L_CURLY@6..7 "{"
+            - WHITESPACE@7..10 "\n  "
+            - FIELD_DEFINITION@10..20
+                - NAME@10..12
+                    - IDENT@10..12 "id"
+                - COLON@12..13 ":"
+                - WHITESPACE@13..14 " "
+                - NAMED_TYPE@14..20
+                    - NAME@14..20
+                        - IDENT@14..20 "String"
+            - WHITESPACE@20..21 "\n"
+            - R_CURLY@21..22 "}"
+- ERROR@6:7 "expected a name" {
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.graphql b/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.graphql
new file mode 100644
index 0000000..0c411b3
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.graphql
@@ -0,0 +1,1 @@
+type Person { id: String
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.txt b/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.txt
new file mode 100644
index 0000000..f12e6f7
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0056_object_definition_with_missing_curly.txt
@@ -0,0 +1,20 @@
+- DOCUMENT@0..24
+    - OBJECT_TYPE_DEFINITION@0..24
+        - type_KW@0..4 "type"
+        - WHITESPACE@4..5 " "
+        - NAME@5..11
+            - IDENT@5..11 "Person"
+        - WHITESPACE@11..12 " "
+        - FIELDS_DEFINITION@12..24
+            - L_CURLY@12..13 "{"
+            - WHITESPACE@13..14 " "
+            - FIELD_DEFINITION@14..24
+                - NAME@14..16
+                    - IDENT@14..16 "id"
+                - COLON@16..17 ":"
+                - WHITESPACE@17..18 " "
+                - NAMED_TYPE@18..24
+                    - NAME@18..24
+                        - IDENT@18..24 "String"
+- ERROR@24:24 "expected R_CURLY, got EOF" EOF
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.graphql b/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.graphql
new file mode 100644
index 0000000..898f304
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.graphql
@@ -0,0 +1,2 @@
+type Person {
+}
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.txt b/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.txt
new file mode 100644
index 0000000..3ee2b1b
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0057_object_definition_with_missing_fields.txt
@@ -0,0 +1,13 @@
+- DOCUMENT@0..15
+    - OBJECT_TYPE_DEFINITION@0..15
+        - type_KW@0..4 "type"
+        - WHITESPACE@4..5 " "
+        - NAME@5..11
+            - IDENT@5..11 "Person"
+        - WHITESPACE@11..12 " "
+        - FIELDS_DEFINITION@12..15
+            - L_CURLY@12..13 "{"
+            - WHITESPACE@13..14 "\n"
+            - R_CURLY@14..15 "}"
+- ERROR@14:15 "expected Field Definition" }
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.graphql b/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.graphql
new file mode 100644
index 0000000..7fd1ce9
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.graphql
@@ -0,0 +1,3 @@
+interface {
+  id: String
+}
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.txt b/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.txt
new file mode 100644
index 0000000..f9ed3ba
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0058_interface_definition_with_missing_name.txt
@@ -0,0 +1,19 @@
+- DOCUMENT@0..26
+    - INTERFACE_TYPE_DEFINITION@0..26
+        - interface_KW@0..9 "interface"
+        - WHITESPACE@9..10 " "
+        - FIELDS_DEFINITION@10..26
+            - L_CURLY@10..11 "{"
+            - WHITESPACE@11..14 "\n  "
+            - FIELD_DEFINITION@14..24
+                - NAME@14..16
+                    - IDENT@14..16 "id"
+                - COLON@16..17 ":"
+                - WHITESPACE@17..18 " "
+                - NAMED_TYPE@18..24
+                    - NAME@18..24
+                        - IDENT@18..24 "String"
+            - WHITESPACE@24..25 "\n"
+            - R_CURLY@25..26 "}"
+- ERROR@10:11 "expected a Name" {
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.graphql b/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.graphql
new file mode 100644
index 0000000..e16cb57
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.graphql
@@ -0,0 +1,2 @@
+interface Person {
+}
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.txt b/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.txt
new file mode 100644
index 0000000..ae89074
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0059_interface_definition_with_missing_fields.txt
@@ -0,0 +1,13 @@
+- DOCUMENT@0..20
+    - INTERFACE_TYPE_DEFINITION@0..20
+        - interface_KW@0..9 "interface"
+        - WHITESPACE@9..10 " "
+        - NAME@10..16
+            - IDENT@10..16 "Person"
+        - WHITESPACE@16..17 " "
+        - FIELDS_DEFINITION@17..20
+            - L_CURLY@17..18 "{"
+            - WHITESPACE@18..19 "\n"
+            - R_CURLY@19..20 "}"
+- ERROR@19:20 "expected Field Definition" }
+recursion limit: 500, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.graphql b/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.graphql
new file mode 100644
index 0000000..69de629
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.graphql
@@ -0,0 +1,1 @@
+interface Person { id: String
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.txt b/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.txt
new file mode 100644
index 0000000..9ed567b
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0060_interface_definition_with_missing_curly.txt
@@ -0,0 +1,20 @@
+- DOCUMENT@0..29
+    - INTERFACE_TYPE_DEFINITION@0..29
+        - interface_KW@0..9 "interface"
+        - WHITESPACE@9..10 " "
+        - NAME@10..16
+            - IDENT@10..16 "Person"
+        - WHITESPACE@16..17 " "
+        - FIELDS_DEFINITION@17..29
+            - L_CURLY@17..18 "{"
+            - WHITESPACE@18..19 " "
+            - FIELD_DEFINITION@19..29
+                - NAME@19..21
+                    - IDENT@19..21 "id"
+                - COLON@21..22 ":"
+                - WHITESPACE@22..23 " "
+                - NAMED_TYPE@23..29
+                    - NAME@23..29
+                        - IDENT@23..29 "String"
+- ERROR@29:29 "expected R_CURLY, got EOF" EOF
+recursion limit: 500, high: 0
\ No newline at end of file