Better error for action == ?action

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

@@ -679,26 +679,22 @@ impl Node<Option<cst::VariableDef>> {
         }
 
         let action_constraint = if let Some((op, rel_expr)) = &vardef.ineq {
-            let refs = rel_expr.to_refs(errs, ast::Var::Action)?;
-            match (op, refs) {
-                (cst::RelOp::In, OneOrMultipleRefs::Multiple(euids)) => {
-                    Some(ActionConstraint::is_in(euids))
-                }
-                (cst::RelOp::In, OneOrMultipleRefs::Single(euid)) => {
-                    Some(ActionConstraint::is_in([euid]))
-                }
-                (cst::RelOp::Eq, OneOrMultipleRefs::Single(euid)) => {
-                    Some(ActionConstraint::is_eq(euid))
-                }
-                (cst::RelOp::Eq, OneOrMultipleRefs::Multiple(_)) => {
-                    errs.push(rel_expr.to_ast_err(ToASTErrorKind::InvalidScopeEqualityRHS));
-                    None
+            match op {
+                cst::RelOp::In => match rel_expr.to_refs(errs, ast::Var::Action)? {
+                    OneOrMultipleRefs::Single(single_ref) => {
+                        Some(ActionConstraint::is_in([single_ref]))
+                    }
+                    OneOrMultipleRefs::Multiple(refs) => Some(ActionConstraint::is_in(refs)),
+                },
+                cst::RelOp::Eq => {
+                    let single_ref = rel_expr.to_ref(ast::Var::Action, errs)?;
+                    Some(ActionConstraint::is_eq(single_ref))
                 }
-                (cst::RelOp::InvalidSingleEq, _) => {
+                cst::RelOp::InvalidSingleEq => {
                     errs.push(self.to_ast_err(ToASTErrorKind::InvalidSingleEq));
                     None
                 }
-                (op, _) => {
+                op => {
                     errs.push(self.to_ast_err(ToASTErrorKind::InvalidConstraintOperator(*op)));
                     None
                 }
@@ -5041,15 +5037,15 @@ mod tests {
 
             (
                 r#"permit(principal, action == ?action, resource);"#,
-                ExpectedErrorMessageBuilder::error("expected single entity uid or set of entity uids, got: template slot").exactly_one_underline("?action").build(),
+                ExpectedErrorMessageBuilder::error("expected single entity uid, got: template slot").exactly_one_underline("?action").build(),
             ),
             (
                 r#"permit(principal, action in ?action, resource);"#,
                 ExpectedErrorMessageBuilder::error("expected single entity uid or set of entity uids, got: template slot").exactly_one_underline("?action").build(),
             ),
             (
                 r#"permit(principal, action == ?principal, resource);"#,
-                ExpectedErrorMessageBuilder::error("expected single entity uid or set of entity uids, got: template slot").exactly_one_underline("?principal").build(),
+                ExpectedErrorMessageBuilder::error("expected single entity uid, got: template slot").exactly_one_underline("?principal").build(),
             ),
             (
                 r#"permit(principal, action in ?principal, resource);"#,
@@ -5204,7 +5200,7 @@ mod tests {
     fn scope_action_eq_set() {
         let p_src = r#"permit(principal, action == [Action::"view", Action::"edit"], resource);"#;
         assert_matches!(parse_policy_template(None, p_src), Err(e) => {
-            expect_err(p_src, &miette::Report::new(e), &ExpectedErrorMessageBuilder::error("the right hand side of equality in the policy scope must be a single entity uid or a template slot").exactly_one_underline(r#"[Action::"view", Action::"edit"]"#).build());
+            expect_err(p_src, &miette::Report::new(e), &ExpectedErrorMessageBuilder::error("expected single entity uid, got: set of entity uids").exactly_one_underline(r#"[Action::"view", Action::"edit"]"#).build());
         });
     }
 

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

@@ -233,12 +233,6 @@ pub enum ToASTErrorKind {
         "policy scope constraints must be either `==`, `in`, `is`, or `_ is _ in _`"
     ))]
     InvalidConstraintOperator(cst::RelOp),
-    /// Returned when the right hand side of `==` in a policy scope clause is not a single Entity UID or a template slot.
-    /// This is valid in Cedar conditions, but not in the Scope
-    #[error(
-        "the right hand side of equality in the policy scope must be a single entity uid or a template slot"
-    )]
-    InvalidScopeEqualityRHS,
     /// Returned when an Entity UID used as an action does not have the type `Action`
     #[error("expected an entity uid with the type `Action` but got `{0}`")]
     #[diagnostic(help("action entities must have type `Action`, optionally in a namespace"))]

cedar-policy/CHANGELOG.md

@@ -40,6 +40,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Validation error message for an invalid attribute access now reports the
   correct attribute and entity type when accessing an optional attribute that is
   itself an entity.
+- The error message returend when parsing an invalid action scope constraint
+  `action == ?action` no longer suggests that `action == [...]` would be a
+  valid scope constraint.
 
 ## [3.1.3] - 2024-04-15