skip explicit StringKey::lookup in some cases

Reviewed By: tyao1

Differential Revision: D25999113

fbshipit-source-id: 7894d9f4db99995b168cac3054f396d8a30338b9

compiler/crates/graphql-ir/src/build.rs

@@ -615,16 +615,8 @@ impl<'schema, 'signatures> Builder<'schema, 'signatures> {
             return Err(vec![Diagnostic::error(
                 ValidationMessage::InvalidFragmentSpreadType {
                     fragment_name: spread.name.value,
-                    parent_type: self
-                        .schema
-                        .get_type_name(parent_type.inner())
-                        .lookup()
-                        .to_owned(),
-                    type_condition: self
-                        .schema
-                        .get_type_name(signature.type_condition)
-                        .lookup()
-                        .to_owned(),
+                    parent_type: self.schema.get_type_name(parent_type.inner()),
+                    type_condition: self.schema.get_type_name(signature.type_condition),
                 },
                 self.location.with_span(spread.span),
             )]);
@@ -751,16 +743,8 @@ impl<'schema, 'signatures> Builder<'schema, 'signatures> {
                 // no possible overlap
                 return Err(vec![Diagnostic::error(
                     ValidationMessage::InvalidInlineFragmentTypeCondition {
-                        parent_type: self
-                            .schema
-                            .get_type_name(parent_type.inner())
-                            .lookup()
-                            .to_owned(),
-                        type_condition: self
-                            .schema
-                            .get_type_name(type_condition)
-                            .lookup()
-                            .to_owned(),
+                        parent_type: self.schema.get_type_name(parent_type.inner()),
+                        type_condition: self.schema.get_type_name(type_condition),
                     },
                     self.location.with_span(fragment.span),
                 )]);
@@ -1373,7 +1357,7 @@ impl<'schema, 'signatures> Builder<'schema, 'signatures> {
             Ok(Value::Object(fields?))
         } else {
             let mut missing: Vec<StringKey> = required_fields.into_iter().map(|x| x).collect();
-            missing.sort_by_key(|x| x.lookup());
+            missing.sort();
             Err(vec![Diagnostic::error(
                 ValidationMessage::MissingRequiredFields(missing, type_definition.name),
                 self.location.with_span(object.span),
@@ -1502,7 +1486,7 @@ impl<'schema, 'signatures> Builder<'schema, 'signatures> {
             Ok(ConstantValue::Object(fields?))
         } else {
             let mut missing: Vec<StringKey> = required_fields.into_iter().map(|x| x).collect();
-            missing.sort_by_key(|x| x.lookup());
+            missing.sort();
             Err(vec![Diagnostic::error(
                 ValidationMessage::MissingRequiredFields(missing, type_definition.name),
                 self.location.with_span(object.span),

compiler/crates/graphql-ir/src/errors.rs

@@ -123,16 +123,16 @@ pub enum ValidationMessage {
         "Invalid type '{type_condition}' in inline fragment, this type can never occur for parent type '{parent_type}'"
     )]
     InvalidInlineFragmentTypeCondition {
-        parent_type: String,
-        type_condition: String,
+        parent_type: StringKey,
+        type_condition: StringKey,
     },
     #[error(
         "Invalid fragment spread '{fragment_name}', the type of this fragment ('{type_condition}') can never occur for parent type '{parent_type}'"
     )]
     InvalidFragmentSpreadType {
         fragment_name: StringKey,
-        parent_type: String,
-        type_condition: String,
+        parent_type: StringKey,
+        type_condition: StringKey,
     },
     #[error("Directive '{0}' not supported in this location")]
     InvalidDirectiveUsageUnsupportedLocation(StringKey),

compiler/crates/graphql-text-printer/src/print_full_operation.rs

@@ -38,7 +38,7 @@ impl<'s> OperationPrinter<'s> {
         self.visit_operation(operation);
         let mut fragments: Vec<(StringKey, Arc<FragmentDefinition>)> =
             self.reachable_fragments.drain().collect();
-        fragments.sort_unstable_by(|a, b| a.0.lookup().cmp(b.0.lookup()));
+        fragments.sort_unstable_by_key(|(name, _)| *name);
         for (_, fragment) in fragments {
             result.push_str("\n\n");
             result.push_str(self.print_fragment(&fragment));

compiler/crates/relay-codegen/src/build_ast.rs

@@ -1225,7 +1225,7 @@ impl<'schema, 'builder> CodegenBuilder<'schema, 'builder> {
 
     fn build_arguments(&mut self, arguments: &[Argument]) -> Option<AstKey> {
         let mut sorted_args: Vec<&Argument> = arguments.iter().map(|arg| arg).collect();
-        sorted_args.sort_unstable_by_key(|arg| arg.name.item.lookup());
+        sorted_args.sort_unstable_by_key(|arg| arg.name.item);
 
         let args = sorted_args
             .into_iter()
@@ -1391,7 +1391,7 @@ impl<'schema, 'builder> CodegenBuilder<'schema, 'builder> {
             }
             ConstantValue::Object(val_object) => {
                 let mut sorted_val_object: Vec<&_> = val_object.iter().collect();
-                sorted_val_object.sort_unstable_by_key(|arg| arg.name.item.lookup());
+                sorted_val_object.sort_unstable_by_key(|arg| arg.name.item);
 
                 let json_values = sorted_val_object
                     .into_iter()

compiler/crates/relay-codegen/src/printer.rs

@@ -225,7 +225,7 @@ impl<'b> JSONPrinter<'b> {
                     for ObjectEntry { key, value } in object {
                         f.push('\n');
                         print_indentation(f, next_indent);
-                        write!(f, "\"{}\": ", key.lookup()).unwrap();
+                        write!(f, "\"{}\": ", key).unwrap();
                         self.print_primitive(f, value, next_indent, is_dedupe_var)
                             .unwrap();
                         f.push(',');

compiler/crates/relay-compiler/tests/compile_relay_artifacts/mod.rs

@@ -64,7 +64,7 @@ pub fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String> {
 
     let mut operations: Vec<&std::sync::Arc<OperationDefinition>> =
         programs.normalization.operations().collect();
-    operations.sort_by(|a, b| a.name.item.lookup().cmp(&b.name.item.lookup()));
+    operations.sort_by_key(|operation| operation.name.item);
     let result = operations
         .into_iter()
         .map(|operation| {
@@ -105,7 +105,7 @@ pub fn transform_fixture(fixture: &Fixture<'_>) -> Result<String, String> {
         .chain({
             let mut fragments: Vec<&std::sync::Arc<FragmentDefinition>> =
                 programs.reader.fragments().collect();
-            fragments.sort_by(|a, b| a.name.item.lookup().cmp(&b.name.item.lookup()));
+            fragments.sort_by_key(|fragment| fragment.name.item);
             fragments
                 .into_iter()
                 .map(|fragment| print_fragment(&schema, fragment))

compiler/crates/relay-transforms/src/react_flight.rs

@@ -206,8 +206,8 @@ impl<'s> ReactFlightTransform<'s> {
     // Generate a metadata directive recording which server components were reachable
     // from the visited IR nodes
     fn generate_flight_local_flight_components_metadata_directive(&self) -> Directive {
-        let mut components: Vec<StringKey> = self.local_components.iter().cloned().collect();
-        components.sort_by_key(|s| s.lookup());
+        let mut components: Vec<StringKey> = self.local_components.iter().copied().collect();
+        components.sort();
         Directive {
             name: WithLocation::generated(*REACT_FLIGHT_LOCAL_COMPONENTS_METADATA_KEY),
             arguments: vec![Argument {
@@ -222,8 +222,8 @@ impl<'s> ReactFlightTransform<'s> {
     // Generate a server directive recording which server components were *transitively* reachable
     // from the visited IR nodes
     fn generate_flight_transitive_flight_components_server_directive(&self) -> Directive {
-        let mut components: Vec<StringKey> = self.transitive_components.iter().cloned().collect();
-        components.sort_by_key(|s| s.lookup());
+        let mut components: Vec<StringKey> = self.transitive_components.iter().copied().collect();
+        components.sort();
         Directive {
             name: WithLocation::generated(*REACT_FLIGHT_TRANSITIVE_COMPONENTS_DIRECTIVE_NAME),
             arguments: vec![Argument {

compiler/crates/relay-transforms/src/refetchable_fragment/fetchable_query_generator.rs

@@ -31,11 +31,8 @@ fn build_refetch_operation(
         let identifier_field_id = get_identifier_field_id(fragment, schema, identifier_field_name)?;
 
         let query_type = schema.query_type().unwrap();
-        let fetch_field_name = format!(
-            "fetch__{}",
-            schema.get_type_name(fragment.type_condition).lookup()
-        )
-        .intern();
+        let fetch_field_name =
+            format!("fetch__{}", schema.get_type_name(fragment.type_condition)).intern();
         let (fetch_field_id, id_arg) =
             get_fetch_field_id_and_id_arg(fragment, schema, query_type, fetch_field_name)?;
 

compiler/crates/relay-transforms/src/validations/validate_connections.rs

@@ -403,8 +403,8 @@ impl<'s> ConnectionValidation<'s> {
             Some((arg, key_val)) => match key_val {
                 ConstantValue::String(string_val) => {
                     let field_alias_or_name = match connection_field.alias {
-                        Some(alias) => alias.item.lookup(),
-                        None => connection_schema_field.name.lookup(),
+                        Some(alias) => alias.item,
+                        None => connection_schema_field.name,
                     };
                     let postfix = format!("_{}", field_alias_or_name);
                     if !string_val.lookup().ends_with(postfix.as_str()) {

compiler/crates/schema-diff/tests/diff_schema_tests.rs

@@ -912,16 +912,16 @@ fn sort_change(change: &mut SchemaChange) {
                     ref mut removed,
                     ..
                 } => {
-                    added.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
-                    removed.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
+                    added.sort_by_key(|item| item.name);
+                    removed.sort_by_key(|item| item.name);
                 }
                 DefinitionChange::InterfaceChanged {
                     ref mut added,
                     ref mut removed,
                     ..
                 } => {
-                    added.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
-                    removed.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
+                    added.sort_by_key(|item| item.name);
+                    removed.sort_by_key(|item| item.name);
                 }
                 DefinitionChange::ObjectChanged {
                     ref mut added,
@@ -931,9 +931,9 @@ fn sort_change(change: &mut SchemaChange) {
                     ref mut interfaces_removed,
                     ..
                 } => {
-                    added.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
-                    removed.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
-                    changed.sort_by(|a, b| a.name.lookup().cmp(b.name.lookup()));
+                    added.sort_by_key(|item| item.name);
+                    removed.sort_by_key(|item| item.name);
+                    changed.sort_by_key(|item| item.name);
                     interfaces_added.sort();
                     interfaces_removed.sort();
                 }

compiler/crates/schema/src/sdl/mod.rs

@@ -172,7 +172,7 @@ impl Schema for SDLSchema {
             Type::Union(_) => return None,
             _ => panic!(
                 "Cannot get field {} on type '{:?}', this type does not have fields",
-                name.lookup(),
+                name,
                 self.get_type_name(parent_type)
             ),
         };