Repository: cedar-policy/cedar
Pull request #1587 (2025-04-25 15:43:47)

Pretty-print the Cedarschema files using two spaces
## Description of changes

The Cedarschema output from generated code is hard to read, e.g. from `cedar translate-schema --direction json-to-cedar -s output.json`

Example from Cedar for Kubernetes:

```
namespace k8s {
type ExtraAttribute = {"key": __cedar::String, 
"values": Set < __cedar::String >};
type FieldRequirement = {"field": __cedar::String, 
"operator": __cedar::String, 
"value": __cedar::String};
type LabelRequirement = {"key": __cedar::String, 
"operator": __cedar::String, 
"values": Set < __cedar::String >};
entity Extra = {"key": __cedar::String, 
"value"?: __cedar::String};
entity Group = {"name": __cedar::String};
entity Node in [Group] = {"extra"?: Set < ExtraAttribute >, 
"name": __cedar::String};
entity NonResourceURL = {"path": __cedar::String};
entity PrincipalUID;
entity Resource = {"apiGroup": __cedar::String, 
"fieldSelector"?: Set < FieldRequirement >, 
"labelSelector"?: Set < LabelRequirement >, 
"name"?: __cedar::String, 
"namespace"?: __cedar::String, 
"resource": __cedar::String, 
"subresource"?: __cedar::String};
entity ServiceAccount in [Group] = {"extra"?: Set < ExtraAttribute >, 
"name": __cedar::String, 
"namespace": __cedar::String};
entity User in [Group] = {"extra"?: Set < ExtraAttribute >, 
"name": __cedar::String};
action "approve" appliesTo {
  principal: [Group, Node, ServiceAccount, User],
  resource: [Resource],
  context: {}
};
action "attest" appliesTo {
  principal: [Group, Node, ServiceAccount, User],
  resource: [Resource],
  context: {}
};
action "bind" appliesTo {
  principal: [Group, Node, ServiceAccount, User],
  resource: [Resource],
  context: {}
};
action "create" appliesTo {
  principal: [Group, Node, ServiceAccount, User],
  resource: [Resource],
  context: {}
};
}
```

I quickly implemented JSON-like "pretty-printing" with four spaces as an example of how it could look like.

The same snippet:

```
namespace k8s {
    type ExtraAttribute = {
        "key": __cedar::String, 
        "values": Set<__cedar::String>
    };

    type FieldRequirement = {
        "field": __cedar::String, 
        "operator": __cedar::String, 
        "value": __cedar::String
    };

    type LabelRequirement = {
        "key": __cedar::String, 
        "operator": __cedar::String, 
        "values": Set<__cedar::String>
    };

    entity Extra = {
        "key": __cedar::String, 
        "value"?: __cedar::String
    };

    entity Group = {
        "name": __cedar::String
    };

    entity Node in [Group] = {
        "extra"?: Set<ExtraAttribute>, 
        "name": __cedar::String
    };

    entity NonResourceURL = {
        "path": __cedar::String
    };

    entity PrincipalUID;

    entity Resource = {
        "apiGroup": __cedar::String, 
        "fieldSelector"?: Set<FieldRequirement>, 
        "labelSelector"?: Set<LabelRequirement>, 
        "name"?: __cedar::String, 
        "namespace"?: __cedar::String, 
        "resource": __cedar::String, 
        "subresource"?: __cedar::String
    };

    entity ServiceAccount in [Group] = {
        "extra"?: Set<ExtraAttribute>, 
        "name": __cedar::String, 
        "namespace": __cedar::String
    };

    entity User in [Group] = {
        "extra"?: Set<ExtraAttribute>, 
        "name": __cedar::String
    };

    action "approve" appliesTo {
        principal: [Group, Node, ServiceAccount, User],
        resource: [Resource],
        context: {}
    };

    action "attest" appliesTo {
        principal: [Group, Node, ServiceAccount, User],
        resource: [Resource],
        context: {}
    };

    action "bind" appliesTo {
        principal: [Group, Node, ServiceAccount, User],
        resource: [Resource],
        context: {}
    };

    action "create" appliesTo {
        principal: [Group, Node, ServiceAccount, User],
        resource: [Resource],
        context: {}
    };
}
```

Happy to discuss if we want configurable indenting, etc., or other style suggestions.

If accepted, I can move on to updating tests, etc.

## Issue #, if available

## Checklist for requesting a review

The change in this PR is (choose one, and delete the other options):

- [x] A backwards-compatible change requiring a minor version bump to `cedar-policy` (e.g., addition of a new API).

Not sure if this output is considered by compability guarantees. It's just spacing

I confirm that this PR (choose one, and delete the other options):

- [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.

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.


luxas: cc @micahhausler as we discussed this
luxas: Cool, thanks for acceptance! 👍  

I'll move the PR into a mergable state in the coming days, and make the code a bit nicer 😄 
github-actions[bot]: 
# Coverage Report
Head Commit: ```fdf1f5cf209bd1fb51e816e74e481491ec0895da```

Base Commit: ```d95df22e17434b4c0cf85d4751ab0660637c24c6```

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

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

**Actual coverage:** 93.50%

**Status:** PASSED :white_check_mark:

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

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy-validator/src/cedar_schema/fmt.rs | :green_circle: | 115/123 | 93.50% | 52, 118, 230-231, 256-257, 287-288
</details>

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

**Actual coverage:** 81.65%

**Status:** PASSED :white_check_mark:

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

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3075/4474 | 68.73% | 68.73%
cedar-policy-cli | :red_circle: | 571/972 | 58.74% | 58.74%
cedar-policy-core | :green_circle: | 11749/13957 | 84.18% | 84.18%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8637/10079 | 85.69% | 85.69%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>

luxas: ```
     Running `/home/runner/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/rustc --crate-name highway --edition=2018 /home/runner/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/highway-0.8.1/src/lib.rs --error-format=json --json=diagnostic-rendered-ansi,artifacts,future-incompat --crate-type lib --emit=dep-info,metadata,link -C embed-bitcode=no -C debuginfo=2 --cfg 'feature="default"' --cfg 'feature="std"' --check-cfg 'cfg(docsrs,test)' --check-cfg 'cfg(feature, values("default", "std"))' -C metadata=37b2c8cb8408c810 -C extra-filename=-b2e5b0490e021a0d --out-dir /home/runner/work/cedar/cedar/cedar-spec/cedar-policy-generators/target/debug/deps -L dependency=/home/runner/work/cedar/cedar/cedar-spec/cedar-policy-generators/target/debug/deps --cap-lints allow -D warnings -F unsafe-code`
error: use of deprecated associated constant `chrono::NaiveDateTime::UNIX_EPOCH`: use `DateTime::UNIX_EPOCH` instead
   --> /home/runner/work/cedar/cedar/cedar-spec/cedar/cedar-policy-core/src/extensions/datetime.rs:333:44
    |
333 |         let delta = value - NaiveDateTime::UNIX_EPOCH;
    |                                            ^^^^^^^^^^
    |
    = note: `-D deprecated` implied by `-D warnings`
    = help: to override `-D warnings` add `#[allow(deprecated)]`

error: could not compile `cedar-policy-core` (lib) due to 1 previous error
```
seems unrelated. Maybe it was fixed in main if I rebase?

Apparently I need to add update a wasm/js unit test as well, will do.
john-h-kastner-aws: We just fixed the `UNIX_EPOCH` deprecation warning. Rebasing should do it 
luxas: @john-h-kastner-aws rebased now. But need to merge https://github.com/cedar-policy/cedar-examples/pull/227 before the wasm example test will be green here.
github-actions[bot]: 
# Coverage Report
Head Commit: ```dade7e7974c98bc496543cee28dff01e9870a14e```

Base Commit: ```5f474662a4659956eb278958e7251dafdd2a092f```

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

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

**Actual coverage:** 93.50%

**Status:** PASSED :white_check_mark:

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

| File | Status | Covered | Coverage | Missed Lines |
|:-----|:------:|:-------:|---------:|:-------------|
cedar-policy-validator/src/cedar_schema/fmt.rs | :green_circle: | 115/123 | 93.50% | 52, 118, 230-231, 256-257, 287-288
</details>

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

**Actual coverage:** 81.65%

**Status:** PASSED :white_check_mark:

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

| Package | Status | Covered | Coverage | Base Coverage |
|:--------|:------:|:-------:|---------:|--------------:|
cedar-policy | :red_circle: | 3075/4474 | 68.73% | 68.73%
cedar-policy-cli | :red_circle: | 571/972 | 58.74% | 58.74%
cedar-policy-core | :green_circle: | 11750/13958 | 84.18% | 84.18%
cedar-policy-formatter | :green_circle: | 914/1043 | 87.63% | 87.63%
cedar-policy-validator | :green_circle: | 8637/10079 | 85.69% | 85.69%
cedar-wasm | :red_circle: | 0/29 | 0.00% | 0.00%
</details>

john-h-kastner-aws: regarding wasm tests: we'll merge this first and then the fixup PR to the examples repo
luxas: Implemented the review feedback:
- de-duplicated the annotations handling without adding a circular dependency, nor moving schema-specific types into core
- made the indentation handling a bit nicer by making a type for wrapping the indentation string


Patch:
diff --git a/cedar-policy-core/src/ast/expr.rs b/cedar-policy-core/src/ast/expr.rs
index 64da7ce4..f9a36720 100644
--- a/cedar-policy-core/src/ast/expr.rs
+++ b/cedar-policy-core/src/ast/expr.rs
@@ -15,7 +15,11 @@
  */
 
 #[cfg(feature = "tolerant-ast")]
-use super::expr_allows_errors::AstExprErrorKind;
+use {
+    super::expr_allows_errors::AstExprErrorKind, crate::parser::err::ToASTError,
+    crate::parser::err::ToASTErrorKind,
+};
+
 use crate::{
     ast::*,
     expr_builder::{self, ExprBuilder as _},
@@ -397,6 +401,90 @@ impl<T> Expr<T> {
             ExprKind::Error { .. } => None,
         }
     }
+
+    /// Converts an `Expr<V>` to `B::Expr` using the provided builder.
+    ///
+    /// Preserves source location information and recursively transforms each expression node.
+    /// Note: Data may be cloned if the source expression is retained elsewhere.
+    pub fn into_expr<B: expr_builder::ExprBuilder>(self) -> B::Expr
+    where
+        T: Clone,
+    {
+        let builder = B::new().with_maybe_source_loc(self.source_loc().cloned().as_ref());
+        match self.into_expr_kind() {
+            ExprKind::Lit(lit) => builder.val(lit),
+            ExprKind::Var(var) => builder.var(var),
+            ExprKind::Slot(slot) => builder.slot(slot),
+            ExprKind::Unknown(u) => builder.unknown(u),
+            ExprKind::If {
+                test_expr,
+                then_expr,
+                else_expr,
+            } => builder.ite(
+                Arc::unwrap_or_clone(test_expr).into_expr::<B>(),
+                Arc::unwrap_or_clone(then_expr).into_expr::<B>(),
+                Arc::unwrap_or_clone(else_expr).into_expr::<B>(),
+            ),
+            ExprKind::And { left, right } => builder.and(
+                Arc::unwrap_or_clone(left).into_expr::<B>(),
+                Arc::unwrap_or_clone(right).into_expr::<B>(),
+            ),
+            ExprKind::Or { left, right } => builder.or(
+                Arc::unwrap_or_clone(left).into_expr::<B>(),
+                Arc::unwrap_or_clone(right).into_expr::<B>(),
+            ),
+            ExprKind::UnaryApp { op, arg } => {
+                let arg = Arc::unwrap_or_clone(arg).into_expr::<B>();
+                builder.unary_app(op, arg)
+            }
+            ExprKind::BinaryApp { op, arg1, arg2 } => {
+                let arg1 = Arc::unwrap_or_clone(arg1).into_expr::<B>();
+                let arg2 = Arc::unwrap_or_clone(arg2).into_expr::<B>();
+                builder.binary_app(op, arg1, arg2)
+            }
+            ExprKind::ExtensionFunctionApp { fn_name, args } => {
+                let args = Arc::unwrap_or_clone(args)
+                    .into_iter()
+                    .map(|e| e.into_expr::<B>());
+                builder.call_extension_fn(fn_name, args)
+            }
+            ExprKind::GetAttr { expr, attr } => {
+                builder.get_attr(Arc::unwrap_or_clone(expr).into_expr::<B>(), attr)
+            }
+            ExprKind::HasAttr { expr, attr } => {
+                builder.has_attr(Arc::unwrap_or_clone(expr).into_expr::<B>(), attr)
+            }
+            ExprKind::Like { expr, pattern } => {
+                builder.like(Arc::unwrap_or_clone(expr).into_expr::<B>(), pattern)
+            }
+            ExprKind::Is { expr, entity_type } => {
+                builder.is_entity_type(Arc::unwrap_or_clone(expr).into_expr::<B>(), entity_type)
+            }
+            ExprKind::Set(set) => builder.set(
+                Arc::unwrap_or_clone(set)
+                    .into_iter()
+                    .map(|e| e.into_expr::<B>()),
+            ),
+            // PANIC SAFETY: `map` is a map, so it will not have duplicates keys, so the `record` constructor cannot error.
+            #[allow(clippy::unwrap_used)]
+            ExprKind::Record(map) => builder
+                .record(
+                    Arc::unwrap_or_clone(map)
+                        .into_iter()
+                        .map(|(k, v)| (k, v.into_expr::<B>())),
+                )
+                .unwrap(),
+            #[cfg(feature = "tolerant-ast")]
+            // PANIC SAFETY: error type is Infallible so can never happen
+            #[allow(clippy::unwrap_used)]
+            ExprKind::Error { .. } => builder
+                .error(ParseErrors::singleton(ToASTError::new(
+                    ToASTErrorKind::ASTErrorNode,
+                    Loc::new(0..1, "AST_ERROR_NODE".into()),
+                )))
+                .unwrap(),
+        }
+    }
 }
 
 #[allow(dead_code)] // some constructors are currently unused, or used only in tests, but provided for completeness
@@ -793,7 +881,7 @@ impl<T: Clone> std::fmt::Display for Expr<T> {
         // To avoid code duplication between pretty-printers for AST Expr and EST Expr,
         // we just convert to EST and use the EST pretty-printer.
         // Note that converting AST->EST is lossless and infallible.
-        write!(f, "{}", crate::est::Expr::from(self.clone()))
+        write!(f, "{}", &self.clone().into_expr::<crate::est::Builder>())
     }
 }
 
@@ -801,7 +889,7 @@ impl<T: Clone> BoundedDisplay for Expr<T> {
     fn fmt(&self, f: &mut impl std::fmt::Write, n: Option<usize>) -> std::fmt::Result {
         // Like the `std::fmt::Display` impl, we convert to EST and use the EST
         // pretty-printer. Note that converting AST->EST is lossless and infallible.
-        BoundedDisplay::fmt(&crate::est::Expr::from(self.clone()), f, n)
+        BoundedDisplay::fmt(&self.clone().into_expr::<crate::est::Builder>(), f, n)
     }
 }
 
@@ -860,7 +948,11 @@ impl std::fmt::Display for Unknown {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         // Like the Display impl for Expr, we delegate to the EST pretty-printer,
         // to avoid code duplication
-        write!(f, "{}", crate::est::Expr::from(Expr::unknown(self.clone())))
+        write!(
+            f,
+            "{}",
+            Expr::unknown(self.clone()).into_expr::<crate::est::Builder>()
+        )
     }
 }
 
diff --git a/cedar-policy-core/src/est.rs b/cedar-policy-core/src/est.rs
index 6219868e..c513692c 100644
--- a/cedar-policy-core/src/est.rs
+++ b/cedar-policy-core/src/est.rs
@@ -379,7 +379,7 @@ impl From<ast::Template> for Policy {
 
 impl<T: Clone> From<ast::Expr<T>> for Clause {
     fn from(expr: ast::Expr<T>) -> Clause {
-        Clause::When(expr.into())
+        Clause::When(expr.into_expr::<Builder>())
     }
 }
 
diff --git a/cedar-policy-core/src/est/annotation.rs b/cedar-policy-core/src/est/annotation.rs
index 847e78fb..14d03a5f 100644
--- a/cedar-policy-core/src/est/annotation.rs
+++ b/cedar-policy-core/src/est/annotation.rs
@@ -43,6 +43,22 @@ impl Annotations {
     pub fn is_empty(&self) -> bool {
         self.0.is_empty()
     }
+    /// Format the annotations using a given level of base indentation
+    pub fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: usize,
+    ) -> std::fmt::Result {
+        let base_indent = " ".repeat(base_indentation);
+        for (k, v) in &self.0 {
+            if let Some(anno) = v {
+                writeln!(f, "{base_indent}@{k}({anno})")?
+            } else {
+                writeln!(f, "{base_indent}@{k}")?
+            }
+        }
+        Ok(())
+    }
 }
 
 impl From<Annotations> for ast::Annotations {
@@ -69,14 +85,7 @@ impl From<ast::Annotations> for Annotations {
 
 impl std::fmt::Display for Annotations {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        for (k, v) in &self.0 {
-            if let Some(anno) = v {
-                writeln!(f, "@{k}({anno})")?
-            } else {
-                writeln!(f, "@{k}")?
-            }
-        }
-        Ok(())
+        self.fmt_indented(f, 0)
     }
 }
 
diff --git a/cedar-policy-core/src/est/expr.rs b/cedar-policy-core/src/est/expr.rs
index 31657651..067f0ba6 100644
--- a/cedar-policy-core/src/est/expr.rs
+++ b/cedar-policy-core/src/est/expr.rs
@@ -29,8 +29,6 @@ use crate::extensions::Extensions;
 use crate::jsonvalue::JsonValueWithNoDuplicateKeys;
 use crate::parser::cst_to_ast;
 use crate::parser::err::ParseErrors;
-#[cfg(feature = "tolerant-ast")]
-use crate::parser::err::{ToASTError, ToASTErrorKind};
 use crate::parser::Node;
 use crate::parser::{cst, Loc};
 use itertools::Itertools;
@@ -1064,82 +1062,6 @@ impl Expr {
     }
 }
 
-// PANIC SAFETY: See comment on `unwrap`
-#[allow(clippy::fallible_impl_from)]
-impl<T: Clone> From<ast::Expr<T>> for Expr {
-    fn from(expr: ast::Expr<T>) -> Expr {
-        match expr.into_expr_kind() {
-            ast::ExprKind::Lit(lit) => lit.into(),
-            ast::ExprKind::Var(var) => var.into(),
-            ast::ExprKind::Slot(slot) => slot.into(),
-            ast::ExprKind::Unknown(u) => Builder::new().unknown(u),
-            ast::ExprKind::If {
-                test_expr,
-                then_expr,
-                else_expr,
-            } => Builder::new().ite(
-                Arc::unwrap_or_clone(test_expr).into(),
-                Arc::unwrap_or_clone(then_expr).into(),
-                Arc::unwrap_or_clone(else_expr).into(),
-            ),
-            ast::ExprKind::And { left, right } => Builder::new().and(
-                Arc::unwrap_or_clone(left).into(),
-                Arc::unwrap_or_clone(right).into(),
-            ),
-            ast::ExprKind::Or { left, right } => Builder::new().or(
-                Arc::unwrap_or_clone(left).into(),
-                Arc::unwrap_or_clone(right).into(),
-            ),
-            ast::ExprKind::UnaryApp { op, arg } => {
-                let arg = Arc::unwrap_or_clone(arg).into();
-                Builder::new().unary_app(op, arg)
-            }
-            ast::ExprKind::BinaryApp { op, arg1, arg2 } => {
-                let arg1 = Arc::unwrap_or_clone(arg1).into();
-                let arg2 = Arc::unwrap_or_clone(arg2).into();
-                Builder::new().binary_app(op, arg1, arg2)
-            }
-            ast::ExprKind::ExtensionFunctionApp { fn_name, args } => {
-                let args = Arc::unwrap_or_clone(args).into_iter().map(Into::into);
-                Builder::new().call_extension_fn(fn_name, args)
-            }
-            ast::ExprKind::GetAttr { expr, attr } => {
-                Builder::new().get_attr(Arc::unwrap_or_clone(expr).into(), attr)
-            }
-            ast::ExprKind::HasAttr { expr, attr } => {
-                Builder::new().has_attr(Arc::unwrap_or_clone(expr).into(), attr)
-            }
-            ast::ExprKind::Like { expr, pattern } => {
-                Builder::new().like(Arc::unwrap_or_clone(expr).into(), pattern)
-            }
-            ast::ExprKind::Is { expr, entity_type } => {
-                Builder::new().is_entity_type(Arc::unwrap_or_clone(expr).into(), entity_type)
-            }
-            ast::ExprKind::Set(set) => {
-                Builder::new().set(Arc::unwrap_or_clone(set).into_iter().map(Into::into))
-            }
-            // PANIC SAFETY: `map` is a map, so it will not have duplicates keys, so the `record` constructor cannot error.
-            #[allow(clippy::unwrap_used)]
-            ast::ExprKind::Record(map) => Builder::new()
-                .record(
-                    Arc::unwrap_or_clone(map)
-                        .into_iter()
-                        .map(|(k, v)| (k, v.into())),
-                )
-                .unwrap(),
-            #[cfg(feature = "tolerant-ast")]
-            // PANIC SAFETY: error type is Infallible so can never happen
-            #[allow(clippy::unwrap_used)]
-            ast::ExprKind::Error { .. } => Builder::new()
-                .error(ParseErrors::singleton(ToASTError::new(
-                    ToASTErrorKind::ASTErrorNode,
-                    Loc::new(0..1, "AST_ERROR_NODE".into()),
-                )))
-                .unwrap(),
-        }
-    }
-}
-
 impl From<ast::Literal> for Expr {
     fn from(lit: ast::Literal) -> Expr {
         Builder::new().val(lit)
@@ -1629,7 +1551,9 @@ mod test {
 
     #[test]
     fn display_and_bounded_display() {
-        let expr = Expr::from(parse_expr(r#"[100, [3, 4, 5], -20, "foo"]"#).unwrap());
+        let expr = parse_expr(r#"[100, [3, 4, 5], -20, "foo"]"#)
+            .unwrap()
+            .into_expr::<Builder>();
         assert_eq!(format!("{expr}"), r#"[100, [3, 4, 5], (-20), "foo"]"#);
         assert_eq!(
             BoundedToString::to_string(&expr, None),
@@ -1650,17 +1574,16 @@ mod test {
         assert_eq!(BoundedToString::to_string(&expr, Some(1)), r#"[100, ..]"#);
         assert_eq!(BoundedToString::to_string(&expr, Some(0)), r#"[..]"#);
 
-        let expr = Expr::from(
-            parse_expr(
-                r#"{
+        let expr = parse_expr(
+            r#"{
             a: 12,
             b: [3, 4, true],
             c: -20,
             "hello ∞ world": "∂µß≈¥"
         }"#,
-            )
-            .unwrap(),
-        );
+        )
+        .unwrap()
+        .into_expr::<Builder>();
         assert_eq!(
             format!("{expr}"),
             r#"{"a": 12, "b": [3, 4, true], "c": (-20), "hello ∞ world": "∂µß≈¥"}"#
diff --git a/cedar-policy-core/src/expr_builder.rs b/cedar-policy-core/src/expr_builder.rs
index ee8cf65b..6d47d785 100644
--- a/cedar-policy-core/src/expr_builder.rs
+++ b/cedar-policy-core/src/expr_builder.rs
@@ -28,7 +28,7 @@ use crate::{
 };
 
 #[cfg(feature = "tolerant-ast")]
-use crate::parser::err::ParseErrors;
+use {crate::parser::err::ParseErrors, std::fmt::Debug};
 
 /// Defines a generic interface for building different expression data
 /// structures.
@@ -46,7 +46,7 @@ pub trait ExprBuilder: Clone {
     ///  By default we fail on errors and this should be a ParseErrors
     ///  But when we run with error parsing enabled, can be Infallible
     #[cfg(feature = "tolerant-ast")]
-    type ErrorType;
+    type ErrorType: Debug;
 
     /// Construct a new expression builder for an expression that will not carry any data.
     fn new() -> Self
diff --git a/cedar-policy-validator/src/cedar_schema/fmt.rs b/cedar-policy-validator/src/cedar_schema/fmt.rs
index de1d5b5b..16733767 100644
--- a/cedar-policy-validator/src/cedar_schema/fmt.rs
+++ b/cedar-policy-validator/src/cedar_schema/fmt.rs
@@ -27,12 +27,70 @@ use thiserror::Error;
 use crate::{json_schema, RawName};
 use cedar_policy_core::{ast::InternalName, impl_diagnostic_from_method_on_nonempty_field};
 
+/// Number of spaces of indentation per level in the Cedarschema file
+pub const NUM_INDENTATION_SPACES: usize = 2;
+
+/// Helper struct to indent Cedarschema files with `NUM_INDENTATION_SPACES` spaces at each level.
+struct BaseIndentation(String);
+
+impl BaseIndentation {
+    /// Do not use any base indentation.
+    fn none() -> Self {
+        BaseIndentation(String::new())
+    }
+
+    /// Indent base using `NUM_INDENTATION_SPACES` more than current self.
+    fn next(&self) -> Self {
+        BaseIndentation(" ".repeat(self.len() + NUM_INDENTATION_SPACES))
+    }
+
+    /// Get the length of how many spaces are used for the base indentation
+    fn len(&self) -> usize {
+        self.0.len()
+    }
+}
+
+impl Display for BaseIndentation {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        f.write_str(&self.0)
+    }
+}
+
+/// Support for formatting with a given base indentation (spaces) that should be applied after newlines.
+trait IndentedDisplay {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result;
+}
+
+/// Display a type supporting indentation with the given amount of base indentation
+struct Indented<'a, T: IndentedDisplay>(&'a T, &'a BaseIndentation);
+
+impl<T: IndentedDisplay> Display for Indented<'_, T> {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.0.fmt_indented(f, self.1)
+    }
+}
+
 impl<N: Display> Display for json_schema::Fragment<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        for (ns, def) in &self.0 {
+        for (i, (ns, def)) in self.0.iter().enumerate() {
             match ns {
-                None => write!(f, "{def}")?,
-                Some(ns) => write!(f, "{}namespace {ns} {{\n{}}}\n", def.annotations, def)?,
+                // Invariant: NamespaceDefinition always prints a newline in the end
+                None => def.fmt(f)?,
+                Some(ns) => writeln!(
+                    f,
+                    "{}namespace {ns} {{\n{}}}",
+                    def.annotations,
+                    Indented(def, &BaseIndentation::none().next())
+                )?,
+            }
+
+            // extra newline to separate namespaces
+            if i < (self.0.len() - 1) {
+                writeln!(f)?
             }
         }
         Ok(())
@@ -41,14 +99,55 @@ impl<N: Display> Display for json_schema::Fragment<N> {
 
 impl<N: Display> Display for json_schema::NamespaceDefinition<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        for (n, ty) in &self.common_types {
-            writeln!(f, "{}type {n} = {};", ty.annotations, ty.ty)?
+        self.fmt_indented(f, &BaseIndentation::none())
+    }
+}
+
+impl<N: Display> IndentedDisplay for json_schema::NamespaceDefinition<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
+        let total_len = self.common_types.len() + self.entity_types.len() + self.actions.len();
+
+        for (i, (n, ty)) in self.common_types.iter().enumerate() {
+            ty.annotations.fmt_indented(f, base_indentation.len())?;
+            writeln!(
+                f,
+                "{base_indentation}type {n} = {};",
+                Indented(&ty.ty, base_indentation)
+            )?;
+
+            if i < (total_len - 1) {
+                // only skip writing an extra newline if this is the last of all items
+                writeln!(f)?
+            }
         }
-        for (n, ty) in &self.entity_types {
-            writeln!(f, "{}entity {n}{};", ty.annotations, ty)?
+        for (i, (n, ty)) in self.entity_types.iter().enumerate() {
+            ty.annotations.fmt_indented(f, base_indentation.len())?;
+            writeln!(
+                f,
+                "{base_indentation}entity {n}{};",
+                Indented(ty, base_indentation)
+            )?;
+
+            if self.common_types.len() + i < (total_len - 1) {
+                writeln!(f)?
+            }
         }
-        for (n, a) in &self.actions {
-            writeln!(f, "{}action \"{}\"{};", a.annotations, n.escape_debug(), a)?
+        for (i, (n, a)) in self.actions.iter().enumerate() {
+            a.annotations.fmt_indented(f, base_indentation.len())?;
+            writeln!(
+                f,
+                "{base_indentation}action \"{}\"{};",
+                n.escape_debug(),
+                Indented(a, base_indentation)
+            )?;
+
+            if self.common_types.len() + self.entity_types.len() + i < (total_len - 1) {
+                writeln!(f)?
+            }
         }
         Ok(())
     }
@@ -56,6 +155,16 @@ impl<N: Display> Display for json_schema::NamespaceDefinition<N> {
 
 impl<N: Display> Display for json_schema::Type<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.fmt_indented(f, &BaseIndentation::none())
+    }
+}
+
+impl<N: Display> IndentedDisplay for json_schema::Type<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
         match self {
             json_schema::Type::Type { ty, .. } => match ty {
                 json_schema::TypeVariant::Boolean => write!(f, "__cedar::Bool"),
@@ -65,8 +174,10 @@ impl<N: Display> Display for json_schema::Type<N> {
                 }
                 json_schema::TypeVariant::Extension { name } => write!(f, "__cedar::{name}"),
                 json_schema::TypeVariant::Long => write!(f, "__cedar::Long"),
-                json_schema::TypeVariant::Record(rty) => write!(f, "{rty}"),
-                json_schema::TypeVariant::Set { element } => write!(f, "Set < {element} >"),
+                json_schema::TypeVariant::Record(rty) => rty.fmt_indented(f, base_indentation),
+                json_schema::TypeVariant::Set { element } => {
+                    write!(f, "Set<{}>", Indented(element.as_ref(), base_indentation))
+                } // It is possible to do Set<{"foo": String}>
                 json_schema::TypeVariant::String => write!(f, "__cedar::String"),
             },
             json_schema::Type::CommonTypeRef { type_name, .. } => write!(f, "{type_name}"),
@@ -74,23 +185,39 @@ impl<N: Display> Display for json_schema::Type<N> {
     }
 }
 
-impl<N: Display> Display for json_schema::RecordType<N> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+impl<N: Display> IndentedDisplay for json_schema::RecordType<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
+        // Record members are indented two spaces more than the base
+        let member_indentation = base_indentation.next();
+
+        // Don't write a newline here, as there might not be attributes, and then we want just "{}"
         write!(f, "{{")?;
         for (i, (n, ty)) in self.attributes.iter().enumerate() {
-            write!(
+            if i == 0 {
+                writeln!(f)?;
+            }
+            ty.annotations.fmt_indented(f, member_indentation.len())?;
+            writeln!(
                 f,
-                "{}\"{}\"{}: {}",
-                ty.annotations,
+                "{member_indentation}\"{}\"{}: {}{}",
                 n.escape_debug(),
                 if ty.required { "" } else { "?" },
-                ty.ty
+                Indented(&ty.ty, &member_indentation),
+                // TODO: Always print trailing commas when
+                // https://github.com/cedar-policy/rfcs/blob/main/text/0071-trailing-commas.md
+                // has been implemented
+                if i < (self.attributes.len() - 1) {
+                    ","
+                } else {
+                    ""
+                }
             )?;
-            if i < (self.attributes.len() - 1) {
-                writeln!(f, ", ")?;
-            }
         }
-        write!(f, "}}")?;
+        write!(f, "{base_indentation}}}")?;
         Ok(())
     }
 }
@@ -108,8 +235,18 @@ fn fmt_non_empty_slice<T: Display>(
 
 impl<N: Display> Display for json_schema::EntityType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.fmt_indented(f, &BaseIndentation::none())
+    }
+}
+
+impl<N: Display> IndentedDisplay for json_schema::EntityType<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
         match &self.kind {
-            json_schema::EntityTypeKind::Standard(ty) => ty.fmt(f),
+            json_schema::EntityTypeKind::Standard(ty) => ty.fmt_indented(f, base_indentation),
             json_schema::EntityTypeKind::Enum { choices } => write!(
                 f,
                 " enum [{}]",
@@ -124,6 +261,16 @@ impl<N: Display> Display for json_schema::EntityType<N> {
 
 impl<N: Display> Display for json_schema::StandardEntityType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.fmt_indented(f, &BaseIndentation::none())
+    }
+}
+
+impl<N: Display> IndentedDisplay for json_schema::StandardEntityType<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
         if let Some(non_empty) = self.member_of_types.split_first() {
             write!(f, " in ")?;
             fmt_non_empty_slice(f, non_empty)?;
@@ -132,7 +279,7 @@ impl<N: Display> Display for json_schema::StandardEntityType<N> {
         let ty = &self.shape;
         // Don't print `= { }`
         if !ty.is_empty_record() {
-            write!(f, " = {ty}")?;
+            write!(f, " = {}", Indented(&ty.0, base_indentation))?;
         }
 
         if let Some(tags) = &self.tags {
@@ -145,6 +292,16 @@ impl<N: Display> Display for json_schema::StandardEntityType<N> {
 
 impl<N: Display> Display for json_schema::ActionType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        self.fmt_indented(f, &BaseIndentation::none())
+    }
+}
+
+impl<N: Display> IndentedDisplay for json_schema::ActionType<N> {
+    fn fmt_indented(
+        &self,
+        f: &mut std::fmt::Formatter<'_>,
+        base_indentation: &BaseIndentation,
+    ) -> std::fmt::Result {
         if let Some(parents) = self.member_of.as_ref().and_then(|refs| refs.split_first()) {
             write!(f, " in ")?;
             fmt_non_empty_slice(f, parents)?;
@@ -162,13 +319,15 @@ impl<N: Display> Display for json_schema::ActionType<N> {
                 }
                 // Both list are non empty
                 (Some(ps), Some(rs)) => {
+                    let member_indent = base_indentation.next();
                     write!(f, " appliesTo {{")?;
-                    write!(f, "\n  principal: ")?;
+                    write!(f, "\n{member_indent}principal: ")?;
                     fmt_non_empty_slice(f, ps)?;
-                    write!(f, ",\n  resource: ")?;
+                    write!(f, ",\n{member_indent}resource: ")?;
                     fmt_non_empty_slice(f, rs)?;
-                    write!(f, ",\n  context: {}", &spec.context.0)?;
-                    write!(f, "\n}}")?;
+                    write!(f, ",\n{member_indent}context: {}", &spec.context.0)?;
+
+                    write!(f, "\n{base_indentation}}}")?;
                 }
             }
         }
@@ -532,4 +691,21 @@ namespace TinyTodo {
 
         assert_eq!(expected_roundtrip, roundtrip_json,);
     }
+
+    #[test]
+    fn test_formatting_roundtrip() {
+        use crate::json_schema::Fragment;
+        let test_schema_str =
+            std::fs::read_to_string("src/cedar_schema/testfiles/example.cedarschema")
+                .expect("missing test schema");
+        println!("{}", test_schema_str);
+
+        let (f, _) = Fragment::from_cedarschema_str(&test_schema_str, Extensions::all_available())
+            .expect("test schema is valid");
+        // assert test schema file is already formatted
+        assert_eq!(
+            f.to_cedarschema().expect("test schema can be displayed"),
+            test_schema_str,
+        )
+    }
 }
diff --git a/cedar-policy/CHANGELOG.md b/cedar-policy/CHANGELOG.md
index 0a16c429..98162319 100644
--- a/cedar-policy/CHANGELOG.md
+++ b/cedar-policy/CHANGELOG.md
@@ -28,6 +28,9 @@ Cedar Language Version: TBD
   from a schema with `Validator::new` afterwhich a reference to the original
   schema can be retrieved using `Validator::schema`. (#1584)
 
+### Fixed
+- Apply entity conformance checking to tags (#1604)
+
 ## [4.4.0] - 2025-04-23
 
 Cedar Language Version: 4.3
diff --git a/cedar-policy/src/ffi/convert.rs b/cedar-policy/src/ffi/convert.rs
index bdabd18c..ff4f6dfb 100644
--- a/cedar-policy/src/ffi/convert.rs
+++ b/cedar-policy/src/ffi/convert.rs
@@ -495,7 +495,16 @@ mod test {
         assert_matches!(result, SchemaToTextAnswer::Success { text, warnings:_ } => {
             assert_eq!(
                 &text,
-                "entity User = {\"name\": __cedar::String};\naction \"sendMessage\" appliesTo {\n  principal: [User],\n  resource: [User],\n  context: {}\n};\n"
+                r#"entity User = {
+  "name": __cedar::String
+};
+
+action "sendMessage" appliesTo {
+  principal: [User],
+  resource: [User],
+  context: {}
+};
+"#
             );
         });
     }


Test patch:
diff --git a/cedar-policy-validator/src/cedar_schema/testfiles/example.cedarschema b/cedar-policy-validator/src/cedar_schema/testfiles/example.cedarschema
new file mode 100644
index 00000000..ed2b673b
--- /dev/null
+++ b/cedar-policy-validator/src/cedar_schema/testfiles/example.cedarschema
@@ -0,0 +1,35 @@
+entity TopLevel = {
+  "obj": {
+    "nestedStr": String
+  }
+};
+
+namespace EmptyNs {
+}
+
+namespace Ns {
+  type Bar = {
+    "obj": {
+      "nestedLong": Long,
+      "nestedObj": {
+        "nestedStr": String
+      }
+    },
+    "setWithAnonymousType": Set<{
+      "key": String,
+      "val": String
+    }>
+  };
+
+  entity Resource = {
+    "bar": Bar
+  };
+
+  entity User;
+
+  action "get" appliesTo {
+    principal: [User],
+    resource: [Resource],
+    context: {}
+  };
+}
diff --git a/cedar-policy/src/test/test.rs b/cedar-policy/src/test/test.rs
index 632037c2..3f4f6134 100644
--- a/cedar-policy/src/test/test.rs
+++ b/cedar-policy/src/test/test.rs
@@ -2203,6 +2203,14 @@ action "g" appliesTo {
         ",
         )
         .unwrap();
+        let entity = Entity::new_with_tags(
+            r#"E::"""#.parse().unwrap(),
+            std::iter::empty(),
+            std::iter::empty(),
+            std::iter::once(("".to_owned(), RestrictedExpression::new_string("".into()))),
+        )
+        .unwrap();
+        assert_matches!(Entities::from_entities([entity], Some(&schema)), Ok(_));
         let entity = Entity::new_with_tags(
             r#"E::"""#.parse().unwrap(),
             std::iter::empty(),
@@ -2227,6 +2235,14 @@ action "g" appliesTo {
         ",
         )
         .unwrap();
+        let entity = Entity::new_with_tags(
+            r#"E::"""#.parse().unwrap(),
+            std::iter::empty(),
+            std::iter::empty(),
+            std::iter::empty(),
+        )
+        .unwrap();
+        assert_matches!(Entities::from_entities([entity], Some(&schema)), Ok(_),);
         let entity = Entity::new_with_tags(
             r#"E::"""#.parse().unwrap(),
             std::iter::empty(),