Repository: apollographql/apollo-rs
Pull request #497 (2023-03-27 16:20:58)

fix(compiler): fix used variables validation for lists and objects
Closes https://github.com/apollographql/apollo-rs/issues/496

Opening this PR to get an early feedback if the general approach is ok.


goto-bus-stop: Thanks for getting started on this 😁 I think we'll need to put a `.variables()` method on the `Value` struct itself. Variables could be used inside lists, but also inside nested lists, and inside input objects. So it needs to recurse to handle deeply nested situations like
```graphql
query ($v: Int) {
  field(opts: {
    prop: [[2], [$v], [4]],
  })
}
```

Patch:
diff --git a/crates/apollo-compiler/CHANGELOG.md b/crates/apollo-compiler/CHANGELOG.md
index f24a2d0..2b1d59c 100644
--- a/crates/apollo-compiler/CHANGELOG.md
+++ b/crates/apollo-compiler/CHANGELOG.md
@@ -18,24 +18,31 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ## Documentation -->
 
-# [0.7.0] (unreleased) - 2023-mm-dd
+# [0.7.0](https://crates.io/crates/apollo-compiler/0.7.0) - 2023-03-27
 
 > Important: X breaking changes below, indicated by **BREAKING**
 
+This release encompasses quite a few validation rules the compiler was missing.
+Here, we primarily focused on field and directive validation, as well as supporting
+multi-file diagnostics.
 ## BREAKING
-
-### Extensions are applied implicitly in HIR, [SimonSapin] in [pull/481], [pull/482], and [pull/484]
+- `find_operation` query now mimics spec's
+[`getOperation`](https://spec.graphql.org/October2021/#GetOperation())
+functionality and returns the anonymous operation if `None` is specified for
+operation name; by [lrlna] in [pull/447]
+- Extensions are applied implicitly in HIR by [SimonSapin] in [pull/481],
+[pull/482], and [pull/484]
 
 Adding a GraphQL type extension is similar to modifying a type.
 This release makes a number of breaking changes to API signatures and behavior
-so these modifications are be accounted for implicitly.
+so these modifications are accounted for implicitly.
 For example, `interface.field(name)` may now return a field
-from an `extend interface` extension as well as from the original `interface` definition.
+from an `extend interface` extension or from the original `interface` definition.
 We expect that most callers don’t need to tell the difference.
 For callers that do, methods with a `self_` prefix are added (or renamed)
 for accessing components of a definition itself as opposed to added by an extension.
 
-Renamed:
+Renamed methods:
 
 * `SchemaDefinition::root_operation_type_definition` → `self_root_operations`
 * `ObjectTypeDefinition::fields_definition` → `self_fields`
@@ -54,7 +61,8 @@ Renamed:
 * `InterfaceTypeDefiniton::directives` → `self_directives`
 * `InputObjectTypeDefiniton::directives` → `self_directives`
 
-Names freed by the above, redefined with new behavior (to consider extensions) and signature:
+Method names freed by the above are now redefined with new behaviour and
+signature, and include extensions:
 
 * `ObjectTypeDefinition::implements_interfaces() -> impl Iterator`
 * `InterfaceTypeDefinition::implements_interfaces() -> impl Iterator`
@@ -67,19 +75,20 @@ Names freed by the above, redefined with new behavior (to consider extensions) a
 * `InterfaceTypeDefiniton::directives() -> impl Iterator`
 * `InputObjectTypeDefiniton::directives() -> impl Iterator`
 
-Behavior and signature change (return the name of an object type instead of its definition):
+Methods whose behaviour and signature changed, where each method now returns the
+name of an object type instead of its definition:
 
 * `SchemaDefinition::query() -> Option<&str>`
 * `SchemaDefinition::mutation() -> Option<&str>`
 * `SchemaDefinition::subscription() -> Option<&str>`
 
-Behavior changed to consider extensions (no signature change):
+Methods whose behaviour changed to consider extensions, and no signature has changed
 
 * `TypeDefinition::field(name) -> Option`
 * `ObjectTypeDefinition::field(name) -> Option`
 * `InterfaceTypeDefinition::field(name) -> Option`
 
-New methods (that consider extensions):
+New methods which take extensions into consideration:
 
 * `SchemaDefinition::root_operations() -> impl Iterator`
 * `ObjectTypeDefinition::fields() -> impl Iterator`
@@ -94,15 +103,73 @@ New methods (that consider extensions):
 * `EnumTypeDefinition::values() -> impl Iterator`
 * `EnumTypeDefinition::value(name) -> Option`
 
-New methods, for every type that has a `directives` method:
+New methods for every type which have a `directives` method:
 
 * `directive_by_name(name) -> Option`
 * `directives_by_name(name) -> impl Iterator`
 
+## Features
+- support mutli-file diagnostics by [goto-bus-stop] in [pull/414]
+- validate directive locations by [lrlna] in [pull/417]
+- validate undefined directives by [lrlna] in [pull/417]
+- validate non-repeatable directives in a given location by [goto-bus-stop] in [pull/488]
+- validate conflicting fields in a selection set (spec: fields can merge) by [goto-bus-stop] in [pull/470]
+- validate introspection fields in subscriptions by [gocamille] in [pull/438]
+- validate required arguments by [goto-bus-stop] in [pull/452]
+- validate unique variables by [lrlna] in [pull/455]
+- validate variables are of input type by [lrlna] in [pull/455]
+- validate root operation type is of Object Type by [lrlna] in [pull/419]
+- validate nested fields in selection sets by [erikwrede] in [pull/441]
+- validate extension existance and kind by [goto-bus-stop] in [pull/458]
+- validate leaf field selection by [yanns] in [pull/465]
+- introduce `op.is_introspection` helper method by [jregistr] in [pull/421]
+- built-in graphql types, including introspection types, are now part of the
+compiler context by [lrlna] in [pull/489]
+
+## Fixes
+- fix variables in directive arguments being reported as unused [goto-bus-stop] in [pull/487]
+- `op.operation_ty()` does not deref [lrlna] in [pull/434]
+
+## Maintenance
+- tests for operation field type resolution v.s. type extensions by [SimonSapin] in [pull/492]
+- using [salsa::invoke] macro to annotate trait function location by [lrlna] in [pull/491]
+- use `Display` for `hir::OperationType` and `hir::DirectiveLocation` by [goto-bus-stop] in [pull/435]
+- rework and simplify validation database by [lrlna] in [pull/436]
+- reset `FileId` between directory tests by [goto-bus-stop] in [pull/437]
+- remove unncessary into_iter() calls by [goto-bus-stop] in [pull/472]
+- check test numbers are unique in test output files by [goto-bus-stop] in [pull/471]
+
 [SimonSapin]: https://github.com/SimonSapin
+[lrlna]: https://github.com/lrlna
+[goto-bus-stop]: https://github.com/goto-bus-stop
+[jregistr]: https://github.com/jregistr
+[gocamille]: https://github.com/gocamille
+[erikwrede]: https://github.com/erikwrede
+[pull/414]: https://github.com/apollographql/apollo-rs/pull/414
+[pull/417]: https://github.com/apollographql/apollo-rs/pull/417
+[pull/419]: https://github.com/apollographql/apollo-rs/pull/419
+[pull/421]: https://github.com/apollographql/apollo-rs/pull/421
+[pull/434]: https://github.com/apollographql/apollo-rs/pull/434
+[pull/435]: https://github.com/apollographql/apollo-rs/pull/435
+[pull/436]: https://github.com/apollographql/apollo-rs/pull/436
+[pull/437]: https://github.com/apollographql/apollo-rs/pull/437
+[pull/441]: https://github.com/apollographql/apollo-rs/pull/441
+[pull/447]: https://github.com/apollographql/apollo-rs/pull/447
+[pull/452]: https://github.com/apollographql/apollo-rs/pull/452
+[pull/455]: https://github.com/apollographql/apollo-rs/pull/455
+[pull/458]: https://github.com/apollographql/apollo-rs/pull/458
+[pull/465]: https://github.com/apollographql/apollo-rs/pull/465
+[pull/470]: https://github.com/apollographql/apollo-rs/pull/470
+[pull/471]: https://github.com/apollographql/apollo-rs/pull/471
+[pull/472]: https://github.com/apollographql/apollo-rs/pull/472
 [pull/481]: https://github.com/apollographql/apollo-rs/pull/481
 [pull/482]: https://github.com/apollographql/apollo-rs/pull/482
 [pull/484]: https://github.com/apollographql/apollo-rs/pull/484
+[pull/487]: https://github.com/apollographql/apollo-rs/pull/487
+[pull/488]: https://github.com/apollographql/apollo-rs/pull/488
+[pull/489]: https://github.com/apollographql/apollo-rs/pull/489
+[pull/491]: https://github.com/apollographql/apollo-rs/pull/491
+[pull/492]: https://github.com/apollographql/apollo-rs/pull/492
 
 # [0.6.0](https://crates.io/crates/apollo-compiler/0.6.0) - 2023-01-18
 
diff --git a/crates/apollo-compiler/Cargo.toml b/crates/apollo-compiler/Cargo.toml
index 6704b49..b8a1bd1 100644
--- a/crates/apollo-compiler/Cargo.toml
+++ b/crates/apollo-compiler/Cargo.toml
@@ -1,6 +1,6 @@
 [package]
 name = "apollo-compiler"
-version = "0.6.0"
+version = "0.7.0"
 authors = ["Irina Shestak <shestak.irina@gmail.com>"]
 license = "MIT OR Apache-2.0"
 repository = "https://github.com/apollographql/apollo-rs"
diff --git a/crates/apollo-compiler/README.md b/crates/apollo-compiler/README.md
index 8439d65..cb621df 100644
--- a/crates/apollo-compiler/README.md
+++ b/crates/apollo-compiler/README.md
@@ -27,7 +27,7 @@ Add this to your `Cargo.toml` to start using `apollo-compiler`:
 ```toml
 # Just an example, change to the necessary package version.
 [dependencies]
-apollo-compiler = "0.6.0"
+apollo-compiler = "0.7.0"
 ```
 
 Or using [cargo-edit]:
diff --git a/crates/apollo-compiler/src/database/hir.rs b/crates/apollo-compiler/src/database/hir.rs
index be43c8d..91bc38c 100644
--- a/crates/apollo-compiler/src/database/hir.rs
+++ b/crates/apollo-compiler/src/database/hir.rs
@@ -197,6 +197,7 @@ impl TypeDefinition {
             Self::ScalarTypeDefinition(def) => def.loc(),
         }
     }
+
     /// Returns `true` if the type definition is [`ScalarTypeDefinition`].
     ///
     /// [`ScalarTypeDefinition`]: TypeDefinition::ScalarTypeDefinition
@@ -844,6 +845,19 @@ impl DirectiveDefinition {
         self.loc
     }
 
+    /// Get the location information for the "head" of the directive definition, namely the
+    /// `directive` keyword and the name.
+    pub(crate) fn head_loc(&self) -> HirNodeLocation {
+        self.name_src()
+            .loc()
+            .map(|name_loc| HirNodeLocation {
+                // Adjust the node length to include the name
+                node_len: name_loc.end_offset() - self.loc.offset(),
+                ..self.loc
+            })
+            .unwrap_or(self.loc)
+    }
+
     /// Checks if current directive is one of built-in directives - `@skip`,
     /// `@include`, `@deprecated`, `@specifiedBy`.
     pub fn is_built_in(&self) -> bool {
@@ -1035,6 +1049,15 @@ impl Value {
             _ => false,
         }
     }
+
+    pub fn variables(&self) -> Vec<Variable> {
+        match self {
+            Value::Variable(var) => vec![var.clone()],
+            Value::List(values) => values.iter().flat_map(|v| v.variables()).collect(),
+            Value::Object(obj) => obj.iter().flat_map(|o| o.1.variables()).collect(),
+            _ => Vec::new(),
+        }
+    }
 }
 
 /// Coerce to a `Float` input type (from either `Float` or `Int` syntax)
@@ -1430,10 +1453,7 @@ impl Field {
                     .iter()
                     .flat_map(|directive| directive.arguments()),
             )
-            .filter_map(|arg| match arg.value() {
-                Value::Variable(var) => Some(var.clone()),
-                _ => None,
-            })
+            .flat_map(|arg| arg.value().variables())
     }
 
     /// Get variables used in the field, including in sub-selections.
@@ -2201,7 +2221,7 @@ impl InputValueDefinition {
         self.description.as_deref()
     }
 
-    /// Get a reference to input value definition's directives.
+    /// Return the directives used on this input value definition.
     pub fn directives(&self) -> &[Directive] {
         self.directives.as_ref()
     }
@@ -3350,6 +3370,11 @@ impl HirNodeLocation {
         self.offset
     }
 
+    /// Get the source offset of the end of the current node.
+    pub fn end_offset(&self) -> usize {
+        self.offset + self.node_len
+    }
+
     /// Get node length.
     pub fn node_len(&self) -> usize {
         self.node_len
diff --git a/crates/apollo-compiler/src/validation/directive.rs b/crates/apollo-compiler/src/validation/directive.rs
index 7105500..874d70c 100644
--- a/crates/apollo-compiler/src/validation/directive.rs
+++ b/crates/apollo-compiler/src/validation/directive.rs
@@ -7,6 +7,130 @@ use crate::{
     ValidationDatabase,
 };
 
+/// Track used names in a recursive function.
+///
+/// Pass the result of `stack.push(name)` to recursive calls. Use `stack.contains(name)` to check
+/// if the name was used somewhere up the call stack.
+struct RecursionStack<'a>(&'a mut Vec<String>);
+impl RecursionStack<'_> {
+    fn push(&mut self, name: String) -> RecursionStack<'_> {
+        self.0.push(name);
+        RecursionStack(self.0)
+    }
+    fn contains(&self, name: &str) -> bool {
+        self.0.iter().any(|seen| seen == name)
+    }
+    fn first(&self) -> Option<&str> {
+        self.0.get(0).map(|s| s.as_str())
+    }
+}
+impl Drop for RecursionStack<'_> {
+    fn drop(&mut self) {
+        self.0.pop();
+    }
+}
+
+/// This struct just groups functions that are used to find self-referential directives.
+/// The way to use it is to call `FindRecursiveDirective::check`.
+struct FindRecursiveDirective<'a> {
+    db: &'a dyn ValidationDatabase,
+}
+impl FindRecursiveDirective<'_> {
+    fn type_definition(
+        &self,
+        seen: &mut RecursionStack<'_>,
+        def: &hir::TypeDefinition,
+    ) -> Result<(), hir::Directive> {
+        for directive in def.directives() {
+            self.directive(seen, directive)?;
+        }
+
+        match def {
+            hir::TypeDefinition::InputObjectTypeDefinition(input_type_definition) => {
+                for input_value in input_type_definition.fields() {
+                    self.input_value(seen, input_value)?;
+                }
+            }
+            hir::TypeDefinition::EnumTypeDefinition(enum_type_definition) => {
+                for enum_value in enum_type_definition.values() {
+                    self.enum_value(seen, enum_value)?;
+                }
+            }
+            _ => (),
+        }
+
+        Ok(())
+    }
+
+    fn input_value(
+        &self,
+        seen: &mut RecursionStack<'_>,
+        input_value: &hir::InputValueDefinition,
+    ) -> Result<(), hir::Directive> {
+        for directive in input_value.directives() {
+            self.directive(seen, directive)?;
+        }
+        if let Some(type_def) = input_value.ty().type_def(self.db.upcast()) {
+            self.type_definition(seen, &type_def)?;
+        }
+
+        Ok(())
+    }
+
+    fn enum_value(
+        &self,
+        seen: &mut RecursionStack<'_>,
+        enum_value: &hir::EnumValueDefinition,
+    ) -> Result<(), hir::Directive> {
+        for directive in enum_value.directives() {
+            self.directive(seen, directive)?;
+        }
+
+        Ok(())
+    }
+
+    fn directive(
+        &self,
+        seen: &mut RecursionStack<'_>,
+        directive: &hir::Directive,
+    ) -> Result<(), hir::Directive> {
+        if !seen.contains(directive.name()) {
+            if let Some(def) = directive.directive(self.db.upcast()) {
+                self.directive_definition(seen.push(directive.name().to_string()), &def)?;
+            }
+        } else if seen.first() == Some(directive.name()) {
+            // Only report an error & bail out early if this is the *initial* directive.
+            // This prevents raising confusing errors when a directive `@b` which is not
+            // self-referential uses a directive `@a` that *is*. The error with `@a` should
+            // only be reported on its definition, not on `@b`'s.
+            return Err(directive.clone());
+        }
+
+        Ok(())
+    }
+
+    fn directive_definition(
+        &self,
+        mut seen: RecursionStack<'_>,
+        def: &hir::DirectiveDefinition,
+    ) -> Result<(), hir::Directive> {
+        let mut guard = seen.push(def.name().to_string());
+        for input_value in def.arguments().input_values() {
+            self.input_value(&mut guard, input_value)?;
+        }
+
+        Ok(())
+    }
+
+    fn check(
+        db: &dyn ValidationDatabase,
+        directive_def: &hir::DirectiveDefinition,
+    ) -> Result<(), hir::Directive> {
+        FindRecursiveDirective { db }
+            .directive_definition(RecursionStack(&mut vec![]), directive_def)
+    }
+}
+
 pub fn validate_directive_definitions(db: &dyn ValidationDatabase) -> Vec<ApolloDiagnostic> {
     let mut diagnostics = Vec::new();
 
@@ -15,23 +139,19 @@ pub fn validate_directive_definitions(db: &dyn ValidationDatabase) -> Vec<Apollo
     //
     // Returns Recursive Definition error.
     for (name, directive_def) in db.directive_definitions().iter() {
-        for input_values in directive_def.arguments().input_values() {
-            for directive in input_values.directives().iter() {
-                let directive_name = directive.name();
-                if name == directive_name {
-                    diagnostics.push(
-                        ApolloDiagnostic::new(
-                            db,
-                            directive.loc().into(),
-                            DiagnosticData::RecursiveDefinition { name: name.clone() },
-                        )
-                        .label(Label::new(
-                            directive.loc(),
-                            "recursive directive definition",
-                        )),
-                    );
-                }
-            }
+        if let Err(directive) = FindRecursiveDirective::check(db, directive_def) {
+            diagnostics.push(
+                ApolloDiagnostic::new(
+                    db,
+                    directive_def.loc().into(),
+                    DiagnosticData::RecursiveDefinition { name: name.clone() },
+                )
+                .label(Label::new(
+                    directive_def.head_loc(),
+                    "recursive directive definition",
+                ))
+                .label(Label::new(directive.loc(), "refers to itself here")),
+            );
         }
 
         // Validate directive definitions' arguments


Test patch:
diff --git a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
index 395c78e..3abb5b0 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
+++ b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.graphql
@@ -3,4 +3,21 @@ type Query {
   field(arg: Int @invalidExample): String
 }
 
-directive @invalidExample(arg: String @invalidExample) on ARGUMENT_DEFINITION
\ No newline at end of file
+# Simple, direct self-reference
+directive @invalidExample(arg: String @invalidExample) on ARGUMENT_DEFINITION
+
+# Some more nested self-references
+extend scalar String @deprecatedType(reason: "use OurCustomString instead")
+directive @deprecatedType(reason: String!) on OBJECT | INTERFACE | ENUM | SCALAR | UNION
+
+directive @loopA(arg: Boolean @loopB) on ARGUMENT_DEFINITION
+directive @loopB(arg: Boolean @loopC) on ARGUMENT_DEFINITION
+directive @loopC(arg: Boolean @loopA) on ARGUMENT_DEFINITION
+
+enum Enum { NON_RECURSIVE }
+extend enum Enum { RECURSIVE @wrong }
+input Name { value: String!, type: Enum }
+input InputObject { name: Name }
+directive @wrong(input: InputObject) on INPUT_FIELD_DEFINITION
+
+directive @thisOneShouldNotError(arg: Boolean @loopC) on FIELD
diff --git a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
index 038c8d2..8475d5e 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0070_self_referential_directive_definition.txt
@@ -8,8 +8,8 @@
             file_id: FileId {
                 id: 70,
             },
-            offset: 106,
-            length: 15,
+            offset: 100,
+            length: 77,
         },
         labels: [
             Label {
@@ -17,15 +17,220 @@
                     file_id: FileId {
                         id: 70,
                     },
-                    offset: 106,
-                    length: 15,
+                    offset: 100,
+                    length: 25,
                 },
                 text: "recursive directive definition",
             },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 138,
+                    length: 15,
+                },
+                text: "refers to itself here",
+            },
         ],
         help: None,
         data: RecursiveDefinition {
             name: "invalidExample",
         },
     },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            70: "0070_self_referential_directive_definition.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 70,
+            },
+            offset: 290,
+            length: 88,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 290,
+                    length: 25,
+                },
+                text: "recursive directive definition",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 235,
+                    length: 54,
+                },
+                text: "refers to itself here",
+            },
+        ],
+        help: None,
+        data: RecursiveDefinition {
+            name: "deprecatedType",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            70: "0070_self_referential_directive_definition.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 70,
+            },
+            offset: 380,
+            length: 60,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 380,
+                    length: 16,
+                },
+                text: "recursive directive definition",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 532,
+                    length: 6,
+                },
+                text: "refers to itself here",
+            },
+        ],
+        help: None,
+        data: RecursiveDefinition {
+            name: "loopA",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            70: "0070_self_referential_directive_definition.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 70,
+            },
+            offset: 441,
+            length: 60,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 441,
+                    length: 16,
+                },
+                text: "recursive directive definition",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 410,
+                    length: 6,
+                },
+                text: "refers to itself here",
+            },
+        ],
+        help: None,
+        data: RecursiveDefinition {
+            name: "loopB",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            70: "0070_self_referential_directive_definition.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 70,
+            },
+            offset: 502,
+            length: 60,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 502,
+                    length: 16,
+                },
+                text: "recursive directive definition",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 471,
+                    length: 6,
+                },
+                text: "refers to itself here",
+            },
+        ],
+        help: None,
+        data: RecursiveDefinition {
+            name: "loopC",
+        },
+    },
+    ApolloDiagnostic {
+        cache: {
+            0: "built_in_types.graphql",
+            70: "0070_self_referential_directive_definition.graphql",
+        },
+        location: DiagnosticLocation {
+            file_id: FileId {
+                id: 70,
+            },
+            offset: 705,
+            length: 62,
+        },
+        labels: [
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 705,
+                    length: 16,
+                },
+                text: "recursive directive definition",
+            },
+            Label {
+                location: DiagnosticLocation {
+                    file_id: FileId {
+                        id: 70,
+                    },
+                    offset: 621,
+                    length: 6,
+                },
+                text: "refers to itself here",
+            },
+        ],
+        help: None,
+        data: RecursiveDefinition {
+            name: "wrong",
+        },
+    },
 ]
diff --git a/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.graphql b/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.graphql
new file mode 100644
index 0000000..4f2e44a
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.graphql
@@ -0,0 +1,21 @@
+type Product {
+  attributes(includeNames: [String!]): [String!]
+}
+
+input Opts {
+  prop: [[Int]!]!
+}
+
+type Query {
+  field(
+    opts: Opts
+  ): [Product]
+}
+
+query ($attributeName: String!, $v: Int) {
+  field(opts: {
+    prop: [[2], [$v], [4]],
+  }) {
+    attributes(includeNames: [$attributeName])
+  }
+}
diff --git a/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.txt b/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.txt
new file mode 100644
index 0000000..5b8f631
--- /dev/null
+++ b/crates/apollo-compiler/test_data/ok/0029_used_variable_in_list_and_input.txt
@@ -0,0 +1,217 @@
+- DOCUMENT@0..303
+    - OBJECT_TYPE_DEFINITION@0..65
+        - type_KW@0..4 "type"
+        - WHITESPACE@4..5 " "
+        - NAME@5..12
+            - IDENT@5..12 "Product"
+        - WHITESPACE@12..13 " "
+        - FIELDS_DEFINITION@13..65
+            - L_CURLY@13..14 "{"
+            - WHITESPACE@14..17 "\n  "
+            - FIELD_DEFINITION@17..63
+                - NAME@17..27
+                    - IDENT@17..27 "attributes"
+                - ARGUMENTS_DEFINITION@27..52
+                    - L_PAREN@27..28 "("
+                    - INPUT_VALUE_DEFINITION@28..51
+                        - NAME@28..40
+                            - IDENT@28..40 "includeNames"
+                        - COLON@40..41 ":"
+                        - WHITESPACE@41..42 " "
+                        - LIST_TYPE@42..51
+                            - L_BRACK@42..43 "["
+                            - NON_NULL_TYPE@43..50
+                                - NAMED_TYPE@43..49
+                                    - NAME@43..49
+                                        - IDENT@43..49 "String"
+                                - BANG@49..50 "!"
+                            - R_BRACK@50..51 "]"
+                    - R_PAREN@51..52 ")"
+                - COLON@52..53 ":"
+                - WHITESPACE@53..54 " "
+                - LIST_TYPE@54..63
+                    - L_BRACK@54..55 "["
+                    - NON_NULL_TYPE@55..62
+                        - NAMED_TYPE@55..61
+                            - NAME@55..61
+                                - IDENT@55..61 "String"
+                        - BANG@61..62 "!"
+                    - R_BRACK@62..63 "]"
+            - WHITESPACE@63..64 "\n"
+            - R_CURLY@64..65 "}"
+    - WHITESPACE@65..67 "\n\n"
+    - INPUT_OBJECT_TYPE_DEFINITION@67..99
+        - input_KW@67..72 "input"
+        - WHITESPACE@72..73 " "
+        - NAME@73..77
+            - IDENT@73..77 "Opts"
+        - WHITESPACE@77..78 " "
+        - INPUT_FIELDS_DEFINITION@78..99
+            - L_CURLY@78..79 "{"
+            - WHITESPACE@79..82 "\n  "
+            - INPUT_VALUE_DEFINITION@82..97
+                - NAME@82..86
+                    - IDENT@82..86 "prop"
+                - COLON@86..87 ":"
+                - WHITESPACE@87..88 " "
+                - NON_NULL_TYPE@88..97
+                    - LIST_TYPE@88..96
+                        - L_BRACK@88..89 "["
+                        - NON_NULL_TYPE@89..95
+                            - LIST_TYPE@89..94
+                                - L_BRACK@89..90 "["
+                                - NAMED_TYPE@90..93
+                                    - NAME@90..93
+                                        - IDENT@90..93 "Int"
+                                - R_BRACK@93..94 "]"
+                            - BANG@94..95 "!"
+                        - R_BRACK@95..96 "]"
+                    - BANG@96..97 "!"
+            - WHITESPACE@97..98 "\n"
+            - R_CURLY@98..99 "}"
+    - WHITESPACE@99..101 "\n\n"
+    - OBJECT_TYPE_DEFINITION@101..154
+        - type_KW@101..105 "type"
+        - WHITESPACE@105..106 " "
+        - NAME@106..111
+            - IDENT@106..111 "Query"
+        - WHITESPACE@111..112 " "
+        - FIELDS_DEFINITION@112..154
+            - L_CURLY@112..113 "{"
+            - WHITESPACE@113..116 "\n  "
+            - FIELD_DEFINITION@116..152
+                - NAME@116..121
+                    - IDENT@116..121 "field"
+                - ARGUMENTS_DEFINITION@121..141
+                    - L_PAREN@121..122 "("
+                    - WHITESPACE@122..127 "\n    "
+                    - INPUT_VALUE_DEFINITION@127..137
+                        - NAME@127..131
+                            - IDENT@127..131 "opts"
+                        - COLON@131..132 ":"
+                        - WHITESPACE@132..133 " "
+                        - NAMED_TYPE@133..137
+                            - NAME@133..137
+                                - IDENT@133..137 "Opts"
+                    - WHITESPACE@137..140 "\n  "
+                    - R_PAREN@140..141 ")"
+                - COLON@141..142 ":"
+                - WHITESPACE@142..143 " "
+                - LIST_TYPE@143..152
+                    - L_BRACK@143..144 "["
+                    - NAMED_TYPE@144..151
+                        - NAME@144..151
+                            - IDENT@144..151 "Product"
+                    - R_BRACK@151..152 "]"
+            - WHITESPACE@152..153 "\n"
+            - R_CURLY@153..154 "}"
+    - WHITESPACE@154..156 "\n\n"
+    - OPERATION_DEFINITION@156..302
+        - OPERATION_TYPE@156..161
+            - query_KW@156..161 "query"
+        - WHITESPACE@161..162 " "
+        - VARIABLE_DEFINITIONS@162..196
+            - L_PAREN@162..163 "("
+            - VARIABLE_DEFINITION@163..186
+                - VARIABLE@163..177
+                    - DOLLAR@163..164 "$"
+                    - NAME@164..177
+                        - IDENT@164..177 "attributeName"
+                - COLON@177..178 ":"
+                - WHITESPACE@178..179 " "
+                - NON_NULL_TYPE@179..186
+                    - NAMED_TYPE@179..185
+                        - NAME@179..185
+                            - IDENT@179..185 "String"
+                    - BANG@185..186 "!"
+            - COMMA@186..187 ","
+            - WHITESPACE@187..188 " "
+            - VARIABLE_DEFINITION@188..195
+                - VARIABLE@188..190
+                    - DOLLAR@188..189 "$"
+                    - NAME@189..190
+                        - IDENT@189..190 "v"
+                - COLON@190..191 ":"
+                - WHITESPACE@191..192 " "
+                - NAMED_TYPE@192..195
+                    - NAME@192..195
+                        - IDENT@192..195 "Int"
+            - R_PAREN@195..196 ")"
+        - WHITESPACE@196..197 " "
+        - SELECTION_SET@197..302
+            - L_CURLY@197..198 "{"
+            - WHITESPACE@198..201 "\n  "
+            - FIELD@201..300
+                - NAME@201..206
+                    - IDENT@201..206 "field"
+                - ARGUMENTS@206..247
+                    - L_PAREN@206..207 "("
+                    - ARGUMENT@207..246
+                        - NAME@207..211
+                            - IDENT@207..211 "opts"
+                        - COLON@211..212 ":"
+                        - WHITESPACE@212..213 " "
+                        - OBJECT_VALUE@213..246
+                            - L_CURLY@213..214 "{"
+                            - WHITESPACE@214..219 "\n    "
+                            - OBJECT_FIELD@219..241
+                                - NAME@219..223
+                                    - IDENT@219..223 "prop"
+                                - COLON@223..224 ":"
+                                - WHITESPACE@224..225 " "
+                                - LIST_VALUE@225..241
+                                    - L_BRACK@225..226 "["
+                                    - LIST_VALUE@226..229
+                                        - L_BRACK@226..227 "["
+                                        - INT_VALUE@227..228
+                                            - INT@227..228 "2"
+                                        - R_BRACK@228..229 "]"
+                                    - COMMA@229..230 ","
+                                    - WHITESPACE@230..231 " "
+                                    - LIST_VALUE@231..235
+                                        - L_BRACK@231..232 "["
+                                        - VARIABLE@232..234
+                                            - DOLLAR@232..233 "$"
+                                            - NAME@233..234
+                                                - IDENT@233..234 "v"
+                                        - R_BRACK@234..235 "]"
+                                    - COMMA@235..236 ","
+                                    - WHITESPACE@236..237 " "
+                                    - LIST_VALUE@237..240
+                                        - L_BRACK@237..238 "["
+                                        - INT_VALUE@238..239
+                                            - INT@238..239 "4"
+                                        - R_BRACK@239..240 "]"
+                                    - R_BRACK@240..241 "]"
+                            - COMMA@241..242 ","
+                            - WHITESPACE@242..245 "\n  "
+                            - R_CURLY@245..246 "}"
+                    - R_PAREN@246..247 ")"
+                - WHITESPACE@247..248 " "
+                - SELECTION_SET@248..300
+                    - L_CURLY@248..249 "{"
+                    - WHITESPACE@249..254 "\n    "
+                    - FIELD@254..296
+                        - NAME@254..264
+                            - IDENT@254..264 "attributes"
+                        - ARGUMENTS@264..296
+                            - L_PAREN@264..265 "("
+                            - ARGUMENT@265..295
+                                - NAME@265..277
+                                    - IDENT@265..277 "includeNames"
+                                - COLON@277..278 ":"
+                                - WHITESPACE@278..279 " "
+                                - LIST_VALUE@279..295
+                                    - L_BRACK@279..280 "["
+                                    - VARIABLE@280..294
+                                        - DOLLAR@280..281 "$"
+                                        - NAME@281..294
+                                            - IDENT@281..294 "attributeName"
+                                    - R_BRACK@294..295 "]"
+                            - R_PAREN@295..296 ")"
+                    - WHITESPACE@296..299 "\n  "
+                    - R_CURLY@299..300 "}"
+            - WHITESPACE@300..301 "\n"
+            - R_CURLY@301..302 "}"
+    - WHITESPACE@302..303 "\n"
+recursion limit: 4096, high: 4