Fix required properties (#439)

* Fix required properties

* Fix tests

* Update dotnet.yml

.github/workflows/dotnet.yml

@@ -12,11 +12,11 @@ jobs:
     strategy:
       matrix:
         include:
-          - os: windows-latest
+          - os: windows-2022
             rid: win-x64
-          - os: ubuntu-latest
+          - os: ubuntu-22.04
             rid: linux-x64
-          - os: macos-latest
+          - os: macos-12
             rid: osx-x64
     runs-on: ${{ matrix.os }}
     env:

src/Directory.Build.props

@@ -18,8 +18,8 @@
   </PropertyGroup>
 
   <PropertyGroup>
-    <Version>7.6.0</Version>
-    <PackageVersion>7.6.0</PackageVersion>
+    <Version>7.7.0</Version>
+    <PackageVersion>7.7.0</PackageVersion>
     <AssemblyVersion>$(Version)</AssemblyVersion>
     <Authors>James Courtney</Authors>
     <Description>FlatSharp is a fast, idiomatic implementation of the FlatBuffer binary format.</Description>

src/FlatSharp.Compiler/SchemaModel/PropertyFieldModel.cs

@@ -132,14 +132,15 @@ public void WriteCode(CodeWriter writer)
 
         this.Attributes.EmitAsMetadata(writer);
 
-        string setter = this.Attributes.SetterKind switch
+        SetterKind setterKind = this.Attributes.SetterKind ?? SetterKind.Public;
+
+        string setter = setterKind switch
         {
             SetterKind.PublicInit => "init;",
             SetterKind.Protected => "protected set;",
             SetterKind.ProtectedInternal => "protected internal set;",
             SetterKind.ProtectedInit => "protected init;",
             SetterKind.ProtectedInternalInit => "protected internal init;",
-            SetterKind.Private => "private set;",
             SetterKind.None => string.Empty,
             SetterKind.Public or _ => "set;",
         };
@@ -153,8 +154,11 @@ public void WriteCode(CodeWriter writer)
         }
 
         string property = $"{access} virtual {typeName} {this.FieldName} {{ get; {setter} }}";
-        if (this.Field.Required == true)
+
+        bool isPublicSetter = setterKind == SetterKind.Public || setterKind == SetterKind.PublicInit;
+        if (this.Field.Required == true && isPublicSetter)
         {
+            // Required keyword is tricky with visibility and setters.
             writer.BeginPreprocessorIf(CSharpHelpers.Net7PreprocessorVariable, $"required {property}")
                   .Else(property)
                   .Flush();

src/FlatSharp.Compiler/SetterKind.cs

@@ -51,13 +51,8 @@ public enum SetterKind
     /// </summary>
     ProtectedInternalInit = 5,
 
-    /// <summary>
-    /// A private setter.
-    /// </summary>
-    Private = 6,
-
     /// <summary>
     /// No setter.
     /// </summary>
-    None = 7,
+    None = 6,
 }

src/FlatSharp/FlatSharp.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <GeneratePackageOnBuild>true</GeneratePackageOnBuild>
-    <TargetFrameworks>netstandard2.0;netstandard2.1;net6.0;net7.0;net8.0</TargetFrameworks>
+    <TargetFrameworks>net6.0;net7.0;net8.0</TargetFrameworks>
     <AssemblyName>FlatSharp</AssemblyName>
     <RootNamespace>FlatSharp</RootNamespace>
     <Description>FlatSharp is an idiomatic C# implementation of the FlatBuffer serialization format. Use attributes to declare your data contracts!</Description>

src/FlatSharp/Serialization/DeserializeClassDefinition.cs

@@ -45,7 +45,7 @@ internal class DeserializeClassDefinition
     protected readonly string vtableAccessor;
     protected readonly string remainingDepthAccessor;
 
-    private DeserializeClassDefinition(
+    public DeserializeClassDefinition(
         string className,
         MethodInfo? onDeserializeMethod,
         ITypeModel typeModel,
@@ -101,16 +101,6 @@ private DeserializeClassDefinition(
         }
     }
 
-    public static DeserializeClassDefinition Create(
-        string className,
-        MethodInfo? onDeserializeMethod,
-        ITypeModel typeModel,
-        int maxVTableIndex,
-        FlatBufferSerializerOptions options)
-    {
-        return new DeserializeClassDefinition(className, onDeserializeMethod, typeModel, maxVTableIndex, options);
-    }
-
     public bool HasEmbeddedBufferReference => !this.options.GreedyDeserialize;
 
     public string ClassName { get; }
@@ -222,8 +212,11 @@ private void AddPropertyDefinitions(ItemMemberModel itemModel)
         }
 
         string required = string.Empty;
-        if (itemModel.IsRequired)
+
+        // Net 6.0 doesn't define this, so leave open the opportunity for users to add their own polyfills.
+        if (itemModel.PropertyInfo.GetCustomAttributes().Any(x => x.GetType().FullName == "System.Runtime.CompilerServices.RequiredMemberAttribute"))
         {
+            FlatSharpInternal.Assert(itemModel.IsRequired, "Expecting the item to be required");
             required = " required";
         }
 

src/FlatSharp/TypeModel/StructTypeModel.cs

@@ -32,9 +32,11 @@ public class StructTypeModel : RuntimeTypeModel
     private ConstructorInfo? preferredConstructor;
     private MethodInfo? onDeserializeMethod;
     private readonly Guid guid = Guid.NewGuid();
+    private readonly FlatBufferStructAttribute? attribute;
 
     internal StructTypeModel(Type clrType, TypeModelContainer container) : base(clrType, container)
     {
+        this.attribute = clrType.GetCustomAttribute<FlatBufferStructAttribute>();
     }
 
     /// <summary>
@@ -102,10 +104,12 @@ public override CodeGeneratedMethod CreateGetMaxSizeMethodBody(GetMaxSizeCodeGen
 
     public override CodeGeneratedMethod CreateParseMethodBody(ParserCodeGenContext context)
     {
+        FlatSharpInternal.Assert(this.attribute is not null, "Attribute shouldn't be null");
+
         // We have to implement two items: The table class and the overall "read" method.
         // Let's start with the read method.
         string className = this.GetDeserializedClassName(context.Options.DeserializationOption);
-        DeserializeClassDefinition classDef = DeserializeClassDefinition.Create(
+        DeserializeClassDefinition classDef = new(
             className,
             this.onDeserializeMethod,
             this,
@@ -191,10 +195,7 @@ public override void Validate()
     {
         base.Validate();
 
-        {
-            FlatBufferStructAttribute? attribute = this.ClrType.GetCustomAttribute<FlatBufferStructAttribute>();
-            FlatSharpInternal.Assert(attribute != null, "Missing attribute.");
-        }
+        FlatSharpInternal.Assert(this.attribute is not null, "Missing attribute");
 
         // Reset in case validation is invoked multiple times.
         this.inlineSize = 0;

src/FlatSharp/TypeModel/TableTypeModel.cs

@@ -669,7 +669,7 @@ public override CodeGeneratedMethod CreateParseMethodBody(ParserCodeGenContext c
 
         // We have to implement two items: The table class and the overall "read" method.
         // Let's start with the read method.
-        var classDef = DeserializeClassDefinition.Create(className, this.onDeserializeMethod, this, this.MaxIndex, context.Options);
+        var classDef = new DeserializeClassDefinition(className, this.onDeserializeMethod, this, this.MaxIndex, context.Options);
 
         // Build up a list of property overrides.
         foreach (var item in this.IndexToMemberMap.Where(x => !x.Value.IsDeprecated))

src/Tests/CompileTests/CSharp8/CSharp8.csproj

@@ -23,6 +23,9 @@
 
     <!-- IsExternalInit not compatible with downstream languages. -->
     <FlatSharpSchema Remove="../../FlatSharpEndToEndTests/**/AccessModifiers.fbs" />
+
+    <!-- Required not compatible with C# 8. -->
+    <FlatSharpSchema Remove="../../FlatSharpEndToEndTests/**/Required.fbs" />
   </ItemGroup>
 
   <ItemGroup>

src/Tests/FlatSharpEndToEndTests/Required/Required.cs

@@ -128,5 +128,63 @@ void ParseAndUse(string fieldName)
         // Finally, make sure something doesn't throw:
         ParseAndUse("__");
     }
+
+    [TestMethod]
+    [DataRow(nameof(RequiredTable_Setters.Pub), true)]
+    [DataRow(nameof(RequiredTable_Setters.PubInit), true)]
+    [DataRow(nameof(RequiredTable_Setters.Prot), false)]
+    [DataRow(nameof(RequiredTable_Setters.ProtectedInit), false)]
+    [DataRow(nameof(RequiredTable_Setters.ProtectedInternal), false)]
+    [DataRow(nameof(RequiredTable_Setters.ProtectedInternalInit), false)]
+    [DataRow(nameof(RequiredTable_Setters.None), false)]
+    public void Only_Public_Setters_Are_CSharp_Required(string propertyName, bool expectRequired)
+    {
+        PropertyInfo property = typeof(RequiredTable_Setters).GetProperty(propertyName);
+
+#if NET7_0_OR_GREATER
+        // Make sure the property is flagged as being required.
+        Assert.AreEqual(
+            expectRequired,
+            typeof(RequiredTable_Setters).GetProperty(propertyName).GetCustomAttributes().Any(x => x.GetType().FullName == "System.Runtime.CompilerServices.RequiredMemberAttribute"));
+#endif
+
+        // Now make sure that FlatSharp still throws the error for required property missing even if the C# property is not required.
+        RequiredTable_Setters table;
+        if (propertyName == nameof(RequiredTable_Setters.None))
+        {
+            table = new(false);
+        }
+        else
+        {
+            table = new(true);
+
+            // Set to null.
+            property.SetMethod.Invoke(table, new object[] { null });
+        }
+
+        // Serialize and expect error.
+        Assert.ThrowsException<InvalidOperationException>(() => RequiredTable_Setters.Serializer.Write(new byte[1024], table));
+    }
+}
+
+public partial class RequiredTable_Setters
+{
+#if NET7_0_OR_GREATER
+    [SetsRequiredMembers]
+#endif
+    public RequiredTable_Setters(bool setNone)
+    {
+        this.Pub = "a";
+        this.PubInit = "b";
+        this.Prot = "c";
+        this.ProtectedInit = "d";
+        this.ProtectedInternal = "e";
+        this.ProtectedInternalInit = "f";
+
+        if (setNone)
+        {
+            this.None = "g";
+        }
+    }
 }
 

src/Tests/FlatSharpEndToEndTests/Required/Required.fbs

@@ -1,6 +1,8 @@
 
 attribute "fs_serializer";
 attribute "fs_valueStruct";
+attribute "fs_setter";
+attribute "fs_defaultCtor";
 
 namespace FlatSharpEndToEndTests.Required;
 
@@ -24,6 +26,7 @@ table RequiredTable (fs_serializer)
     F : ValueStruct (required);
 }
 
+
 table NonRequiredTable (fs_serializer)
 {
     A : string;
@@ -32,4 +35,15 @@ table NonRequiredTable (fs_serializer)
     D : [ ubyte ];
     E : RefStruct;
     F : ValueStruct;
-}
+}
+
+table RequiredTable_Setters (fs_serializer, fs_defaultCtor:"None")
+{
+    pub : string (required, fs_setter:"Public");
+    pub_init : string (required, fs_setter:"PublicInit");
+    prot : string (required, fs_setter:"Protected");
+    protected_internal : string (required, fs_setter:"ProtectedInternal");
+    protected_init : string (required, fs_setter:"ProtectedInit");
+    protected_internal_init : string (required, fs_setter:"ProtectedInternalInit");
+    none : string (required, fs_setter:"None");
+}