Repository: apollographql/apollo-rs
Pull request #263 (2022-07-27 09:58:02)

feat(apollo-compiler): additional union validations
- validates that union members are defined in scope
- validates that union members are of object type


Patch:
diff --git a/crates/apollo-compiler/src/diagnostics.rs b/crates/apollo-compiler/src/diagnostics.rs
index 8488f96..e741d54 100644
--- a/crates/apollo-compiler/src/diagnostics.rs
+++ b/crates/apollo-compiler/src/diagnostics.rs
@@ -22,6 +22,7 @@ pub enum ApolloDiagnostic {
     CapitalizedValue(CapitalizedValue),
     UnusedVariable(UnusedVariable),
     OutputType(OutputType),
+    ObjectType(ObjectType),
 }
 
 impl ApolloDiagnostic {
@@ -42,6 +43,7 @@ impl ApolloDiagnostic {
                 | ApolloDiagnostic::UndefinedField(_)
                 | ApolloDiagnostic::BuiltInScalarDefinition(_)
                 | ApolloDiagnostic::OutputType(_)
+                | ApolloDiagnostic::ObjectType(_)
         )
     }
 
@@ -80,6 +82,7 @@ impl fmt::Display for ApolloDiagnostic {
             ApolloDiagnostic::UnusedVariable(diagnostic) => Report::new(diagnostic.clone()),
             ApolloDiagnostic::MissingField(diagnostic) => Report::new(diagnostic.clone()),
             ApolloDiagnostic::OutputType(diagnostic) => Report::new(diagnostic.clone()),
+            ApolloDiagnostic::ObjectType(diagnostic) => Report::new(diagnostic.clone()),
             ApolloDiagnostic::UndefinedField(diagnostic) => Report::new(diagnostic.clone()),
         };
 
@@ -317,7 +320,6 @@ pub struct UnusedVariable {
 
 #[derive(Diagnostic, Debug, Error, Clone, Hash, PartialEq, Eq)]
 #[error("`{}` field must return an output type", self.name)]
-/// Returns `true` if the definition is either a [`ScalarTypeDefinition`],
 #[diagnostic(
     code("apollo-compiler validation error"),
     help("Scalars, Objects, Interfaces, Unions and Enums are output types. Change `{}` field to return one of these output types.", self.name)
@@ -335,6 +337,25 @@ pub struct OutputType {
     pub definition: SourceSpan,
 }
 
+#[derive(Diagnostic, Debug, Error, Clone, Hash, PartialEq, Eq)]
+#[error("`{}` field must return an output type", self.name)]
+#[diagnostic(
+    code("apollo-compiler validation error"),
+    help("Union members must be of base Object Type. `{}` is of `{}` type", self.name, self.ty)
+)]
+pub struct ObjectType {
+    // union member
+    pub name: String,
+    // actual type
+    pub ty: String,
+
+    #[source_code]
+    pub src: String,
+
+    #[label("this is of `{}` type", self.ty)]
+    pub definition: SourceSpan,
+}
+
 #[derive(Diagnostic, Debug, Error, Clone, Hash, PartialEq, Eq)]
 #[error("Cannot query `{}` field", self.field)]
 /// Returns `true` if the definition is either a [`ScalarTypeDefinition`],
@@ -352,4 +373,3 @@ pub struct UndefinedField {
     #[help]
     pub help: String,
 }
-
diff --git a/crates/apollo-compiler/src/lib.rs b/crates/apollo-compiler/src/lib.rs
index 4666d71..e217610 100644
--- a/crates/apollo-compiler/src/lib.rs
+++ b/crates/apollo-compiler/src/lib.rs
@@ -591,7 +591,7 @@ type Book @directiveA(name: "pageCount") @directiveB(name: "author") {
 
         let book_obj = ctx.db.find_object_type_by_name("Book".to_string()).unwrap();
 
-        let directive_names : Vec<&str> = book_obj.directives().iter().map(|d|d.name()).collect();
+        let directive_names: Vec<&str> = book_obj.directives().iter().map(|d| d.name()).collect();
         assert_eq!(directive_names, ["directiveA", "directiveB"]);
     }
 
diff --git a/crates/apollo-compiler/src/validation/interfaces.rs b/crates/apollo-compiler/src/validation/interfaces.rs
index 6db9663..f700edc 100644
--- a/crates/apollo-compiler/src/validation/interfaces.rs
+++ b/crates/apollo-compiler/src/validation/interfaces.rs
@@ -2,8 +2,8 @@ use std::collections::{HashMap, HashSet};
 
 use crate::{
     diagnostics::{
-        MissingField, RecursiveDefinition, TransitiveImplementedInterfaces, UndefinedDefinition,
-        UniqueDefinition, UniqueField, OutputType,
+        MissingField, OutputType, RecursiveDefinition, TransitiveImplementedInterfaces,
+        UndefinedDefinition, UniqueDefinition, UniqueField,
     },
     validation::ValidationSet,
     values::{FieldDefinition, InterfaceTypeDefinition},
@@ -133,7 +133,6 @@ pub fn check(db: &dyn SourceDatabase) -> Vec<ApolloDiagnostic> {
                     src: db.input_string(()).to_string(),
                     definition: (offset, len).into(),
                 }))
-
             }
         }
     }
@@ -423,6 +422,11 @@ type Person {
   age: Int
 }
 
+type Photo {
+  size: Int
+  type: String
+}
+
 interface NamedEntity {
   name: String
 }
diff --git a/crates/apollo-compiler/src/validation/objects.rs b/crates/apollo-compiler/src/validation/objects.rs
index 6509687..d409928 100644
--- a/crates/apollo-compiler/src/validation/objects.rs
+++ b/crates/apollo-compiler/src/validation/objects.rs
@@ -94,7 +94,6 @@ pub fn check(db: &dyn SourceDatabase) -> Vec<ApolloDiagnostic> {
                     src: db.input_string(()).to_string(),
                     definition: (offset, len).into(),
                 }))
-
             }
         }
     }
@@ -249,6 +248,11 @@ type Person {
   age: Int
 }
 
+type Photo {
+  size: Int
+  type: String
+}
+
 interface NamedEntity {
   name: String
 }
diff --git a/crates/apollo-compiler/src/validation/operations.rs b/crates/apollo-compiler/src/validation/operations.rs
index 49b8ca3..33d0623 100644
--- a/crates/apollo-compiler/src/validation/operations.rs
+++ b/crates/apollo-compiler/src/validation/operations.rs
@@ -284,7 +284,23 @@ type Subscription {
   newMessage: Result
 }
 
-union Pet = Cat | Dog
+interface Pet {
+  name: String
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+type Cat implements Pet {
+  name: String
+  nickname: String
+  meowVolume: Int
+}
+
+union CatOrDog = Cat | Dog
 "#;
         let ctx = ApolloCompiler::new(input);
         let diagnostics = ctx.validate();
@@ -315,7 +331,23 @@ type Query {
   cat: Pet
 }
 
-union Pet = Cat | Dog
+union CatOrDog = Cat | Dog
+
+interface Pet {
+  name: String
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+type Cat implements Pet {
+  name: String
+  nickname: String
+  meowVolume: Int
+}
 "#;
         let ctx = ApolloCompiler::new(input);
         let diagnostics = ctx.validate();
@@ -346,7 +378,23 @@ type Query {
   cat: Pet
 }
 
-union Pet = Cat | Dog
+union CatOrDog = Cat | Dog
+
+interface Pet {
+  name: String
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+type Cat implements Pet {
+  name: String
+  nickname: String
+  meowVolume: Int
+}
 "#;
         let ctx = ApolloCompiler::new(input);
         let diagnostics = ctx.validate();
@@ -367,7 +415,23 @@ type Query {
   cat: Pet
 }
 
-union Pet = Cat | Dog
+union CatOrDog = Cat | Dog
+
+interface Pet {
+  name: String
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+type Cat implements Pet {
+  name: String
+  nickname: String
+  meowVolume: Int
+}
 "#;
         let ctx = ApolloCompiler::new(input);
         let diagnostics = ctx.validate();
diff --git a/crates/apollo-compiler/src/validation/unions.rs b/crates/apollo-compiler/src/validation/unions.rs
index edc3eb5..225c2cc 100644
--- a/crates/apollo-compiler/src/validation/unions.rs
+++ b/crates/apollo-compiler/src/validation/unions.rs
@@ -1,29 +1,33 @@
 use std::collections::HashMap;
 
-use crate::{diagnostics::UniqueDefinition, values::UnionMember, ApolloDiagnostic, SourceDatabase};
+use crate::{
+    diagnostics::{ObjectType, UndefinedDefinition, UniqueDefinition},
+    values::UnionMember,
+    ApolloDiagnostic, SourceDatabase,
+};
 
 pub fn check(db: &dyn SourceDatabase) -> Vec<ApolloDiagnostic> {
     let mut diagnostics = Vec::new();
 
-    // A Union type must include one or more unique member types.
-    //
-    // Return a Unique Value error in case of a duplicate member.
     for union_def in db.unions().iter() {
         let mut seen: HashMap<&str, &UnionMember> = HashMap::new();
         for union_member in union_def.union_members().iter() {
             let name = union_member.name();
+            let offset: usize = union_member.ast_node(db).text_range().start().into();
+            let len: usize = union_member.ast_node(db).text_range().len().into();
+            // A Union type must include one or more unique member types.
+            //
+            // Return a Unique Value error in case of a duplicate member.
             if let Some(prev_def) = seen.get(&name) {
                 let prev_offset: usize = prev_def.ast_node(db).text_range().start().into();
                 let prev_node_len: usize = prev_def.ast_node(db).text_range().len().into();
 
-                let current_offset: usize = union_member.ast_node(db).text_range().start().into();
-                let current_node_len: usize = union_member.ast_node(db).text_range().len().into();
                 diagnostics.push(ApolloDiagnostic::UniqueDefinition(UniqueDefinition {
                     name: name.into(),
                     ty: "union member".into(),
                     src: db.input_string(()).to_string(),
                     original_definition: (prev_offset, prev_node_len).into(),
-                    redefined_definition: (current_offset, current_node_len).into(),
+                    redefined_definition: (offset, len).into(),
                     help: Some(format!(
                         "`{name}` must only be defined once in this document."
                     )),
@@ -31,6 +35,26 @@ pub fn check(db: &dyn SourceDatabase) -> Vec<ApolloDiagnostic> {
             } else {
                 seen.insert(name, union_member);
             }
+
+            // Union member must be defined.
+            let union_member_type = db.find_type_system_definition_by_name(name.to_string());
+            if union_member_type.is_none() {
+                diagnostics.push(ApolloDiagnostic::UndefinedDefinition(UndefinedDefinition {
+                    ty: name.into(),
+                    src: db.input_string(()).to_string(),
+                    definition: (offset, len).into(),
+                }))
+            } else if let Some(ty) = union_member_type {
+                // Union member must be of object type.
+                if !ty.is_object_type_definition() {
+                    diagnostics.push(ApolloDiagnostic::ObjectType(ObjectType {
+                        name: name.into(),
+                        ty: ty.ty(),
+                        src: db.input_string(()).to_string(),
+                        definition: (offset, len).into(),
+                    }))
+                }
+            }
         }
     }
 


Test patch:
diff --git a/crates/apollo-compiler/test_data/diagnostics/0036_object_type_with_non_output_field_types.txt b/crates/apollo-compiler/test_data/diagnostics/0036_object_type_with_non_output_field_types.txt
index 24fcbe2..d425eb9 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0036_object_type_with_non_output_field_types.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0036_object_type_with_non_output_field_types.txt
@@ -1,4 +1,18 @@
 [
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Photo",
+            src: "query mainPage {\n  width\n  result\n  entity\n}\n\ntype Query {\n  width: Int\n  img: Url\n  relationship: Person\n  entity: NamedEntity\n  depth: Number\n  result: SearchResult\n  permissions: Auth\n  coordinates: Point2D\n  main: mainPage\n}\n\ntype Person {\n  name: String\n  age: Int\n}\n\ninterface NamedEntity {\n  name: String\n}\n\nenum Number {\n  INT\n  FLOAT\n}\n\nunion SearchResult = Photo | Person\n\ndirective @Auth(username: String!) repeatable on OBJECT | INTERFACE\n\ninput Point2D {\n  x: Float\n  y: Float\n}\n\nscalar Url @specifiedBy(url: \"https://tools.ietf.org/html/rfc3986\")",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    367,
+                ),
+                length: SourceOffset(
+                    6,
+                ),
+            },
+        },
+    ),
     OutputType(
         OutputType {
             name: "permissions",
diff --git a/crates/apollo-compiler/test_data/diagnostics/0037_interface_with_non_output_fields.txt b/crates/apollo-compiler/test_data/diagnostics/0037_interface_with_non_output_fields.txt
index b9b5a96..36b8043 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0037_interface_with_non_output_fields.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0037_interface_with_non_output_fields.txt
@@ -1,4 +1,18 @@
 [
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Photo",
+            src: "query mainPage {\n  name\n}\n\ntype Query {\n  name: mainInterface\n}\n\ninterface mainInterface {\n  width: Int\n  img: Url\n  relationship: Person\n  entity: NamedEntity\n  depth: Number\n  result: SearchResult\n  permissions: Auth\n  coordinates: Point2D\n  main: mainPage\n}\n\ntype Person {\n  name: String\n  age: Int\n}\n\ninterface NamedEntity {\n  name: String\n}\n\nenum Number {\n  INT\n  FLOAT\n}\n\nunion SearchResult = Photo | Person\n\ndirective @Auth(username: String!) repeatable on OBJECT | INTERFACE\n\ninput Point2D {\n  x: Float\n  y: Float\n}\n\nscalar Url @specifiedBy(url: \"https://tools.ietf.org/html/rfc3986\")",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    399,
+                ),
+                length: SourceOffset(
+                    6,
+                ),
+            },
+        },
+    ),
     OutputType(
         OutputType {
             name: "permissions",
diff --git a/crates/apollo-compiler/test_data/diagnostics/0042_mutation_operation_with_undefined_fields.txt b/crates/apollo-compiler/test_data/diagnostics/0042_mutation_operation_with_undefined_fields.txt
index 0e3e63e..4994696 100644
--- a/crates/apollo-compiler/test_data/diagnostics/0042_mutation_operation_with_undefined_fields.txt
+++ b/crates/apollo-compiler/test_data/diagnostics/0042_mutation_operation_with_undefined_fields.txt
@@ -1,4 +1,32 @@
 [
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Cat",
+            src: "mutation adoptAPetMutation {\n  undefinedMutationField\n}\n\ntype Query {\n  name: String,\n  age: Int\n}\n\ntype Mutation {\n  addPet (name: String!, petType: PetType): Result!\n}\n\ntype Result {\n  id: String\n}\n\nunion PetType = Cat | Dog\n",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    217,
+                ),
+                length: SourceOffset(
+                    4,
+                ),
+            },
+        },
+    ),
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Dog",
+            src: "mutation adoptAPetMutation {\n  undefinedMutationField\n}\n\ntype Query {\n  name: String,\n  age: Int\n}\n\ntype Mutation {\n  addPet (name: String!, petType: PetType): Result!\n}\n\ntype Result {\n  id: String\n}\n\nunion PetType = Cat | Dog\n",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    223,
+                ),
+                length: SourceOffset(
+                    4,
+                ),
+            },
+        },
+    ),
     UndefinedField(
         UndefinedField {
             field: "undefinedMutationField",
diff --git a/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.graphql b/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.graphql
new file mode 100644
index 0000000..eae7153
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.graphql
@@ -0,0 +1,20 @@
+interface Pet {
+  name: String
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+union CatOrDog = Cat | Dog
+
+type Human {
+  name: String
+  pets: [Pet]
+}
+
+type Query {
+  human: Human
+}
\ No newline at end of file
diff --git a/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.txt b/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.txt
new file mode 100644
index 0000000..b82a580
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0043_undefined_union_member.txt
@@ -0,0 +1,16 @@
+[
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Cat",
+            src: "interface Pet {\n  name: String\n}\n\ntype Dog implements Pet {\n  name: String\n  nickname: String\n  barkVolume: Int\n}\n\nunion CatOrDog = Cat | Dog\n\ntype Human {\n  name: String\n  pets: [Pet]\n}\n\ntype Query {\n  human: Human\n}",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    132,
+                ),
+                length: SourceOffset(
+                    4,
+                ),
+            },
+        },
+    ),
+]
diff --git a/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.graphql b/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.graphql
new file mode 100644
index 0000000..96a8c54
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.graphql
@@ -0,0 +1,26 @@
+interface Pet {
+  name: String
+}
+
+type Cat implements Pet {
+  name: String
+  nickname: String
+  meowVolume: Int
+}
+
+type Dog implements Pet {
+  name: String
+  nickname: String
+  barkVolume: Int
+}
+
+union CatOrDog = Cat | Pet
+
+type Human {
+  name: String
+  pets: [Pet]
+}
+
+type Query {
+  human: Human
+}
\ No newline at end of file
diff --git a/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.txt b/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.txt
new file mode 100644
index 0000000..163f114
--- /dev/null
+++ b/crates/apollo-compiler/test_data/diagnostics/0044_union_member_not_of_object_type.txt
@@ -0,0 +1,17 @@
+[
+    ObjectType(
+        ObjectType {
+            name: "Pet",
+            ty: "InterfaceTypeDefinition",
+            src: "interface Pet {\n  name: String\n}\n\ntype Cat implements Pet {\n  name: String\n  nickname: String\n  meowVolume: Int\n}\n\ntype Dog implements Pet {\n  name: String\n  nickname: String\n  barkVolume: Int\n}\n\nunion CatOrDog = Cat | Pet\n\ntype Human {\n  name: String\n  pets: [Pet]\n}\n\ntype Query {\n  human: Human\n}",
+            definition: SourceSpan {
+                offset: SourceOffset(
+                    219,
+                ),
+                length: SourceOffset(
+                    5,
+                ),
+            },
+        },
+    ),
+]