Apply clippy lint suggestions (#1372)

Signed-off-by: John Kastner <[email protected]>

cedar-policy-cli/src/main.rs

@@ -75,7 +75,7 @@ mod test {
         let test_data_root = PathBuf::from(r"../sample-data/sandbox_b");
         let mut schema_file = test_data_root.clone();
         schema_file.push("schema.cedarschema");
-        let mut old_policies_file = test_data_root.clone();
+        let mut old_policies_file = test_data_root;
         old_policies_file.push("policies_4.cedar");
         let new_policies_file = old_policies_file.clone();
 

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

@@ -2246,7 +2246,7 @@ impl AsRef<str> for PolicyID {
 }
 
 #[cfg(feature = "arbitrary")]
-impl<'u> arbitrary::Arbitrary<'u> for PolicyID {
+impl arbitrary::Arbitrary<'_> for PolicyID {
     fn arbitrary(u: &mut arbitrary::Unstructured<'_>) -> arbitrary::Result<PolicyID> {
         let s: String = u.arbitrary()?;
         Ok(PolicyID::from_string(s))

cedar-policy-core/src/parser/err.rs

@@ -259,7 +259,7 @@ pub enum ToASTErrorKind {
     InvalidPattern(String),
     /// Returned when the right hand side of a `is` expression is not an entity type name
     #[error("right hand side of an `is` expression must be an entity type name, but got `{rhs}`")]
-    #[diagnostic(help("{}", invalid_is_help(&.lhs, &.rhs)))]
+    #[diagnostic(help("{}", invalid_is_help(lhs, rhs)))]
     InvalidIsType {
         /// LHS of the invalid `is` expression, as a string
         lhs: String,

cedar-policy-validator/src/cedar_schema/err.rs

@@ -635,7 +635,7 @@ pub mod schema_warnings {
     // CAUTION: this type is publicly exported in `cedar-policy`.
     // Don't make fields `pub`, don't make breaking changes, and use caution
     // when adding public methods.
-    #[derive(Debug, Clone, Error)]
+    #[derive(Eq, PartialEq, Debug, Clone, Error)]
     #[error("The name `{name}` shadows a builtin Cedar name. You'll have to refer to the builtin as `__cedar::{name}`.")]
     pub struct ShadowsBuiltinWarning {
         pub(crate) name: SmolStr,
@@ -655,7 +655,7 @@ pub mod schema_warnings {
     // CAUTION: this type is publicly exported in `cedar-policy`.
     // Don't make fields `pub`, don't make breaking changes, and use caution
     // when adding public methods.
-    #[derive(Debug, Clone, Error)]
+    #[derive(Eq, PartialEq, Debug, Clone, Error)]
     #[error("The common type name {name} shadows an entity name")]
     pub struct ShadowsEntityWarning {
         pub(crate) name: SmolStr,
@@ -690,7 +690,7 @@ pub mod schema_warnings {
 // CAUTION: this type is publicly exported in `cedar-policy`.
 // Don't make fields `pub`, don't make breaking changes, and use caution
 // when adding public methods.
-#[derive(Debug, Clone, Error, Diagnostic)]
+#[derive(Eq, PartialEq, Debug, Clone, Error, Diagnostic)]
 #[non_exhaustive]
 pub enum SchemaWarning {
     /// Warning when a declaration shadows a builtin type

cedar-policy-validator/src/cedar_schema/test.rs

@@ -528,7 +528,7 @@ namespace Baz {action "Foo" appliesTo {
             Extensions::all_available(),
         )
         .expect("Schema should parse");
-        assert!(warnings.collect::<Vec<_>>().is_empty());
+        assert_eq!(warnings.collect::<Vec<_>>(), vec![]);
         let github = fragment
             .0
             .get(&Some("GitHub".parse().unwrap()))
@@ -657,7 +657,7 @@ namespace Baz {action "Foo" appliesTo {
             Extensions::all_available(),
         )
         .expect("failed to parse");
-        assert!(warnings.collect::<Vec<_>>().is_empty());
+        assert_eq!(warnings.collect::<Vec<_>>(), vec![]);
         let doccloud = fragment
             .0
             .get(&Some("DocCloud".parse().unwrap()))
@@ -785,7 +785,7 @@ namespace Baz {action "Foo" appliesTo {
         "#;
         let (_, warnings) =
             json_schema::Fragment::from_cedarschema_str(src, Extensions::all_available()).unwrap();
-        assert!(warnings.collect::<Vec<_>>().is_empty());
+        assert_eq!(warnings.collect::<Vec<_>>(), vec![]);
     }
 
     #[test]
@@ -855,7 +855,7 @@ namespace Baz {action "Foo" appliesTo {
 
         let (_, warnings) =
             json_schema::Fragment::from_cedarschema_str(src, Extensions::all_available()).unwrap();
-        assert!(warnings.collect::<Vec<_>>().is_empty());
+        assert_eq!(warnings.collect::<Vec<_>>(), vec![]);
     }
 
     #[test]
@@ -876,7 +876,7 @@ namespace Baz {action "Foo" appliesTo {
         "#;
         let (fragment, warnings) =
             json_schema::Fragment::from_cedarschema_str(src, Extensions::all_available()).unwrap();
-        assert!(warnings.collect::<Vec<_>>().is_empty());
+        assert_eq!(warnings.collect::<Vec<_>>(), vec![]);
         let service = fragment.0.get(&Some("Service".parse().unwrap())).unwrap();
         let resource = service
             .entity_types

cedar-policy-validator/src/coreschema.rs

@@ -358,7 +358,7 @@ pub mod request_validation_errors {
     /// not valid for the request action
     #[derive(Debug, Error, Diagnostic)]
     #[error("principal type `{principal_ty}` is not valid for `{action}`")]
-    #[diagnostic(help("{}", invalid_principal_type_help(&.valid_principal_tys, .action.as_ref())))]
+    #[diagnostic(help("{}", invalid_principal_type_help(valid_principal_tys, .action.as_ref())))]
     pub struct InvalidPrincipalTypeError {
         /// Principal type which is not valid
         pub(super) principal_ty: ast::EntityType,
@@ -407,7 +407,7 @@ pub mod request_validation_errors {
     /// not valid for the request action
     #[derive(Debug, Error, Diagnostic)]
     #[error("resource type `{resource_ty}` is not valid for `{action}`")]
-    #[diagnostic(help("{}", invalid_resource_type_help(&.valid_resource_tys, .action.as_ref())))]
+    #[diagnostic(help("{}", invalid_resource_type_help(valid_resource_tys, .action.as_ref())))]
     pub struct InvalidResourceTypeError {
         /// Resource type which is not valid
         pub(super) resource_ty: ast::EntityType,
@@ -888,7 +888,7 @@ mod test {
                 (ast::EntityUID::with_eid_and_type("User", "abc123").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Action", "edit_photo").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Photo", "vacationphoto94.jpg").unwrap(), None),
-                context_with_extra_attr.clone(),
+                context_with_extra_attr,
                 Some(&schema()),
                 Extensions::all_available(),
             ),
@@ -914,7 +914,7 @@ mod test {
                 (ast::EntityUID::with_eid_and_type("User", "abc123").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Action", "edit_photo").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Photo", "vacationphoto94.jpg").unwrap(), None),
-                context_with_wrong_type_attr.clone(),
+                context_with_wrong_type_attr,
                 Some(&schema()),
                 Extensions::all_available(),
             ),
@@ -943,7 +943,7 @@ mod test {
                 (ast::EntityUID::with_eid_and_type("User", "abc123").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Action", "edit_photo").unwrap(), None),
                 (ast::EntityUID::with_eid_and_type("Photo", "vacationphoto94.jpg").unwrap(), None),
-                context_with_heterogeneous_set.clone(),
+                context_with_heterogeneous_set,
                 Some(&schema()),
                 Extensions::all_available(),
             ),

cedar-policy-validator/src/entity_manifest.rs

@@ -304,7 +304,7 @@ fn union_fields(first: &Fields, second: &Fields) -> Fields {
     for (key, value) in second {
         res.entry(key.clone())
             .and_modify(|existing| existing.union_mut(value))
-            .or_insert(value.clone());
+            .or_insert_with(|| value.clone());
     }
     res
 }
@@ -375,7 +375,7 @@ impl RootAccessTrie {
             self.trie
                 .entry(key.clone())
                 .and_modify(|existing| existing.union_mut(value))
-                .or_insert(value.clone());
+                .or_insert_with(|| value.clone());
         }
     }
 }

cedar-policy-validator/src/entity_manifest/loader.rs

@@ -58,7 +58,7 @@ pub(crate) struct EntityRequestRef<'a> {
     access_trie: &'a AccessTrie,
 }
 
-impl<'a> EntityRequestRef<'a> {
+impl EntityRequestRef<'_> {
     fn to_request(&self) -> EntityRequest {
         EntityRequest {
             entity_id: self.entity_id.clone(),
@@ -251,7 +251,7 @@ pub(crate) fn load_entities(
         }
 
         let mut next_to_load = vec![];
-        for (entity_request, loaded_maybe) in to_load.drain(..).zip(new_entities) {
+        for (entity_request, loaded_maybe) in to_load.into_iter().zip(new_entities) {
             if let Some(loaded) = loaded_maybe {
                 next_to_load.extend(find_remaining_entities(
                     &loaded,
@@ -435,7 +435,7 @@ fn compute_ancestors_request(
 
     while !to_visit.is_empty() {
         let mut next_to_visit = vec![];
-        for entity_request in to_visit.drain(..) {
+        for entity_request in to_visit {
             // check the is_ancestor flag for entities
             // the is_ancestor flag on sets of entities is handled by find_remaining_entities
             if entity_request.access_trie.is_ancestor {

cedar-policy-validator/src/entity_manifest/slicing.rs

@@ -140,7 +140,7 @@ struct EntitySlicer<'a> {
     entities: &'a Entities,
 }
 
-impl<'a> EntityLoader for EntitySlicer<'a> {
+impl EntityLoader for EntitySlicer<'_> {
     fn load_entities(
         &mut self,
         to_load: &[EntityRequest],

cedar-policy-validator/src/entity_manifest/type_annotations.rs

@@ -69,16 +69,16 @@ impl RootAccessTrie {
                         match key {
                             EntityRoot::Literal(lit) => slice.to_typed(
                                 request_type,
-                                &Type::euid_literal(lit.clone(), schema).ok_or(
+                                &Type::euid_literal(lit.clone(), schema).ok_or_else(|| {
                                     MismatchedMissingEntityError {
                                         entity: lit.clone(),
-                                    },
-                                )?,
+                                    }
+                                })?,
                                 schema,
                             )?,
                             EntityRoot::Var(Var::Action) => {
                                 let ty = Type::euid_literal(request_type.action.clone(), schema)
-                                    .ok_or(MismatchedMissingEntityError {
+                                    .ok_or_else(|| MismatchedMissingEntityError {
                                         entity: request_type.action.clone(),
                                     })?;
                                 slice.to_typed(request_type, &ty, schema)?
@@ -96,7 +96,7 @@ impl RootAccessTrie {
                             EntityRoot::Var(Var::Context) => {
                                 let ty = &schema
                                     .get_action_id(&request_type.action.clone())
-                                    .ok_or(MismatchedMissingEntityError {
+                                    .ok_or_else(|| MismatchedMissingEntityError {
                                         entity: request_type.action.clone(),
                                     })?
                                     .context;

cedar-policy-validator/src/lib.rs

@@ -323,13 +323,13 @@ mod test {
         let policy_a_src = r#"permit(principal in foo_type::"a", action == Action::"actin", resource == bar_type::"b");"#;
         let policy_a = parser::parse_policy(Some(PolicyID::from_string("pola")), policy_a_src)
             .expect("Test Policy Should Parse");
-        set.add_static(policy_a.clone())
+        set.add_static(policy_a)
             .expect("Policy already present in PolicySet");
 
         let policy_b_src = r#"permit(principal in foo_tye::"a", action == Action::"action", resource == br_type::"b");"#;
         let policy_b = parser::parse_policy(Some(PolicyID::from_string("polb")), policy_b_src)
             .expect("Test Policy Should Parse");
-        set.add_static(policy_b.clone())
+        set.add_static(policy_b)
             .expect("Policy already present in PolicySet");
 
         let result = validator.validate(&set, ValidationMode::default());
@@ -508,7 +508,7 @@ mod test {
         // `result` contains the two prior error messages plus one new one
         assert_eq!(result.validation_errors().count(), 3);
         let invalid_action_err = ValidationError::invalid_action_application(
-            loc.clone(),
+            loc,
             PolicyID::from_string("link3"),
             false,
             false,

cedar-policy-validator/src/schema.rs

@@ -2788,8 +2788,7 @@ pub(crate) mod test {
                 }
               }
         );
-        let schema =
-            ValidatorSchema::from_json_value(src.clone(), Extensions::all_available()).unwrap();
+        let schema = ValidatorSchema::from_json_value(src, Extensions::all_available()).unwrap();
         let mut attributes = assert_entity_type_exists(&schema, "Demo::User").attributes();
         let (attr_name, attr_ty) = attributes.next().unwrap();
         assert_eq!(attr_name, "id");
@@ -3511,7 +3510,7 @@ pub(crate) mod test {
                 "actions": { },
             }
         });
-        let schema = ValidatorSchema::from_json_value(src.clone(), Extensions::all_available());
+        let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::JsonDeserialization(_)));
 
         let src: serde_json::Value = json!({
@@ -3521,7 +3520,7 @@ pub(crate) mod test {
                 "actions": { },
             }
         });
-        let schema = ValidatorSchema::from_json_value(src.clone(), Extensions::all_available());
+        let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::JsonDeserialization(_)));
 
         let src: serde_json::Value = json!({
@@ -3535,7 +3534,7 @@ pub(crate) mod test {
                 "actions": { },
             }
         });
-        let schema = ValidatorSchema::from_json_value(src.clone(), Extensions::all_available());
+        let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::JsonDeserialization(_)));
 
         let src: serde_json::Value = json!({
@@ -3549,7 +3548,7 @@ pub(crate) mod test {
                 "actions": { },
             }
         });
-        let schema = ValidatorSchema::from_json_value(src.clone(), Extensions::all_available());
+        let schema = ValidatorSchema::from_json_value(src, Extensions::all_available());
         assert_matches!(schema, Err(SchemaError::JsonDeserialization(_)));
 
         let src: serde_json::Value = json!({
@@ -4650,7 +4649,7 @@ mod entity_tags {
             },
             "actions": {}
         }});
-        assert_matches!(ValidatorSchema::from_json_value(json.clone(), Extensions::all_available()), Ok(schema) => {
+        assert_matches!(ValidatorSchema::from_json_value(json, Extensions::all_available()), Ok(schema) => {
             let user = assert_entity_type_exists(&schema, "User");
             assert_matches!(user.tag_type(), Some(Type::Set { element_type: Some(el_ty) }) => {
                 assert_matches!(&**el_ty, Type::Primitive { primitive_type: Primitive::String });
@@ -5136,8 +5135,8 @@ action CreateList in Create appliesTo {
     #[test]
     fn empty_schema_principals_and_resources() {
         let empty: ValidatorSchema = "".parse().unwrap();
-        assert!(empty.principals().collect::<Vec<_>>().is_empty());
-        assert!(empty.resources().collect::<Vec<_>>().is_empty());
+        assert!(empty.principals().next().is_none());
+        assert!(empty.resources().next().is_none());
     }
 
     #[test]
@@ -5350,8 +5349,8 @@ action CreateList in Create appliesTo {
     #[test]
     fn empty_schema_principals_and_resources() {
         let empty: ValidatorSchema = "".parse().unwrap();
-        assert!(empty.principals().collect::<Vec<_>>().is_empty());
-        assert!(empty.resources().collect::<Vec<_>>().is_empty());
+        assert!(empty.principals().next().is_none());
+        assert!(empty.resources().next().is_none());
     }
 
     #[test]

cedar-policy-validator/src/typecheck/test/expr.rs

@@ -1158,7 +1158,7 @@ fn less_than_typecheck_fails() {
             ValidationError::expected_one_of_types(
                 get_loc(src, "\"\""),
                 expr_id_placeholder(),
-                expected_types.clone(),
+                expected_types,
                 Type::primitive_string(),
                 None,
             ),