Add support for annotations without explicit value

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

@@ -1085,7 +1085,11 @@ impl From<StaticPolicy> for TemplateBody {
 impl std::fmt::Display for TemplateBody {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for (k, v) in self.annotations.iter() {
-            writeln!(f, "@{}(\"{}\")", k, v.val.escape_debug())?
+            write!(f, "@{k}")?;
+            if let Some(v) = &v.raw_val {
+                write!(f, "(\"{}\")", v.escape_debug())?;
+            }
+            writeln!(f)?;
         }
         write!(
             f,
@@ -1163,16 +1167,51 @@ impl From<BTreeMap<AnyId, Annotation>> for Annotations {
 /// Struct which holds the value of a particular annotation
 #[derive(Serialize, Deserialize, Clone, Hash, Eq, PartialEq, Debug, PartialOrd, Ord)]
 pub struct Annotation {
-    /// Annotation value
-    pub val: SmolStr,
+    /// Annotation value. `None` for annotations without a value, i.e., `@foo`.
+    /// An annotation without a value should be treated as equivalent to the
+    /// value being `""`. This interpretation is implemented by the `AsRef<str>`
+    /// impl an `val` method below.
+    raw_val: Option<SmolStr>,
     /// Source location. Note this is the location of _the entire key-value
     /// pair_ for the annotation, not just `val` above
-    pub loc: Option<Loc>,
+    loc: Option<Loc>,
+}
+
+impl Annotation {
+    /// Construct a new annotation.
+    pub fn new(val: Option<SmolStr>, loc: Option<Loc>) -> Self {
+        Self { raw_val: val, loc }
+    }
+
+    /// Get the annotation value, treating an annotation without a value as
+    /// being equivalent to the value `""`.
+    pub fn val(&self) -> &str {
+        self.as_ref()
+    }
+
+    /// Get the annotation value, returning `None` for annotations without a
+    /// value. We generally want to treat this as equivalent to `""`, but we
+    /// occasionally want a lossless representation.
+    pub fn raw_val(&self) -> Option<&SmolStr> {
+        self.raw_val.as_ref()
+    }
+
+    /// Get the annotation value, returning `None` for annotations without a
+    /// value. We generally want to treat this as equivalent to `""`, but we
+    /// occasionally want a lossless representation.
+    pub fn into_raw_val(self) -> Option<SmolStr> {
+        self.raw_val
+    }
+
+    /// Get the location for the whole annotation (not just the value).
+    pub fn loc(&self) -> Option<&Loc> {
+        self.loc.as_ref()
+    }
 }
 
 impl AsRef<str> for Annotation {
     fn as_ref(&self) -> &str {
-        &self.val
+        self.raw_val.as_ref().map(SmolStr::as_str).unwrap_or("")
     }
 }
 
@@ -1704,7 +1743,11 @@ impl ActionConstraint {
 impl std::fmt::Display for StaticPolicy {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for (k, v) in self.0.annotations.iter() {
-            writeln!(f, "@{}(\"{}\")", k, v.val.escape_debug())?
+            write!(f, "@{k}")?;
+            if let Some(v) = &v.raw_val {
+                write!(f, "(\"{}\")", v.escape_debug())?;
+            }
+            writeln!(f)?;
         }
         write!(
             f,

cedar-policy-core/src/est.rs

@@ -66,7 +66,7 @@ pub struct Policy {
     #[serde(skip_serializing_if = "BTreeMap::is_empty")]
     #[serde_as(as = "serde_with::MapPreventDuplicates<_,_>")]
     #[cfg_attr(feature = "wasm", tsify(type = "Record<string, string>"))]
-    annotations: BTreeMap<ast::AnyId, SmolStr>,
+    annotations: BTreeMap<ast::AnyId, Option<SmolStr>>,
 }
 
 /// Serde JSON structure for a `when` or `unless` clause in the EST format
@@ -140,7 +140,10 @@ impl TryFrom<cst::Policy> for Policy {
             action: action.into(),
             resource: resource.into(),
             conditions,
-            annotations: annotations.into_iter().map(|(k, v)| (k, v.val)).collect(),
+            annotations: annotations
+                .into_iter()
+                .map(|(k, v)| (k, v.into_raw_val()))
+                .collect(),
         })
     }
 }
@@ -230,7 +233,7 @@ impl Policy {
             None,
             self.annotations
                 .into_iter()
-                .map(|(key, val)| (key, ast::Annotation { val, loc: None }))
+                .map(|(key, val)| (key, ast::Annotation::new(val, None)))
                 .collect(),
             self.effect,
             self.principal.try_into()?,
@@ -276,7 +279,7 @@ impl From<ast::Policy> for Policy {
             conditions: vec![ast.non_scope_constraints().clone().into()],
             annotations: ast
                 .annotations()
-                .map(|(k, v)| (k.clone(), v.val.clone()))
+                .map(|(k, v)| (k.clone(), v.raw_val().cloned()))
                 .collect(),
         }
     }
@@ -293,7 +296,7 @@ impl From<ast::Template> for Policy {
             conditions: vec![ast.non_scope_constraints().clone().into()],
             annotations: ast
                 .annotations()
-                .map(|(k, v)| (k.clone(), v.val.clone()))
+                .map(|(k, v)| (k.clone(), v.raw_val().cloned()))
                 .collect(),
         }
     }
@@ -308,7 +311,11 @@ impl<T: Clone> From<ast::Expr<T>> for Clause {
 impl std::fmt::Display for Policy {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         for (k, v) in self.annotations.iter() {
-            writeln!(f, "@{k}(\"{}\") ", v.escape_debug())?;
+            write!(f, "@{k}")?;
+            if let Some(v) = v {
+                write!(f, "(\"{}\")", v.escape_debug())?;
+            }
+            writeln!(f)?;
         }
         write!(
             f,
@@ -586,6 +593,86 @@ mod test {
         );
     }
 
+    #[test]
+    fn annotated_without_value_policy() {
+        let policy = r#"@foo permit(principal, action, resource);"#;
+        let cst = parser::text_to_cst::parse_policy(policy)
+            .unwrap()
+            .node
+            .unwrap();
+        let est: Policy = cst.try_into().unwrap();
+        let expected_json = json!(
+            {
+                "effect": "permit",
+                "principal": {
+                    "op": "All",
+                },
+                "action": {
+                    "op": "All",
+                },
+                "resource": {
+                    "op": "All",
+                },
+                "conditions": [],
+                "annotations": { "foo": null, }
+            }
+        );
+        assert_eq!(
+            serde_json::to_value(&est).unwrap(),
+            expected_json,
+            "\nExpected:\n{}\n\nActual:\n{}\n\n",
+            serde_json::to_string_pretty(&expected_json).unwrap(),
+            serde_json::to_string_pretty(&est).unwrap()
+        );
+        let old_est = est.clone();
+        let roundtripped = est_roundtrip(est);
+        assert_eq!(&old_est, &roundtripped);
+        let est = text_roundtrip(&old_est);
+        assert_eq!(&old_est, &est);
+
+        // during the lossy transform to AST, the only difference for this policy is that
+        // a `when { true }` is added
+        let expected_json_after_roundtrip = json!(
+            {
+                "effect": "permit",
+                "principal": {
+                    "op": "All",
+                },
+                "action": {
+                    "op": "All",
+                },
+                "resource": {
+                    "op": "All",
+                },
+                "conditions": [
+                    {
+                        "kind": "when",
+                        "body": {
+                            "Value": true
+                        }
+                    }
+                ],
+                "annotations": { "foo": null, }
+            }
+        );
+        let roundtripped = serde_json::to_value(ast_roundtrip(est.clone())).unwrap();
+        assert_eq!(
+            roundtripped,
+            expected_json_after_roundtrip,
+            "\nExpected after roundtrip:\n{}\n\nActual after roundtrip:\n{}\n\n",
+            serde_json::to_string_pretty(&expected_json_after_roundtrip).unwrap(),
+            serde_json::to_string_pretty(&roundtripped).unwrap()
+        );
+        let roundtripped = serde_json::to_value(circular_roundtrip(est)).unwrap();
+        assert_eq!(
+            roundtripped,
+            expected_json_after_roundtrip,
+            "\nExpected after roundtrip:\n{}\n\nActual after roundtrip:\n{}\n\n",
+            serde_json::to_string_pretty(&expected_json_after_roundtrip).unwrap(),
+            serde_json::to_string_pretty(&roundtripped).unwrap()
+        );
+    }
+
     /// Test that we can use Cedar reserved words like `if` and `has` as annotation keys
     #[test]
     fn reserved_words_as_annotations() {

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

@@ -31,7 +31,7 @@ pub struct Annotation {
     /// key
     pub key: Node<Ident>,
     /// value
-    pub value: Node<Str>,
+    pub value: Option<Node<Str>>,
 }
 
 /// Literal strings