Avoid some clones constructing error enum (#1384)

Signed-off-by: John Kastner <jkastner@amazon.com>
cedar-policy · Dec 27, 2024 · 78abd25 · 78abd25
1 parent 6f28be2
commit 78abd25
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 22 deletions.
diff --git a/cedar-policy-validator/src/cedar_schema/err.rs b/cedar-policy-validator/src/cedar_schema/err.rs
@@ -437,15 +437,15 @@ pub enum ToJsonSchemaError {
 }
 
 impl ToJsonSchemaError {
-    pub(crate) fn duplicate_context(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_context(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicateContext(DuplicateContext {
             name: name.to_smolstr(),
             loc1,
             loc2,
         })
     }
 
-    pub(crate) fn duplicate_decls(decl: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_decls(decl: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicateDeclarations(DuplicateDeclarations {
             decl: decl.to_smolstr(),
             loc1,
@@ -454,7 +454,7 @@ impl ToJsonSchemaError {
     }
 
     pub(crate) fn duplicate_namespace(
-        namespace_id: impl ToSmolStr,
+        namespace_id: &impl ToSmolStr,
         loc1: Option<Loc>,
         loc2: Option<Loc>,
     ) -> Self {
@@ -465,7 +465,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn duplicate_principal(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_principal(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicatePrincipalOrResource(DuplicatePrincipalOrResource {
             name: name.to_smolstr(),
             kind: PR::Principal,
@@ -474,7 +474,7 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn duplicate_resource(name: impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
+    pub(crate) fn duplicate_resource(name: &impl ToSmolStr, loc1: Loc, loc2: Loc) -> Self {
         Self::DuplicatePrincipalOrResource(DuplicatePrincipalOrResource {
             name: name.to_smolstr(),
             kind: PR::Resource,
@@ -483,30 +483,30 @@ impl ToJsonSchemaError {
         })
     }
 
-    pub(crate) fn no_principal(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_principal(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Principal,
             name: name.to_smolstr(),
             loc,
         })
     }
 
-    pub(crate) fn no_resource(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn no_resource(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::NoPrincipalOrResource(NoPrincipalOrResource {
             kind: PR::Resource,
             name: name.to_smolstr(),
             loc,
         })
     }
 
-    pub(crate) fn reserved_name(name: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn reserved_name(name: &impl ToSmolStr, loc: Loc) -> Self {
         Self::ReservedName(ReservedName {
             name: name.to_smolstr(),
             loc,
         })
     }
 
-    pub(crate) fn reserved_keyword(keyword: impl ToSmolStr, loc: Loc) -> Self {
+    pub(crate) fn reserved_keyword(keyword: &impl ToSmolStr, loc: Loc) -> Self {
         Self::ReservedSchemaKeyword(ReservedSchemaKeyword {
             keyword: keyword.to_smolstr(),
             loc,

diff --git a/cedar-policy-validator/src/cedar_schema/to_json_schema.rs b/cedar-policy-validator/src/cedar_schema/to_json_schema.rs
@@ -197,7 +197,7 @@ impl TryFrom<Annotated<Namespace>> for json_schema::NamespaceDefinition<RawName>
                 let id = UnreservedId::try_from(decl.data.name.node)
                     .map_err(|e| ToJsonSchemaError::reserved_name(e.name(), name_loc.clone()))?;
                 let ctid = json_schema::CommonTypeId::new(id)
-                    .map_err(|e| ToJsonSchemaError::reserved_keyword(e.id, name_loc))?;
+                    .map_err(|e| ToJsonSchemaError::reserved_keyword(&e.id, name_loc))?;
                 Ok((
                     ctid,
                     CommonType {
@@ -273,7 +273,7 @@ fn convert_app_decls(
             } => match context {
                 Some(existing_context) => {
                     return Err(ToJsonSchemaError::duplicate_context(
-                        name.clone(),
+                        name,
                         existing_context.loc,
                         loc,
                     )
@@ -300,7 +300,7 @@ fn convert_app_decls(
             } => match principal_types {
                 Some(existing_tys) => {
                     return Err(ToJsonSchemaError::duplicate_principal(
-                        name.clone(),
+                        name,
                         existing_tys.loc,
                         loc,
                     )
@@ -325,12 +325,9 @@ fn convert_app_decls(
                 loc,
             } => match resource_types {
                 Some(existing_tys) => {
-                    return Err(ToJsonSchemaError::duplicate_resource(
-                        name.clone(),
-                        existing_tys.loc,
-                        loc,
-                    )
-                    .into());
+                    return Err(
+                        ToJsonSchemaError::duplicate_resource(name, existing_tys.loc, loc).into(),
+                    );
                 }
                 None => {
                     resource_types = Some(Node::with_source_loc(
@@ -344,10 +341,10 @@ fn convert_app_decls(
     Ok(json_schema::ApplySpec {
         resource_types: resource_types
             .map(|node| node.node)
-            .ok_or_else(|| ToJsonSchemaError::no_resource(name.clone(), name_loc.clone()))?,
+            .ok_or_else(|| ToJsonSchemaError::no_resource(&name, name_loc.clone()))?,
         principal_types: principal_types
             .map(|node| node.node)
-            .ok_or_else(|| ToJsonSchemaError::no_principal(name.clone(), name_loc.clone()))?,
+            .ok_or_else(|| ToJsonSchemaError::no_principal(&name, name_loc.clone()))?,
         context: context.map(|c| c.node).unwrap_or_default(),
     })
 }
@@ -524,7 +521,7 @@ where
     for (key, node) in i {
         match map.entry(key.clone()) {
             Entry::Occupied(entry) => Err(ToJsonSchemaError::duplicate_decls(
-                key,
+                &key,
                 entry.get().loc.clone(),
                 node.loc,
             )),
@@ -614,7 +611,7 @@ fn update_namespace_record(
 ) -> Result<(), ToJsonSchemaErrors> {
     match map.entry(name.clone()) {
         Entry::Occupied(entry) => Err(ToJsonSchemaError::duplicate_namespace(
-            name.map_or("".into(), |n| n.to_smolstr()),
+            &name.map_or("".into(), |n| n.to_smolstr()),
             record.loc,
             entry.get().loc.clone(),
         )