Repository: apollographql/apollo-rs
Pull request #447 (2023-02-03 17:01:46)

breaking(compiler): unify find_operation
Mimics spec's [`getOperation`](https://spec.graphql.org/October2021/#GetOperation()) functionality and returns the anonymous operation if `None` is specified for operation name.


Patch:
diff --git a/crates/apollo-compiler/README.md b/crates/apollo-compiler/README.md
index 7d73371..8439d65 100644
--- a/crates/apollo-compiler/README.md
+++ b/crates/apollo-compiler/README.md
@@ -142,7 +142,7 @@ fn main() -> Result<()> {
     }
     assert!(diagnostics.is_empty());
 
-    let op = compiler.db.find_operation_by_name(query_id, String::from("getProduct"))
+    let op = compiler.db.find_operation(query_id, Some("getProduct".into()))
         .expect("getProduct query does not exist");
     let fragment_in_op: Vec<hir::FragmentDefinition> = op.selection_set().selection().iter().filter_map(|sel| match sel {
         hir::Selection::FragmentSpread(frag) => {
@@ -299,7 +299,7 @@ let diagnostics = compiler.validate();
 for diagnostic in &diagnostics {
     println!("{}", diagnostic)
 }
-assert_eq!(diagnostics.len(), 5)
+assert_eq!(diagnostics.len(), 9)
 ```
 
 ## License
diff --git a/crates/apollo-compiler/src/database/document.rs b/crates/apollo-compiler/src/database/document.rs
index 91c3fd7..b81f087 100644
--- a/crates/apollo-compiler/src/database/document.rs
+++ b/crates/apollo-compiler/src/database/document.rs
@@ -38,25 +38,20 @@ pub(crate) fn find_type_definition_by_name(
     db.types_definitions_by_name().get(&name).cloned()
 }
 
-pub(crate) fn find_operation_by_name(
+pub(crate) fn find_operation(
     db: &dyn HirDatabase,
     file_id: FileId,
-    name: String,
+    name: Option<String>,
 ) -> Option<Arc<OperationDefinition>> {
-    db.operations(file_id)
-        .iter()
-        .find(|def| def.name() == Some(&*name))
-        .cloned()
-}
-
-pub(crate) fn find_anonymous_operation(
-    db: &dyn HirDatabase,
-    file_id: FileId,
-) -> Option<Arc<OperationDefinition>> {
-    db.operations(file_id)
-        .iter()
-        .find(|def| def.name().is_none())
-        .cloned()
+    let ops = db.operations(file_id);
+
+    if let Some(name) = name {
+        ops.iter().find(|def| def.name() == Some(&*name)).cloned()
+    } else if ops.len() == 1 {
+        Some(ops[0].clone())
+    } else {
+        None
+    }
 }
 
 pub(crate) fn find_fragment_by_name(
@@ -323,6 +318,44 @@ mod tests {
     use crate::ApolloCompiler;
     use crate::HirDatabase;
 
+    #[test]
+    fn find_operations() {
+        let type_system = r#"
+type Query {
+  name: String
+}
+        "#;
+        let op = r#"{ name }"#;
+        let named_op = r#"query getName { name } "#;
+        let several_named_op = r#"query getName { name } query getAnotherName { name }"#;
+        let noop = r#""#;
+
+        let mut compiler = ApolloCompiler::new();
+        compiler.add_type_system(type_system, "ts.graphql");
+        let op_id = compiler.add_executable(op, "op.graphql");
+        let op = compiler.db.find_operation(op_id, None);
+        assert!(op.is_some());
+
+        compiler.update_executable(op_id, named_op);
+        let op = compiler.db.find_operation(op_id, Some("getName".into()));
+        assert!(op.is_some());
+        let op = compiler.db.find_operation(op_id, None);
+        assert!(op.is_some());
+
+        compiler.update_executable(op_id, several_named_op);
+        let op = compiler.db.find_operation(op_id, Some("getName".into()));
+        assert!(op.is_some());
+        let op = compiler.db.find_operation(op_id, None);
+        assert!(op.is_none());
+
+        compiler.update_executable(op_id, noop);
+        let op = compiler.db.find_operation(op_id, Some("getName".into()));
+        assert!(op.is_none());
+
+        let op = compiler.db.find_operation(op_id, None);
+        assert!(op.is_none());
+    }
+
     #[test]
     fn find_definitions_with_directive() {
         let schema = r#"
diff --git a/crates/apollo-compiler/src/database/hir.rs b/crates/apollo-compiler/src/database/hir.rs
index c658130..6018376 100644
--- a/crates/apollo-compiler/src/database/hir.rs
+++ b/crates/apollo-compiler/src/database/hir.rs
@@ -130,6 +130,17 @@ impl TypeDefinition {
             _ => None,
         }
     }
+
+    pub fn loc(&self) -> Option<HirNodeLocation> {
+        match self {
+            Self::ObjectTypeDefinition(def) => Some(def.loc()),
+            Self::InterfaceTypeDefinition(def) => Some(def.loc()),
+            Self::UnionTypeDefinition(def) => Some(def.loc()),
+            Self::EnumTypeDefinition(def) => Some(def.loc()),
+            Self::InputObjectTypeDefinition(def) => Some(def.loc()),
+            Self::ScalarTypeDefinition(def) => def.loc(),
+        }
+    }
 }
 
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
@@ -1455,6 +1466,12 @@ impl InputValueDefinition {
     pub fn default_value(&self) -> Option<&DefaultValue> {
         self.default_value.as_ref()
     }
+
+    /// If the argument does not have a default value and has a non-null type,
+    /// a value must be provided by users.
+    pub fn is_required(&self) -> bool {
+        self.ty().is_non_null() && self.default_value.is_none()
+    }
 }
 
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
@@ -2166,7 +2183,7 @@ mod tests {
 
         let db = compiler.db;
         let type_introspect: Arc<OperationDefinition> = db
-            .find_operation_by_name(query_id, String::from("TypeIntrospect"))
+            .find_operation(query_id, Some(String::from("TypeIntrospect")))
             .expect("TypeIntrospect operation does not exist");
 
         assert!(type_introspect.is_introspection(&db));
@@ -2202,11 +2219,11 @@ mod tests {
 
         let db = compiler.db;
         let check_stock: Arc<OperationDefinition> = db
-            .find_operation_by_name(query_id, String::from("CheckStock"))
+            .find_operation(query_id, Some("CheckStock".into()))
             .expect("CheckStock operation does not exist");
 
         let purchase_operation: Arc<OperationDefinition> = db
-            .find_operation_by_name(mutation_id, String::from("PurchaseBasket"))
+            .find_operation(mutation_id, Some("PurchaseBasket".into()))
             .expect("CheckStock operation does not exist");
 
         assert!(!check_stock.is_introspection(&db));
@@ -2279,15 +2296,15 @@ mod tests {
 
         let db = compiler.db;
         let deep_introspect: Arc<OperationDefinition> = db
-            .find_operation_by_name(query_id, String::from("IntrospectDeepFragments"))
+            .find_operation(query_id, Some("IntrospectDeepFragments".into()))
             .expect("IntrospectDeepFragments operation does not exist");
 
         assert!(deep_introspect.is_introspection(&db));
 
         let deep_introspect: Arc<OperationDefinition> = db
-            .find_operation_by_name(
+            .find_operation(
                 query_id_not_introspect,
-                String::from("IntrospectDeepFragments"),
+                Some("IntrospectDeepFragments".into()),
             )
             .expect("IntrospectDeepFragments operation does not exist");
         assert!(!deep_introspect.is_introspection(&db));
diff --git a/crates/apollo-compiler/src/database/hir_db.rs b/crates/apollo-compiler/src/database/hir_db.rs
index 59536da..7b035ce 100644
--- a/crates/apollo-compiler/src/database/hir_db.rs
+++ b/crates/apollo-compiler/src/database/hir_db.rs
@@ -65,16 +65,18 @@ pub trait HirDatabase: InputDatabase + AstDatabase {
     fn input_objects(&self) -> ByName<InputObjectTypeDefinition>;
 
     // Derived from above queries:
+
     /// Return an operation definition corresponding to the name and file id.
-    fn find_operation_by_name(
+    /// If `name` is `None`, and there is only one operation, that operation will
+    /// be returned.
+    /// If `name` is `None`, and there is more than one operation, `None` will
+    /// be returned.
+    fn find_operation(
         &self,
         file_id: FileId,
-        name: String,
+        name: Option<String>,
     ) -> Option<Arc<OperationDefinition>>;
 
-    /// Return an operation definition without a name, corresponding to the file id.
-    fn find_anonymous_operation(&self, file_id: FileId) -> Option<Arc<OperationDefinition>>;
-
     /// Return an fragment definition corresponding to the name and file id.
     /// Result of this query is not cached internally.
     #[salsa::transparent]
diff --git a/crates/apollo-compiler/src/diagnostics.rs b/crates/apollo-compiler/src/diagnostics.rs
index ddbd33f..d8805f8 100644
--- a/crates/apollo-compiler/src/diagnostics.rs
+++ b/crates/apollo-compiler/src/diagnostics.rs
@@ -167,6 +167,8 @@ pub enum DiagnosticData {
         /// Field name
         field: String,
     },
+    #[error("the argument `{name}` is not supported")]
+    UndefinedArgument { name: String },
     #[error("cannot find type `{name}` in this document")]
     UndefinedDefinition {
         /// Name of the type not in scope
@@ -190,6 +192,8 @@ pub enum DiagnosticData {
         // current field that should be defined
         field: String,
     },
+    #[error("the required argument `{name}` is not provided")]
+    RequiredArgument { name: String },
     #[error(
         "Transitively implemented interfaces must also be defined on an implementing interface or object"
     )]
diff --git a/crates/apollo-compiler/src/lib.rs b/crates/apollo-compiler/src/lib.rs
index 61b64e6..abf6521 100644
--- a/crates/apollo-compiler/src/lib.rs
+++ b/crates/apollo-compiler/src/lib.rs
@@ -272,7 +272,7 @@ fragment vipCustomer on User {
 }
 
 type Query {
-  topProducts: Product
+  topProducts(first: Int): Product
   customer: User
 }
 
@@ -634,7 +634,7 @@ fragment vipCustomer on User {
 
         let op = compiler
             .db
-            .find_operation_by_name(query_id, String::from("getProduct"));
+            .find_operation(query_id, Some("getProduct".into()));
         let fragment_in_op: Vec<crate::hir::FragmentDefinition> = op
             .unwrap()
             .selection_set()
@@ -1229,7 +1229,7 @@ type Query {
                     compiler.set_type_system_hir(cloned);
                     compiler
                         .db
-                        .find_anonymous_operation(query_id)
+                        .find_operation(query_id, None)
                         .unwrap()
                         .fields(&compiler.db)[0]
                         .ty(&compiler.db)
diff --git a/crates/apollo-compiler/src/validation/directive.rs b/crates/apollo-compiler/src/validation/directive.rs
index 84fa87b..21074ee 100644
--- a/crates/apollo-compiler/src/validation/directive.rs
+++ b/crates/apollo-compiler/src/validation/directive.rs
@@ -97,9 +97,9 @@ pub fn validate_directives(
         let name = dir.name();
         let loc = dir.loc();
 
-        if let Some(directive) = db.find_directive_definition_by_name(name.into()) {
+        if let Some(directive_definition) = db.find_directive_definition_by_name(name.into()) {
             let allowed_loc: HashSet<hir::DirectiveLocation> =
-                HashSet::from_iter(directive.directive_locations().iter().cloned());
+                HashSet::from_iter(directive_definition.directive_locations().iter().cloned());
             if !allowed_loc.contains(&dir_loc) {
                 let mut diag = ApolloDiagnostic::new(
                         db,
@@ -107,12 +107,12 @@ pub fn validate_directives(
                         DiagnosticData::UnsupportedLocation {
                             name: name.into(),
                             dir_loc,
-                            directive_def: directive.loc.map(|loc| loc.into()),
+                            directive_def: directive_definition.loc.map(|loc| loc.into()),
                         },
                 )
                     .label(Label::new(loc, format!("{dir_loc} is not a valid location")))
                     .help("the directive must be used in a location that the service has declared support for");
-                if let Some(directive_def_loc) = directive.loc {
+                if let Some(directive_def_loc) = directive_definition.loc {
                     diag = diag.label(Label::new(
                         directive_def_loc,
                         format!("consider adding {dir_loc} directive location here"),
@@ -120,6 +120,63 @@ pub fn validate_directives(
                 }
                 diagnostics.push(diag)
             }
+
+            for arg in dir.arguments() {
+                let exists = directive_definition
+                    .arguments()
+                    .input_values()
+                    .iter()
+                    .any(|arg_def| arg.name() == arg_def.name());
+
+                if !exists {
+                    let mut diagnostic = ApolloDiagnostic::new(
+                        db,
+                        arg.loc.into(),
+                        DiagnosticData::UndefinedArgument {
+                            name: arg.name().into(),
+                        },
+                    )
+                    .label(Label::new(arg.loc, "argument by this name not found"));
+                    if let Some(loc) = directive_definition.loc {
+                        diagnostic = diagnostic.label(Label::new(loc, "directive declared here"));
+                    }
+
+                    diagnostics.push(diagnostic);
+                }
+            }
+
+            for arg_def in directive_definition.arguments().input_values() {
+                let arg_value = dir
+                    .arguments()
+                    .iter()
+                    .find(|value| value.name() == arg_def.name());
+                let is_null = match arg_value {
+                    None => true,
+                    // Prevents explicitly providing `requiredArg: null`,
+                    // but you can still indirectly do the wrong thing by typing `requiredArg: $mayBeNull`
+                    // and it won't raise a validation error at this stage.
+                    Some(value) => value.value() == &hir::Value::Null,
+                };
+
+                if arg_def.is_required() && is_null {
+                    let mut diagnostic = ApolloDiagnostic::new(
+                        db,
+                        dir.loc.into(),
+                        DiagnosticData::RequiredArgument {
+                            name: arg_def.name().into(),
+                        },
+                    );
+                    diagnostic = diagnostic.label(Label::new(
+                        dir.loc,
+                        format!("missing value for argument `{}`", arg_def.name()),
+                    ));
+                    if let Some(loc) = arg_def.loc {
+                        diagnostic = diagnostic.label(Label::new(loc, "argument defined here"));
+                    }
+
+                    diagnostics.push(diagnostic);
+                }
+            }
         } else {
             diagnostics.push(
                 ApolloDiagnostic::new(
diff --git a/crates/apollo-compiler/src/validation/field.rs b/crates/apollo-compiler/src/validation/field.rs
index fa35da5..f52f1a1 100644
--- a/crates/apollo-compiler/src/validation/field.rs
+++ b/crates/apollo-compiler/src/validation/field.rs
@@ -12,7 +12,105 @@ pub fn validate_field(
 ) -> Vec<ApolloDiagnostic> {
     let mut diagnostics =
         db.validate_directives(field.directives().to_vec(), hir::DirectiveLocation::Field);
-    diagnostics.extend(db.validate_arguments(field.arguments().to_vec()));
+
+    if !field.arguments().is_empty() {
+        diagnostics.extend(db.validate_arguments(field.arguments().to_vec()));
+    }
+
+    if let Some(field_definition) = field.field_definition(db.upcast()) {
+        for arg in field.arguments() {
+            let exists = field_definition
+                .arguments()
+                .input_values()
+                .iter()
+                .any(|arg_def| arg.name() == arg_def.name());
+
+            if !exists {
+                let diagnostic = ApolloDiagnostic::new(
+                    db,
+                    arg.loc.into(),
+                    DiagnosticData::UndefinedArgument {
+                        name: arg.name().into(),
+                    },
+                )
+                .label(Label::new(arg.loc, "argument by this name not found"))
+                .label(Label::new(field_definition.loc, "field declared here"));
+
+                diagnostics.push(diagnostic);
+            }
+        }
+
+        for arg_def in field_definition.arguments().input_values() {
+            let arg_value = field
+                .arguments()
+                .iter()
+                .find(|value| value.name() == arg_def.name());
+            let is_null = match arg_value {
+                None => true,
+                // Prevents explicitly providing `requiredArg: null`,
+                // but you can still indirectly do the wrong thing by typing `requiredArg: $mayBeNull`
+                // and it won't raise a validation error at this stage.
+                Some(value) => value.value() == &hir::Value::Null,
+            };
+
+            if arg_def.is_required() && is_null {
+                let mut diagnostic = ApolloDiagnostic::new(
+                    db,
+                    field.loc.into(),
+                    DiagnosticData::RequiredArgument {
+                        name: arg_def.name().into(),
+                    },
+                );
+                diagnostic = diagnostic.label(Label::new(
+                    field.loc,
+                    format!("missing value for argument `{}`", arg_def.name()),
+                ));
+                if let Some(loc) = arg_def.loc {
+                    diagnostic = diagnostic.label(Label::new(loc, "argument defined here"));
+                }
+
+                diagnostics.push(diagnostic);
+            }
+        }
+    }
+
+    let field_type = field.ty(db.upcast());
+    if field_type.is_some() {
+        diagnostics.extend(db.validate_selection_set(field.selection_set().clone()));
+    } else {
+        let help = format!(
+            "`{}` is not defined on `{}` type",
+            field.name(),
+            field.parent_obj.as_ref().unwrap(),
+        );
+        let field_name = field.name();
+        let diagnostic = ApolloDiagnostic::new(
+            db,
+            field.loc().into(),
+            DiagnosticData::UndefinedField {
+                field: field_name.into(),
+            },
+        )
+        .label(Label::new(
+            field.loc(),
+            format!("`{field_name}` field is not defined"),
+        ))
+        .help(help);
+
+        let parent_type_loc = db
+            .find_type_definition_by_name(field.parent_obj.clone().unwrap())
+            .and_then(|type_def| type_def.loc());
+
+        let diagnostic = if let Some(parent_type_loc) = parent_type_loc {
+            diagnostic.label(Label::new(
+                parent_type_loc,
+                format!("`{}` declared here", field.parent_obj.as_ref().unwrap()),
+            ))
+        } else {
+            diagnostic
+        };
+        diagnostics.push(diagnostic);
+    }
 
     diagnostics
 }
@@ -61,13 +159,13 @@ pub fn validate_field_definitions(
                         field: field_name.into(),
                         original_definition: original_definition.into(),
                         redefined_definition: redefined_definition.into(),
-                    }
+                    },
                 )
-                .labels([
-                    Label::new(original_definition, format!("previous definition of `{field_name}` here")),
-                    Label::new(redefined_definition, format!("`{field_name}` redefined here")),
-                ])
-                .help(format!("`{field_name}` field must only be defined once in this input object definition."))
+                    .labels([
+                        Label::new(original_definition, format!("previous definition of `{field_name}` here")),
+                        Label::new(redefined_definition, format!("`{field_name}` redefined here")),
+                    ])
+                    .help(format!("`{field_name}` field must only be defined once in this input object definition."))
             );
         } else {
             seen.insert(field_name, field);
@@ -81,8 +179,8 @@ pub fn validate_field_definitions(
                         name: field.name().into(),
                         ty: field_ty.kind(),
                     })
-                    .label(Label::new(field.loc(), format!("this is of `{}` type", field_ty.kind())))
-                    .help(format!("Scalars, Objects, Interfaces, Unions and Enums are output types. Change `{}` field to return one of these output types.", field.name())),
+                        .label(Label::new(field.loc(), format!("this is of `{}` type", field_ty.kind())))
+                        .help(format!("Scalars, Objects, Interfaces, Unions and Enums are output types. Change `{}` field to return one of these output types.", field.name())),
                 );
             }
         } else if let Some(field_ty_loc) = field.ty().loc() {
diff --git a/crates/apollo-compiler/src/validation/fragment.rs b/crates/apollo-compiler/src/validation/fragment.rs
index e36ac9c..6022673 100644
--- a/crates/apollo-compiler/src/validation/fragment.rs
+++ b/crates/apollo-compiler/src/validation/fragment.rs
@@ -10,8 +10,48 @@ pub fn validate_fragment_definitions(
             def.directives().to_vec(),
             DirectiveLocation::FragmentDefinition,
         ));
-        diagnostics.extend(db.validate_selection_set(def.selection_set().clone()));
+
+        let fragment_type_def = db.find_type_definition_by_name(def.type_condition().to_string());
+        // Make sure the fragment type exists in the schema
+        if fragment_type_def.is_some() {
+            // TODO handle cases where the type does not support fragments (Enum, Scalar...)
+            diagnostics.extend(db.validate_selection_set(def.selection_set().clone()));
+        }
     }
 
     diagnostics
 }
+
+#[cfg(test)]
+mod test {
+    use crate::ApolloCompiler;
+
+    #[test]
+    fn it_validates_fields_in_fragment_definitions() {
+        let input = r#"
+type Query {
+  name: String
+  topProducts: Product
+}
+
+type Product {
+  inStock: Boolean
+  name: String
+}
+
+fragment XY on Product {
+  notExistingField
+}
+"#;
+
+        let mut compiler = ApolloCompiler::new();
+        compiler.add_document(input, "schema.graphql");
+
+        let diagnostics = compiler.validate();
+        for diagnostic in &diagnostics {
+            println!("{diagnostic}");
+        }
+
+        assert_eq!(diagnostics.len(), 1)
+    }
+}
diff --git a/crates/apollo-compiler/src/validation/operation.rs b/crates/apollo-compiler/src/validation/operation.rs
index d3e25c4..006557d 100644
--- a/crates/apollo-compiler/src/validation/operation.rs
+++ b/crates/apollo-compiler/src/validation/operation.rs
@@ -16,7 +16,12 @@ pub fn validate_operation_definitions(
         diagnostics
             .extend(db.validate_directives(def.directives().to_vec(), def.operation_ty().into()));
         diagnostics.extend(db.validate_variable_definitions(def.variables.as_ref().clone()));
-        diagnostics.extend(db.validate_selection_set(def.selection_set().clone()));
+
+        // Validate the Selection Set recursively
+        // Check that the root type exists
+        if def.object_type(db.upcast()).is_some() {
+            diagnostics.extend(db.validate_selection_set(def.selection_set().clone()));
+        }
     }
 
     let subscription_operations = db.upcast().subscription_operations(file_id);
@@ -93,50 +98,6 @@ pub fn validate_operation_definitions(
         }
     }
 
-    // Fields must exist on the type being queried.
-    for op in operations.iter() {
-        for selection in op.selection_set().selection() {
-            let obj_type = op.object_type(db.upcast());
-            let obj_name = obj_type.as_ref().map(|obj| obj.name().to_owned());
-            if let hir::Selection::Field(field) = selection {
-                if field.ty(db.upcast()).is_none() {
-                    let field_name = field.name();
-                    let help = if let Some(actual_name) = obj_name {
-                        format!("`{field_name}` is not defined on `{actual_name}` type")
-                    } else {
-                        format!(
-                            "`{}` is not defined on the current {} root operation type.",
-                            field_name,
-                            op.operation_ty()
-                        )
-                    };
-                    let diagnostic = ApolloDiagnostic::new(
-                        db,
-                        field.loc().into(),
-                        DiagnosticData::UndefinedField {
-                            field: field_name.into(),
-                        },
-                    )
-                    .label(Label::new(
-                        field.loc(),
-                        format!("`{field_name}` field is not defined"),
-                    ))
-                    .help(help);
-
-                    let diagnostic = if let Some(ty) = obj_type {
-                        diagnostic.label(Label::new(
-                            ty.loc(),
-                            format!("`{}` declared here", ty.name()),
-                        ))
-                    } else {
-                        diagnostic
-                    };
-                    diagnostics.push(diagnostic)
-                }
-            }
-        }
-    }
-
     diagnostics
 }
 
@@ -317,51 +278,25 @@ query {
 
 query getPet {
   cat {
-    owner {
-      name
-    }
+    name
   }
 }
 
-query getPet {
+query getOtherPet {
   cat {
-    treat
-  }
-}
-
-subscription sub {
-  newMessage {
-    body
-    sender
+    nickname
   }
-  disallowedSecondRootField
 }
 
 type Query {
-  cat: Pet
+  cat: Cat
 }
 
-type Subscription {
-  newMessage: Result
-}
-
-interface Pet {
-  name: String
-}
-
-type Dog implements Pet {
-  name: String
-  nickname: String
-  barkVolume: Int
-}
-
-type Cat implements Pet {
+type Cat {
   name: String
   nickname: String
   meowVolume: Int
 }
-
-union CatOrDog = Cat | Dog
 "#;
         let mut compiler = ApolloCompiler::new();
         compiler.add_document(input, "schema.graphql");
@@ -370,7 +305,7 @@ union CatOrDog = Cat | Dog
         for diagnostic in &diagnostics {
             println!("{diagnostic}")
         }
-        assert_eq!(diagnostics.len(), 5)
+        assert_eq!(diagnostics.len(), 1)
     }
 
     #[test]
@@ -384,9 +319,8 @@ query getName {
 
 query getName {
   cat {
-    owner {
-      name
-    }
+    name
+    nickname
   }
 }
 
@@ -398,6 +332,7 @@ union CatOrDog = Cat | Dog
 
 interface Pet {
   name: String
+  nickname: String
 }
 
 type Dog implements Pet {
@@ -431,11 +366,9 @@ query getCatName {
   }
 }
 
-query getOwnerName {
+query getPetNickname {
   cat {
-    owner {
-      name
-    }
+    nickname
   }
 }
 
@@ -447,6 +380,7 @@ union CatOrDog = Cat | Dog
 
 interface Pet {
   name: String
+  nickname: String
 }
 
 type Dog implements Pet {
@@ -465,6 +399,9 @@ type Cat implements Pet {
         compiler.add_document(input, "schema.graphql");
 
         let diagnostics = compiler.validate();
+        for diagnostic in &diagnostics {
+            println!("{diagnostic}")
+        }
         assert!(diagnostics.is_empty());
     }
 
@@ -508,15 +445,19 @@ type Cat implements Pet {
             println!("{diagnostic}")
         }
 
-        assert_eq!(diagnostics.len(), 2)
+        assert_eq!(diagnostics.len(), 1)
     }
 
     #[test]
     fn it_validates_fields_in_operations() {
         let input = r#"
 query getProduct {
-  size
-  weight
+  name
+  noName
+  topProducts {
+    inStock
+    price
+  }
 }
 
 type Query {
@@ -525,11 +466,9 @@ type Query {
 }
 
 type Product {
-  inStock: Boolean @join__field(graph: INVENTORY)
-  name: String @join__field(graph: PRODUCTS)
+  inStock: Boolean
+  name: String
 }
-
-directive @join__field(graph: join__Graph, requires: join__FieldSet, provides: join__FieldSet) on FIELD_DEFINITION
 "#;
 
         let mut compiler = ApolloCompiler::new();
diff --git a/crates/apollo-compiler/src/validation/selection.rs b/crates/apollo-compiler/src/validation/selection.rs
index 80f4dfc..311018e 100644
--- a/crates/apollo-compiler/src/validation/selection.rs
+++ b/crates/apollo-compiler/src/validation/selection.rs
@@ -11,12 +11,10 @@ pub fn validate_selection(
     for sel in selection.iter() {
         match sel {
             hir::Selection::Field(field) => {
-                if !field.selection_set().selection().is_empty() {
-                    diagnostics
-                        .extend(db.validate_selection(field.selection_set().selection.clone()))
-                }
                 diagnostics.extend(db.validate_field(field.clone()));
             }
+
+            // TODO handle fragment spreads on invalid parent types
             hir::Selection::FragmentSpread(frag) => diagnostics.extend(db.validate_directives(
                 frag.directives().to_vec(),
                 hir::DirectiveLocation::FragmentSpread,
@@ -35,5 +33,11 @@ pub fn validate_selection_set(
     db: &dyn ValidationDatabase,
     selection_set: hir::SelectionSet,
 ) -> Vec<ApolloDiagnostic> {
-    db.validate_selection(selection_set.selection)
+    let mut diagnostics = Vec::new();
+
+    // TODO handle Enums, Scalar (no selection set content allowed), Unions
+
+    diagnostics.extend(db.validate_selection(selection_set.selection));
+
+    diagnostics
 }
diff --git a/crates/apollo-compiler/src/validation/variable.rs b/crates/apollo-compiler/src/validation/variable.rs
index 4071200..858c5ef 100644
--- a/crates/apollo-compiler/src/validation/variable.rs
+++ b/crates/apollo-compiler/src/validation/variable.rs
@@ -112,7 +112,7 @@ query ExampleQuery {
 }
 
 type Query {
-  topProducts: Products
+  topProducts(first: Int): Products
 }
 
 type Products {
@@ -187,6 +187,7 @@ fragment fragmentOne on Query {
 
 type Query {
   topProducts: Product
+  profilePic(size: Int): String
 }
 
 type Product {
diff --git a/crates/apollo-encoder/src/from_parser.rs b/crates/apollo-encoder/src/from_parser.rs
index 446a7c3..bf1f756 100644
--- a/crates/apollo-encoder/src/from_parser.rs
+++ b/crates/apollo-encoder/src/from_parser.rs
@@ -10,10 +10,13 @@ use thiserror::Error;
 // occurred
 #[derive(Debug, Clone, Error)]
 pub enum FromError {
+    /// the parse tree is missing a node
     #[error("parse tree is missing a node")]
     MissingNode,
+    /// expected to parse a `i32`
     #[error("invalid i32")]
     ParseIntError(#[from] std::num::ParseIntError),
+    /// expected to parse a `f64`
     #[error("invalid f64")]
     ParseFloatError(#[from] std::num::ParseFloatError),
 }
diff --git a/crates/apollo-encoder/src/lib.rs b/crates/apollo-encoder/src/lib.rs
index 9b5d864..b8fbfba 100644
--- a/crates/apollo-encoder/src/lib.rs
+++ b/crates/apollo-encoder/src/lib.rs
@@ -37,6 +37,7 @@ pub use enum_def::EnumDefinition;
 pub use enum_value::EnumValue;
 pub use field::{Field, FieldDefinition};
 pub use fragment::{FragmentDefinition, FragmentSpread, InlineFragment, TypeCondition};
+pub use from_parser::FromError;
 pub use input_field::InputField;
 pub use input_object_def::InputObjectDefinition;
 pub use input_value::InputValueDefinition;
diff --git a/crates/apollo-parser/src/parser/generated/syntax_kind.rs b/crates/apollo-parser/src/parser/generated/syntax_kind.rs
index 763190d..9cd0b9e 100644
--- a/crates/apollo-parser/src/parser/generated/syntax_kind.rs
+++ b/crates/apollo-parser/src/parser/generated/syntax_kind.rs
@@ -76,6 +76,7 @@ pub enum SyntaxKind {
     IDENT,
     WHITESPACE,
     COMMENT,
+    ERROR,
     NAME,
     INTEGER_PART,
     NEGATIVE_SIGN,
diff --git a/crates/apollo-parser/src/parser/mod.rs b/crates/apollo-parser/src/parser/mod.rs
index 68b6025..1af9de6 100644
--- a/crates/apollo-parser/src/parser/mod.rs
+++ b/crates/apollo-parser/src/parser/mod.rs
@@ -181,7 +181,7 @@ impl<'a> Parser<'a> {
         }
 
         let token = self.pop();
-        self.builder.borrow_mut().token(kind, token.data());
+        self.push_ast(kind, token);
     }
 
     /// Create a parser limit error and push it into the error vector.
@@ -235,16 +235,20 @@ impl<'a> Parser<'a> {
         }
 
         let current = self.pop();
-        // we usually bump ignored after we pop a token, so make sure we also do
-        // this when we create an error and pop.
-        self.skip_ignored();
         let err = if current.kind == TokenKind::Eof {
             Error::eof(message, current.index())
         } else {
             // this needs to be the computed location
             Error::with_loc(message, current.data().to_string(), current.index())
         };
+
+        // Keep the error in the parse tree for position information
+        self.push_ast(SyntaxKind::ERROR, current);
         self.push_err(err);
+
+        // we usually skip ignored tokens after we pop each token, so make sure we also do
+        // this when we create an error and pop.
+        self.skip_ignored();
     }
 
     /// Consume the next token if it is `kind` or emit an error
@@ -561,4 +565,28 @@ mod tests {
         "##]];
         tree.assert_eq(&format!("{:#?}", ast.document().syntax));
     }
+
+    #[test]
+    fn tree_with_syntax_errors() {
+        use crate::ast::Definition;
+
+        // Some arbitrary token spam in incorrect places--this test uses
+        // valid tokens only
+        let source = r#"
+            garbage type Query implements X {
+                field(arg: Int): Int
+            } garbage :,, (|) interface X {}
+        "#;
+        let ast = Parser::new(source).parse();
+
+        let mut definitions = ast.document().definitions();
+        let query_def = definitions.next().unwrap();
+        let interface_def = definitions.next().unwrap();
+        assert_eq!(definitions.next(), None);
+        assert!(matches!(query_def, Definition::ObjectTypeDefinition(_)));
+        assert!(matches!(
+            interface_def,
+            Definition::InterfaceTypeDefinition(_)
+        ));
+    }
 }
diff --git a/xtask/src/ast_src.rs b/xtask/src/ast_src.rs
index bb2c1b2..0b0025c 100644
--- a/xtask/src/ast_src.rs
+++ b/xtask/src/ast_src.rs
@@ -65,7 +65,7 @@ pub(crate) const KINDS_SRC: KindsSrc = KindsSrc {
         "INPUT_FIELD_DEFINITION",
     ],
     literals: &["INT", "FLOAT", "STRING"],
-    tokens: &["IDENT", "WHITESPACE", "COMMENT"],
+    tokens: &["IDENT", "WHITESPACE", "COMMENT", "ERROR"],
     // These are all the "DOCUMENT" items defined in the GraphQL spec --
     // https://spec.graphql.org/October2021/#sec-Appendix-Grammar-Summary.Document,
     // as well as items listed in the ungrammar file in the root directory.


Test patch:
diff --git a/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.graphql b/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.graphql
index b1ac364..dcc364e 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.graphql
@@ -1,15 +1,9 @@
 query getName {
-  cat {
-    name
-  }
+  cat
 }
 
 query getName {
-  cat {
-    owner {
-      name
-    }
-  }
+  cat
 }
 
 type Query {
diff --git a/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.txt b/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.txt
index aee8cd1..cf46bf3 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0001_duplicate_operatoin_names.txt
@@ -7,7 +7,7 @@
             file_id: FileId {
                 id: 0,
             },
-            offset: 46,
+            offset: 31,
             length: 7,
         },
         labels: [
@@ -26,7 +26,7 @@
                     file_id: FileId {
                         id: 0,
                     },
-                    offset: 46,
+                    offset: 31,
                     length: 7,
                 },
                 text: "`getName` redefined here",
@@ -49,7 +49,7 @@
                 file_id: FileId {
                     id: 0,
                 },
-                offset: 46,
+                offset: 31,
                 length: 7,
             },
         },
diff --git a/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.graphql b/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.graphql
index ee5985f..a61d4f2 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.graphql
@@ -1,14 +1,10 @@
 query {
-  cat {
-    name
-  }
+  cat
 }
 
 mutation {
-  addPet {
-    owner {
-      name
-    }
+  addPet(name: "Example") {
+    response
   }
 }
 
@@ -23,4 +19,4 @@ type Mutation {
 type Result {
   pageNumber: Int
   response: String
-}
\ No newline at end of file
+}
diff --git a/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.txt b/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.txt
index cffe8f0..07976ad 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0002_multiple_anonymous_operations.txt
@@ -8,7 +8,7 @@
                 id: 1,
             },
             offset: 0,
-            length: 30,
+            length: 15,
         },
         labels: [
             Label {
@@ -17,7 +17,7 @@
                         id: 1,
                     },
                     offset: 0,
-                    length: 30,
+                    length: 15,
                 },
                 text: "provide a name for this definition",
             },
@@ -35,8 +35,8 @@
             file_id: FileId {
                 id: 1,
             },
-            offset: 32,
-            length: 56,
+            offset: 17,
+            length: 57,
         },
         labels: [
             Label {
@@ -44,8 +44,8 @@
                     file_id: FileId {
                         id: 1,
                     },
-                    offset: 32,
-                    length: 56,
+                    offset: 17,
+                    length: 57,
                 },
                 text: "provide a name for this definition",
             },
diff --git a/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.graphql b/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.graphql
index 80354f4..8d6bd73 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.graphql
@@ -1,14 +1,10 @@
 query {
-  cat {
-    name
-  }
+  cat
 }
 
 mutation getName {
   addPet {
-    owner {
-      name
-    }
+    response
   }
 }
 
diff --git a/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.txt b/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.txt
index 7cac0ce..10dc12e 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0003_anonymous_and_named_operation.txt
@@ -1,4 +1,42 @@
 [
+    ApolloDiagnostic {
+        cache: {
+            2: "0003_anonymous_and_named_operation.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 2,
+            },
+            offset: 38,
+            length: 25,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 2,
+                    },
+                    offset: 38,
+                    length: 25,
+                },
+                text: "missing value for argument `name`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 2,
+                    },
+                    offset: 123,
+                    length: 13,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "name",
+        },
+    },
     ApolloDiagnostic {
         cache: {
             2: "0003_anonymous_and_named_operation.graphql",
@@ -8,7 +46,7 @@
                 id: 2,
             },
             offset: 0,
-            length: 30,
+            length: 15,
         },
         labels: [
             Label {
@@ -17,7 +55,7 @@
                         id: 2,
                     },
                     offset: 0,
-                    length: 30,
+                    length: 15,
                 },
                 text: "provide a name for this definition",
             },
diff --git a/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.graphql b/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.graphql
index c5cf392..e8aef3d 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.graphql
@@ -8,6 +8,7 @@ subscription sub {
 
 type Subscription {
   newMessage: Result
+  disallowedSecondRootField: String
 }
 
 type Result {
diff --git a/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.txt b/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.txt
index 156cb65..e151cff 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0004_subscription_with_multiple_root_fields.txt
@@ -36,44 +36,4 @@
             },
         },
     },
-    ApolloDiagnostic {
-        cache: {
-            3: "0004_subscription_with_multiple_root_fields.graphql",
-        },
-        location: DiagnosticLocation {
-            file_id: FileId {
-                id: 3,
-            },
-            offset: 60,
-            length: 25,
-        },
-        labels: [
-            Label {
-                location: DiagnosticLocation {
-                    file_id: FileId {
-                        id: 3,
-                    },
-                    offset: 60,
-                    length: 25,
-                },
-                text: "`disallowedSecondRootField` field is not defined",
-            },
-            Label {
-                location: DiagnosticLocation {
-                    file_id: FileId {
-                        id: 3,
-                    },
-                    offset: 89,
-                    length: 42,
-                },
-                text: "`Subscription` declared here",
-            },
-        ],
-        help: Some(
-            "`disallowedSecondRootField` is not defined on `Subscription` type",
-        ),
-        data: UndefinedField {
-            field: "disallowedSecondRootField",
-        },
-    },
 ]
diff --git a/crates/apollo-compiler/test_data/diagnostics/0005_subscription_with_multiple_root_fields_in_fragment_spreads.graphql b/crates/apollo-compiler/test_data/diagnostics/0005_subscription_with_multiple_root_fields_in_fragment_spreads.graphql
index a4e608a..c0d8464 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0005_subscription_with_multiple_root_fields_in_fragment_spreads.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0005_subscription_with_multiple_root_fields_in_fragment_spreads.graphql
@@ -12,6 +12,7 @@ fragment multipleSubscriptions on Subscription {
 
 type Subscription {
   newMessage: Result
+  disallowedSecondRootField: String
 }
 
 type Result {
diff --git a/crates/apollo-compiler/test_data/diagnostics/0011_syntactic_errors_from_parser.graphql b/crates/apollo-compiler/test_data/diagnostics/0011_syntactic_errors_from_parser.graphql
index 36c714e..207c226 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0011_syntactic_errors_from_parser.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0011_syntactic_errors_from_parser.graphql
@@ -11,4 +11,5 @@ type Query {
 
 type Product {
   name: String
+  birthday: String
 }
\ No newline at end of file
diff --git a/crates/apollo-compiler/test_data/diagnostics/0014_subscription_operation_without_subscription_schema_operation_type.txt b/crates/apollo-compiler/test_data/diagnostics/0014_subscription_operation_without_subscription_schema_operation_type.txt
index 97cf048..0a4438f 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0014_subscription_operation_without_subscription_schema_operation_type.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0014_subscription_operation_without_subscription_schema_operation_type.txt
@@ -37,34 +37,4 @@
             ty: "subscription",
         },
     },
-    ApolloDiagnostic {
-        cache: {
-            13: "0014_subscription_operation_without_subscription_schema_operation_type.graphql",
-        },
-        location: DiagnosticLocation {
-            file_id: FileId {
-                id: 13,
-            },
-            offset: 37,
-            length: 36,
-        },
-        labels: [
-            Label {
-                location: DiagnosticLocation {
-                    file_id: FileId {
-                        id: 13,
-                    },
-                    offset: 37,
-                    length: 36,
-                },
-                text: "`newMessage` field is not defined",
-            },
-        ],
-        help: Some(
-            "`newMessage` is not defined on the current Subscription root operation type.",
-        ),
-        data: UndefinedField {
-            field: "newMessage",
-        },
-    },
 ]
diff --git a/crates/apollo-compiler/test_data/diagnostics/0015_mutation_operation_without_mutation_schema_definition.txt b/crates/apollo-compiler/test_data/diagnostics/0015_mutation_operation_without_mutation_schema_definition.txt
index d95a0f4..7567792 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0015_mutation_operation_without_mutation_schema_definition.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0015_mutation_operation_without_mutation_schema_definition.txt
@@ -29,34 +29,4 @@
             ty: "mutation",
         },
     },
-    ApolloDiagnostic {
-        cache: {
-            14: "0015_mutation_operation_without_mutation_schema_definition.graphql",
-        },
-        location: DiagnosticLocation {
-            file_id: FileId {
-                id: 14,
-            },
-            offset: 31,
-            length: 41,
-        },
-        labels: [
-            Label {
-                location: DiagnosticLocation {
-                    file_id: FileId {
-                        id: 14,
-                    },
-                    offset: 31,
-                    length: 41,
-                },
-                text: "`addPet` field is not defined",
-            },
-        ],
-        help: Some(
-            "`addPet` is not defined on the current Mutation root operation type.",
-        ),
-        data: UndefinedField {
-            field: "addPet",
-        },
-    },
 ]
diff --git a/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.graphql b/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.graphql
index 9b26e98..210459c 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.graphql
@@ -1,4 +1,4 @@
-query queryA($status: String @skip) @skip(if: $foo){
+query queryA($status: String @skip(if: true)) @skip(if: $foo){
   field
   response(status: $status) @deprecated
   human {
@@ -24,7 +24,7 @@ mutation myMutation @skip(if: true) {
   setMessage (message: "Hello, World! Yours, GraphQL.")
 }
 
-interface Pet @skip {
+interface Pet @skip(if: true) {
   name: String
 }
 
@@ -40,8 +40,8 @@ type Cat implements Pet {
   meowVolume: Int
 }
 
-input Example @include {
-  self: Example @include
+input Example @include(if: true) {
+  self: Example @include(if: true)
   value: String
 }
 
@@ -72,7 +72,7 @@ type Mutation {
   setMessage(message: String): String
 }
 
-schema @include {
+schema @include(if: true) {
   query: Query
   subscription: Subscription
   mutation: Mutation
@@ -86,4 +86,4 @@ type Result {
 scalar spec @directiveB @specifiedBy(url: "https://spec.graphql.org/")
 
 directive @directiveA on UNION
-directive @directiveB on ENUM
\ No newline at end of file
+directive @directiveB on ENUM
diff --git a/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.txt b/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.txt
index 6067537..318e150 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0050_directives_in_invalid_locations.txt
@@ -7,8 +7,8 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 1105,
-            length: 8,
+            offset: 1145,
+            length: 18,
         },
         labels: [
             Label {
@@ -16,8 +16,8 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1105,
-                    length: 8,
+                    offset: 1145,
+                    length: 18,
                 },
                 text: "SCHEMA is not a valid location",
             },
@@ -39,7 +39,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 1246,
+            offset: 1296,
             length: 11,
         },
         labels: [
@@ -48,7 +48,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1246,
+                    offset: 1296,
                     length: 11,
                 },
                 text: "SCALAR is not a valid location",
@@ -58,7 +58,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
                 text: "consider adding SCALAR directive location here",
@@ -75,7 +75,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
             ),
@@ -89,7 +89,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 791,
+            offset: 831,
             length: 11,
         },
         labels: [
@@ -98,7 +98,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 791,
+                    offset: 831,
                     length: 11,
                 },
                 text: "ENUM is not a valid location",
@@ -108,7 +108,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
                 text: "consider adding ENUM directive location here",
@@ -125,7 +125,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
             ),
@@ -139,7 +139,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 813,
+            offset: 853,
             length: 11,
         },
         labels: [
@@ -148,7 +148,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 813,
+                    offset: 853,
                     length: 11,
                 },
                 text: "ENUM_VALUE is not a valid location",
@@ -158,7 +158,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
                 text: "consider adding ENUM_VALUE directive location here",
@@ -175,7 +175,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
             ),
@@ -189,7 +189,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 709,
+            offset: 749,
             length: 11,
         },
         labels: [
@@ -198,7 +198,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 709,
+                    offset: 749,
                     length: 11,
                 },
                 text: "UNION is not a valid location",
@@ -208,7 +208,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
                 text: "consider adding UNION directive location here",
@@ -225,7 +225,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
             ),
@@ -239,8 +239,8 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 425,
-            length: 5,
+            offset: 435,
+            length: 15,
         },
         labels: [
             Label {
@@ -248,8 +248,8 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 425,
-                    length: 5,
+                    offset: 435,
+                    length: 15,
                 },
                 text: "INTERFACE is not a valid location",
             },
@@ -271,8 +271,8 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 639,
-            length: 8,
+            offset: 659,
+            length: 18,
         },
         labels: [
             Label {
@@ -280,8 +280,8 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 639,
-                    length: 8,
+                    offset: 659,
+                    length: 18,
                 },
                 text: "INPUT_OBJECT is not a valid location",
             },
@@ -303,8 +303,8 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 666,
-            length: 8,
+            offset: 696,
+            length: 18,
         },
         labels: [
             Label {
@@ -312,8 +312,8 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 666,
-                    length: 8,
+                    offset: 696,
+                    length: 18,
                 },
                 text: "INPUT_FIELD_DEFINITION is not a valid location",
             },
@@ -335,7 +335,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 492,
+            offset: 512,
             length: 11,
         },
         labels: [
@@ -344,7 +344,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 492,
+                    offset: 512,
                     length: 11,
                 },
                 text: "FIELD_DEFINITION is not a valid location",
@@ -354,7 +354,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
                 text: "consider adding FIELD_DEFINITION directive location here",
@@ -371,7 +371,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
             ),
@@ -385,7 +385,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 856,
+            offset: 896,
             length: 11,
         },
         labels: [
@@ -394,7 +394,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 856,
+                    offset: 896,
                     length: 11,
                 },
                 text: "OBJECT is not a valid location",
@@ -417,7 +417,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 928,
+            offset: 968,
             length: 56,
         },
         labels: [
@@ -426,7 +426,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 928,
+                    offset: 968,
                     length: 56,
                 },
                 text: "ARGUMENT_DEFINITION is not a valid location",
@@ -449,7 +449,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 36,
+            offset: 46,
             length: 15,
         },
         labels: [
@@ -458,7 +458,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 36,
+                    offset: 46,
                     length: 15,
                 },
                 text: "QUERY is not a valid location",
@@ -482,7 +482,7 @@
                 id: 50,
             },
             offset: 29,
-            length: 5,
+            length: 15,
         },
         labels: [
             Label {
@@ -491,7 +491,7 @@
                         id: 50,
                     },
                     offset: 29,
-                    length: 5,
+                    length: 15,
                 },
                 text: "VARIABLE_DEFINITION is not a valid location",
             },
@@ -513,7 +513,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 89,
+            offset: 99,
             length: 11,
         },
         labels: [
@@ -522,7 +522,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 89,
+                    offset: 99,
                     length: 11,
                 },
                 text: "FIELD is not a valid location",
@@ -545,7 +545,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 123,
+            offset: 133,
             length: 11,
         },
         labels: [
@@ -554,7 +554,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 123,
+                    offset: 133,
                     length: 11,
                 },
                 text: "FRAGMENT_SPREAD is not a valid location",
@@ -564,7 +564,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
                 text: "consider adding FRAGMENT_SPREAD directive location here",
@@ -581,7 +581,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
             ),
@@ -595,7 +595,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 258,
+            offset: 268,
             length: 11,
         },
         labels: [
@@ -604,7 +604,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 258,
+                    offset: 268,
                     length: 11,
                 },
                 text: "SUBSCRIPTION is not a valid location",
@@ -614,7 +614,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
                 text: "consider adding SUBSCRIPTION directive location here",
@@ -631,7 +631,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
             ),
@@ -645,7 +645,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 334,
+            offset: 344,
             length: 15,
         },
         labels: [
@@ -654,7 +654,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 334,
+                    offset: 344,
                     length: 15,
                 },
                 text: "MUTATION is not a valid location",
@@ -677,7 +677,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 162,
+            offset: 172,
             length: 11,
         },
         labels: [
@@ -686,7 +686,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 162,
+                    offset: 172,
                     length: 11,
                 },
                 text: "FRAGMENT_DEFINITION is not a valid location",
@@ -696,7 +696,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
                 text: "consider adding FRAGMENT_DEFINITION directive location here",
@@ -713,7 +713,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1337,
+                    offset: 1387,
                     length: 29,
                 },
             ),
@@ -727,7 +727,7 @@
             file_id: FileId {
                 id: 50,
             },
-            offset: 201,
+            offset: 211,
             length: 11,
         },
         labels: [
@@ -736,7 +736,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 201,
+                    offset: 211,
                     length: 11,
                 },
                 text: "INLINE_FRAGMENT is not a valid location",
@@ -746,7 +746,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
                 text: "consider adding INLINE_FRAGMENT directive location here",
@@ -763,7 +763,7 @@
                     file_id: FileId {
                         id: 50,
                     },
-                    offset: 1306,
+                    offset: 1356,
                     length: 30,
                 },
             ),
diff --git a/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.graphql b/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.graphql
new file mode 100644
index 0000000..7a6ac00
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.graphql
@@ -0,0 +1,11 @@
+type Query {
+  field(arg1: Int): Int
+}
+
+fragment X on Query {
+  field(arg1: 1, arg2: 2, arg3: 3)
+}
+
+query {
+  field(arg2: 3)
+}
diff --git a/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.txt b/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.txt
new file mode 100644
index 0000000..a0caa78
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0053_argument_name_is_not_defined.txt
@@ -0,0 +1,116 @@
+[
+    ApolloDiagnostic {
+        cache: {
+            53: "0053_argument_name_is_not_defined.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 53,
+            },
+            offset: 116,
+            length: 7,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 116,
+                    length: 7,
+                },
+                text: "argument by this name not found",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 15,
+                    length: 21,
+                },
+                text: "field declared here",
+            },
+        ],
+        help: None,
+        data: UndefinedArgument {
+            name: "arg2",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            53: "0053_argument_name_is_not_defined.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 53,
+            },
+            offset: 79,
+            length: 7,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 79,
+                    length: 7,
+                },
+                text: "argument by this name not found",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 15,
+                    length: 21,
+                },
+                text: "field declared here",
+            },
+        ],
+        help: None,
+        data: UndefinedArgument {
+            name: "arg2",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            53: "0053_argument_name_is_not_defined.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 53,
+            },
+            offset: 88,
+            length: 7,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 88,
+                    length: 7,
+                },
+                text: "argument by this name not found",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 53,
+                    },
+                    offset: 15,
+                    length: 21,
+                },
+                text: "field declared here",
+            },
+        ],
+        help: None,
+        data: UndefinedArgument {
+            name: "arg3",
+        },
+    },
+]
diff --git a/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.graphql b/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.graphql
new file mode 100644
index 0000000..b0cadff
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.graphql
@@ -0,0 +1,26 @@
+type ComplicatedArgs {
+  multipleReqs(req1: Int!, req2: Int!): String
+  multipleOptAndReq(req1: Int!, req2: Int!, opt1: Int = 0, opt2: Int = 0): String
+}
+
+type Basic { a: Int! }
+
+type Query {
+  basic: Basic
+  complicatedArgs: ComplicatedArgs
+}
+
+fragment Y on ComplicatedArgs {
+    multipleReqs
+    multipleReqs(req2: 1)
+}
+
+{
+  basic @skip @include(wrong: false)
+
+  complicatedArgs {
+    multipleReqs(req2: 1)
+    multipleOptAndReq(req1: 1)
+    multipleOptAndReq(opt1: 1, opt2: 1, req1: 1)
+  }
+}
diff --git a/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.txt b/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.txt
new file mode 100644
index 0000000..0539f8f
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0054_argument_not_provided.txt
@@ -0,0 +1,314 @@
+[
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 333,
+            length: 5,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 333,
+                    length: 5,
+                },
+                text: "missing value for argument `if`",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "if",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 348,
+            length: 12,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 348,
+                    length: 12,
+                },
+                text: "argument by this name not found",
+            },
+        ],
+        help: None,
+        data: UndefinedArgument {
+            name: "wrong",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 339,
+            length: 22,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 339,
+                    length: 22,
+                },
+                text: "missing value for argument `if`",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "if",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 387,
+            length: 21,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 387,
+                    length: 21,
+                },
+                text: "missing value for argument `req1`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 38,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req1",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 413,
+            length: 26,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 413,
+                    length: 26,
+                },
+                text: "missing value for argument `req2`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 102,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req2",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 444,
+            length: 44,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 444,
+                    length: 44,
+                },
+                text: "missing value for argument `req2`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 102,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req2",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 281,
+            length: 12,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 281,
+                    length: 12,
+                },
+                text: "missing value for argument `req1`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 38,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req1",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 281,
+            length: 12,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 281,
+                    length: 12,
+                },
+                text: "missing value for argument `req2`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 50,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req2",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            54: "0054_argument_not_provided.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 54,
+            },
+            offset: 298,
+            length: 21,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 298,
+                    length: 21,
+                },
+                text: "missing value for argument `req1`",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 54,
+                    },
+                    offset: 38,
+                    length: 10,
+                },
+                text: "argument defined here",
+            },
+        ],
+        help: None,
+        data: RequiredArgument {
+            name: "req1",
+        },
+    },
+]
diff --git a/crates/apollo-parser/test_data/parser/err/0024_document_with_incorrect_definition.txt b/crates/apollo-parser/test_data/parser/err/0024_document_with_incorrect_definition.txt
index a55c06d..bce5220 100644
--- a/crates/apollo-parser/test_data/parser/err/0024_document_with_incorrect_definition.txt
+++ b/crates/apollo-parser/test_data/parser/err/0024_document_with_incorrect_definition.txt
@@ -1,3 +1,4 @@
-- DOCUMENT@0..0
+- DOCUMENT@0..21
+    - ERROR@0..21 "awsas8d2934213hkj0987"
 - ERROR@0:21 "expected definition" awsas8d2934213hkj0987
 recursion limit: 4096, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0025_document_with_incorrect_definition_and_selection_set.txt b/crates/apollo-parser/test_data/parser/err/0025_document_with_incorrect_definition_and_selection_set.txt
index 2fc7c67..a7cc237 100644
--- a/crates/apollo-parser/test_data/parser/err/0025_document_with_incorrect_definition_and_selection_set.txt
+++ b/crates/apollo-parser/test_data/parser/err/0025_document_with_incorrect_definition_and_selection_set.txt
@@ -1,17 +1,18 @@
-- DOCUMENT@0..27
-    - WHITESPACE@0..2 "\n\n"
-    - OPERATION_DEFINITION@2..27
-        - SELECTION_SET@2..27
-            - L_CURLY@2..3 "{"
-            - WHITESPACE@3..8 "\n    "
-            - FIELD@8..11
-                - NAME@8..11
-                    - IDENT@8..11 "pet"
-            - WHITESPACE@11..16 "\n    "
-            - FIELD@16..25
-                - NAME@16..25
-                    - IDENT@16..25 "faveSnack"
-            - WHITESPACE@25..26 "\n"
-            - R_CURLY@26..27 "}"
+- DOCUMENT@0..41
+    - ERROR@0..14 "uasdf21230jkdw"
+    - WHITESPACE@14..16 "\n\n"
+    - OPERATION_DEFINITION@16..41
+        - SELECTION_SET@16..41
+            - L_CURLY@16..17 "{"
+            - WHITESPACE@17..22 "\n    "
+            - FIELD@22..25
+                - NAME@22..25
+                    - IDENT@22..25 "pet"
+            - WHITESPACE@25..30 "\n    "
+            - FIELD@30..39
+                - NAME@30..39
+                    - IDENT@30..39 "faveSnack"
+            - WHITESPACE@39..40 "\n"
+            - R_CURLY@40..41 "}"
 - ERROR@0:14 "expected definition" uasdf21230jkdw
 recursion limit: 4096, high: 3
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0026_invalid_definition_squished_between_two_valid_definitions.txt b/crates/apollo-parser/test_data/parser/err/0026_invalid_definition_squished_between_two_valid_definitions.txt
index b03b322..0ac9a05 100644
--- a/crates/apollo-parser/test_data/parser/err/0026_invalid_definition_squished_between_two_valid_definitions.txt
+++ b/crates/apollo-parser/test_data/parser/err/0026_invalid_definition_squished_between_two_valid_definitions.txt
@@ -1,4 +1,4 @@
-- DOCUMENT@0..126
+- DOCUMENT@0..140
     - ENUM_TYPE_DEFINITION@0..97
         - enum_KW@0..4 "enum"
         - WHITESPACE@4..5 " "
@@ -40,19 +40,20 @@
             - WHITESPACE@95..96 "\n"
             - R_CURLY@96..97 "}"
     - WHITESPACE@97..99 "\n\n"
-    - WHITESPACE@99..101 "\n\n"
-    - OPERATION_DEFINITION@101..126
-        - SELECTION_SET@101..126
-            - L_CURLY@101..102 "{"
-            - WHITESPACE@102..107 "\n    "
-            - FIELD@107..110
-                - NAME@107..110
-                    - IDENT@107..110 "pet"
-            - WHITESPACE@110..115 "\n    "
-            - FIELD@115..124
-                - NAME@115..124
-                    - IDENT@115..124 "faveSnack"
-            - WHITESPACE@124..125 "\n"
-            - R_CURLY@125..126 "}"
+    - ERROR@99..113 "uasdf21230jkdw"
+    - WHITESPACE@113..115 "\n\n"
+    - OPERATION_DEFINITION@115..140
+        - SELECTION_SET@115..140
+            - L_CURLY@115..116 "{"
+            - WHITESPACE@116..121 "\n    "
+            - FIELD@121..124
+                - NAME@121..124
+                    - IDENT@121..124 "pet"
+            - WHITESPACE@124..129 "\n    "
+            - FIELD@129..138
+                - NAME@129..138
+                    - IDENT@129..138 "faveSnack"
+            - WHITESPACE@138..139 "\n"
+            - R_CURLY@139..140 "}"
 - ERROR@99:113 "expected definition" uasdf21230jkdw
 recursion limit: 4096, high: 3
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0027_invalid_type_system_extension.txt b/crates/apollo-parser/test_data/parser/err/0027_invalid_type_system_extension.txt
index 2e826ac..b9ec4b4 100644
--- a/crates/apollo-parser/test_data/parser/err/0027_invalid_type_system_extension.txt
+++ b/crates/apollo-parser/test_data/parser/err/0027_invalid_type_system_extension.txt
@@ -1,5 +1,7 @@
-- DOCUMENT@0..1
-    - WHITESPACE@0..1 " "
+- DOCUMENT@0..10
+    - ERROR@0..6 "extend"
+    - WHITESPACE@6..7 " "
+    - ERROR@7..10 "Cat"
 - ERROR@0:6 "Invalid Type System Extension. This extension cannot be applied." extend
 - ERROR@7:10 "expected definition" Cat
 recursion limit: 4096, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0028_invalid_type_system_extension_followed_by_valid.txt b/crates/apollo-parser/test_data/parser/err/0028_invalid_type_system_extension_followed_by_valid.txt
index 715bbb5..d8aa085 100644
--- a/crates/apollo-parser/test_data/parser/err/0028_invalid_type_system_extension_followed_by_valid.txt
+++ b/crates/apollo-parser/test_data/parser/err/0028_invalid_type_system_extension_followed_by_valid.txt
@@ -1,27 +1,29 @@
-- DOCUMENT@0..52
-    - WHITESPACE@0..1 " "
-    - WHITESPACE@1..3 "\n\n"
-    - INTERFACE_TYPE_EXTENSION@3..52
-        - extend_KW@3..9 "extend"
-        - WHITESPACE@9..10 " "
-        - interface_KW@10..19 "interface"
-        - WHITESPACE@19..20 " "
-        - NAME@20..31
-            - IDENT@20..31 "NamedEntity"
-        - WHITESPACE@31..32 " "
-        - FIELDS_DEFINITION@32..52
-            - L_CURLY@32..33 "{"
-            - WHITESPACE@33..38 "\n    "
-            - FIELD_DEFINITION@38..50
-                - NAME@38..42
-                    - IDENT@38..42 "name"
-                - COLON@42..43 ":"
-                - WHITESPACE@43..44 " "
-                - NAMED_TYPE@44..50
-                    - NAME@44..50
-                        - IDENT@44..50 "String"
-            - WHITESPACE@50..51 "\n"
-            - R_CURLY@51..52 "}"
+- DOCUMENT@0..61
+    - ERROR@0..6 "extend"
+    - WHITESPACE@6..7 " "
+    - ERROR@7..10 "Cat"
+    - WHITESPACE@10..12 "\n\n"
+    - INTERFACE_TYPE_EXTENSION@12..61
+        - extend_KW@12..18 "extend"
+        - WHITESPACE@18..19 " "
+        - interface_KW@19..28 "interface"
+        - WHITESPACE@28..29 " "
+        - NAME@29..40
+            - IDENT@29..40 "NamedEntity"
+        - WHITESPACE@40..41 " "
+        - FIELDS_DEFINITION@41..61
+            - L_CURLY@41..42 "{"
+            - WHITESPACE@42..47 "\n    "
+            - FIELD_DEFINITION@47..59
+                - NAME@47..51
+                    - IDENT@47..51 "name"
+                - COLON@51..52 ":"
+                - WHITESPACE@52..53 " "
+                - NAMED_TYPE@53..59
+                    - NAME@53..59
+                        - IDENT@53..59 "String"
+            - WHITESPACE@59..60 "\n"
+            - R_CURLY@60..61 "}"
 - ERROR@0:6 "Invalid Type System Extension. This extension cannot be applied." extend
 - ERROR@7:10 "expected definition" Cat
 recursion limit: 4096, high: 0
\ No newline at end of file
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 9fb8e06..c8c635a 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
@@ -1,15 +1,16 @@
-- DOCUMENT@0..15
-    - WHITESPACE@0..1 "\n"
-    - OPERATION_DEFINITION@1..15
-        - OPERATION_TYPE@1..6
-            - query_KW@1..6 "query"
-        - WHITESPACE@6..7 " "
-        - NAME@7..12
-            - IDENT@7..12 "empty"
-        - WHITESPACE@12..13 " "
-        - SELECTION_SET@13..15
-            - L_CURLY@13..14 "{"
-            - R_CURLY@14..15 "}"
+- DOCUMENT@0..108
+    - ERROR@0..93 "\"after this PR this should not be an issue: https://github.com/graphql/graphql-spec/pull/892\""
+    - WHITESPACE@93..94 "\n"
+    - OPERATION_DEFINITION@94..108
+        - OPERATION_TYPE@94..99
+            - query_KW@94..99 "query"
+        - WHITESPACE@99..100 " "
+        - NAME@100..105
+            - IDENT@100..105 "empty"
+        - WHITESPACE@105..106 " "
+        - SELECTION_SET@106..108
+            - L_CURLY@106..107 "{"
+            - R_CURLY@107..108 "}"
 - 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 "expected at least one Selection in Selection Set" }
 recursion limit: 4096, high: 1
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0040_operation_definition_missing_selection_set.txt b/crates/apollo-parser/test_data/parser/err/0040_operation_definition_missing_selection_set.txt
index 2bf2066..d710be0 100644
--- a/crates/apollo-parser/test_data/parser/err/0040_operation_definition_missing_selection_set.txt
+++ b/crates/apollo-parser/test_data/parser/err/0040_operation_definition_missing_selection_set.txt
@@ -1,10 +1,11 @@
-- DOCUMENT@0..17
-    - OPERATION_DEFINITION@0..17
+- DOCUMENT@0..18
+    - OPERATION_DEFINITION@0..18
         - OPERATION_TYPE@0..5
             - query_KW@0..5 "query"
         - WHITESPACE@5..6 " "
         - NAME@6..16
             - IDENT@6..16 "__typename"
         - WHITESPACE@16..17 " "
+        - ERROR@17..18 "}"
 - ERROR@17:18 "expected a Selection Set" }
 recursion limit: 4096, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0041_operation_definition_with_missing_selection_set.txt b/crates/apollo-parser/test_data/parser/err/0041_operation_definition_with_missing_selection_set.txt
index b145bfb..4913f56 100644
--- a/crates/apollo-parser/test_data/parser/err/0041_operation_definition_with_missing_selection_set.txt
+++ b/crates/apollo-parser/test_data/parser/err/0041_operation_definition_with_missing_selection_set.txt
@@ -1,9 +1,10 @@
-- DOCUMENT@0..16
-    - OPERATION_DEFINITION@0..16
+- DOCUMENT@0..19
+    - OPERATION_DEFINITION@0..19
         - OPERATION_TYPE@0..5
             - query_KW@0..5 "query"
         - WHITESPACE@5..6 " "
         - NAME@6..16
             - IDENT@6..16 "__typename"
+        - ERROR@16..19 "EOF"
 - ERROR@16:16 "expected a Selection Set" EOF
 recursion limit: 4096, high: 0
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0042_document_with_incorrect_token.txt b/crates/apollo-parser/test_data/parser/err/0042_document_with_incorrect_token.txt
index b025b34..99349b9 100644
--- a/crates/apollo-parser/test_data/parser/err/0042_document_with_incorrect_token.txt
+++ b/crates/apollo-parser/test_data/parser/err/0042_document_with_incorrect_token.txt
@@ -1,43 +1,45 @@
-- DOCUMENT@0..81
-    - WHITESPACE@0..1 "\n"
-    - COMMENT@1..10 "# comment"
-    - WHITESPACE@10..11 "\n"
-    - OPERATION_DEFINITION@11..36
-        - SELECTION_SET@11..36
-            - L_CURLY@11..12 "{"
-            - WHITESPACE@12..17 "\n    "
-            - FIELD@17..20
-                - NAME@17..20
-                    - IDENT@17..20 "pet"
-            - WHITESPACE@20..25 "\n    "
-            - FIELD@25..34
-                - NAME@25..34
-                    - IDENT@25..34 "faveSnack"
-            - WHITESPACE@34..35 "\n"
-            - R_CURLY@35..36 "}"
-    - WHITESPACE@36..38 "\n\n"
-    - COMMENT@38..47 "# comment"
-    - WHITESPACE@47..48 "\n"
-    - WHITESPACE@48..50 "\n\n"
-    - OBJECT_TYPE_DEFINITION@50..81
-        - type_KW@50..54 "type"
-        - WHITESPACE@54..55 " "
-        - NAME@55..60
-            - IDENT@55..60 "Query"
-        - WHITESPACE@60..61 " "
-        - FIELDS_DEFINITION@61..81
-            - L_CURLY@61..62 "{"
-            - WHITESPACE@62..67 "\n    "
-            - FIELD_DEFINITION@67..79
-                - NAME@67..71
-                    - IDENT@67..71 "name"
-                - COLON@71..72 ":"
-                - WHITESPACE@72..73 " "
-                - NAMED_TYPE@73..79
-                    - NAME@73..79
-                        - IDENT@73..79 "String"
-            - WHITESPACE@79..80 "\n"
-            - R_CURLY@80..81 "}"
+- DOCUMENT@0..83
+    - ERROR@0..1 "@"
+    - WHITESPACE@1..2 "\n"
+    - COMMENT@2..11 "# comment"
+    - WHITESPACE@11..12 "\n"
+    - OPERATION_DEFINITION@12..37
+        - SELECTION_SET@12..37
+            - L_CURLY@12..13 "{"
+            - WHITESPACE@13..18 "\n    "
+            - FIELD@18..21
+                - NAME@18..21
+                    - IDENT@18..21 "pet"
+            - WHITESPACE@21..26 "\n    "
+            - FIELD@26..35
+                - NAME@26..35
+                    - IDENT@26..35 "faveSnack"
+            - WHITESPACE@35..36 "\n"
+            - R_CURLY@36..37 "}"
+    - WHITESPACE@37..39 "\n\n"
+    - COMMENT@39..48 "# comment"
+    - WHITESPACE@48..49 "\n"
+    - ERROR@49..50 "}"
+    - WHITESPACE@50..52 "\n\n"
+    - OBJECT_TYPE_DEFINITION@52..83
+        - type_KW@52..56 "type"
+        - WHITESPACE@56..57 " "
+        - NAME@57..62
+            - IDENT@57..62 "Query"
+        - WHITESPACE@62..63 " "
+        - FIELDS_DEFINITION@63..83
+            - L_CURLY@63..64 "{"
+            - WHITESPACE@64..69 "\n    "
+            - FIELD_DEFINITION@69..81
+                - NAME@69..73
+                    - IDENT@69..73 "name"
+                - COLON@73..74 ":"
+                - WHITESPACE@74..75 " "
+                - NAMED_TYPE@75..81
+                    - NAME@75..81
+                        - IDENT@75..81 "String"
+            - WHITESPACE@81..82 "\n"
+            - R_CURLY@82..83 "}"
 - ERROR@0:1 "expected a StringValue, Name or OperationDefinition" @
 - ERROR@49:50 "expected a StringValue, Name or OperationDefinition" }
 recursion limit: 4096, high: 3
\ No newline at end of file
diff --git a/crates/apollo-parser/test_data/parser/err/0044_list_type_with_no_type.txt b/crates/apollo-parser/test_data/parser/err/0044_list_type_with_no_type.txt
index 0e307d6..1b5d513 100644
--- a/crates/apollo-parser/test_data/parser/err/0044_list_type_with_no_type.txt
+++ b/crates/apollo-parser/test_data/parser/err/0044_list_type_with_no_type.txt
@@ -1,4 +1,4 @@
-- DOCUMENT@0..65
+- DOCUMENT@0..69
     - OBJECT_TYPE_DEFINITION@0..63
         - type_KW@0..4 "type"
         - WHITESPACE@4..5 " "
@@ -40,8 +40,12 @@
             - FIELD_DEFINITION@59..63
                 - NAME@59..63
                     - IDENT@59..63 "Type"
-    - WHITESPACE@63..64 "\n"
-    - WHITESPACE@64..65 "\n"
+    - ERROR@63..64 "]"
+    - ERROR@64..65 "]"
+    - ERROR@65..66 "]"
+    - WHITESPACE@66..67 "\n"
+    - ERROR@67..68 "}"
+    - WHITESPACE@68..69 "\n"
 - ERROR@22:23 "expected item type" ]
 - ERROR@64:65 "expected a type" ]
 - ERROR@64:65 "expected R_CURLY, got ]" ]
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
new file mode 100644
index 0000000..03ac94f
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.graphql
@@ -0,0 +1,9 @@
+cats
+# https://github.com/apollographql/apollo-rs/issues/325
+
+interface X {}
+
+cats
+type Query {
+    name: String
+}
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
new file mode 100644
index 0000000..e0823e7
--- /dev/null
+++ b/crates/apollo-parser/test_data/parser/err/0045_ignored_token_spans.txt
@@ -0,0 +1,40 @@
+- DOCUMENT@0..115
+    - 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_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"
+- ERROR@0:4 "expected definition" cats
+- ERROR@78:82 "expected definition" cats
+recursion limit: 4096, high: 0
\ No newline at end of file