Avoid creating and throwing away anyhow::Errors in ObjectShape::shape_of (#41729)

anyhow::Error takes a backtrace on construction which is quite expensive.
GitOrigin-RevId: b26c197980df29c1466b8ce3ef9ad89350590a73

Author: goffrie

crates/convex/sync_types/src/identifier.rs

@@ -1,5 +1,8 @@
 use std::{
-    fmt::Display,
+    fmt::{
+        self,
+        Display,
+    },
     ops::Deref,
     str::FromStr,
 };
@@ -28,11 +31,11 @@ pub const IDENTIFIER_REQUIREMENTS: &str =
 /// [^3]: <https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5B%3AXID_START%3A%5D&g=&i=>
 /// [^4]: <https://util.unicode.org/UnicodeJsps/list-unicodeset.jsp?a=%5B%3AXID_CONTINUE%3A%5D&g=&i=>
 pub fn check_valid_identifier(s: &str) -> anyhow::Result<()> {
-    check_valid_identifier_inner(s).map_err(|e| anyhow::anyhow!(e))
+    check_valid_identifier_inner(s, |e| anyhow::anyhow!(e.to_string()))
 }
 
 pub fn is_valid_identifier(s: &str) -> bool {
-    check_valid_identifier_inner(s).is_ok()
+    check_valid_identifier_inner(s, |_| ()).is_ok()
 }
 
 #[derive(Clone, Debug, PartialEq, PartialOrd, Ord, Eq, Hash)]
@@ -77,38 +80,41 @@ impl Deref for Identifier {
     }
 }
 
-fn check_valid_identifier_inner(s: &str) -> Result<(), String> {
+fn check_valid_identifier_inner<E>(
+    s: &str,
+    error: impl FnOnce(fmt::Arguments<'_>) -> E,
+) -> Result<(), E> {
     let mut chars = s.chars();
     match chars.next() {
         Some(c) if c.is_ascii_alphabetic() => (),
         Some('_') => (),
         Some(c) => {
-            return Err(format!(
+            return Err(error(format_args!(
                 "Invalid first character '{c}' in {s}: Identifiers must start with an alphabetic \
                  character or underscore"
-            ))
+            )))
         },
-        None => return Err(format!("Identifier cannot be empty")),
+        None => return Err(error(format_args!("Identifier cannot be empty"))),
     };
     for c in chars {
         if !c.is_ascii_alphanumeric() && c != '_' {
-            return Err(format!(
+            return Err(error(format_args!(
                 "Identifier {s} has invalid character '{c}': Identifiers can only contain \
                  alphanumeric characters or underscores"
-            ));
+            )));
         }
     }
     if s.len() > MAX_IDENTIFIER_LEN {
-        return Err(format!(
+        return Err(error(format_args!(
             "Identifier is too long ({} > maximum {})",
             s.len(),
             MAX_IDENTIFIER_LEN
-        ));
+        )));
     }
     if s.chars().all(|c| c == '_') {
-        return Err(format!(
+        return Err(error(format_args!(
             "Identifier {s} cannot have exclusively underscores"
-        ));
+        )));
     }
     Ok(())
 }

crates/shape_inference/src/object.rs

@@ -4,6 +4,7 @@ use std::{
 };
 
 use value::{
+    identifier::is_valid_identifier,
     ConvexObject,
     IdentifierFieldName,
 };
@@ -75,22 +76,25 @@ impl<C: ShapeConfig> ObjectShape<C, u64> {
     }
 
     pub fn shape_of(object: &ConvexObject) -> CountedShapeEnum<C> {
-        if object.len() <= C::MAX_OBJECT_FIELDS {
-            if let Ok(fields) = object
+        // N.B.: check is_valid_identifier separately to avoid creating
+        // anyhow::Errors, which is expensive
+        if object.len() <= C::MAX_OBJECT_FIELDS
+            && object.iter().all(|(f, _)| is_valid_identifier(f))
+        {
+            let field_shapes: BTreeMap<_, _> = object
                 .iter()
-                .map(|(f, v)| Ok((IdentifierFieldName::try_from(f.clone())?, v)))
-                .collect::<anyhow::Result<BTreeMap<_, _>>>()
-            {
-                let mut field_shapes = BTreeMap::new();
-                for (field_name, value) in fields {
-                    let field = ObjectField {
-                        value_shape: Shape::shape_of(value),
-                        optional: false,
-                    };
-                    field_shapes.insert(field_name.clone(), field);
-                }
-                return ShapeEnum::Object(Self::new(field_shapes));
-            }
+                .map(|(field_name, value)| {
+                    (
+                        IdentifierFieldName::try_from(field_name.clone())
+                            .expect("checked is_valid_identifier above"),
+                        ObjectField {
+                            value_shape: Shape::shape_of(value),
+                            optional: false,
+                        },
+                    )
+                })
+                .collect();
+            return ShapeEnum::Object(Self::new(field_shapes));
         }
         let mut fields = UnionBuilder::new();
         let mut values = UnionBuilder::new();