diff --git a/crates/apollo-compiler/src/diagnostics.rs b/crates/apollo-compiler/src/diagnostics.rs index cd36c78b8..f3cf3715f 100644 --- a/crates/apollo-compiler/src/diagnostics.rs +++ b/crates/apollo-compiler/src/diagnostics.rs @@ -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")] diff --git a/crates/apollo-compiler/src/validation/directive.rs b/crates/apollo-compiler/src/validation/directive.rs index 874d70cbf..8969bcafa 100644 --- a/crates/apollo-compiler/src/validation/directive.rs +++ b/crates/apollo-compiler/src/validation/directive.rs @@ -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); -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, @@ -144,7 +122,7 @@ pub fn validate_directive_definitions(db: &dyn ValidationDatabase) -> Vec { + 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 { let mut diagnostics = Vec::new(); @@ -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. @@ -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( diff --git a/crates/apollo-compiler/src/validation/mod.rs b/crates/apollo-compiler/src/validation/mod.rs index e7a453ea1..a3897202b 100644 --- a/crates/apollo-compiler/src/validation/mod.rs +++ b/crates/apollo-compiler/src/validation/mod.rs @@ -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); +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(); + } +} 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 8475d5e07..a334058ca 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 @@ -34,7 +34,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "invalidExample", }, }, @@ -73,7 +73,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "deprecatedType", }, }, @@ -112,7 +112,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "loopA", }, }, @@ -151,7 +151,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "loopB", }, }, @@ -190,7 +190,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "loopC", }, }, @@ -229,7 +229,7 @@ }, ], help: None, - data: RecursiveDefinition { + data: RecursiveDirectiveDefinition { name: "wrong", }, }, diff --git a/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.graphql b/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.graphql new file mode 100644 index 000000000..65d651046 --- /dev/null +++ b/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.graphql @@ -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! +} \ No newline at end of file diff --git a/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.txt b/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.txt new file mode 100644 index 000000000..a65d5db9a --- /dev/null +++ b/crates/apollo-compiler/test_data/diagnostics/0084_circular_non_nullable_input_objects.txt @@ -0,0 +1,158 @@ +[ + ApolloDiagnostic { + cache: { + 0: "built_in_types.graphql", + 81: "0084_circular_non_nullable_input_objects.graphql", + }, + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 62, + length: 49, + }, + labels: [ + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 62, + length: 49, + }, + text: "cyclical input object definition", + }, + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 231, + length: 13, + }, + text: "refers to itself here", + }, + ], + help: None, + data: RecursiveInputObjectDefinition { + name: "First", + }, + }, + ApolloDiagnostic { + cache: { + 0: "built_in_types.graphql", + 81: "0084_circular_non_nullable_input_objects.graphql", + }, + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 113, + length: 48, + }, + labels: [ + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 113, + length: 48, + }, + text: "cyclical input object definition", + }, + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 78, + length: 15, + }, + text: "refers to itself here", + }, + ], + help: None, + data: RecursiveInputObjectDefinition { + name: "Second", + }, + }, + ApolloDiagnostic { + cache: { + 0: "built_in_types.graphql", + 81: "0084_circular_non_nullable_input_objects.graphql", + }, + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 163, + length: 49, + }, + labels: [ + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 163, + length: 49, + }, + text: "cyclical input object definition", + }, + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 130, + length: 13, + }, + text: "refers to itself here", + }, + ], + help: None, + data: RecursiveInputObjectDefinition { + name: "Third", + }, + }, + ApolloDiagnostic { + cache: { + 0: "built_in_types.graphql", + 81: "0084_circular_non_nullable_input_objects.graphql", + }, + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 214, + length: 32, + }, + labels: [ + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 214, + length: 32, + }, + text: "cyclical input object definition", + }, + Label { + location: DiagnosticLocation { + file_id: FileId { + id: 81, + }, + offset: 179, + length: 15, + }, + text: "refers to itself here", + }, + ], + help: None, + data: RecursiveInputObjectDefinition { + name: "Fourth", + }, + }, +] diff --git a/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.graphql b/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.graphql new file mode 100644 index 000000000..dd970dbb3 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.graphql @@ -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 +} \ No newline at end of file diff --git a/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.txt b/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.txt new file mode 100644 index 000000000..155964dd8 --- /dev/null +++ b/crates/apollo-compiler/test_data/ok/0030_cyclical_nullable_input_objects.txt @@ -0,0 +1,155 @@ +- DOCUMENT@0..246 + - OBJECT_TYPE_DEFINITION@0..60 + - type_KW@0..4 "type" + - WHITESPACE@4..5 " " + - NAME@5..10 + - IDENT@5..10 "Query" + - WHITESPACE@10..11 " " + - FIELDS_DEFINITION@11..60 + - L_CURLY@11..12 "{" + - WHITESPACE@12..17 "\n " + - FIELD_DEFINITION@17..29 + - NAME@17..21 + - IDENT@17..21 "name" + - COLON@21..22 ":" + - WHITESPACE@22..23 " " + - NAMED_TYPE@23..29 + - NAME@23..29 + - IDENT@23..29 "String" + - WHITESPACE@29..34 "\n " + - FIELD_DEFINITION@34..58 + - NAME@34..41 + - IDENT@34..41 "example" + - ARGUMENTS_DEFINITION@41..53 + - L_PAREN@41..42 "(" + - INPUT_VALUE_DEFINITION@42..52 + - NAME@42..45 + - IDENT@42..45 "arg" + - COLON@45..46 ":" + - WHITESPACE@46..47 " " + - NAMED_TYPE@47..52 + - NAME@47..52 + - IDENT@47..52 "First" + - R_PAREN@52..53 ")" + - COLON@53..54 ":" + - WHITESPACE@54..55 " " + - NAMED_TYPE@55..58 + - NAME@55..58 + - IDENT@55..58 "Int" + - WHITESPACE@58..59 "\n" + - R_CURLY@59..60 "}" + - WHITESPACE@60..62 "\n\n" + - INPUT_OBJECT_TYPE_DEFINITION@62..110 + - input_KW@62..67 "input" + - WHITESPACE@67..68 " " + - NAME@68..73 + - IDENT@68..73 "First" + - WHITESPACE@73..74 " " + - INPUT_FIELDS_DEFINITION@74..110 + - L_CURLY@74..75 "{" + - WHITESPACE@75..78 "\n " + - INPUT_VALUE_DEFINITION@78..92 + - NAME@78..84 + - IDENT@78..84 "second" + - COLON@84..85 ":" + - WHITESPACE@85..86 " " + - NAMED_TYPE@86..92 + - NAME@86..92 + - IDENT@86..92 "Second" + - WHITESPACE@92..95 "\n " + - INPUT_VALUE_DEFINITION@95..108 + - NAME@95..100 + - IDENT@95..100 "value" + - COLON@100..101 ":" + - WHITESPACE@101..102 " " + - NAMED_TYPE@102..108 + - NAME@102..108 + - IDENT@102..108 "String" + - WHITESPACE@108..109 "\n" + - R_CURLY@109..110 "}" + - WHITESPACE@110..112 "\n\n" + - INPUT_OBJECT_TYPE_DEFINITION@112..163 + - input_KW@112..117 "input" + - WHITESPACE@117..118 " " + - NAME@118..124 + - IDENT@118..124 "Second" + - WHITESPACE@124..125 " " + - INPUT_FIELDS_DEFINITION@125..163 + - L_CURLY@125..126 "{" + - WHITESPACE@126..129 "\n " + - INPUT_VALUE_DEFINITION@129..145 + - NAME@129..134 + - IDENT@129..134 "third" + - COLON@134..135 ":" + - WHITESPACE@135..136 " " + - NON_NULL_TYPE@136..145 + - LIST_TYPE@136..144 + - L_BRACK@136..137 "[" + - NON_NULL_TYPE@137..143 + - NAMED_TYPE@137..142 + - NAME@137..142 + - IDENT@137..142 "Third" + - BANG@142..143 "!" + - R_BRACK@143..144 "]" + - BANG@144..145 "!" + - WHITESPACE@145..148 "\n " + - INPUT_VALUE_DEFINITION@148..161 + - NAME@148..153 + - IDENT@148..153 "value" + - COLON@153..154 ":" + - WHITESPACE@154..155 " " + - NAMED_TYPE@155..161 + - NAME@155..161 + - IDENT@155..161 "String" + - WHITESPACE@161..162 "\n" + - R_CURLY@162..163 "}" + - WHITESPACE@163..165 "\n\n" + - INPUT_OBJECT_TYPE_DEFINITION@165..213 + - input_KW@165..170 "input" + - WHITESPACE@170..171 " " + - NAME@171..176 + - IDENT@171..176 "Third" + - WHITESPACE@176..177 " " + - INPUT_FIELDS_DEFINITION@177..213 + - L_CURLY@177..178 "{" + - WHITESPACE@178..181 "\n " + - INPUT_VALUE_DEFINITION@181..195 + - NAME@181..187 + - IDENT@181..187 "fourth" + - COLON@187..188 ":" + - WHITESPACE@188..189 " " + - NAMED_TYPE@189..195 + - NAME@189..195 + - IDENT@189..195 "Fourth" + - WHITESPACE@195..198 "\n " + - INPUT_VALUE_DEFINITION@198..211 + - NAME@198..203 + - IDENT@198..203 "value" + - COLON@203..204 ":" + - WHITESPACE@204..205 " " + - NAMED_TYPE@205..211 + - NAME@205..211 + - IDENT@205..211 "String" + - WHITESPACE@211..212 "\n" + - R_CURLY@212..213 "}" + - WHITESPACE@213..215 "\n\n" + - INPUT_OBJECT_TYPE_DEFINITION@215..246 + - input_KW@215..220 "input" + - WHITESPACE@220..221 " " + - NAME@221..227 + - IDENT@221..227 "Fourth" + - WHITESPACE@227..228 " " + - INPUT_FIELDS_DEFINITION@228..246 + - L_CURLY@228..229 "{" + - WHITESPACE@229..232 "\n " + - INPUT_VALUE_DEFINITION@232..244 + - NAME@232..237 + - IDENT@232..237 "first" + - COLON@237..238 ":" + - WHITESPACE@238..239 " " + - NAMED_TYPE@239..244 + - NAME@239..244 + - IDENT@239..244 "First" + - WHITESPACE@244..245 "\n" + - R_CURLY@245..246 "}" +recursion limit: 4096, high: 0