Addressing Comments

Co-authored-by: John Kastner <[email protected]>
Co-authored-by: Craig Disselkoen <[email protected]>
Signed-off-by: Aaron Eline <[email protected]>

Author: 3 people

cedar-policy-core/src/ast/entity.rs

@@ -327,7 +327,7 @@ impl Entity {
     /// Create a new `Entity` with this UID, attributes, and ancestors
     ///
     /// # Errors
-    /// - Will error if any of the Restricted Expressions in `attrs` error when evaluated
+    /// - Will error if any of the [`RestrictedExpr]`s in `attrs` error when evaluated
     pub fn new(
         uid: EntityUID,
         attrs: HashMap<SmolStr, RestrictedExpr>,
@@ -355,17 +355,11 @@ impl Entity {
         })
     }
 
-    /// Creates a new `Entity` with this UID, ancestors, and an empty set of attributes
-    /// Since it lacks any attributes, this method returns `Self` instead of `Result<Self>`
-    pub fn new_empty_attrs(
-        uid: EntityUID,
-        ancestors: HashSet<EntityUID>,
-        extensions: &Extensions<'_>,
-    ) -> Self {
-        // PANIC SAFETY
-        // Safe as the hashmap is empty
-        #[allow(clippy::unwrap_used)]
-        Self::new(uid, HashMap::new(), ancestors, extensions).unwrap()
+    /// Create a new `Entity` with this UID, ancestors, and an empty set of attributes
+    ///
+    /// Since there are no attributes, this method does not error, and returns `Self` instead of `Result<Self>`
+    pub fn new_empty_attrs(uid: EntityUID, ancestors: HashSet<EntityUID>) -> Self {
+        Self::new_with_attr_partial_value(uid, HashMap::new(), ancestors)
     }
 
     /// Create a new `Entity` with this UID, attributes, and ancestors.

cedar-policy-core/src/entities.rs

@@ -164,10 +164,10 @@ impl Entities {
     /// responsible for ensuring that TC and DAG hold before calling this method.
     ///
     /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
     /// - [`EntitiesError::TransitiveClosureError`] if `tc_computation ==
     ///   TCComputation::EnforceAlreadyComputed` and the entities are not transitivly closed
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     pub fn from_entities(
         entities: impl IntoIterator<Item = Entity>,
@@ -824,7 +824,7 @@ mod json_parsing_tests {
         parser.from_json_value(json).expect("JSON is correct")
     }
 
-    /// Ensure the initial conditions of the entiites still hold
+    /// Ensure the initial conditions of the entities still hold
     fn simple_entities_still_sane(e: &Entities) {
         let bob = r#"Test::"bob""#.parse().unwrap();
         let alice = e.entity(&r#"Test::"alice""#.parse().unwrap()).unwrap();

cedar-policy/src/api.rs

@@ -105,13 +105,12 @@ impl Entity {
     }
 
     /// Create a new `Entity` with this Uid, parents, and no attributes.
-    /// This is the same as `Self::new` except the attributes are empty, and therefor it can
+    /// This is the same as `Self::new` except the attributes are empty, and therefore it can
     /// return `Self` instead of `Result<Self>`
     pub fn new_empty_attrs(uid: EntityUid, parents: HashSet<EntityUid>) -> Self {
         Self(ast::Entity::new_empty_attrs(
             uid.into(),
             parents.into_iter().map(EntityUid::into).collect(),
-            Extensions::all_available(),
         ))
     }
 
@@ -356,9 +355,9 @@ impl Entities {
     /// error if attributes have the wrong types (e.g., string instead of
     /// integer), or if required attributes are missing or superfluous
     /// attributes are provided.
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     pub fn from_entities(
         entities: impl IntoIterator<Item = Entity>,
@@ -386,9 +385,9 @@ impl Entities {
     ///
     /// Re-computing the transitive closure can be expensive, so it is advised
     /// to not call this method in a loop.
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     pub fn add_entities(
         self,
@@ -420,9 +419,9 @@ impl Entities {
     ///
     /// Re-computing the transitive closure can be expensive, so it is advised
     /// to not call this method in a loop.
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     pub fn add_entities_from_json_str(
@@ -458,9 +457,9 @@ impl Entities {
     ///
     /// Re-computing the transitive closure can be expensive, so it is advised
     /// to not call this method in a loop.
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     pub fn add_entities_from_json_value(
@@ -497,9 +496,9 @@ impl Entities {
     /// Re-computing the transitive closure can be expensive, so it is advised
     /// to not call this method in a loop.
     ///
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     pub fn add_entities_from_json_file(
@@ -539,9 +538,9 @@ impl Entities {
     /// instead of integer), or if required attributes are missing or
     /// superfluous attributes are provided.
     ///
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     ///
@@ -600,9 +599,9 @@ impl Entities {
     /// instead of integer), or if required attributes are missing or
     /// superfluous attributes are provided.
     ///
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`]if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     ///
@@ -658,9 +657,9 @@ impl Entities {
     /// instead of integer), or if required attributes are missing or
     /// superfluous attributes are provided.
     ///
-    /// # Errors
-    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entiites`
-    /// - [`EntitiesError::InvalidEntity` if `schema` is not none and any entities do not conform
+    /// ## Errors
+    /// - [`EntitiesError::Duplicate`] if there are any duplicate entities in `entities`
+    /// - [`EntitiesError::InvalidEntity`] if `schema` is not none and any entities do not conform
     ///   to the schema
     /// - [`EntitiesError::Deserialization`] if there are errors while parsing the json
     pub fn from_json_file(
@@ -1545,7 +1544,7 @@ impl Schema {
 
     /// Returns an iterator over every entity type that can be a principal for `action` in this schema
     ///
-    /// # Errors
+    /// ## Errors
     ///
     /// Returns [`None`] if `action` is not found in the schema
     pub fn principals_for_action(
@@ -1559,7 +1558,7 @@ impl Schema {
 
     /// Returns an iterator over every entity type that can be a resource for `action` in this schema
     ///
-    /// # Errors
+    /// ## Errors
     ///
     /// Returns [`None`] if `action` is not found in the schema
     pub fn resources_for_action(
@@ -1573,7 +1572,7 @@ impl Schema {
 
     /// Returns an iterator over all the entity types that can be an ancestor of `ty`
     ///
-    /// # Errors
+    /// ## Errors
     ///
     /// Returns [`None`] if the `ty` is not found in the schema
     pub fn ancestors<'a>(