Revert "implement schema-format parsers for RFC 68 (#1136)" (#1202)

Signed-off-by: Craig Disselkoen <[email protected]>

Author: cdisselkoen

cedar-policy-validator/src/cedar_schema/ast.rs

@@ -251,28 +251,15 @@ impl<N> From<PrimitiveType> for json_schema::TypeVariant<N> {
 }
 
 /// Attribute declarations, used in records and entity types.
-/// One [`AttrDecl`] is one key-value pair, or an embedded attribute map pair `?: value_ty`.
-///
-/// The data structures in this file, and the Cedar schema format parser in
-/// general, permissively allow `EAMap`s to appear anywhere an [`AttrDecl`] is
-/// expected (including inside other `EAMap`s), in order to provide better error
-/// messages when `EAMap`s are encountered in illegal but plausible positions
+/// One [`AttrDecl`] is one key-value pair.
 #[derive(Debug, Clone)]
-pub enum AttrDecl {
-    /// A normal attribute declaration `name: ty` or `name?: ty`
-    Concrete {
-        /// Name of this attribute
-        name: Node<SmolStr>,
-        /// Whether or not it is a required attribute (default `true`)
-        required: bool,
-        /// The type of this attribute
-        ty: Node<Type>,
-    },
-    /// An `EAMap` declaration `?: ty`
-    EAMap {
-        /// Value type of the `EAMap`
-        value_ty: Node<Type>,
-    },
+pub struct AttrDecl {
+    /// Name of this attribute
+    pub name: Node<SmolStr>,
+    /// Whether or not it is a required attribute (default `true`)
+    pub required: bool,
+    /// The type of this attribute
+    pub ty: Node<Type>,
 }
 
 /// The target of a [`PRAppDecl`]

cedar-policy-validator/src/cedar_schema/err.rs

@@ -298,12 +298,6 @@ impl From<ToJsonSchemaError> for ToJsonSchemaErrors {
     }
 }
 
-impl From<EAMapError> for ToJsonSchemaErrors {
-    fn from(e: EAMapError) -> Self {
-        Self::from(ToJsonSchemaError::from(e))
-    }
-}
-
 impl Display for ToJsonSchemaErrors {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{}", self.0.first()) // intentionally showing only the first error; see #326 for discussion on a similar error type
@@ -405,10 +399,6 @@ pub enum ToJsonSchemaError {
     #[error(transparent)]
     #[diagnostic(transparent)]
     ReservedSchemaKeyword(#[from] ReservedSchemaKeyword),
-    /// Errors relating to embedded attribute maps
-    #[error(transparent)]
-    #[diagnostic(transparent)]
-    EAMap(#[from] EAMapError),
 }
 
 impl ToJsonSchemaError {
@@ -521,86 +511,6 @@ impl Diagnostic for ReservedName {
     }
 }
 
-/// An embedded attribute map (RFC 68) was encountered where one is not allowed
-#[derive(Debug, Clone, Error, PartialEq, Eq)]
-#[error("found an embedded attribute map type, but embedded attribute maps are not allowed in this position")]
-pub struct EAMapNotAllowedHereError {
-    /// Source location of the `EAMap`
-    pub(crate) source_loc: Loc,
-    /// Context-dependent help text
-    pub(crate) help: Option<EAMapNotAllowedHereHelp>,
-}
-
-impl Diagnostic for EAMapNotAllowedHereError {
-    impl_diagnostic_from_source_loc_field!(source_loc);
-
-    fn help<'a>(&'a self) -> Option<Box<dyn Display + 'a>> {
-        self.help.as_ref().map(|help| Box::new(help) as _)
-    }
-}
-
-#[derive(Debug, Clone, Error, PartialEq, Eq)]
-pub(crate) enum EAMapNotAllowedHereHelp {
-    #[error("Embedded attribute maps are not allowed as the top-level descriptor of all attributes of an entity. Try making an entity attribute to hold the embedded attribute map. E.g., `attributes: {{ ?: someType }}`.")]
-    TopLevelEAMap,
-}
-
-/// Encountered a type like `{ foo: Long, ?: String }` that mixes concrete attributes with an `EAMap`.
-/// This is currently not allowed.
-#[derive(Debug, Clone, Error, PartialEq, Eq)]
-#[error("this type contains both concrete attributes and an embedded attribute map (`?:`), which is not allowed")]
-pub struct EAMapWithConcreteAttributesError {
-    /// Source location of the `EAMap` declaration
-    pub(crate) source_loc: Loc,
-}
-
-impl Diagnostic for EAMapWithConcreteAttributesError {
-    impl_diagnostic_from_source_loc_field!(source_loc);
-}
-
-/// Encountered a type like `{ ?: Long, ?: String }` that mixes two or more `EAMap` declarations.
-/// This is currently not allowed.
-#[derive(Debug, Clone, Error, PartialEq, Eq)]
-#[error("this type contains two or more different embedded attribute map declarations (`?:`), which is not allowed")]
-pub struct MultipleEAMapDeclarationsError {
-    /// Source location of the first `EAMap` declaration
-    pub(crate) source_loc_1: Loc,
-    /// Source location of the second `EAMap` declaration
-    pub(crate) source_loc_2: Loc,
-}
-
-impl Diagnostic for MultipleEAMapDeclarationsError {
-    impl_diagnostic_from_two_source_loc_fields!(source_loc_1, source_loc_2);
-
-    fn help<'a>(&'a self) -> Option<Box<dyn std::fmt::Display + 'a>> {
-        Some(Box::new(
-            "try separating this into two different attributes",
-        ))
-    }
-}
-
-/// Errors relating to embedded attribute maps (`EAMap`s)
-//
-// This is NOT a publicly exported error type.
-#[derive(Debug, Clone, Diagnostic, Error, PartialEq, Eq)]
-#[non_exhaustive]
-pub enum EAMapError {
-    /// An embedded attribute map (RFC 68) was encountered where one is not allowed
-    #[error(transparent)]
-    #[diagnostic(transparent)]
-    NotAllowedHere(#[from] EAMapNotAllowedHereError),
-    /// Encountered a type like `{ foo: Long, ?: String }` that mixes concrete attributes with an `EAMap`.
-    /// This is currently not allowed.
-    #[error(transparent)]
-    #[diagnostic(transparent)]
-    WithConcreteAttributes(#[from] EAMapWithConcreteAttributesError),
-    /// Encountered a type like `{ ?: Long, ?: String }` that mixes two or more `EAMap` declarations.
-    /// This is currently not allowed.
-    #[error(transparent)]
-    #[diagnostic(transparent)]
-    MultipleEAMapDeclarations(#[from] MultipleEAMapDeclarationsError),
-}
-
 #[derive(Debug, Clone, PartialEq, Eq, Error)]
 #[error("unknown type name: `{name}`")]
 pub struct UnknownTypeName {

cedar-policy-validator/src/cedar_schema/fmt.rs

@@ -74,7 +74,7 @@ impl<N: Display> Display for json_schema::Type<N> {
     }
 }
 
-impl<N: Display> Display for json_schema::RecordType<json_schema::RecordAttributeType<N>> {
+impl<N: Display> Display for json_schema::RecordType<N> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         write!(f, "{{")?;
         for (i, (n, ty)) in self.attributes.iter().enumerate() {
@@ -94,37 +94,6 @@ impl<N: Display> Display for json_schema::RecordType<json_schema::RecordAttribut
     }
 }
 
-impl<N: Display> Display for json_schema::RecordType<json_schema::EntityAttributeType<N>> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{{")?;
-        for (i, (n, ty)) in self.attributes.iter().enumerate() {
-            write!(
-                f,
-                "\"{}\"{}: {}",
-                n.escape_debug(),
-                if ty.required { "" } else { "?" },
-                ty.ty
-            )?;
-            if i < (self.attributes.len() - 1) {
-                write!(f, ", ")?;
-            }
-        }
-        write!(f, "}}")?;
-        Ok(())
-    }
-}
-
-impl<N: Display> Display for json_schema::EntityAttributeTypeInternal<N> {
-    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        match self {
-            json_schema::EntityAttributeTypeInternal::Type(ty) => ty.fmt(f),
-            json_schema::EntityAttributeTypeInternal::EAMap { value_type } => {
-                write!(f, "{{ ?: {value_type} }}")
-            }
-        }
-    }
-}
-
 /// Create a non-empty with borrowed contents from a slice
 fn non_empty_slice<T>(v: &[T]) -> Option<NonEmpty<&T>> {
     let vs: Vec<&T> = v.iter().collect();

cedar-policy-validator/src/cedar_schema/grammar.lalrpop

@@ -183,18 +183,15 @@ pub Type: Node<SType> = {
         => Node::with_source_loc(SType::Record(ds.unwrap_or_default()), Loc::new(l..r, Arc::clone(src))),
 }
 
-// AttrDecls := [Name ['?'] | '?'] ':' Type [',' | ',' AttrDecls]
+// AttrDecls := Name ['?'] ':' Type [',' | ',' AttrDecls]
 AttrDecls: Vec<Node<AttrDecl>> = {
     <l:@L> <name: Name> <required:"?"?> ":" <ty:Type> ","? <r:@R>
-        => vec![Node::with_source_loc(AttrDecl::Concrete { name, required: required.is_none(), ty}, Loc::new(l..r, Arc::clone(src)))],
+        => vec![Node::with_source_loc(AttrDecl { name, required: required.is_none(), ty}, Loc::new(l..r, Arc::clone(src)))],
     <l:@L> <name: Name> <required:"?"?> ":" <ty:Type> "," <r:@R> <mut ds: AttrDecls>
-        => {ds.insert(0, Node::with_source_loc(AttrDecl::Concrete { name, required: required.is_none(), ty}, Loc::new(l..r, Arc::clone(src)))); ds},
-    <l:@L> "?" ":" <value_ty:Type> ","? <r:@R>
-        => vec![Node::with_source_loc(AttrDecl::EAMap { value_ty }, Loc::new(l..r, Arc::clone(src)))],
-    <l:@L> "?" ":" <value_ty:Type> "," <r:@R> <mut ds: AttrDecls>
-        => {ds.insert(0, Node::with_source_loc(AttrDecl::EAMap { value_ty }, Loc::new(l..r, Arc::clone(src)))); ds},
+        => {ds.insert(0, Node::with_source_loc(AttrDecl { name, required: required.is_none(), ty}, Loc::new(l..r, Arc::clone(src)))); ds},
 }
 
+
 Comma<E>: Vec<E> = {
     <e:E?> => e.into_iter().collect(),
     <mut es:(<E> ",")+> <e:E> => {