Repository: apollographql/apollo-rs
Pull request #389 (2022-12-19 11:10:13)

Move named types to a separate HIR enum and separate namespacing
* Split `hir::Definition` enum variants that define types into a new `NamedTypeDefinition` enum.

* Add Salsa queries for finding a type by its name, (excluding non-type named definitions), and use them where appropriate. This changes some diagnostics from "wrong definition kind" to "no such type".

* Remove `find_type_system_definition_by_name` and `find_definition_by_name` queries since they mix up the four namespaces of a GraphQL document that should really be separate. Instead its callers should probably use one of:

  * `find_directive_definition_by_name`
  * `find_operation_by_name`
  * `find_fragment_by_name`
  * `find_type_definition_by_name`


SimonSapin: I've renamed `NamedTypeDefinition` to `TypeDefinition`
SimonSapin: (The two commits make sense separately, so this could be rebased instead of squashed.)

Patch:
diff --git a/crates/apollo-compiler/src/database/document.rs b/crates/apollo-compiler/src/database/document.rs
index b2746f1..77d0cb1 100644
--- a/crates/apollo-compiler/src/database/document.rs
+++ b/crates/apollo-compiler/src/database/document.rs
@@ -4,6 +4,7 @@ use std::{
 };
 
 use crate::{hir::*, AstDatabase, FileId, HirDatabase, InputDatabase};
+use indexmap::IndexMap;
 
 #[salsa::query_group(DocumentStorage)]
 pub trait DocumentDatabase: InputDatabase + AstDatabase + HirDatabase {
@@ -37,6 +38,10 @@ pub trait DocumentDatabase: InputDatabase + AstDatabase + HirDatabase {
 
     fn find_type_system_definition_by_name(&self, name: String) -> Option<Definition>;
 
+    fn types_definitions_by_name(&self) -> Arc<IndexMap<String, TypeDefinition>>;
+
+    fn find_type_definition_by_name(&self, name: String) -> Option<TypeDefinition>;
+
     fn query_operations(&self, file_id: FileId) -> Arc<Vec<Arc<OperationDefinition>>>;
 
     fn mutation_operations(&self, file_id: FileId) -> Arc<Vec<Arc<OperationDefinition>>>;
@@ -78,6 +83,29 @@ fn find_type_system_definition_by_name(
         .cloned()
 }
 
+fn types_definitions_by_name(db: &dyn DocumentDatabase) -> Arc<IndexMap<String, TypeDefinition>> {
+    let mut map = IndexMap::new();
+    macro_rules! add {
+        ($get: ident, $variant: ident) => {
+            for (name, def) in db.$get().iter() {
+                map.entry(name.clone())
+                    .or_insert_with(|| TypeDefinition::$variant(def.clone()));
+            }
+        };
+    }
+    add!(scalars, ScalarTypeDefinition);
+    add!(object_types, ObjectTypeDefinition);
+    add!(interfaces, InterfaceTypeDefinition);
+    add!(unions, UnionTypeDefinition);
+    add!(enums, EnumTypeDefinition);
+    add!(input_objects, InputObjectTypeDefinition);
+    Arc::new(map)
+}
+
+fn find_type_definition_by_name(db: &dyn DocumentDatabase, name: String) -> Option<TypeDefinition> {
+    db.types_definitions_by_name().get(&name).cloned()
+}
+
 fn find_operation_by_name(
     db: &dyn DocumentDatabase,
     file_id: FileId,
diff --git a/crates/apollo-compiler/src/database/hir.rs b/crates/apollo-compiler/src/database/hir.rs
index c1b499d..c093ce9 100644
--- a/crates/apollo-compiler/src/database/hir.rs
+++ b/crates/apollo-compiler/src/database/hir.rs
@@ -15,124 +15,89 @@ pub enum Definition {
     OperationDefinition(Arc<OperationDefinition>),
     FragmentDefinition(Arc<FragmentDefinition>),
     DirectiveDefinition(Arc<DirectiveDefinition>),
+    TypeDefinition(TypeDefinition),
+    SchemaDefinition(Arc<SchemaDefinition>),
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
+pub enum TypeDefinition {
     ScalarTypeDefinition(Arc<ScalarTypeDefinition>),
     ObjectTypeDefinition(Arc<ObjectTypeDefinition>),
     InterfaceTypeDefinition(Arc<InterfaceTypeDefinition>),
     UnionTypeDefinition(Arc<UnionTypeDefinition>),
     EnumTypeDefinition(Arc<EnumTypeDefinition>),
     InputObjectTypeDefinition(Arc<InputObjectTypeDefinition>),
-    SchemaDefinition(Arc<SchemaDefinition>),
 }
 
 impl Definition {
     // Get a reference to definition's name.
     pub fn name(&self) -> Option<&str> {
         match self {
-            Definition::OperationDefinition(def) => def.name(),
-            Definition::FragmentDefinition(def) => Some(def.name()),
-            Definition::DirectiveDefinition(def) => Some(def.name()),
-            Definition::ScalarTypeDefinition(def) => Some(def.name()),
-            Definition::ObjectTypeDefinition(def) => Some(def.name()),
-            Definition::InterfaceTypeDefinition(def) => Some(def.name()),
-            Definition::UnionTypeDefinition(def) => Some(def.name()),
-            Definition::EnumTypeDefinition(def) => Some(def.name()),
-            Definition::InputObjectTypeDefinition(def) => Some(def.name()),
-            Definition::SchemaDefinition(_) => None,
+            Self::OperationDefinition(def) => def.name(),
+            Self::FragmentDefinition(def) => Some(def.name()),
+            Self::DirectiveDefinition(def) => Some(def.name()),
+            Self::TypeDefinition(def) => Some(def.name()),
+            Self::SchemaDefinition(_) => None,
         }
     }
 
     pub fn name_src(&self) -> Option<&Name> {
         match self {
-            Definition::OperationDefinition(def) => def.name_src(),
-            Definition::FragmentDefinition(def) => Some(def.name_src()),
-            Definition::DirectiveDefinition(def) => Some(def.name_src()),
-            Definition::ScalarTypeDefinition(def) => Some(def.name_src()),
-            Definition::ObjectTypeDefinition(def) => Some(def.name_src()),
-            Definition::InterfaceTypeDefinition(def) => Some(def.name_src()),
-            Definition::UnionTypeDefinition(def) => Some(def.name_src()),
-            Definition::EnumTypeDefinition(def) => Some(def.name_src()),
-            Definition::InputObjectTypeDefinition(def) => Some(def.name_src()),
-            Definition::SchemaDefinition(_) => None,
+            Self::OperationDefinition(def) => def.name_src(),
+            Self::FragmentDefinition(def) => Some(def.name_src()),
+            Self::DirectiveDefinition(def) => Some(def.name_src()),
+            Self::TypeDefinition(def) => match def {
+                TypeDefinition::ScalarTypeDefinition(def) => Some(def.name_src()),
+                TypeDefinition::ObjectTypeDefinition(def) => Some(def.name_src()),
+                TypeDefinition::InterfaceTypeDefinition(def) => Some(def.name_src()),
+                TypeDefinition::UnionTypeDefinition(def) => Some(def.name_src()),
+                TypeDefinition::EnumTypeDefinition(def) => Some(def.name_src()),
+                TypeDefinition::InputObjectTypeDefinition(def) => Some(def.name_src()),
+            },
+            Self::SchemaDefinition(_) => None,
         }
     }
 
     // Get the current definition type, e..g OperationDefinition,
     // EnumTypeDefinition, ObjectTypeDefinition etc.
-    pub fn ty(&self) -> String {
+    pub fn ty(&self) -> &'static str {
         match self {
-            Definition::OperationDefinition(_) => "OperationDefinition".to_string(),
-            Definition::FragmentDefinition(_) => "FragmentDefinition".to_string(),
-            Definition::DirectiveDefinition(_) => "DirectiveDefinition".to_string(),
-            Definition::ScalarTypeDefinition(_) => "ScalarTypeDefinition".to_string(),
-            Definition::ObjectTypeDefinition(_) => "ObjectTypeDefinition".to_string(),
-            Definition::InterfaceTypeDefinition(_) => "InterfaceTypeDefinition".to_string(),
-            Definition::UnionTypeDefinition(_) => "UnionTypeDefinition".to_string(),
-            Definition::EnumTypeDefinition(_) => "EnumTypeDefinition".to_string(),
-            Definition::InputObjectTypeDefinition(_) => "InputObjectTypeDefinition".to_string(),
-            Definition::SchemaDefinition(_) => "SchemaDefinition".to_string(),
+            Self::OperationDefinition(_) => "OperationDefinition",
+            Self::FragmentDefinition(_) => "FragmentDefinition",
+            Self::DirectiveDefinition(_) => "DirectiveDefinition",
+            Self::TypeDefinition(def) => def.ty(),
+            Self::SchemaDefinition(_) => "SchemaDefinition",
         }
     }
 
     pub fn field(&self, name: &str) -> Option<&FieldDefinition> {
         match self {
-            Definition::ObjectTypeDefinition(def) => def.field(name),
-            Definition::InterfaceTypeDefinition(def) => def.field(name),
+            Self::TypeDefinition(def) => match def {
+                TypeDefinition::ObjectTypeDefinition(def) => def.field(name),
+                TypeDefinition::InterfaceTypeDefinition(def) => def.field(name),
+                _ => None,
+            },
             _ => None,
         }
     }
 
     pub fn directives(&self) -> &[Directive] {
         match self {
-            Definition::OperationDefinition(def) => def.directives(),
-            Definition::FragmentDefinition(def) => def.directives(),
-            Definition::DirectiveDefinition(_) => &[],
-            Definition::ScalarTypeDefinition(def) => def.directives(),
-            Definition::ObjectTypeDefinition(def) => def.directives(),
-            Definition::InterfaceTypeDefinition(def) => def.directives(),
-            Definition::UnionTypeDefinition(def) => def.directives(),
-            Definition::EnumTypeDefinition(def) => def.directives(),
-            Definition::InputObjectTypeDefinition(def) => def.directives(),
-            Definition::SchemaDefinition(def) => def.directives(),
+            Self::OperationDefinition(def) => def.directives(),
+            Self::FragmentDefinition(def) => def.directives(),
+            Self::DirectiveDefinition(_) => &[],
+            Self::TypeDefinition(def) => match def {
+                TypeDefinition::ScalarTypeDefinition(def) => def.directives(),
+                TypeDefinition::ObjectTypeDefinition(def) => def.directives(),
+                TypeDefinition::InterfaceTypeDefinition(def) => def.directives(),
+                TypeDefinition::UnionTypeDefinition(def) => def.directives(),
+                TypeDefinition::EnumTypeDefinition(def) => def.directives(),
+                TypeDefinition::InputObjectTypeDefinition(def) => def.directives(),
+            },
+            Self::SchemaDefinition(def) => def.directives(),
         }
     }
 
-    /// Returns `true` if the definition is either a [`ScalarTypeDefinition`],
-    /// [`ObjectTypeDefinition`], [`InterfaceTypeDefinition`],
-    /// [`UnionTypeDefinition`], [`EnumTypeDefinition`].
-    ///
-    /// [`ScalarTypeDefinition`]: Definition::ScalarTypeDefinition
-    /// [`ObjectTypeDefinition`]: Definition::ObjectTypeDefinition
-    /// [`InterfaceTypeDefinition`]: Definition::InterfaceTypeDefinition
-    /// [`UnionTypeDefinition`]: Definition::UnionTypeDefinition
-    /// [`EnumTypeDefinition`]: Definition::EnumTypeDefinition
-    #[must_use]
-    pub fn is_output_definition(&self) -> bool {
-        matches!(
-            self,
-            Self::ScalarTypeDefinition(..)
-                | Self::ObjectTypeDefinition(..)
-                | Self::InterfaceTypeDefinition(..)
-                | Self::UnionTypeDefinition(..)
-                | Self::EnumTypeDefinition(..)
-        )
-    }
-
-    /// Returns `true` if the definition is either a [`ScalarTypeDefinition`],
-    /// [`EnumTypeDefinition`], [`InputObjectTypeDefinition`].
-    ///
-    /// [`ScalarTypeDefinition`]: Definition::ScalarTypeDefinition
-    /// [`EnumTypeDefinition`]: Definition::EnumTypeDefinition
-    /// [`InputObjectTypeDefinition`]: Definition::ObjectTypeDefinition
-    #[must_use]
-    pub fn is_input_definition(&self) -> bool {
-        matches!(
-            self,
-            Self::ScalarTypeDefinition(..)
-                | Self::EnumTypeDefinition(..)
-                | Self::InputObjectTypeDefinition(..)
-        )
-    }
-
     /// Returns `true` if the definition is [`OperationDefinition`].
     ///
     /// [`OperationDefinition`]: Definition::OperationDefinition
@@ -162,7 +127,10 @@ impl Definition {
     /// [`ScalarTypeDefinition`]: Definition::ScalarTypeDefinition
     #[must_use]
     pub fn is_scalar_type_definition(&self) -> bool {
-        matches!(self, Self::ScalarTypeDefinition(..))
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::ScalarTypeDefinition(..))
+        )
     }
 
     /// Returns `true` if the definition is [`ObjectTypeDefinition`].
@@ -170,7 +138,10 @@ impl Definition {
     /// [`ObjectTypeDefinition`]: Definition::ObjectTypeDefinition
     #[must_use]
     pub fn is_object_type_definition(&self) -> bool {
-        matches!(self, Self::ObjectTypeDefinition { .. })
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::ObjectTypeDefinition { .. })
+        )
     }
 
     /// Returns `true` if the definition is [`InterfaceTypeDefinition`].
@@ -178,7 +149,10 @@ impl Definition {
     /// [`InterfaceTypeDefinition`]: Definition::InterfaceTypeDefinition
     #[must_use]
     pub fn is_interface_type_definition(&self) -> bool {
-        matches!(self, Self::InterfaceTypeDefinition(..))
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::InterfaceTypeDefinition(..))
+        )
     }
 
     /// Returns `true` if the definition is [`UnionTypeDefinition`].
@@ -186,7 +160,10 @@ impl Definition {
     /// [`UnionTypeDefinition`]: Definition::UnionTypeDefinition
     #[must_use]
     pub fn is_union_type_definition(&self) -> bool {
-        matches!(self, Self::UnionTypeDefinition(..))
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::UnionTypeDefinition(..))
+        )
     }
 
     /// Returns `true` if the definition is [`EnumTypeDefinition`].
@@ -194,7 +171,10 @@ impl Definition {
     /// [`EnumTypeDefinition`]: Definition::EnumTypeDefinition
     #[must_use]
     pub fn is_enum_type_definition(&self) -> bool {
-        matches!(self, Self::EnumTypeDefinition(..))
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::EnumTypeDefinition(..))
+        )
     }
 
     /// Returns `true` if the definition is [`InputObjectTypeDefinition`].
@@ -202,7 +182,10 @@ impl Definition {
     /// [`InputObjectTypeDefinition`]: Definition::InputObjectTypeDefinition
     #[must_use]
     pub fn is_input_object_type_definition(&self) -> bool {
-        matches!(self, Self::InputObjectTypeDefinition(..))
+        matches!(
+            self,
+            Self::TypeDefinition(TypeDefinition::InputObjectTypeDefinition(..))
+        )
     }
 
     /// Returns `true` if the definition is [`SchemaDefinition`].
@@ -213,6 +196,67 @@ impl Definition {
         matches!(self, Self::SchemaDefinition(..))
     }
 }
+
+impl TypeDefinition {
+    pub fn name(&self) -> &str {
+        match self {
+            Self::ScalarTypeDefinition(def) => def.name(),
+            Self::ObjectTypeDefinition(def) => def.name(),
+            Self::InterfaceTypeDefinition(def) => def.name(),
+            Self::UnionTypeDefinition(def) => def.name(),
+            Self::EnumTypeDefinition(def) => def.name(),
+            Self::InputObjectTypeDefinition(def) => def.name(),
+        }
+    }
+
+    pub fn ty(&self) -> &'static str {
+        match self {
+            Self::ScalarTypeDefinition(_) => "ScalarTypeDefinition",
+            Self::ObjectTypeDefinition(_) => "ObjectTypeDefinition",
+            Self::InterfaceTypeDefinition(_) => "InterfaceTypeDefinition",
+            Self::UnionTypeDefinition(_) => "UnionTypeDefinition",
+            Self::EnumTypeDefinition(_) => "EnumTypeDefinition",
+            Self::InputObjectTypeDefinition(_) => "InputObjectTypeDefinition",
+        }
+    }
+
+    /// Returns whether this definition is a scalar, object, interface, union, or enum.
+    #[must_use]
+    pub fn is_output_definition(&self) -> bool {
+        matches!(
+            self,
+            Self::ScalarTypeDefinition(..)
+                | Self::ObjectTypeDefinition(..)
+                | Self::InterfaceTypeDefinition(..)
+                | Self::UnionTypeDefinition(..)
+                | Self::EnumTypeDefinition(..)
+        )
+    }
+
+    /// Returns whether this definition is an input object, scalar, or enum.
+    ///
+    /// [`ScalarTypeDefinition`]: Definition::ScalarTypeDefinition
+    /// [`EnumTypeDefinition`]: Definition::EnumTypeDefinition
+    /// [`InputObjectTypeDefinition`]: Definition::ObjectTypeDefinition
+    #[must_use]
+    pub fn is_input_definition(&self) -> bool {
+        matches!(
+            self,
+            Self::ScalarTypeDefinition(..)
+                | Self::EnumTypeDefinition(..)
+                | Self::InputObjectTypeDefinition(..)
+        )
+    }
+
+    pub fn field(&self, name: &str) -> Option<&FieldDefinition> {
+        match self {
+            Self::ObjectTypeDefinition(def) => def.field(name),
+            Self::InterfaceTypeDefinition(def) => def.field(name),
+            _ => None,
+        }
+    }
+}
+
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct FragmentDefinition {
     pub(crate) name: Name,
@@ -267,8 +311,8 @@ impl FragmentDefinition {
             .collect()
     }
 
-    pub fn ty(&self, db: &dyn DocumentDatabase) -> Option<Definition> {
-        db.find_type_system_definition_by_name(self.name().to_string())
+    pub fn ty(&self, db: &dyn DocumentDatabase) -> Option<TypeDefinition> {
+        db.find_type_definition_by_name(self.name().to_string())
     }
 
     /// Get fragment definition's hir node location.
@@ -536,41 +580,24 @@ impl Type {
     /// Get a reference to location information of the current HIR node.
     pub fn loc(&self) -> Option<&HirNodeLocation> {
         match self {
-            Type::NonNull { ty: _, loc } => loc.as_ref(),
-            Type::List { ty: _, loc } => loc.as_ref(),
-            Type::Named { name: _, loc } => loc.as_ref(),
+            Type::NonNull { loc, .. } | Type::List { loc, .. } | Type::Named { loc, .. } => {
+                loc.as_ref()
+            }
         }
     }
 
-    pub fn ty(&self, db: &dyn DocumentDatabase) -> Option<Definition> {
-        db.find_definition_by_name(self.name())
+    pub fn ty(&self, db: &dyn DocumentDatabase) -> Option<TypeDefinition> {
+        db.find_type_definition_by_name(self.name())
     }
 
     pub fn name(&self) -> String {
         match self {
-            Type::NonNull { ty, loc: _ } => get_name(*ty.clone()),
-            Type::List { ty, loc: _ } => get_name(*ty.clone()),
-            Type::Named { name, loc: _ } => name.to_owned(),
+            Type::NonNull { ty, .. } | Type::List { ty, .. } => ty.name(),
+            Type::Named { name, .. } => name.clone(),
         }
     }
 }
 
-fn get_name(ty: Type) -> String {
-    match ty {
-        Type::NonNull { ty, loc: _ } => match *ty {
-            Type::NonNull { ty, loc: _ } => get_name(*ty),
-            Type::List { ty, loc: _ } => get_name(*ty),
-            Type::Named { name, loc: _ } => name,
-        },
-        Type::List { ty, loc: _ } => match *ty {
-            Type::NonNull { ty, loc: _ } => get_name(*ty),
-            Type::List { ty, loc: _ } => get_name(*ty),
-            Type::Named { name, loc: _ } => name,
-        },
-        Type::Named { name, loc: _ } => name,
-    }
-}
-
 #[derive(Clone, Debug, Hash, PartialEq, Eq)]
 pub struct Directive {
     pub(crate) name: Name,
@@ -1039,7 +1066,7 @@ impl Field {
     /// Get a reference to field's type.
     pub fn ty(&self, db: &dyn DocumentDatabase) -> Option<Type> {
         let def = db
-            .find_type_system_definition_by_name(self.parent_obj.as_ref()?.to_string())?
+            .find_type_definition_by_name(self.parent_obj.as_ref()?.to_string())?
             .field(self.name())?
             .ty()
             .to_owned();
diff --git a/crates/apollo-compiler/src/database/hir_db.rs b/crates/apollo-compiler/src/database/hir_db.rs
index 6176a0a..b969035 100644
--- a/crates/apollo-compiler/src/database/hir_db.rs
+++ b/crates/apollo-compiler/src/database/hir_db.rs
@@ -82,37 +82,43 @@ fn type_system_definitions(db: &dyn HirDatabase) -> Arc<Vec<Definition>> {
         db.scalars()
             .values()
             .cloned()
-            .map(Definition::ScalarTypeDefinition),
+            .map(TypeDefinition::ScalarTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.extend(
         db.object_types()
             .values()
             .cloned()
-            .map(Definition::ObjectTypeDefinition),
+            .map(TypeDefinition::ObjectTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.extend(
         db.interfaces()
             .values()
             .cloned()
-            .map(Definition::InterfaceTypeDefinition),
+            .map(TypeDefinition::InterfaceTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.extend(
         db.unions()
             .values()
             .cloned()
-            .map(Definition::UnionTypeDefinition),
+            .map(TypeDefinition::UnionTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.extend(
         db.enums()
             .values()
             .cloned()
-            .map(Definition::EnumTypeDefinition),
+            .map(TypeDefinition::EnumTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.extend(
         db.input_objects()
             .values()
             .cloned()
-            .map(Definition::InputObjectTypeDefinition),
+            .map(TypeDefinition::InputObjectTypeDefinition)
+            .map(Definition::TypeDefinition),
     );
     definitions.push(Definition::SchemaDefinition(db.schema()));
 
@@ -1128,7 +1134,7 @@ fn field(
 ) -> Option<Arc<Field>> {
     let name = name(field.name(), file_id)?;
     let alias = alias(field.alias());
-    let new_parent_obj = parent_ty(db, name.src(), parent_obj.clone());
+    let new_parent_obj = parent_ty(db, name.src(), parent_obj.as_deref());
     let selection_set = selection_set(db, field.selection_set(), new_parent_obj, file_id);
     let directives = directives(field.directives(), file_id);
     let arguments = arguments(field.arguments(), file_id);
@@ -1146,19 +1152,18 @@ fn field(
     Some(Arc::new(field_data))
 }
 
-fn parent_ty(db: &dyn HirDatabase, field_name: &str, parent_obj: Option<String>) -> Option<String> {
-    if let Some(name) = parent_obj {
-        db.type_system_definitions().iter().find_map(|def| {
-            if let Some(n) = def.name() {
-                if name == n {
-                    return Some(def.field(field_name)?.ty().name());
-                }
+fn parent_ty(db: &dyn HirDatabase, field_name: &str, parent_obj: Option<&str>) -> Option<String> {
+    let name = parent_obj?;
+    // TODO: this should use `db.find_type_definition_by_name(name)`
+    // but can’t (yet?) because `HirDatabase` is not `DocumentDatabase`.
+    db.type_system_definitions()
+        .iter()
+        .find_map(|def| match def {
+            Definition::TypeDefinition(def) if def.name() == name => {
+                Some(def.field(field_name)?.ty().name())
             }
-            None
+            _ => None,
         })
-    } else {
-        None
-    }
 }
 
 fn name(name: Option<ast::Name>, file_id: FileId) -> Option<Name> {
diff --git a/crates/apollo-compiler/src/diagnostics.rs b/crates/apollo-compiler/src/diagnostics.rs
index 51ee33f..d0fdc11 100644
--- a/crates/apollo-compiler/src/diagnostics.rs
+++ b/crates/apollo-compiler/src/diagnostics.rs
@@ -333,7 +333,7 @@ pub struct OutputType {
     // field name
     pub name: String,
     // field type
-    pub ty: String,
+    pub ty: &'static str,
 
     #[source_code]
     pub src: Arc<str>,
@@ -352,7 +352,7 @@ pub struct ObjectType {
     // union member
     pub name: String,
     // actual type
-    pub ty: String,
+    pub ty: &'static str,
 
     #[source_code]
     pub src: Arc<str>,
diff --git a/crates/apollo-compiler/src/lib.rs b/crates/apollo-compiler/src/lib.rs
index 91f7273..7ae2d81 100644
--- a/crates/apollo-compiler/src/lib.rs
+++ b/crates/apollo-compiler/src/lib.rs
@@ -209,7 +209,7 @@ impl Default for ApolloCompiler {
 mod test {
     use std::collections::HashMap;
 
-    use crate::{hir::Definition, ApolloCompiler, DocumentDatabase, HirDatabase};
+    use crate::{hir::TypeDefinition, ApolloCompiler, DocumentDatabase, HirDatabase};
 
     #[test]
     fn it_creates_compiler_from_multiple_sources() {
@@ -923,7 +923,7 @@ scalar Url @specifiedBy(url: "https://tools.ietf.org/html/rfc3986")
                     if let Some(field_ty) = f.ty().ty(&compiler.db) {
                         match field_ty {
                             // get that definition's directives, for example
-                            Definition::ScalarTypeDefinition(scalar) => {
+                            TypeDefinition::ScalarTypeDefinition(scalar) => {
                                 let dir_names: Vec<String> = scalar
                                     .directives()
                                     .iter()
@@ -952,7 +952,7 @@ scalar Url @specifiedBy(url: "https://tools.ietf.org/html/rfc3986")
                             if let Some(input_ty) = val.ty().ty(&compiler.db) {
                                 match input_ty {
                                     // get that definition's directives, for example
-                                    Definition::EnumTypeDefinition(enum_) => {
+                                    TypeDefinition::EnumTypeDefinition(enum_) => {
                                         let dir_names: Vec<String> = enum_
                                             .enum_values_definition()
                                             .iter()
@@ -1003,7 +1003,7 @@ scalar Url @specifiedBy(url: "https://tools.ietf.org/html/rfc3986")
                 .filter_map(|f| {
                     if let Some(field_ty) = f.ty().ty(&compiler.db) {
                         match field_ty {
-                            Definition::ScalarTypeDefinition(scalar) => {
+                            TypeDefinition::ScalarTypeDefinition(scalar) => {
                                 let dir_names: Vec<String> = scalar
                                     .directives()
                                     .iter()
diff --git a/crates/apollo-compiler/src/validation/union_.rs b/crates/apollo-compiler/src/validation/union_.rs
index 069cb3f..b287016 100644
--- a/crates/apollo-compiler/src/validation/union_.rs
+++ b/crates/apollo-compiler/src/validation/union_.rs
@@ -2,7 +2,7 @@ use std::collections::HashMap;
 
 use crate::{
     diagnostics::{ObjectType, UndefinedDefinition, UniqueDefinition},
-    hir::UnionMember,
+    hir::{TypeDefinition, UnionMember},
     ApolloDiagnostic, ValidationDatabase,
 };
 
@@ -36,19 +36,18 @@ pub fn check(db: &dyn ValidationDatabase) -> Vec<ApolloDiagnostic> {
                 seen.insert(name, union_member);
             }
 
-            // Union member must be defined.
-            let union_member_type = db
-                .upcast()
-                .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.source_code(union_member.loc().file_id()),
-                    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() {
+            match db.upcast().find_type_definition_by_name(name.to_string()) {
+                None => {
+                    // Union member must be defined.
+                    diagnostics.push(ApolloDiagnostic::UndefinedDefinition(UndefinedDefinition {
+                        ty: name.into(),
+                        src: db.source_code(union_member.loc().file_id()),
+                        definition: (offset, len).into(),
+                    }))
+                }
+                Some(TypeDefinition::ObjectTypeDefinition { .. }) => {} // good
+                Some(ty) => {
+                    // Union member must be of object type.
                     diagnostics.push(ApolloDiagnostic::ObjectType(ObjectType {
                         name: name.into(),
                         ty: ty.ty(),
diff --git a/crates/apollo-compiler/src/validation/validation_db.rs b/crates/apollo-compiler/src/validation/validation_db.rs
index 3740e5d..4de92db 100644
--- a/crates/apollo-compiler/src/validation/validation_db.rs
+++ b/crates/apollo-compiler/src/validation/validation_db.rs
@@ -238,6 +238,7 @@ pub fn check_db_definitions(
 
         // TODO: validate extensions too
         use hir::Definition::*;
+        use hir::TypeDefinition::*;
         match definition {
             OperationDefinition(def) => {
                 diagnostics.extend(db.check_selection_set(def.selection_set().clone()));
@@ -248,22 +249,24 @@ pub fn check_db_definitions(
             DirectiveDefinition(def) => {
                 diagnostics.extend(db.check_directive_definition(def.clone()));
             }
-            ScalarTypeDefinition(_def) => {}
-            ObjectTypeDefinition(def) => {
-                diagnostics.extend(db.check_object_type_definition(def.clone()));
-            }
-            InterfaceTypeDefinition(def) => {
-                diagnostics.extend(db.check_interface_type_definition(def.clone()));
-            }
-            UnionTypeDefinition(def) => {
-                diagnostics.extend(db.check_union_type_definition(def.clone()));
-            }
-            EnumTypeDefinition(def) => {
-                diagnostics.extend(db.check_enum_type_definition(def.clone()));
-            }
-            InputObjectTypeDefinition(def) => {
-                diagnostics.extend(db.check_input_object_type_definition(def.clone()));
-            }
+            TypeDefinition(def) => match def {
+                ScalarTypeDefinition(_def) => {}
+                ObjectTypeDefinition(def) => {
+                    diagnostics.extend(db.check_object_type_definition(def.clone()));
+                }
+                InterfaceTypeDefinition(def) => {
+                    diagnostics.extend(db.check_interface_type_definition(def.clone()));
+                }
+                UnionTypeDefinition(def) => {
+                    diagnostics.extend(db.check_union_type_definition(def.clone()));
+                }
+                EnumTypeDefinition(def) => {
+                    diagnostics.extend(db.check_enum_type_definition(def.clone()));
+                }
+                InputObjectTypeDefinition(def) => {
+                    diagnostics.extend(db.check_input_object_type_definition(def.clone()));
+                }
+            },
             SchemaDefinition(def) => {
                 diagnostics.extend(db.check_schema_definition(def.clone()));
             }


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 d425eb9..a4fb2b5 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
@@ -13,17 +13,16 @@
             },
         },
     ),
-    OutputType(
-        OutputType {
-            name: "permissions",
-            ty: "DirectiveDefinition",
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Auth",
             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(
-                    169,
+                    182,
                 ),
                 length: SourceOffset(
-                    20,
+                    4,
                 ),
             },
         },
@@ -43,17 +42,16 @@
             },
         },
     ),
-    OutputType(
-        OutputType {
-            name: "main",
-            ty: "OperationDefinition",
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "mainPage",
             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(
-                    212,
+                    218,
                 ),
                 length: SourceOffset(
-                    15,
+                    8,
                 ),
             },
         },
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 36b8043..c692860 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
@@ -13,17 +13,16 @@
             },
         },
     ),
-    OutputType(
-        OutputType {
-            name: "permissions",
-            ty: "DirectiveDefinition",
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "Auth",
             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(
-                    201,
+                    214,
                 ),
                 length: SourceOffset(
-                    20,
+                    4,
                 ),
             },
         },
@@ -43,17 +42,16 @@
             },
         },
     ),
-    OutputType(
-        OutputType {
-            name: "main",
-            ty: "OperationDefinition",
+    UndefinedDefinition(
+        UndefinedDefinition {
+            ty: "mainPage",
             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(
-                    244,
+                    250,
                 ),
                 length: SourceOffset(
-                    15,
+                    8,
                 ),
             },
         },