cedar-policy/cedar

More restrictions on type shadowing / collisions

Closed this issue · 2 comments

Category

Cedar language or syntax features/changes

Describe the feature you'd like to request

We recently decided to disallow Set, Record, Entity, and Extension as common type names, reverting a change that was originally planned for 4.0. (#1070)

Since that issue was created, #1060 improved our typename resolution algorithms considerably, with two consequences relevant for this issue:

  1. on main (but in no release as of this writing), primitive type names like Long and String are also allowed as common type names
  2. on main (but in no release as of this writing), both primitive type names (Long, String, etc) and the other reserved names (Set, Entity, etc) are allowed as common type names in nonempty namespaces. #1070 was ambiguous about whether it applies only to common types in the empty namespaces, or to common types in all namespaces.

This issue asks that we revert the two consequences listed above, returning to the released behavior. Specifically, both primitive type names (Long, String, etc) and the other reserved names in #1070 (Set, Entity, etc) should be disallowed as common type names, in all namespaces.

This issue specifically does not ask to disallow any of these names as entity type names, only common type names. All of these are allowed today (in released versions of Cedar) as entity type names, so that would be a breaking change. One consequence of this is that we will still need the shadowing checks proposed in RFC 70.

Describe alternatives you've considered

We could make more of these names legal as common types, but that would make it harder for folks programmatically consuming the JSON schema format, and there doesn't seem to be a good reason to allow these names as common types (in any namespace), as it's more likely to create confusion than be useful.

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

Upon checking the status quo, it seems that we do not forbid declarations of common types of such names. Instead, we disallow (i.e., throw an unknown field ... error) when they are referenced as common types. I assume that we want to error on such declarations.

I believe that's the status quo only on current main. The 3.3.0 status quo, in my understanding (I haven't confirmed), is that we do forbid declarations of common types of these names?