Repository: apollographql/apollo-rs
Pull request #551 (2023-05-11 10:50:08)

feat(compiler): support `@deprecated` on arguments and input values
Per draft spec: https://spec.graphql.org/draft/#sec--deprecated



Patch:
diff --git a/crates/apollo-compiler/src/built_in_types.graphql b/crates/apollo-compiler/src/built_in_types.graphql
index 16fb023..744e101 100644
--- a/crates/apollo-compiler/src/built_in_types.graphql
+++ b/crates/apollo-compiler/src/built_in_types.graphql
@@ -114,7 +114,7 @@ directive @deprecated(
   [CommonMark](https://commonmark.org/).
   """
   reason: String = "No longer supported"
-) on FIELD_DEFINITION | ENUM_VALUE
+) on FIELD_DEFINITION | ARGUMENT_DEFINITION | INPUT_FIELD_DEFINITION | ENUM_VALUE
 
 "Exposes a URL that specifies the behaviour of this scalar."
 directive @specifiedBy(
@@ -151,4 +151,4 @@ String; however, it is not intended to be human-readable. When expected as an
 input type, any string (such as `\"4\"`) or integer (such as `4`) input value
 will be accepted as an ID.
 """
-scalar ID
\ No newline at end of file
+scalar ID
diff --git a/crates/apollo-compiler/src/database/document.rs b/crates/apollo-compiler/src/database/document.rs
index c7d90fd..c36f45c 100644
--- a/crates/apollo-compiler/src/database/document.rs
+++ b/crates/apollo-compiler/src/database/document.rs
@@ -199,6 +199,21 @@ pub(crate) fn operation_inline_fragment_fields(
     Arc::new(fields)
 }
 
+pub(crate) fn operation_fragment_references(
+    db: &dyn HirDatabase,
+    selection_set: SelectionSet,
+) -> Arc<Vec<Arc<FragmentDefinition>>> {
+    let fields = selection_set
+        .selection()
+        .iter()
+        .filter_map(|sel| match sel {
+            Selection::FragmentSpread(fragment_spread) => Some(fragment_spread.fragment(db)?),
+            _ => None,
+        })
+        .collect();
+    Arc::new(fields)
+}
+
 pub(crate) fn operation_fragment_spread_fields(
     db: &dyn HirDatabase,
     selection_set: SelectionSet,
diff --git a/crates/apollo-compiler/src/database/hir.rs b/crates/apollo-compiler/src/database/hir.rs
index 50c450b..08b840c 100644
--- a/crates/apollo-compiler/src/database/hir.rs
+++ b/crates/apollo-compiler/src/database/hir.rs
@@ -529,6 +529,11 @@ impl OperationDefinition {
         db.operation_fragment_spread_fields(self.selection_set.clone())
     }
 
+    /// Get all fragment definitions referenced by the operation.
+    pub fn fragment_references(&self, db: &dyn HirDatabase) -> Arc<Vec<Arc<FragmentDefinition>>> {
+        db.operation_fragment_references(self.selection_set.clone())
+    }
+
     /// Get the AST location information for this HIR node.
     pub fn loc(&self) -> HirNodeLocation {
         self.loc
diff --git a/crates/apollo-compiler/src/database/hir_db.rs b/crates/apollo-compiler/src/database/hir_db.rs
index c5ae4e6..064bca6 100644
--- a/crates/apollo-compiler/src/database/hir_db.rs
+++ b/crates/apollo-compiler/src/database/hir_db.rs
@@ -206,6 +206,13 @@ pub trait HirDatabase: InputDatabase + AstDatabase {
     #[salsa::invoke(document::operation_fragment_spread_fields)]
     fn operation_fragment_spread_fields(&self, selection_set: SelectionSet) -> Arc<Vec<Field>>;
 
+    /// Return all fragment definitions referenced in the operation.
+    #[salsa::invoke(document::operation_fragment_references)]
+    fn operation_fragment_references(
+        &self,
+        selection_set: SelectionSet,
+    ) -> Arc<Vec<Arc<FragmentDefinition>>>;
+
     /// Return the fields that `selection_set` selects including visiting fragments and inline fragments.
     #[salsa::invoke(document::flattened_operation_fields)]
     fn flattened_operation_fields(&self, selection_set: SelectionSet) -> Vec<Arc<Field>>;
diff --git a/crates/apollo-compiler/src/validation/operation.rs b/crates/apollo-compiler/src/validation/operation.rs
index da663f0..89f859e 100644
--- a/crates/apollo-compiler/src/validation/operation.rs
+++ b/crates/apollo-compiler/src/validation/operation.rs
@@ -174,7 +174,12 @@ pub fn validate_query_operations(
             queries
                 .iter()
                 .filter_map(|op| {
-                    if !op.is_introspection(db.upcast()) {
+                    let has_cycles = op
+                        .fragment_references(db.upcast())
+                        .iter()
+                        .any(|fragment| !db.validate_fragment_cycles(fragment.clone()).is_empty());
+
+                    if !has_cycles && !op.is_introspection(db.upcast()) {
                         let diagnostic = ApolloDiagnostic::new(
                             db,
                             op.loc().into(),
diff --git a/crates/apollo-compiler/src/validation/validation_db.rs b/crates/apollo-compiler/src/validation/validation_db.rs
index f8ad4af..f3c1da6 100644
--- a/crates/apollo-compiler/src/validation/validation_db.rs
+++ b/crates/apollo-compiler/src/validation/validation_db.rs
@@ -512,4 +512,71 @@ query {
             "executable documents must not contain ObjectTypeDefinition"
         );
     }
+
+    #[test]
+    fn executable_definition_with_cycles_do_not_overflow_stack() {
+        let input_type_system = r#"
+type Query {
+    name: String
+}
+"#;
+
+        let input_executable = r#"
+{
+    ...q
+}
+fragment q on Query {
+    ...q
+}
+"#;
+
+        let mut compiler = ApolloCompiler::new();
+        compiler.add_type_system(input_type_system, "schema.graphql");
+        compiler.add_executable(input_executable, "query.graphql");
+
+        let diagnostics = compiler.validate();
+        // TODO(@goto-bus-stop) this should be 1, but both uses of ...q cause a report
+        assert_eq!(diagnostics.len(), 2);
+        assert_eq!(
+            diagnostics[0].data.to_string(),
+            "`q` fragment cannot reference itself"
+        );
+    }
+
+    #[test]
+    fn executable_definition_with_nested_cycles_do_not_overflow_stack() {
+        let input_type_system = r#"
+type Query {
+    obj: TestObject
+}
+
+type TestObject {
+    name: String
+}
+"#;
+
+        let input_executable = r#"
+{
+    obj {
+        ...q
+    }
+}
+
+fragment q on TestObject {
+    ...q
+}
+"#;
+
+        let mut compiler = ApolloCompiler::new();
+        compiler.add_type_system(input_type_system, "schema.graphql");
+        compiler.add_executable(input_executable, "query.graphql");
+
+        let diagnostics = compiler.validate();
+        // TODO(@goto-bus-stop) this should be 1, but both uses of ...q cause a report
+        assert_eq!(diagnostics.len(), 2);
+        assert_eq!(
+            diagnostics[0].data.to_string(),
+            "`q` fragment cannot reference itself"
+        );
+    }
 }


Test patch:
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 d99d4df..1d115be 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
@@ -435,7 +435,7 @@
                     id: 0,
                 },
                 offset: 2347,
-                length: 393,
+                length: 440,
             },
         },
     },
@@ -473,7 +473,7 @@
                 file_id: FileId {
                     id: 0,
                 },
-                offset: 2742,
+                offset: 2789,
                 length: 172,
             },
         },
@@ -591,7 +591,7 @@
                     id: 0,
                 },
                 offset: 2347,
-                length: 393,
+                length: 440,
             },
         },
     },
diff --git a/crates/apollo-compiler/test_data/diagnostics/0087_fragment_type_condition_on_composite_types.txt b/crates/apollo-compiler/test_data/diagnostics/0087_fragment_type_condition_on_composite_types.txt
index 1aad669..c938497 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0087_fragment_type_condition_on_composite_types.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0087_fragment_type_condition_on_composite_types.txt
@@ -27,7 +27,7 @@
                     file_id: FileId {
                         id: 0,
                     },
-                    offset: 2916,
+                    offset: 2963,
                     length: 150,
                 },
                 text: "`Int` declared here",
@@ -64,7 +64,7 @@
                     file_id: FileId {
                         id: 0,
                     },
-                    offset: 2916,
+                    offset: 2963,
                     length: 150,
                 },
                 text: "`Int` is defined here",
@@ -105,7 +105,7 @@
                     file_id: FileId {
                         id: 0,
                     },
-                    offset: 2916,
+                    offset: 2963,
                     length: 150,
                 },
                 text: "`Int` is defined here",