Skip to content

Commit

Permalink
feat(compiler): validate circular input objects (#505)
Browse files Browse the repository at this point in the history
  • Loading branch information
lrlna authored Mar 31, 2023
1 parent 2a32af7 commit ca4cd8b
Show file tree
Hide file tree
Showing 9 changed files with 479 additions and 36 deletions.
6 changes: 4 additions & 2 deletions crates/apollo-compiler/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,10 +195,12 @@ pub enum DiagnosticData {
/// Location of the extension
extension: DiagnosticLocation,
},
#[error("{name} directive definition cannot reference itself")]
RecursiveDefinition { name: String },
#[error("`{name}` directive definition cannot reference itself")]
RecursiveDirectiveDefinition { name: String },
#[error("interface {name} cannot implement itself")]
RecursiveInterfaceDefinition { name: String },
#[error("`{name}` input object cannot reference itself")]
RecursiveInputObjectDefinition { name: String },
#[error("values in an Enum Definition should be capitalized")]
CapitalizedValue { value: String },
#[error("fields must be unique in a definition")]
Expand Down
28 changes: 3 additions & 25 deletions crates/apollo-compiler/src/validation/directive.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,16 @@ use std::collections::HashSet;
use crate::{
diagnostics::{ApolloDiagnostic, DiagnosticData, Label},
hir,
validation::ValidationSet,
validation::{RecursionStack, ValidationSet},
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,
Expand Down Expand Up @@ -144,7 +122,7 @@ pub fn validate_directive_definitions(db: &dyn ValidationDatabase) -> Vec<Apollo
ApolloDiagnostic::new(
db,
directive_def.loc().into(),
DiagnosticData::RecursiveDefinition { name: name.clone() },
DiagnosticData::RecursiveDirectiveDefinition { name: name.clone() },
)
.label(Label::new(
directive_def.head_loc(),
Expand Down
87 changes: 84 additions & 3 deletions crates/apollo-compiler/src/validation/input_object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,70 @@ use std::{collections::HashMap, sync::Arc};

use crate::{
diagnostics::{ApolloDiagnostic, DiagnosticData, Label},
hir, ValidationDatabase,
hir,
validation::RecursionStack,
ValidationDatabase,
};

// Implements [Circular References](https://spec.graphql.org/October2021/#sec-Input-Objects.Circular-References)
// part of the input object validation spec.
struct FindRecursiveInputValue<'a> {
db: &'a dyn ValidationDatabase,
}

impl FindRecursiveInputValue<'_> {
fn input_value_definition(
&self,
seen: &mut RecursionStack<'_>,
def: &hir::InputValueDefinition,
) -> Result<(), hir::InputValueDefinition> {
let ty = def.ty();
return match ty {
hir::Type::NonNull { ty, loc: _ } => match ty.as_ref() {
// NonNull type followed by Named type is the one that's not allowed
// to be cyclical, so this is only case we care about.
//
// Everything else may be a cyclical input value.
hir::Type::Named { name, loc: _ } => {
if !seen.contains(name) {
if let Some(def) = self.db.find_input_object_by_name(name.into()) {
self.input_object_definition(seen.push(name.into()), def.as_ref())?
}
} else if seen.first() == Some(name) {
return Err(def.clone());
}

Ok(())
}
hir::Type::NonNull { .. } | hir::Type::List { .. } => Ok(()),
},
hir::Type::List { .. } => Ok(()),
hir::Type::Named { .. } => Ok(()),
};
}

fn input_object_definition(
&self,
mut seen: RecursionStack<'_>,
def: &hir::InputObjectTypeDefinition,
) -> Result<(), hir::InputValueDefinition> {
let mut guard = seen.push(def.name().to_string());
for input_value in def.fields() {
self.input_value_definition(&mut guard, input_value)?;
}

Ok(())
}

fn check(
db: &dyn ValidationDatabase,
input_obj: &hir::InputObjectTypeDefinition,
) -> Result<(), hir::InputValueDefinition> {
FindRecursiveInputValue { db }
.input_object_definition(RecursionStack(&mut vec![]), input_obj)
}
}

pub fn validate_input_object_definitions(db: &dyn ValidationDatabase) -> Vec<ApolloDiagnostic> {
let mut diagnostics = Vec::new();

Expand All @@ -25,6 +86,26 @@ pub fn validate_input_object_definition(
hir::DirectiveLocation::InputObject,
);

if let Err(input_val) = FindRecursiveInputValue::check(db, input_obj.as_ref()) {
let mut labels = vec![Label::new(
input_obj.loc(),
"cyclical input object definition",
)];
if let Some(loc) = input_val.loc() {
labels.push(Label::new(loc, "refers to itself here"));
};
diagnostics.push(
ApolloDiagnostic::new(
db,
input_obj.loc().into(),
DiagnosticData::RecursiveInputObjectDefinition {
name: input_obj.name().into(),
},
)
.labels(labels),
)
}

// Fields in an Input Object Definition must be unique
//
// Returns Unique Definition error.
Expand All @@ -49,9 +130,9 @@ pub fn validate_input_values(
diagnostics.extend(db.validate_directives(input_value.directives().to_vec(), dir_loc));

let name = input_value.name();
if let Some(prev_arg) = seen.get(name) {
if let Some(prev_value) = seen.get(name) {
if let (Some(original_value), Some(redefined_value)) =
(prev_arg.loc(), input_value.loc())
(prev_value.loc(), input_value.loc())
{
diagnostics.push(
ApolloDiagnostic::new(
Expand Down
23 changes: 23 additions & 0 deletions crates/apollo-compiler/src/validation/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,26 @@ impl PartialEq for ValidationSet {
self.name == other.name
}
}

/// 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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "invalidExample",
},
},
Expand Down Expand Up @@ -73,7 +73,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "deprecatedType",
},
},
Expand Down Expand Up @@ -112,7 +112,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "loopA",
},
},
Expand Down Expand Up @@ -151,7 +151,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "loopB",
},
},
Expand Down Expand Up @@ -190,7 +190,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "loopC",
},
},
Expand Down Expand Up @@ -229,7 +229,7 @@
},
],
help: None,
data: RecursiveDefinition {
data: RecursiveDirectiveDefinition {
name: "wrong",
},
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
type Query {
name: String
example(arg: First): Int
}

input First {
second: Second!
value: String
}

input Second {
third: Third!
value: String
}

input Third {
fourth: Fourth!
value: String
}

input Fourth {
first: First!
}
Loading

0 comments on commit ca4cd8b

Please sign in to comment.