Enum with Fields Cleanup (#685)

src/diagnostics/errors.rs

@@ -79,8 +79,8 @@ pub enum Error {
         enumerator_identifier: String,
     },
 
-    /// Enumerators cannot declare associated fields when their enclosing enum has an underlying type.
-    EnumeratorCannotDeclareAssociatedFields {
+    /// Enumerators cannot contain fields when their enclosing enum has an underlying type.
+    EnumeratorCannotContainFields {
         enumerator_identifier: String,
     },
 
@@ -549,8 +549,8 @@ implement_diagnostic_functions!(
     ),
     (
         "E054",
-        EnumeratorCannotDeclareAssociatedFields,
-        format!("invalid enumerator '{enumerator_identifier}': associated fields cannot be declared within enums that specify an underlying type"),
+        EnumeratorCannotContainFields,
+        format!("invalid enumerator '{enumerator_identifier}': fields cannot be declared within enums that specify an underlying type"),
         enumerator_identifier
     )
 );

src/grammar/elements/enumerator.rs

@@ -8,7 +8,7 @@ use crate::utils::ptr_util::WeakPtr;
 pub struct Enumerator {
     pub identifier: Identifier,
     pub value: EnumeratorValue,
-    pub associated_fields: Option<Vec<WeakPtr<Field>>>,
+    pub fields: Option<Vec<WeakPtr<Field>>>,
     pub parent: WeakPtr<Enum>,
     pub scope: Scope,
     pub attributes: Vec<WeakPtr<Attribute>>,
@@ -24,10 +24,11 @@ impl Enumerator {
         }
     }
 
-    pub fn associated_fields(&self) -> Option<Vec<&Field>> {
-        self.associated_fields
+    pub fn fields(&self) -> Vec<&Field> {
+        self.fields
             .as_ref()
             .map(|fields| fields.iter().map(WeakPtr::borrow).collect())
+            .unwrap_or_default()
     }
 }
 
@@ -39,7 +40,7 @@ pub enum EnumeratorValue {
 
 impl Container<Field> for Enumerator {
     fn contents(&self) -> Vec<&Field> {
-        self.associated_fields().unwrap_or_default()
+        self.fields()
     }
 }
 

src/parsers/slice/grammar.rs

@@ -396,7 +396,7 @@ fn construct_enumerator(
     parser: &mut Parser,
     (raw_comment, attributes): (RawDocComment, Vec<WeakPtr<Attribute>>),
     identifier: Identifier,
-    associated_fields: Option<Vec<OwnedPtr<Field>>>,
+    fields: Option<Vec<OwnedPtr<Field>>>,
     enumerator_value: Option<Integer<i128>>,
     span: Span,
 ) -> OwnedPtr<Enumerator> {
@@ -413,7 +413,7 @@ fn construct_enumerator(
     let mut enumerator = OwnedPtr::new(Enumerator {
         identifier,
         value,
-        associated_fields: None,
+        fields: None,
         parent: WeakPtr::create_uninitialized(), // Patched by its container.
         scope: parser.current_scope.clone(),
         attributes,
@@ -422,14 +422,14 @@ fn construct_enumerator(
     });
 
     // Add any associated fields to the enumerator.
-    if let Some(fields) = associated_fields {
+    if let Some(fields) = fields {
         let downgraded = downgrade_as!(enumerator, dyn Container<Field>);
         unsafe {
             let converted_fields = fields.into_iter().map(|mut field| {
                 field.borrow_mut().parent = downgraded.clone();
                 parser.ast.add_named_element(field)
             });
-            enumerator.borrow_mut().associated_fields = Some(converted_fields.collect());
+            enumerator.borrow_mut().fields = Some(converted_fields.collect());
         }
     }
 

src/patchers/encoding_patcher.rs

@@ -416,13 +416,13 @@ impl ComputeSupportedEncodings for Enum {
         }
 
         for enumerator in self.enumerators() {
-            // Enums with associated fields are not allowed in Slice1 mode.
-            if enumerator.associated_fields().is_some() {
+            // Enums with fields are not allowed in Slice1 mode.
+            if enumerator.fields.is_some() {
                 supported_encodings.disable(Encoding::Slice1);
                 if compilation_mode == CompilationMode::Slice1 {
-                    return Some("enumerators declared in Slice1 mode cannot have associated fields");
+                    return Some("field syntax cannot be used with enumerators declared in Slice1 mode");
                 }
-                break; // Once we've found a single enumerator with associated fields, we can stop checking.
+                break; // Once we've found a single enumerator with fields, we can stop checking.
             }
         }
 

src/validators/cycle_detection.rs

@@ -14,7 +14,7 @@ pub(super) fn detect_cycles(ast: &Ast, diagnostics: &mut Diagnostics) {
     for node in ast.as_slice() {
         cycle_detector.dependency_stack.clear(); // Make sure the detector is cleared between checks.
         match node {
-            // Only structs and enumerators (with associated fields) need to be checked for cycles.
+            // Only structs and enumerators (with fields) need to be checked for cycles.
             // It is safe for classes to contain cycles since they use reference semantics,
             // and exceptions can't cause cycles since they cannot be used as types.
             Node::Struct(struct_def) => cycle_detector.check_for_cycles(struct_def.borrow()),

src/validators/dictionary.rs

@@ -48,8 +48,21 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option<Diagnostic> {
             }
             true
         }
+
+        // Only enums with underlying types can be used as dictionary keys. Fields aren't allowed.
+        Types::Enum(enum_def) => {
+            if enum_def.underlying.is_none() {
+                let error = Diagnostic::new(Error::KeyTypeNotSupported {
+                    kind: formatted_kind(definition),
+                })
+                .set_span(type_ref.span())
+                .add_note("only enums with underlying types can be used as dictionary keys", None);
+                return Some(error);
+            }
+            true
+        }
+
         Types::Class(_) => false,
-        Types::Enum(_) => true,
         Types::CustomType(_) => true,
         Types::Sequence(_) => false,
         Types::Dictionary(_) => false,
@@ -70,9 +83,9 @@ fn check_dictionary_key_type(type_ref: &TypeRef) -> Option<Diagnostic> {
 }
 
 fn formatted_kind(definition: &dyn Type) -> String {
-    if let Types::Class(class_def) = definition.concrete_type() {
-        format!("class '{}'", class_def.identifier())
-    } else {
-        definition.kind().to_owned()
+    match definition.concrete_type() {
+        Types::Class(class_def) => format!("class '{}'", class_def.identifier()),
+        Types::Enum(enum_def) => format!("enum '{}'", enum_def.identifier()),
+        _ => definition.kind().to_owned(),
     }
 }

src/validators/enums.rs

@@ -12,11 +12,11 @@ pub fn validate_enum(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     underlying_type_cannot_be_optional(enum_def, diagnostics);
     nonempty_if_checked(enum_def, diagnostics);
 
-    // If the enum wasn't defined in a Slice1 file, validate whether associated fields or explicit values are allowed,
-    // based on whether it has an underlying type. Associated fields in Slice1 files are rejected by `encoding_patcher`.
+    // If the enum wasn't defined in a Slice1 file, validate whether fields or explicit values are allowed,
+    // based on whether it has an underlying type. Fields in Slice1 files are already rejected by `encoding_patcher`.
     if !enum_def.supported_encodings().supports(Encoding::Slice1) {
         if enum_def.underlying.is_some() {
-            cannot_contain_associated_fields(enum_def, diagnostics);
+            cannot_contain_fields(enum_def, diagnostics);
         } else {
             cannot_contain_explicit_values(enum_def, diagnostics);
         }
@@ -138,14 +138,14 @@ fn nonempty_if_checked(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     }
 }
 
-/// Validate that this enum's enumerators don't specify any associated fields.
+/// Validate that this enum's enumerators don't specify any fields.
 /// This function should only be called for enums with underlying types.
-fn cannot_contain_associated_fields(enum_def: &Enum, diagnostics: &mut Diagnostics) {
+fn cannot_contain_fields(enum_def: &Enum, diagnostics: &mut Diagnostics) {
     debug_assert!(enum_def.underlying.is_some());
 
     for enumerator in enum_def.enumerators() {
-        if enumerator.associated_fields().is_some() {
-            Diagnostic::new(Error::EnumeratorCannotDeclareAssociatedFields {
+        if enumerator.fields.is_some() {
+            Diagnostic::new(Error::EnumeratorCannotContainFields {
                 enumerator_identifier: enumerator.identifier().to_owned(),
             })
             .set_span(enumerator.span())

src/visitor.rs

@@ -261,8 +261,8 @@ impl Enumerator {
     /// This function delegates to `visitor.visit_enumerator`.
     pub fn visit_with(&self, visitor: &mut impl Visitor) {
         visitor.visit_enumerator(self);
-        if let Some(associated_fields) = &self.associated_fields {
-            for field in associated_fields {
+        if let Some(fields) = &self.fields {
+            for field in fields {
                 field.borrow().visit_with(visitor);
             }
         }

tests/dictionaries/key_type.rs

@@ -91,7 +91,8 @@ fn collections_are_disallowed(key_type: &str, key_kind: &str) {
     check_diagnostics(diagnostics, [expected]);
 }
 
-#[test_case("MyEnum", "unchecked enum MyEnum: int8 {}" ; "enums")]
+#[test_case("MyEnum", "enum MyEnum: int8 { A }" ; "simple enums")]
+#[test_case("MyEnum", "unchecked enum MyEnum: int8 {}" ; "unchecked simple enums")]
 #[test_case("MyCustom", "custom MyCustom" ; "custom_types")]
 fn allowed_constructed_types(key_type: &str, key_type_def: &str) {
     // Arrange
@@ -107,15 +108,17 @@ fn allowed_constructed_types(key_type: &str, key_type_def: &str) {
     assert_parses(slice);
 }
 
-#[test_case("MyClass", "class", "Slice1"; "classes")]
-fn disallowed_constructed_types(key_type: &str, key_kind: &str, mode: &str) {
+#[test_case("MyEnum", "enum MyEnum { A }", "enum", "Slice2" ; "enums")]
+#[test_case("MyEnum", "unchecked enum MyEnum {}", "enum", "Slice2" ; "unchecked enums")]
+#[test_case("MyClass", "class MyClass {}", "class", "Slice1"; "classes")]
+fn disallowed_constructed_types(key_type: &str, key_type_def: &str, key_kind: &str, mode: &str) {
     // Arrange
     let slice = format!(
         "
             mode = {mode}
             module Test
 
-            {key_kind} {key_type} {{}}
+            {key_type_def}
             typealias Dict = Dictionary<{key_type}, uint8>
         "
     );

tests/enums/container.rs

@@ -92,19 +92,19 @@ mod associated_fields {
         // Assert
         let a = ast.find_element::<Enumerator>("Test::E::A").unwrap();
         assert!(matches!(a.value, EnumeratorValue::Implicit(0)));
-        assert!(a.associated_fields().is_none());
+        assert!(a.fields.is_none());
 
         let b = ast.find_element::<Enumerator>("Test::E::B").unwrap();
         assert!(matches!(b.value, EnumeratorValue::Implicit(1)));
-        assert!(b.associated_fields().unwrap().len() == 1);
+        assert!(b.fields.as_ref().unwrap().len() == 1);
 
         let c = ast.find_element::<Enumerator>("Test::E::C").unwrap();
         assert!(matches!(c.value, EnumeratorValue::Implicit(2)));
-        assert!(c.associated_fields().unwrap().len() == 2);
+        assert!(c.fields.as_ref().unwrap().len() == 2);
 
         let d = ast.find_element::<Enumerator>("Test::E::D").unwrap();
         assert!(matches!(d.value, EnumeratorValue::Implicit(3)));
-        assert!(d.associated_fields().unwrap().len() == 0);
+        assert!(d.fields.as_ref().unwrap().len() == 0);
     }
 
     #[test_case("unchecked enum", true ; "unchecked")]
@@ -207,7 +207,7 @@ mod underlying_type {
         let diagnostics = parse_for_diagnostics(slice);
 
         // Assert
-        let expected = Diagnostic::new(Error::EnumeratorCannotDeclareAssociatedFields {
+        let expected = Diagnostic::new(Error::EnumeratorCannotContainFields {
             enumerator_identifier: "B".to_owned(),
         });
         check_diagnostics(diagnostics, [expected]);

tests/enums/mode_compatibility.rs

@@ -54,7 +54,7 @@ mod slice1 {
         check_diagnostics(diagnostics, [expected]);
     }
 
-    /// Verifies that associated fields are disallowed in Slice1 mode.
+    /// Verifies that enumerators with fields are disallowed in Slice1 mode.
     #[test]
     fn associated_fields_fail() {
         // Arrange
@@ -77,13 +77,13 @@ mod slice1 {
             mode: CompilationMode::Slice1,
         })
         .add_note(
-            "enumerators declared in Slice1 mode cannot have associated fields",
+            "field syntax cannot be used with enumerators declared in Slice1 mode",
             None,
         );
         check_diagnostics(diagnostics, [expected]);
     }
 
-    /// Verifies that even empty associated field lists are disallowed in Slice1 mode.
+    /// Verifies that even empty field lists are disallowed in Slice1 mode.
     #[test]
     fn empty_associated_fields_fail() {
         // Arrange
@@ -106,7 +106,7 @@ mod slice1 {
             mode: CompilationMode::Slice1,
         })
         .add_note(
-            "enumerators declared in Slice1 mode cannot have associated fields",
+            "field syntax cannot be used with enumerators declared in Slice1 mode",
             None,
         );
         check_diagnostics(diagnostics, [expected]);