Rework generated unknown, remove top-level Discriminant property (#3871)

icerpc · Dec 13, 2023 · 1f32830 · 1f32830
1 parent 5933ac1
commit 1f32830
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 90 deletions.
diff --git a/tests/ZeroC.Slice.Tests/EnumWithFieldsTests.cs b/tests/ZeroC.Slice.Tests/EnumWithFieldsTests.cs
@@ -62,7 +62,7 @@ public void Decode_unchecked_enum_returns_unknown()
         // Assert
         Assert.That(decoded, Is.InstanceOf<Shape.Unknown>());
         Assert.That(decoder.Consumed, Is.EqualTo(encoder.EncodedByteCount));
-        Assert.That(decoded.Discriminant, Is.EqualTo(RevisedShape.Square.Discriminant));
+        Assert.That(((Shape.Unknown)decoded).Discriminant, Is.EqualTo(RevisedShape.Square.Discriminant));
     }
 
     [Test]

diff --git a/tools/slicec-cs/src/builders.rs b/tools/slicec-cs/src/builders.rs
@@ -132,6 +132,11 @@ impl ContainerBuilder {
         self
     }
 
+    pub fn add_field(&mut self, field: String) -> &mut Self {
+        self.fields.push(field);
+        self
+    }
+
     pub fn add_fields(&mut self, fields: &[&Field]) -> &mut Self {
         for field in fields {
             let type_string = field

diff --git a/tools/slicec-cs/src/generators/enum_generator.rs b/tools/slicec-cs/src/generators/enum_generator.rs
@@ -50,8 +50,7 @@ fn enum_declaration(enum_def: &Enum) -> CodeBlock {
 
         let mut builder = ContainerBuilder::new(
             &format!(
-                r#"
-[Dunet.Union]
+                r#"[Dunet.Union]
 {} abstract partial record class"#,
                 enum_def.access_modifier(),
             ),
@@ -72,33 +71,13 @@ fn enum_declaration(enum_def: &Enum) -> CodeBlock {
             builder.add_attribute(attribute);
         }
 
-        builder.add_block({
-            let mut code = CodeBlock::default();
-            code.writeln(&CommentTag::new(
-                "summary",
-                "The discriminant of this enumerator.".into(),
-            ));
-            code.writeln("public int Discriminant => GetDiscriminant();");
-            code
-        });
-
         builder.add_block(
             FunctionBuilder::new("internal abstract", "void", "Encode", FunctionType::Declaration)
                 .add_never_editor_browsable_attribute()
                 .add_parameter("ref SliceEncoder", "encoder", None, None)
                 .build(),
         );
 
-        builder.add_block(
-            FunctionBuilder::new(
-                "private protected abstract",
-                "int",
-                "GetDiscriminant",
-                FunctionType::Declaration,
-            )
-            .build(),
-        );
-
         builder.build()
     }
 }
@@ -165,9 +144,9 @@ fn enumerators_as_nested_records(enum_def: &Enum) -> CodeBlock {
             let mut code = CodeBlock::default();
             code.writeln(&CommentTag::new(
                 "summary",
-                "The discriminant of this enumerator.".to_owned(),
+                "The discriminant of this enumerator, used for encoding/decoding.".to_owned(),
             ));
-            writeln!(code, "public new const int Discriminant = {};", enumerator.value());
+            writeln!(code, "public const int Discriminant = {};", enumerator.value());
             code
         });
 
@@ -211,8 +190,6 @@ fn enumerators_as_nested_records(enum_def: &Enum) -> CodeBlock {
                 .build()
         );
 
-        builder.add_block("private protected override int GetDiscriminant() => Discriminant;".into());
-
         code.add_block(&builder.build());
     }
 
@@ -221,72 +198,44 @@ fn enumerators_as_nested_records(enum_def: &Enum) -> CodeBlock {
 
         let mut builder = ContainerBuilder::new("public partial record class", "Unknown");
 
-        builder.add_comment(
-            "summary",
-            format!(
-                "Represents an enumerator not defined in the local Slice definition of unchecked enum '{enum_name}'.",
-                enum_name = enum_def.identifier(),
-            ),
-        );
-
-        builder.add_block({
-            let mut code = CodeBlock::default();
-            code.writeln(&CommentTag::new(
+        builder
+            .add_field("int Discriminant".to_owned())
+            .add_field("global::System.ReadOnlyMemory<byte> Fields".to_owned())
+            .add_base(enum_def.escape_identifier())
+            .add_comment(
                 "summary",
-                "The fields of this unknown enumerator.".into(),
-            ));
-            writeln!(
-                code,
-                "public global::System.ReadOnlyMemory<byte> Fields {{ get; init; }}"
-            );
-            code
-        });
-
-        builder.add_block("private readonly int _discriminant;".into());
-
-        builder.add_block(
-            FunctionBuilder::new("public", "", "Unknown", FunctionType::BlockBody)
-                .add_comment("summary", "Constructs a new instance of Unknown.")
-                .add_comment_with_attribute(
-                    "param",
-                    "name",
-                    "discriminant",
-                    "The discriminant of this unknown enumerator.",
-                )
-                .add_comment_with_attribute(
-                    "param",
-                    "name",
-                    "fields",
-                    "The encoded fields of this unknown enumerator.",
-                )
-                .add_parameter("int", "discriminant", None, None)
-                .add_parameter("global::System.ReadOnlyMemory<byte>", "fields", None, None)
-                .set_body({
-                    let mut code = CodeBlock::default();
-                    code.writeln("_discriminant = discriminant;");
-                    code.writeln("Fields = fields;");
-                    code
-                })
-                .build(),
-        );
-
-        builder.add_block(
-            FunctionBuilder::new("internal override", "void", "Encode", FunctionType::BlockBody)
-                .add_parameter("ref SliceEncoder", "encoder", None, None)
-                .set_body({
-                    let mut code = CodeBlock::default();
-                    code.writeln("encoder.EncodeVarInt32(_discriminant);");
-                    code.writeln("encoder.EncodeSize(Fields.Length);");
-                    code.writeln("encoder.WriteByteSpan(Fields.Span);");
-
-                    code
-                })
-                .build(),
-        );
+                format!(
+                    "Represents an enumerator not defined in the local Slice definition of unchecked enum '{enum_name}'.",
+                    enum_name = enum_def.identifier(),
+                ),
+            )
+            .add_comment_with_attribute(
+                "param",
+                "name",
+                "Discriminant",
+                "The discriminant of this unknown enumerator.",
+            )
+            .add_comment_with_attribute(
+                "param",
+                "name",
+                "Fields",
+                "The encoded fields of this unknown enumerator.",
+            )
+            .add_block(
+                FunctionBuilder::new("internal override", "void", "Encode", FunctionType::BlockBody)
+                    .add_parameter("ref SliceEncoder", "encoder", None, None)
+                    .add_never_editor_browsable_attribute()
+                    .set_body({
+                        let mut code = CodeBlock::default();
+                        code.writeln("encoder.EncodeVarInt32(Discriminant);");
+                        code.writeln("encoder.EncodeSize(Fields.Length);");
+                        code.writeln("encoder.WriteByteSpan(Fields.Span);");
 
-        builder.add_block("private protected override int GetDiscriminant() => _discriminant;".into());
+                        code
+                    })
+                    .build(),
+            );
 
-        builder.build();
         code.add_block(&builder.build());
     }