Misleading messages for errors on `==` in action scope

[john-h-kastner-aws]

Category

Other

Describe the feature you'd like to request

The error message for an invalid equality in the action scope is misleading in two cases.

If you write an equality with an invalid expression on the right, it expects an entity uid or set of entity uids, but == doesn't accept a set.

jkastner@88665a03aa97 cedar % cargo run check-parse <<<'permit(principal, action == 1, resource);'
    Finished dev [unoptimized + debuginfo] target(s) in 0.58s
     Running `target/debug/cedar check-parse`
  × failed to parse policy set
  ╰─▶ expected an entity uid or set of entity uids, found literal `1`
   ╭─[<stdin>:1:1]
 1 │ permit(principal, action == 1, resource);
   ·                             ─
   ╰────

If we follow the advice and try it with a set, it now claims to want a single entity uid or a template slot, but a slot isn't allowed in the action scope.

jkastner@88665a03aa97 cedar % cargo run check-parse <<<'permit(principal, action == [Action::"foo"], resource);'
    Finished dev [unoptimized + debuginfo] target(s) in 0.27s
     Running `target/debug/cedar check-parse`
  × failed to parse policy set
  ╰─▶ the right hand side of equality in the policy scope must be a single entity uid or a template slot
   ╭─[<stdin>:1:1]
 1 │ permit(principal, action == [Action::"foo"], resource);
   ·                             ───────────────
   ╰────

Describe alternatives you've considered

.

Additional context

No response

Is this something that you'd be interested in working on?

• 👋 I may be able to implement this feature request
• ⚠️ This feature might incur a breaking change

[john-h-kastner-aws]

Part of the issue here seems to be that the error variant used for action == [...] is InvalidScopeEqualityRHS, but this should instead be reported as RefCreationError. This is the only place we use InvalidScopeEqualityRHS, so we should then remove that variant.

[cdisselkoen]

Just checking that #818 addressed both of the cases described above? The description of #818 doesn't explicitly say it addresses the second case.

[john-h-kastner-aws]

Both are fixed

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

Lines 5201 to 5204 in 489af84

	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("expected single entity uid, got: set of entity uids").exactly_one_underline(r#"[Action::"view", Action::"edit"]"#).build());
	});