Add newtype for Union::name

Summary: : Create a new type wrapper struct UnionName for Union.name type and updates callers. Reviewed By: captbaritone Differential Revision: D47526520 fbshipit-source-id: 6ee8973b92c53ae80499a2acb2cf636a2e0958af
facebook · Jul 19, 2023 · a55a3fa · a55a3fa
1 parent 64b707d
commit a55a3fa
Show file tree

Hide file tree

Showing 15 changed files with 60 additions and 22 deletions.
diff --git a/compiler/crates/common/src/lib.rs b/compiler/crates/common/src/lib.rs
@@ -54,6 +54,7 @@ pub use named_item::Named;
 pub use named_item::NamedItem;
 pub use named_item::ObjectName;
 pub use named_item::ScalarName;
+pub use named_item::UnionName;
 pub use perf_logger::NoopPerfLogger;
 pub use perf_logger::NoopPerfLoggerEvent;
 pub use perf_logger::PerfLogEvent;

diff --git a/compiler/crates/common/src/named_item.rs b/compiler/crates/common/src/named_item.rs
@@ -136,3 +136,25 @@ impl fmt::Display for InterfaceName {
 }
 
 impl_lookup!(InterfaceName);
+
+#[derive(
+    Clone,
+    Copy,
+    Debug,
+    Eq,
+    PartialEq,
+    Ord,
+    PartialOrd,
+    Hash,
+    Serialize,
+    Deserialize
+)]
+pub struct UnionName(pub StringKey);
+
+impl fmt::Display for UnionName {
+    fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(fmt, "{}", self.0)
+    }
+}
+
+impl_lookup!(UnionName);
diff --git a/compiler/crates/relay-lsp/src/search_schema_items/mod.rs b/compiler/crates/relay-lsp/src/search_schema_items/mod.rs
@@ -72,7 +72,7 @@ pub(crate) fn on_search_schema_items(
         &filter,
     );
     let unions = filter_and_transform_items(
-        schema.unions().map(|u| u.name.item),
+        schema.unions().map(|u| u.name.item.0),
         &schema_documentation,
         &filter,
     );

diff --git a/compiler/crates/schema-print/src/print_schema.rs b/compiler/crates/schema-print/src/print_schema.rs
@@ -303,7 +303,7 @@ impl<'schema, 'writer, 'curent_writer> Printer<'schema, 'writer> {
 
     fn print_union(&mut self, id: UnionID) -> FmtResult {
         let union_ = self.schema.union(id);
-        write!(self.writer(), "union {}", union_.name.item)?;
+        write!(self.writer(), "union {}", union_.name.item.0)?;
         self.print_directive_values(&union_.directives)?;
         if !union_.members.is_empty() {
             let union_members = union_

diff --git a/compiler/crates/schema-validate/src/errors.rs b/compiler/crates/schema-validate/src/errors.rs
@@ -8,6 +8,7 @@
 use common::ArgumentName;
 use common::InterfaceName;
 use common::ObjectName;
+use common::UnionName;
 use intern::string_key::StringKey;
 use schema::Type;
 use schema::TypeReference;
@@ -73,8 +74,8 @@ pub enum SchemaValidationError {
     )]
     MissingRequiredArgument(StringKey, StringKey, ArgumentName, InterfaceName),
 
-    #[error("Union type must define one or more member types.")]
-    UnionWithNoMembers(StringKey),
+    #[error("Union type {0} must define one or more member types.")]
+    UnionWithNoMembers(UnionName),
 
     #[error("Union can only include member {0} once.")]
     DuplicateMember(ObjectName),

diff --git a/compiler/crates/schema-validate/src/lib.rs b/compiler/crates/schema-validate/src/lib.rs
@@ -252,7 +252,7 @@ impl<'schema> ValidationContext<'schema> {
 
     fn validate_union_members(&mut self, id: UnionID) {
         let union = self.schema.union(id);
-        let context = ValidationContextType::TypeNode(union.name.item);
+        let context = ValidationContextType::TypeNode(union.name.item.0);
         if union.members.is_empty() {
             self.report_error(
                 SchemaValidationError::UnionWithNoMembers(union.name.item),

diff --git a/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_object.expected b/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_object.expected
@@ -66,7 +66,7 @@ type Fur {
 Type EmptyFurType with definition:
 	union EmptyFurType
 had errors:
-	* Union type must define one or more member types.
+	* Union type EmptyFurType must define one or more member types.
 
 Type Human with definition:
 	type Human implements Hominid @fetchable(field_name: "id") {

diff --git a/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_union.expected b/compiler/crates/schema-validate/tests/validate_schema/fixtures/validate_union.expected
@@ -54,7 +54,7 @@ type Fur {
 Type EmptyFurType with definition:
 	union EmptyFurType
 had errors:
-	* Union type must define one or more member types.
+	* Union type EmptyFurType must define one or more member types.
 
 Type InvalidFurType with definition:
 	union InvalidFurType = Hair | Fur | Hair

diff --git a/compiler/crates/schema/src/definitions/mod.rs b/compiler/crates/schema/src/definitions/mod.rs
@@ -21,6 +21,7 @@ use common::Named;
 use common::NamedItem;
 use common::ObjectName;
 use common::ScalarName;
+use common::UnionName;
 use common::WithLocation;
 use graphql_syntax::ConstantValue;
 use graphql_syntax::DirectiveLocation;
@@ -355,7 +356,7 @@ pub struct Enum {
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct Union {
-    pub name: WithLocation<StringKey>,
+    pub name: WithLocation<UnionName>,
     pub is_extension: bool,
     pub members: Vec<ObjectID>,
     pub directives: Vec<DirectiveValue>,
@@ -553,7 +554,7 @@ impl_named_for_with_location!(Object, ObjectName);
 impl_named_for_with_location!(Field, StringKey);
 impl_named_for_with_location!(InputObject, InputObjectName);
 impl_named_for_with_location!(Interface, InterfaceName);
-impl_named_for_with_location!(Union, StringKey);
+impl_named_for_with_location!(Union, UnionName);
 impl_named_for_with_location!(Scalar, ScalarName);
 impl_named_for_with_location!(Enum, EnumName);
 

diff --git a/compiler/crates/schema/src/flatbuffer/mod.rs b/compiler/crates/schema/src/flatbuffer/mod.rs
@@ -18,6 +18,7 @@ use common::InterfaceName;
 use common::ObjectName;
 use common::ScalarName;
 use common::Span;
+use common::UnionName;
 use common::WithLocation;
 use flatbuffers::ForwardsUOffset;
 use flatbuffers::Vector;
@@ -291,7 +292,7 @@ impl<'fb> FlatBufferSchema<'fb> {
 
     fn parse_union(&self, id: UnionID) -> Option<Union> {
         let union = self.unions.get(id.0.try_into().unwrap());
-        let name = union.name()?.intern();
+        let name = UnionName(union.name()?.intern());
         let parsed_union = Union {
             name: WithLocation::generated(name),
             is_extension: union.is_extension(),

diff --git a/compiler/crates/schema/src/flatbuffer/serialize.rs b/compiler/crates/schema/src/flatbuffer/serialize.rs
@@ -339,7 +339,7 @@ impl<'fb, 'schema> Serializer<'fb, 'schema> {
         let union = self.schema.union(id);
         let name = union.name;
         let idx = self.unions.len();
-        self.add_to_type_map(idx, schema_flatbuffer::TypeKind::Union, name.item);
+        self.add_to_type_map(idx, schema_flatbuffer::TypeKind::Union, name.item.0);
         self.unions.push(schema_flatbuffer::Union::create(
             &mut self.bldr,
             &schema_flatbuffer::UnionArgs::default(),

diff --git a/compiler/crates/schema/src/flatbuffer/wrapper.rs b/compiler/crates/schema/src/flatbuffer/wrapper.rs
@@ -313,7 +313,7 @@ impl Schema for SchemaWrapper {
             Type::Interface(id) => self.interface(id).name.item.0,
             Type::Object(id) => self.object(id).name.item.0,
             Type::Scalar(id) => self.scalar(id).name.item.0,
-            Type::Union(id) => self.union(id).name.item,
+            Type::Union(id) => self.union(id).name.item.0,
         }
     }
 

diff --git a/compiler/crates/schema/src/in_memory/mod.rs b/compiler/crates/schema/src/in_memory/mod.rs
@@ -19,6 +19,7 @@ use common::Location;
 use common::ObjectName;
 use common::ScalarName;
 use common::SourceLocationKey;
+use common::UnionName;
 use common::WithLocation;
 use graphql_syntax::*;
 use intern::string_key::Intern;
@@ -148,7 +149,7 @@ impl Schema for InMemorySchema {
             Type::Interface(id) => self.interfaces[id.as_usize()].name.item.0,
             Type::Object(id) => self.objects[id.as_usize()].name.item.0,
             Type::Scalar(id) => self.scalars[id.as_usize()].name.item.0,
-            Type::Union(id) => self.unions[id.as_usize()].name.item,
+            Type::Union(id) => self.unions[id.as_usize()].name.item.0,
         }
     }
 
@@ -456,13 +457,13 @@ impl InMemorySchema {
     }
 
     pub fn add_union(&mut self, union: Union) -> DiagnosticsResult<UnionID> {
-        if self.type_map.contains_key(&union.name.item) {
-            return todo_add_location(SchemaError::DuplicateType(union.name.item));
+        if self.type_map.contains_key(&union.name.item.0) {
+            return todo_add_location(SchemaError::DuplicateType(union.name.item.0));
         }
         let index: u32 = self.unions.len().try_into().unwrap();
         let name = union.name.item;
         self.unions.push(union);
-        self.type_map.insert(name, Type::Union(UnionID(index)));
+        self.type_map.insert(name.0, Type::Union(UnionID(index)));
         Ok(UnionID(index))
     }
 
@@ -635,7 +636,7 @@ impl InMemorySchema {
             ));
         }
         self.type_map.remove(&self.get_type_name(Type::Union(id)));
-        self.type_map.insert(union.name.item, Type::Union(id));
+        self.type_map.insert(union.name.item.0, Type::Union(id));
         self.unions[id.as_usize()] = union;
         Ok(())
     }
@@ -1288,7 +1289,10 @@ impl InMemorySchema {
                     .collect::<DiagnosticsResult<Vec<_>>>()?;
                 let directives = self.build_directive_values(directives);
                 self.unions.push(Union {
-                    name: WithLocation::new(Location::new(*location_key, name.span), name.value),
+                    name: WithLocation::new(
+                        Location::new(*location_key, name.span),
+                        UnionName(name.value),
+                    ),
                     is_extension,
                     members,
                     directives,

diff --git a/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected b/compiler/crates/schema/tests/build_schema/fixtures/directives-for-external-types.expected
@@ -1006,7 +1006,9 @@ Text Schema:Schema {
     Union {
         name: WithLocation {
             location: <generated>:993:1000,
-            item: "Address",
+            item: UnionName(
+                "Address",
+            ),
         },
         is_extension: false,
         members: [
@@ -1885,7 +1887,9 @@ unions: [
     Union {
         name: WithLocation {
             location: <generated>:0:0,
-            item: "Address",
+            item: UnionName(
+                "Address",
+            ),
         },
         is_extension: false,
         members: [

diff --git a/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected b/compiler/crates/schema/tests/build_schema/fixtures/kitchen-sink.expected
@@ -909,7 +909,9 @@ Text Schema:Schema {
     Union {
         name: WithLocation {
             location: <generated>:431:436,
-            item: "Actor",
+            item: UnionName(
+                "Actor",
+            ),
         },
         is_extension: false,
         members: [
@@ -1616,7 +1618,9 @@ unions: [
     Union {
         name: WithLocation {
             location: <generated>:0:0,
-            item: "Actor",
+            item: UnionName(
+                "Actor",
+            ),
         },
         is_extension: false,
         members: [