Repository: cedar-policy/cedar
Pull request #1561 (2025-04-01 15:29:04)

Enable more pedantic lints on all crates
## Description of changes

Progress towards #1560 by making lints more consistent across our crates 

## Issue #, if available

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [ ] A breaking change requiring a major version bump to `cedar-policy` (e.g., changes to the signature of an existing API).
- [ ] A backwards-compatible change requiring a minor version bump to `cedar-policy` (e.g., addition of a new API).
- [ ] A bug fix or other functionality change requiring a patch to `cedar-policy`.
- [x] A change "invisible" to users (e.g., documentation, changes to "internal" crates like `cedar-policy-core`, `cedar-validator`, etc.)
- [ ] A change (breaking or otherwise) that only impacts unreleased or experimental code.

I confirm that this PR (choose one, and delete the other options):

- [ ] Updates the "Unreleased" section of the CHANGELOG with a description of my change (required for major/minor version bumps).
- [x] Does not update the CHANGELOG because my change does not significantly impact released code.

I confirm that [`cedar-spec`](https://github.com/cedar-policy/cedar-spec) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar formal model or DRT infrastructure.
- [ ] Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in `cedar-spec`, and how you have tested that your updates are correct.)
- [ ] Requires updates, but I do not plan to make them in the near future. (Make sure that your changes are hidden behind a feature flag to mark them as experimental.)
- [ ] I'm not sure how my change impacts `cedar-spec`. (Post your PR anyways, and we'll discuss in the comments.)

I confirm that [`docs.cedarpolicy.com`](https://docs.cedarpolicy.com/) (choose one, and delete the other options):

- [x] Does not require updates because my change does not impact the Cedar language specification.
- [ ] Requires updates, and I have made / will make these updates myself. (Please include in your description a timeline or link to the relevant PR in [`cedar-docs`](https://github.com/cedar-policy/cedar-docs). PRs should be targeted at a `staging-X.Y` branch, not `main`.)
- [ ] I'm not sure how my change impacts the documentation. (Post your PR anyways, and we'll discuss in the comments.)


github-actions[bot]: 
# Coverage Report
Head Commit: ```237e0fca4fd622011808493b1821e3013f664c72```

Base Commit: ```38d445767c7f20f7d561b226392c38295357711e```

[Download the full coverage report.](https://github.com/cedar-policy/cedar/actions/runs/14199723144/artifacts/2860637414)

## Coverage of Added or Modified Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 100.00%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy-core/src/ast/extension.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-core/src/ast/pattern.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy-core/src/ast/policy.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-core/src/entities.rs | :green_circle: | 23/23 | 100.00% | 
cedar-policy-core/src/evaluator.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-core/src/evaluator/err.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy-core/src/extensions/datetime.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-validator/src/diagnostics/validation_errors.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/entity_manifest/loader.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/extensions.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/level_validate.rs | :green_circle: | 6/6 | 100.00% | 
cedar-policy-validator/src/schema.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy-validator/src/types.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy/src/api.rs | :green_circle: | 4/4 | 100.00% | 
cedar-policy/src/proto/ast.rs | :green_circle: | 3/3 | 100.00% | 
</details>

## Coverage of All Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 81.58%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3066/4466 | 68.65% | 68.50%
cedar-policy-cli | :red_circle: | 573/976 | 58.71% | 57.99%
cedar-policy-core | :green_circle: | 11697/13897 | 84.17% | 83.87%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8509/9940 | 85.60% | 85.61%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>

github-actions[bot]: 
# Coverage Report
Head Commit: ```47205863c500c0e3e5476cb4ca954c696d1a968c```

Base Commit: ```38d445767c7f20f7d561b226392c38295357711e```

[Download the full coverage report.](https://github.com/cedar-policy/cedar/actions/runs/14221197817/artifacts/2867747108)

## Coverage of Added or Modified Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 100.00%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy-core/src/ast/extension.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-core/src/ast/pattern.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy-core/src/ast/policy.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-core/src/entities.rs | :green_circle: | 23/23 | 100.00% | 
cedar-policy-core/src/evaluator.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-core/src/evaluator/err.rs | :green_circle: | 4/4 | 100.00% | 
cedar-policy-core/src/extensions/datetime.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy-validator/src/diagnostics/validation_errors.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/entity_manifest/loader.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/extensions.rs | :green_circle: | 1/1 | 100.00% | 
cedar-policy-validator/src/level_validate.rs | :green_circle: | 6/6 | 100.00% | 
cedar-policy-validator/src/schema.rs | :green_circle: | 3/3 | 100.00% | 
cedar-policy-validator/src/types.rs | :green_circle: | 2/2 | 100.00% | 
cedar-policy/src/api.rs | :green_circle: | 5/5 | 100.00% | 
cedar-policy/src/proto/ast.rs | :green_circle: | 3/3 | 100.00% | 
</details>

## Coverage of All Lines of Rust Code
**Required coverage:** 80.00%

**Actual coverage:** 81.58%

**Status:** PASSED :white_check_mark:

<details>
<summary><b>Details</b></summary>

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3067/4467 | 68.66% | 68.50%
cedar-policy-cli | :red_circle: | 573/976 | 58.71% | 57.99%
cedar-policy-core | :green_circle: | 11698/13898 | 84.17% | 83.87%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8509/9940 | 85.60% | 85.61%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>


Patch:
diff --git a/Cargo.lock b/Cargo.lock
index e0536c54..2f9396ad 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -303,6 +303,7 @@ dependencies = [
  "cedar-policy-formatter",
  "clap",
  "glob",
+ "graphviz-rust",
  "miette",
  "predicates",
  "rstest",
@@ -501,7 +502,7 @@ dependencies = [
  "heck",
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -660,7 +661,7 @@ dependencies = [
  "proc-macro2",
  "quote",
  "strsim",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -671,7 +672,7 @@ checksum = "fc34b93ccb385b40dc71c6fceac4b2ad23662c7eeb248cf10d529b7e055b6ead"
 dependencies = [
  "darling_core",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -692,7 +693,7 @@ checksum = "30542c1ad912e0e3d22a1935c290e12e8a29d704a420177a31faad4a601a0800"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -733,6 +734,21 @@ version = "0.3.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fea41bba32d969b513997752735605054bc0dfa92b4c56bf1189f2e174be7a10"
 
+[[package]]
+name = "dot-generator"
+version = "0.2.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0aaac7ada45f71873ebce336491d1c1bc4a7c8042c7cea978168ad59e805b871"
+dependencies = [
+ "dot-structures",
+]
+
+[[package]]
+name = "dot-structures"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "675e35c02a51bb4d4618cb4885b3839ce6d1787c97b664474d9208d074742e20"
+
 [[package]]
 name = "educe"
 version = "0.6.0"
@@ -742,7 +758,7 @@ dependencies = [
  "enum-ordinalize",
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -783,7 +799,7 @@ checksum = "0d28318a75d4aead5c4db25382e8ef717932d0346600cacae6357eb5941bc5ff"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -843,7 +859,7 @@ checksum = "162ee34ebcb7c64a8abebc059ce0fee27c2262618d7b60ed8faf72fef13c3650"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -943,6 +959,22 @@ dependencies = [
  "web-sys",
 ]
 
+[[package]]
+name = "graphviz-rust"
+version = "0.9.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8f7892a4538fc9aa3658ae6962774428487c16e11663c58c594357975377c901"
+dependencies = [
+ "dot-generator",
+ "dot-structures",
+ "into-attr",
+ "into-attr-derive",
+ "pest",
+ "pest_derive",
+ "rand",
+ "tempfile",
+]
+
 [[package]]
 name = "half"
 version = "2.5.0"
@@ -1065,6 +1097,28 @@ dependencies = [
  "walkdir",
 ]
 
+[[package]]
+name = "into-attr"
+version = "0.1.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "18b48c537e49a709e678caec3753a7dba6854661a1eaa27675024283b3f8b376"
+dependencies = [
+ "dot-structures",
+]
+
+[[package]]
+name = "into-attr-derive"
+version = "0.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ecac7c1ae6cd2c6a3a64d1061a8bdc7f52ff62c26a831a2301e54c1b5d70d5b1"
+dependencies = [
+ "dot-generator",
+ "dot-structures",
+ "into-attr",
+ "quote",
+ "syn 1.0.109",
+]
+
 [[package]]
 name = "is-terminal"
 version = "0.4.16"
@@ -1225,7 +1279,7 @@ dependencies = [
  "quote",
  "regex-syntax",
  "rustc_version",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1272,7 +1326,7 @@ checksum = "bf45bf44ab49be92fd1227a3be6fc6f617f1a337c06af54981048574d8783147"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1392,6 +1446,51 @@ dependencies = [
  "windows-targets",
 ]
 
+[[package]]
+name = "pest"
+version = "2.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "198db74531d58c70a361c42201efde7e2591e976d518caf7662a47dc5720e7b6"
+dependencies = [
+ "memchr",
+ "thiserror 2.0.12",
+ "ucd-trie",
+]
+
+[[package]]
+name = "pest_derive"
+version = "2.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d725d9cfd79e87dccc9341a2ef39d1b6f6353d68c4b33c177febbe1a402c97c5"
+dependencies = [
+ "pest",
+ "pest_generator",
+]
+
+[[package]]
+name = "pest_generator"
+version = "2.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "db7d01726be8ab66ab32f9df467ae8b1148906685bbe75c82d1e65d7f5b3f841"
+dependencies = [
+ "pest",
+ "pest_meta",
+ "proc-macro2",
+ "quote",
+ "syn 2.0.100",
+]
+
+[[package]]
+name = "pest_meta"
+version = "2.8.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7f9f832470494906d1fca5329f8ab5791cc60beb230c74815dff541cbd2b5ca0"
+dependencies = [
+ "once_cell",
+ "pest",
+ "sha2",
+]
+
 [[package]]
 name = "petgraph"
 version = "0.7.1"
@@ -1434,7 +1533,7 @@ checksum = "6e918e4ff8c4549eb882f14b3a4bc8c8bc93de829416eacf579f1207a8fbf861"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1546,7 +1645,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "5316f57387668042f561aae71480de936257848f9c43ce528e311d89a07cadeb"
 dependencies = [
  "proc-macro2",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1613,7 +1712,7 @@ dependencies = [
  "prost",
  "prost-types",
  "regex",
- "syn",
+ "syn 2.0.100",
  "tempfile",
 ]
 
@@ -1627,7 +1726,7 @@ dependencies = [
  "itertools 0.14.0",
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1748,7 +1847,7 @@ checksum = "1165225c21bff1f3bbce98f5a1f889949bc902d3575308cc7b0de30b4f6d27c7"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1812,7 +1911,7 @@ dependencies = [
  "regex",
  "relative-path",
  "rustc_version",
- "syn",
+ "syn 2.0.100",
  "unicode-ident",
 ]
 
@@ -1932,7 +2031,7 @@ checksum = "5b0276cf7f2c73365f7157c8123c21cd9a50fbbd844757af28ca1f5925fc2a00"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1943,7 +2042,7 @@ checksum = "e578a843d40b4189a4d66bba51d7684f57da5bd7c304c64e14bd63efbef49509"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -1986,7 +2085,18 @@ dependencies = [
  "darling",
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
+]
+
+[[package]]
+name = "sha2"
+version = "0.10.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "793db75ad2bcafc3ffa7c68b215fee268f537982cd901d132f89c6343f3a3dc8"
+dependencies = [
+ "cfg-if",
+ "cpufeatures",
+ "digest",
 ]
 
 [[package]]
@@ -2108,6 +2218,17 @@ version = "3.0.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "b7401a30af6cb5818bb64852270bb722533397edcfc7344954a38f420819ece2"
 
+[[package]]
+name = "syn"
+version = "1.0.109"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "unicode-ident",
+]
+
 [[package]]
 name = "syn"
 version = "2.0.100"
@@ -2194,7 +2315,7 @@ checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -2205,7 +2326,7 @@ checksum = "7f7cf42b4507d8ea322120659672cf1b9dbb93f8f2d4ecfd6e51350ff5b17a1d"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -2309,7 +2430,7 @@ dependencies = [
  "proc-macro2",
  "quote",
  "serde_derive_internals",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -2324,6 +2445,12 @@ version = "1.18.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "1dccffe3ce07af9386bfd29e80c0ab1a8205a2fc34e4bcd40364df902cfa8f3f"
 
+[[package]]
+name = "ucd-trie"
+version = "0.1.7"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971"
+
 [[package]]
 name = "unarray"
 version = "0.1.4"
@@ -2459,7 +2586,7 @@ dependencies = [
  "log",
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
  "wasm-bindgen-shared",
 ]
 
@@ -2494,7 +2621,7 @@ checksum = "8ae87ea40c9f689fc23f209965b6fb8a99ad69aeeb0231408be24920604395de"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
  "wasm-bindgen-backend",
  "wasm-bindgen-shared",
 ]
@@ -2529,7 +2656,7 @@ checksum = "17d5042cc5fa009658f9a7333ef24291b1291a25b6382dd68862a7f3b969f69b"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
 
 [[package]]
@@ -2683,5 +2810,5 @@ checksum = "a996a8f63c5c4448cd959ac1bab0aaa3306ccfd060472f85943ee0750f0169be"
 dependencies = [
  "proc-macro2",
  "quote",
- "syn",
+ "syn 2.0.100",
 ]
diff --git a/Cargo.toml b/Cargo.toml
index 7b2db52e..f3a10ff5 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -44,8 +44,21 @@ rust-2018-idioms = "deny"
 [workspace.lints.clippy]
 nursery = { level = "warn", priority = -1 }
 # Enabling some pedantic lints incrementally
+inefficient_to_string = "warn"
+implicit_clone = "warn"
+cloned_instead_of_copied = "warn"
+format_collect = "warn"
+format_push_string = "warn"
+trivially_copy_pass_by_ref = "warn"
+large_types_passed_by_value = "warn"
 needless_pass_by_value = "warn"
+option_as_ref_cloned = "warn"
+ref_option_ref = "warn"
 ref_option = "warn"
+option_option = "warn"
+inconsistent_struct_constructor = "warn"
+cast_lossless = "warn"
+cast_possible_truncation = "warn"
 
 # These lints may be worth enforcing, but cause a lot of noise at the moment.
 use_self = "allow"
diff --git a/cedar-policy-cli/Cargo.toml b/cedar-policy-cli/Cargo.toml
index 7fc29304..caa31541 100644
--- a/cedar-policy-cli/Cargo.toml
+++ b/cedar-policy-cli/Cargo.toml
@@ -34,6 +34,7 @@ tempfile = "3"
 glob = "0.3.2"
 predicates = "3.1.3"
 rstest = "0.25.0"
+graphviz-rust = {version = "0.9.3",  default-features = false }
 
 # We override the name of the binary for src/main.rs, which otherwise would be
 # cedar-policy-cli (matching the crate name).
diff --git a/cedar-policy-core/src/ast/extension.rs b/cedar-policy-core/src/ast/extension.rs
index d8161ca8..a02f7ea6 100644
--- a/cedar-policy-core/src/ast/extension.rs
+++ b/cedar-policy-core/src/ast/extension.rs
@@ -166,8 +166,8 @@ impl ExtensionFunction {
     ) -> Self {
         Self {
             name,
-            func,
             style,
+            func,
             return_type,
             arg_types,
         }
@@ -403,7 +403,7 @@ impl RepresentableExtensionValue {
         func: Name,
         args: Vec<RestrictedExpr>,
     ) -> Self {
-        Self { value, func, args }
+        Self { func, args, value }
     }
 
     /// Get the internal value
diff --git a/cedar-policy-core/src/ast/pattern.rs b/cedar-policy-core/src/ast/pattern.rs
index 560947f3..9012a626 100644
--- a/cedar-policy-core/src/ast/pattern.rs
+++ b/cedar-policy-core/src/ast/pattern.rs
@@ -93,13 +93,13 @@ impl std::fmt::Display for Pattern {
 }
 
 impl PatternElem {
-    fn match_char(&self, text_char: &char) -> bool {
+    fn match_char(self, text_char: char) -> bool {
         match self {
             PatternElem::Char(c) => text_char == c,
             PatternElem::Wildcard => true,
         }
     }
-    fn is_wildcard(&self) -> bool {
+    fn is_wildcard(self) -> bool {
         matches!(self, PatternElem::Wildcard)
     }
 }
@@ -139,7 +139,7 @@ impl Pattern {
                 star_idx = j;
                 tmp_idx = i;
                 j += 1;
-            } else if j < pattern_len && pattern[j].match_char(&text[i]) {
+            } else if j < pattern_len && pattern[j].match_char(text[i]) {
                 i += 1;
                 j += 1;
             } else if contains_star {
diff --git a/cedar-policy-core/src/ast/policy.rs b/cedar-policy-core/src/ast/policy.rs
index 00744ee9..af5c0ca0 100644
--- a/cedar-policy-core/src/ast/policy.rs
+++ b/cedar-policy-core/src/ast/policy.rs
@@ -1509,9 +1509,7 @@ impl EntityReference {
     pub fn into_expr(&self, slot: SlotId) -> Expr {
         match self {
             EntityReference::EUID(euid) => Expr::val(euid.clone()),
-            EntityReference::Slot(loc) => {
-                Expr::slot(slot).with_maybe_source_loc(loc.as_ref().cloned())
-            }
+            EntityReference::Slot(loc) => Expr::slot(slot).with_maybe_source_loc(loc.clone()),
         }
     }
 }
diff --git a/cedar-policy-core/src/entities.rs b/cedar-policy-core/src/entities.rs
index 928894a7..ce5f5edd 100644
--- a/cedar-policy-core/src/entities.rs
+++ b/cedar-policy-core/src/entities.rs
@@ -303,11 +303,15 @@ impl Entities {
         entities_by_type
     }
 
-    /// Write entities into a DOT graph
-    pub fn to_dot_str(&self) -> String {
-        let mut dot_str = String::new();
+    /// Write entities into a DOT graph.  This function only returns an `Err`
+    /// result on a failing `write!` to `f`, so it is infallible if the `Write`
+    /// implementation cannot fail (e.g., `String`).
+    pub fn to_dot_str(&self, f: &mut impl std::fmt::Write) -> std::fmt::Result {
         // write prelude
-        dot_str.push_str("strict digraph {\n\tordering=\"out\"\n\tnode[shape=box]\n");
+        write!(
+            f,
+            "strict digraph {{\n\tordering=\"out\"\n\tnode[shape=box]\n"
+        )?;
 
         // From DOT language reference:
         // An ID is one of the following:
@@ -317,39 +321,39 @@ impl Entities {
         // an HTML string (<...>).
         // The best option to convert a `Name` or an `EntityUid` is to use double-quoted string.
         // The `escape_debug` method should be sufficient for our purpose.
-        fn to_dot_id(v: &impl std::fmt::Display) -> String {
-            format!("\"{}\"", v.to_string().escape_debug())
+        fn to_dot_id(f: &mut impl std::fmt::Write, v: &impl std::fmt::Display) -> std::fmt::Result {
+            write!(f, "\"{}\"", v.to_string().escape_debug())
         }
 
         // write clusters (subgraphs)
         let entities_by_type = self.get_entities_by_entity_type();
 
         for (et, entities) in entities_by_type {
-            dot_str.push_str(&format!(
-                "\tsubgraph \"cluster_{et}\" {{\n\t\tlabel={}\n",
-                to_dot_id(&et)
-            ));
+            write!(f, "\tsubgraph \"cluster_{et}\" {{\n\t\tlabel=",)?;
+            to_dot_id(f, &et)?;
+            writeln!(f)?;
             for entity in entities {
-                let euid = to_dot_id(&entity.uid());
-                let label = format!(r#"[label={}]"#, to_dot_id(&entity.uid().eid().escaped()));
-                dot_str.push_str(&format!("\t\t{euid} {label}\n"));
+                write!(f, "\t\t")?;
+                to_dot_id(f, &entity.uid())?;
+                write!(f, " [label=")?;
+                to_dot_id(f, &entity.uid().eid().escaped())?;
+                writeln!(f, "]")?;
             }
-            dot_str.push_str("\t}\n");
+            writeln!(f, "\t}}")?;
         }
 
         // adding edges
         for entity in self.iter() {
             for ancestor in entity.ancestors() {
-                dot_str.push_str(&format!(
-                    "\t{} -> {}\n",
-                    to_dot_id(&entity.uid()),
-                    to_dot_id(&ancestor)
-                ));
+                write!(f, "\t")?;
+                to_dot_id(f, &entity.uid())?;
+                write!(f, " -> ")?;
+                to_dot_id(f, &ancestor)?;
+                writeln!(f)?;
             }
         }
-
-        dot_str.push_str("}\n");
-        dot_str
+        writeln!(f, "}}")?;
+        Ok(())
     }
 }
 
diff --git a/cedar-policy-core/src/evaluator.rs b/cedar-policy-core/src/evaluator.rs
index 8f3633f8..e97a9815 100644
--- a/cedar-policy-core/src/evaluator.rs
+++ b/cedar-policy-core/src/evaluator.rs
@@ -100,7 +100,7 @@ impl<'e> RestrictedEvaluator<'e> {
     pub fn partial_interpret(&self, expr: BorrowedRestrictedExpr<'_>) -> Result<PartialValue> {
         stack_size_check()?;
 
-        let res = self.partial_interpret_internal(&expr);
+        let res = self.partial_interpret_internal(expr);
 
         // set the returned value's source location to the same source location
         // as the input expression had.
@@ -127,10 +127,7 @@ impl<'e> RestrictedEvaluator<'e> {
     /// `partial_interpret()`.
     ///
     /// INVARIANT: If this returns a residual, the residual expression must be a valid restricted expression.
-    fn partial_interpret_internal(
-        &self,
-        expr: &BorrowedRestrictedExpr<'_>,
-    ) -> Result<PartialValue> {
+    fn partial_interpret_internal(&self, expr: BorrowedRestrictedExpr<'_>) -> Result<PartialValue> {
         match expr.as_ref().expr_kind() {
             ExprKind::Lit(lit) => Ok(lit.clone().into()),
             ExprKind::Set(items) => {
diff --git a/cedar-policy-core/src/evaluator/err.rs b/cedar-policy-core/src/evaluator/err.rs
index 20c10597..9072645c 100644
--- a/cedar-policy-core/src/evaluator/err.rs
+++ b/cedar-policy-core/src/evaluator/err.rs
@@ -320,8 +320,8 @@ impl EvaluationError {
         evaluation_errors::ExtensionFunctionExecutionError {
             extension_name,
             msg,
-            source_loc,
             advice,
+            source_loc,
         }
         .into()
     }
@@ -346,6 +346,7 @@ pub mod evaluation_errors {
     use miette::Diagnostic;
     use nonempty::NonEmpty;
     use smol_str::SmolStr;
+    use std::fmt::Write;
     use std::sync::Arc;
     use thiserror::Error;
 
@@ -429,7 +430,10 @@ pub mod evaluation_errors {
                 )
             };
             if self.exists_the_other_kind {
-                help_text.push_str(&format!(
+                // PANIC SAFETY: A `write!` to a `String` cannot fail
+                #[allow(clippy::unwrap_used)]
+                write!(
+                    &mut help_text,
                     "; note that {} (not {}) named `{}` does exist",
                     if self.was_attr {
                         "a tag"
@@ -442,7 +446,8 @@ pub mod evaluation_errors {
                         "a tag"
                     },
                     self.attr_or_tag,
-                ));
+                )
+                .unwrap()
             }
             Some(Box::new(help_text))
         }
diff --git a/cedar-policy-core/src/extensions/datetime.rs b/cedar-policy-core/src/extensions/datetime.rs
index afef6b6f..3c7a43d0 100644
--- a/cedar-policy-core/src/extensions/datetime.rs
+++ b/cedar-policy-core/src/extensions/datetime.rs
@@ -469,7 +469,7 @@ fn parse_duration(s: &str) -> Result<Duration, DurationParseError> {
         .ok_or(DurationParseError::Overflow)
     };
     let mut ms = if s.starts_with('-') {
-        i64::try_from(-(ms as i128)).map_err(|_| DurationParseError::Overflow)?
+        i64::try_from(-i128::from(ms)).map_err(|_| DurationParseError::Overflow)?
     } else {
         i64::try_from(ms).map_err(|_| DurationParseError::Overflow)?
     };
@@ -512,7 +512,7 @@ impl UTCOffset {
     const MAX_MM: u32 = 60;
 
     fn to_seconds(&self) -> i64 {
-        let offset_in_seconds_unsigned = (self.hh * 3600 + self.mm * 60) as i64;
+        let offset_in_seconds_unsigned = i64::from(self.hh * 3600 + self.mm * 60);
         if self.positive {
             offset_in_seconds_unsigned
         } else {
@@ -983,8 +983,8 @@ mod tests {
         ] {
             assert!(parse_duration(s).is_err());
         }
-        assert!(parse_duration(&milliseconds_to_duration(i64::MAX as i128 + 1)).is_err());
-        assert!(parse_duration(&milliseconds_to_duration(i64::MIN as i128 - 1)).is_err());
+        assert!(parse_duration(&milliseconds_to_duration(i128::from(i64::MAX) + 1)).is_err());
+        assert!(parse_duration(&milliseconds_to_duration(i128::from(i64::MIN) - 1)).is_err());
     }
 
     #[test]
@@ -1004,7 +1004,8 @@ mod tests {
             Some(
                 unix_epoch
                     .offset(
-                        parse_duration(&milliseconds_to_duration(i64::MAX as i128 - 1)).unwrap()
+                        parse_duration(&milliseconds_to_duration(i128::from(i64::MAX) - 1))
+                            .unwrap()
                     )
                     .expect("valid datetime")
             )
@@ -1017,7 +1018,8 @@ mod tests {
             Some(
                 unix_epoch
                     .offset(
-                        parse_duration(&milliseconds_to_duration(i64::MIN as i128 + 1)).unwrap()
+                        parse_duration(&milliseconds_to_duration(i128::from(i64::MIN) + 1))
+                            .unwrap()
                     )
                     .expect("valid datetime")
             )
diff --git a/cedar-policy-validator/src/diagnostics/validation_errors.rs b/cedar-policy-validator/src/diagnostics/validation_errors.rs
index 9b4cb901..c9f94464 100644
--- a/cedar-policy-validator/src/diagnostics/validation_errors.rs
+++ b/cedar-policy-validator/src/diagnostics/validation_errors.rs
@@ -496,7 +496,7 @@ impl Display for EntityDerefLevel {
 impl From<u32> for EntityDerefLevel {
     fn from(value: u32) -> Self {
         EntityDerefLevel {
-            level: value as i64,
+            level: i64::from(value),
         }
     }
 }
diff --git a/cedar-policy-validator/src/entity_manifest/loader.rs b/cedar-policy-validator/src/entity_manifest/loader.rs
index 623235b9..d3eb835b 100644
--- a/cedar-policy-validator/src/entity_manifest/loader.rs
+++ b/cedar-policy-validator/src/entity_manifest/loader.rs
@@ -575,8 +575,8 @@ fn compute_ancestors_request(
     }
 
     Ok(AncestorsRequest {
-        ancestors,
         entity_id,
+        ancestors,
     })
 }
 
diff --git a/cedar-policy-validator/src/extensions.rs b/cedar-policy-validator/src/extensions.rs
index 937a0db5..ac3d5ec1 100644
--- a/cedar-policy-validator/src/extensions.rs
+++ b/cedar-policy-validator/src/extensions.rs
@@ -125,7 +125,7 @@ impl<'a> ExtensionSchemas<'a> {
 
     /// Get all extension types that support operator overloading
     pub fn types_with_operator_overloading(&self) -> impl Iterator<Item = &Name> + '_ {
-        self.types_with_operator_overloading.iter().cloned()
+        self.types_with_operator_overloading.iter().copied()
     }
 }
 
diff --git a/cedar-policy-validator/src/json_schema.rs b/cedar-policy-validator/src/json_schema.rs
index 31695bc4..aae88228 100644
--- a/cedar-policy-validator/src/json_schema.rs
+++ b/cedar-policy-validator/src/json_schema.rs
@@ -2050,6 +2050,10 @@ impl TypeVariant<ConditionalName> {
 }
 
 // Only used for serialization
+#[allow(
+    clippy::trivially_copy_pass_by_ref,
+    reason = "Reference required to work with derived serde serialize implementation"
+)]
 fn is_partial_schema_default(b: &bool) -> bool {
     *b == partial_schema_default()
 }
@@ -2218,6 +2222,10 @@ impl<'a> arbitrary::Arbitrary<'a> for TypeOfAttribute<RawName> {
 }
 
 // Only used for serialization
+#[allow(
+    clippy::trivially_copy_pass_by_ref,
+    reason = "Reference required to work with derived serde serialize implementation"
+)]
 fn is_record_attribute_required_default(b: &bool) -> bool {
     *b == record_attribute_required_default()
 }
diff --git a/cedar-policy-validator/src/level_validate.rs b/cedar-policy-validator/src/level_validate.rs
index 9f5aad7d..33ae4698 100644
--- a/cedar-policy-validator/src/level_validate.rs
+++ b/cedar-policy-validator/src/level_validate.rs
@@ -42,7 +42,7 @@ impl Validator {
         // Only perform level validation if validation passed.
         if peekable_errors.peek().is_none() {
             let levels_errors =
-                self.check_entity_deref_level(p, mode, &EntityDerefLevel::from(max_deref_level));
+                self.check_entity_deref_level(p, mode, EntityDerefLevel::from(max_deref_level));
             (peekable_errors.chain(levels_errors), warnings)
         } else {
             (peekable_errors.chain(vec![]), warnings)
@@ -55,7 +55,7 @@ impl Validator {
         &'a self,
         t: &'a Template,
         mode: ValidationMode,
-        max_allowed_level: &EntityDerefLevel,
+        max_allowed_level: EntityDerefLevel,
     ) -> Vec<ValidationError> {
         let typechecker = Typechecker::new(&self.schema, mode);
         let type_annotated_asts = typechecker.typecheck_by_request_env(t);
@@ -90,7 +90,7 @@ impl Validator {
     /// Returns a tuple of `(actual level used, optional violation information)`
     fn check_entity_deref_level_helper(
         e: &cedar_policy_core::ast::Expr<Option<crate::types::Type>>,
-        max_allowed_level: &EntityDerefLevel,
+        max_allowed_level: EntityDerefLevel,
         policy_id: &PolicyID,
     ) -> (EntityDerefLevel, Option<ValidationError>) {
         use crate::types::{EntityRecordKind, Type};
@@ -100,7 +100,7 @@ impl Validator {
                 EntityDerefLevel { level: 0 }, //Literals can't be dereferenced
                 None,
             ),
-            ExprKind::Var(_) => (*max_allowed_level, None), //Roots start at `max_allowed_level`
+            ExprKind::Var(_) => (max_allowed_level, None), //Roots start at `max_allowed_level`
             ExprKind::Slot(_) => (EntityDerefLevel { level: 0 }, None), //Slot will be replaced by Entity literal so treat the same
             ExprKind::Unknown(_) => (
                 EntityDerefLevel { level: 0 }, //Can't dereference an unknown
@@ -143,7 +143,7 @@ impl Validator {
                                 source_loc: e.source_loc().cloned(),
                                 policy_id: policy_id.clone(),
                                 actual_level: new_level,
-                                allowed_level: *max_allowed_level,
+                                allowed_level: max_allowed_level,
                             }
                             .into(),
                         ),
@@ -205,7 +205,7 @@ impl Validator {
                                             source_loc: e.source_loc().cloned(),
                                             policy_id: policy_id.clone(),
                                             actual_level: new_level,
-                                            allowed_level: *max_allowed_level,
+                                            allowed_level: max_allowed_level,
                                         }
                                         .into(),
                                     ),
@@ -315,7 +315,7 @@ mod levels_validation_tests {
         let result = validator.check_entity_deref_level(
             set.get_template(&PolicyID::from_string("policy0")).unwrap(),
             ValidationMode::default(),
-            &EntityDerefLevel { level: 0 },
+            EntityDerefLevel { level: 0 },
         );
         assert!(result.is_empty());
     }
@@ -333,7 +333,7 @@ mod levels_validation_tests {
         let result = validator.check_entity_deref_level(
             set.get_template(&PolicyID::from_string("policy0")).unwrap(),
             ValidationMode::default(),
-            &EntityDerefLevel { level: 0 },
+            EntityDerefLevel { level: 0 },
         );
         assert!(result.len() == 1);
     }
diff --git a/cedar-policy-validator/src/schema.rs b/cedar-policy-validator/src/schema.rs
index 40b1dee4..c56f1a7b 100644
--- a/cedar-policy-validator/src/schema.rs
+++ b/cedar-policy-validator/src/schema.rs
@@ -802,8 +802,7 @@ impl ValidatorSchema {
             .filter(|ct| {
                 // Only collect common types that are not primitives and have location data
                 let ct_name = ct.0.clone();
-                ct_name.loc().is_some()
-                    && !Primitive::is_primitive(ct_name.basename().clone().into_smolstr())
+                ct_name.loc().is_some() && !Primitive::is_primitive(ct_name.basename().as_ref())
             })
             .map(|ct| ValidatorCommonType::new(ct.0, ct.1))
             .collect();
@@ -1447,7 +1446,7 @@ impl<'a> CommonTypeResolver<'a> {
         }
 
         // Pop a node
-        while let Some(name) = work_set.iter().next().cloned() {
+        while let Some(name) = work_set.iter().next().copied() {
             work_set.remove(name);
             if let Some(deps) = self.graph.get(name) {
                 for dep in deps {
@@ -1475,7 +1474,7 @@ impl<'a> CommonTypeResolver<'a> {
 
         // The set of nodes that have not been added to the result
         // i.e., there are still in-coming edges and hence exists a cycle
-        let mut set: HashSet<&InternalName> = HashSet::from_iter(self.graph.keys().cloned());
+        let mut set: HashSet<&InternalName> = HashSet::from_iter(self.graph.keys().copied());
         for name in res.iter() {
             set.remove(name);
         }
diff --git a/cedar-policy-validator/src/types.rs b/cedar-policy-validator/src/types.rs
index f579e424..66c52d0d 100644
--- a/cedar-policy-validator/src/types.rs
+++ b/cedar-policy-validator/src/types.rs
@@ -1509,12 +1509,8 @@ pub enum Primitive {
 impl Primitive {
     /// Check if a string is a primitive
     #[cfg(feature = "extended-schema")]
-    pub(crate) fn is_primitive(s: SmolStr) -> bool {
-        let s = s.as_str();
-        match s {
-            "Bool" | "Long" | "String" => true,
-            _ => false,
-        }
+    pub(crate) fn is_primitive(s: &str) -> bool {
+        matches!(s, "Bool" | "Long" | "String")
     }
 }
 
@@ -2754,6 +2750,6 @@ mod test {
     #[test]
     #[cfg(feature = "extended-schema")]
     fn test_matches_name() {
-        assert!(Primitive::is_primitive(SmolStr::from("Long")))
+        assert!(Primitive::is_primitive("Long"))
     }
 }
diff --git a/cedar-policy/src/api.rs b/cedar-policy/src/api.rs
index eadb654d..c1bc92a2 100644
--- a/cedar-policy/src/api.rs
+++ b/cedar-policy/src/api.rs
@@ -792,7 +792,11 @@ impl Entities {
     /// format. Entity visualization is best-effort and not well tested.
     /// Feel free to submit an issue if you are using this feature and would like it improved.
     pub fn to_dot_str(&self) -> String {
-        self.0.to_dot_str()
+        let mut dot_str = String::new();
+        // PANIC SAFETY: Writing to the String `dot_str` cannot fail, so `to_dot_str` will not return an `Err` result.
+        #[allow(clippy::unwrap_used)]
+        self.0.to_dot_str(&mut dot_str).unwrap();
+        dot_str
     }
 }
 
@@ -4744,12 +4748,12 @@ action CreateList in Create appliesTo {
         let t = attrs.get_attr("tasks").unwrap();
         assert!(t.loc.is_some());
         assert_matches!(&t.attr_type, cedar_policy_validator::types::Type::Set { ref element_type } => {
-            let el = *element_type.clone().unwrap().to_owned();
+            let el = *element_type.clone().unwrap();
             assert_matches!(el, Type::EntityOrRecord(EntityRecordKind::Record { attrs, .. }) => {
                 assert!(attrs.get_attr("name").unwrap().loc.is_some());
                 assert!(attrs.get_attr("id").unwrap().loc.is_some());
                 assert!(attrs.get_attr("state").unwrap().loc.is_some());
-            })
+            });
         });
     }
 
@@ -4761,7 +4765,7 @@ action CreateList in Create appliesTo {
         let default_namespace = schema.0.namespaces().last().unwrap();
         assert_eq!(default_namespace.name, SmolStr::from("__cedar"));
         assert!(default_namespace.name_loc.is_none());
-        assert!(default_namespace.def_loc.is_none())
+        assert!(default_namespace.def_loc.is_none());
     }
 
     #[test]
@@ -5176,7 +5180,7 @@ action CreateList in Create appliesTo {
         let default_namespace = schema
             .0
             .namespaces()
-            .filter(|n| n.name == SmolStr::from("__cedar"))
+            .filter(|n| n.name == *"__cedar")
             .last()
             .unwrap();
         assert!(default_namespace.name_loc.is_none());
@@ -5185,11 +5189,11 @@ action CreateList in Create appliesTo {
         let default_namespace = schema
             .0
             .namespaces()
-            .filter(|n| n.name == SmolStr::from("Foo"))
+            .filter(|n| n.name == *"Foo")
             .last()
             .unwrap();
         assert!(default_namespace.name_loc.is_some());
-        assert!(default_namespace.def_loc.is_some())
+        assert!(default_namespace.def_loc.is_some());
     }
 }
 
diff --git a/cedar-policy/src/proto/ast.rs b/cedar-policy/src/proto/ast.rs
index 175532b4..e91a27ff 100644
--- a/cedar-policy/src/proto/ast.rs
+++ b/cedar-policy/src/proto/ast.rs
@@ -646,19 +646,16 @@ impl From<&models::Request> for ast::Request {
 
 impl From<&ast::Request> for models::Request {
     // PANIC SAFETY: experimental feature
-    #[allow(clippy::unimplemented)]
+    #[allow(clippy::expect_used)]
     fn from(v: &ast::Request) -> Self {
         Self {
             principal: Some(models::EntityUid::from(v.principal())),
             action: Some(models::EntityUid::from(v.action())),
             resource: Some(models::EntityUid::from(v.resource())),
             context: {
-                let ctx = match v.context() {
-                    Some(ctx) => ctx,
-                    None => unimplemented!(
-                        "Requests with unknown context currently cannot be modeled in protobuf"
-                    ),
-                };
+                let ctx = v.context().expect(
+                    "Requests with unknown context currently cannot be modeled in protobuf",
+                );
                 match ctx {
                     ast::Context::Value(map) => map
                         .iter()


Test patch:
diff --git a/cedar-policy-cli/tests/sample.rs b/cedar-policy-cli/tests/sample.rs
index c7c4b455..84718e04 100644
--- a/cedar-policy-cli/tests/sample.rs
+++ b/cedar-policy-cli/tests/sample.rs
@@ -1289,3 +1289,20 @@ fn test_translate_schema() {
         .assert()
         .code(0);
 }
+
+#[rstest]
+fn visualize_entities_parses_as_dot(
+    #[files("sample-data/**/entities.json")]
+    #[files("sample-data/**/entity.json")]
+    path: PathBuf,
+) {
+    let visualize = assert_cmd::Command::cargo_bin("cedar")
+        .expect("bin exists")
+        .arg("visualize")
+        .arg("--entities")
+        .arg(path)
+        .assert()
+        .code(0);
+    let visualized = std::str::from_utf8(&visualize.get_output().stdout).unwrap();
+    graphviz_rust::parse(visualized).unwrap();
+}