Delete Legacy Tests (#398)

Delete legacy tests, replace ifs with asserts, and more

.github/workflows/codecov.yml

@@ -95,10 +95,6 @@ jobs:
             --output stryker.coverage.xml `
             --targetargs "src/FlatSharp.Compiler/bin/Debug/net7.0/FlatSharp.Compiler.dll --nullable-warnings false --normalize-field-names true --input `"$fbs`" -o src/tests/Stryker/CodeGen --mutation-testing-mode"
       
-    - name: Legacy Tests
-      working-directory: src
-      run: dotnet test Tests/FlatSharpTests -c Debug -p:SignAssembly=false --collect:"XPlat Code Coverage" --settings Tests/coverlet.runsettings -f net7.0
-
     - name: E2E Tests
       working-directory: src
       run: dotnet test Tests/FlatSharpEndToEndTests -c Debug -p:SignAssembly=false --collect:"XPlat Code Coverage" --settings Tests/coverlet.runsettings -f net7.0

.github/workflows/dotnet.yml

@@ -72,11 +72,6 @@ jobs:
       working-directory: src/Tests/FlatSharpCompilerTests
       run: dotnet test -c Release /p:SignAssembly=true --verbosity normal
 
-    - if: runner.os == 'Windows'
-      name: Legacy Test
-      working-directory: src/Tests/FlatSharpTests
-      run: dotnet test -c Release /p:SignAssembly=true --verbosity normal
-
     - if: runner.os == 'Windows'
       name: Upload Packages
       uses: actions/upload-artifact@v2

src/FlatSharp.Compiler/FlatSharp.Compiler.csproj

@@ -10,6 +10,7 @@
     <CopyLocalLockFileAssemblies>true</CopyLocalLockFileAssemblies>
     <NuspecProperties>$(NuspecProperties);OutDir=$(MSBuildThisFileDirectory)\bin\$(Configuration)</NuspecProperties>
     <Nullable>annotations</Nullable>
+    <Deterministic>true</Deterministic>
 
     <!-- NU5127: Nuget package doesn't contain any frameworks. This is intentional. -->
     <NoWarn>$(NoWarn);CS3021;3021;NU5127</NoWarn>
@@ -53,6 +54,7 @@
 
   <ItemGroup>
     <ProjectReference Include="..\FlatSharp.Runtime\FlatSharp.Runtime.csproj" />
+    <ProjectReference Include="..\FlatSharp.UnityPolyfills\FlatSharp.UnityPolyfills.csproj" />
     <ProjectReference Include="..\FlatSharp\FlatSharp.csproj" />
   </ItemGroup>
 </Project>

src/FlatSharp.Compiler/FlatSharpCompiler.cs

@@ -594,7 +594,7 @@ private static string GetFlatcPath()
                                 Options = localOptions,
                                 InputHash = inputHash,
                                 PreviousAssembly = assembly,
-                                TypeModelContainer = TypeModelContainer.CreateDefault().WithUnitySupport(localOptions.UnityAssemblyPath is not null),
+                                TypeModelContainer = TypeModelContainer.CreateDefault().WithUnitySupport(true),
                             });
 
                         return true;
@@ -679,6 +679,10 @@ private static Assembly[] BuildAdditionalReferences(CompilerOptions options, IEn
         {
             references.Add(Assembly.LoadFrom(options.UnityAssemblyPath));
         }
+        else
+        {
+            references.Add(typeof(Unity.Collections.NativeArray<>).Assembly);
+        }
 
         return references.ToArray();
     }

src/FlatSharp.Compiler/SchemaModel/BaseReferenceTypeSchemaModel.cs

@@ -195,7 +195,7 @@ protected virtual void EmitStaticConstructor(CodeWriter writer, CompileContext c
         using (writer.WithBlock())
         {
             var keyProperty = this.properties.Values.SingleOrDefault(p => p.Field.Key);
-            if (keyProperty is not null)
+            if (keyProperty is not null && context.CompilePass == CodeWritingPass.LastPass)
             {
                 writer.AppendLine($"global::FlatSharp.SortedVectorHelpers.RegisterKeyLookup<{this.Name}, {keyProperty.Field.Type.ResolveTypeOrElementTypeName(this.Schema, keyProperty.Attributes)}>(x => x.{keyProperty.FieldName}, {keyProperty.Index});");
             }

src/FlatSharp.Compiler/SchemaModel/ValueUnionSchemaModel.cs

@@ -71,6 +71,7 @@ protected override void OnWriteCode(CodeWriter writer, CompileContext context)
         bool generateUnsafeItemsOriginal = context.CompilePass >= CodeWritingPass.LastPass && this.Attributes.UnsafeUnion == true;
         bool generateUnsafeItems = generateUnsafeItemsOriginal;
 
+        HashSet<string> seenTypes = new();
         List<(string resolvedType, EnumVal value, int? size)> innerTypes = new List<(string, EnumVal, int?)>();
         foreach (var inner in this.union.Values.Select(x => x.Value))
         {
@@ -85,6 +86,11 @@ protected override void OnWriteCode(CodeWriter writer, CompileContext context)
 
             long discriminator = inner.Value;
             string typeName = inner.UnionType.ResolveTypeOrElementTypeName(this.Schema, this.Attributes);
+            if (!seenTypes.Add(typeName))
+            {
+                ErrorContext.Current.RegisterError($"FlatSharp unions may not contain duplicate types. Union = {this.FullName}");
+                continue;
+            }
 
             int? size = null;
             Type? type = context.PreviousAssembly?.GetType(typeName);

src/FlatSharp.Runtime/FlatSharpInternal.cs

@@ -71,6 +71,19 @@ public static void AssertLittleEndian()
             throw new FlatSharpInternalException(message);
         }
     }
+
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    public static void AssertWellAligned<TElement>(int alignment)
+        where TElement : unmanaged
+    {
+        var size = Unsafe.SizeOf<TElement>();
+
+        // Inlining this method should allow this check to be elided as the alignment is a constant from the callsite.
+        if (size % alignment != 0)
+        {
+            throw new InvalidOperationException($"Type '{typeof(TElement).FullName}' does not support Unsafe Span operations because the size ({size}) is not a multiple of the alignment ({alignment}).");
+        }
+    }
 }
 
 [ExcludeFromCodeCoverage]

src/FlatSharp.Runtime/GeneratedSerializerWrapper.cs

@@ -57,8 +57,6 @@ private GeneratedSerializerWrapper(GeneratedSerializerWrapper<T> template)
 
     public Type RootType => typeof(T);
 
-    public string? CSharp => this.lazyCSharp.Value;
-
     public FlatBufferDeserializationOption DeserializationOption => this.option;
 
     public int GetMaxSize(T item)
@@ -223,11 +221,13 @@ int ISerializer.Write<TSpanWriter>(TSpanWriter writer, Span<byte> destination, o
         };
     }
 
+    [ExcludeFromCodeCoverage]
     public ISerializer<T> WithSettings(Action<SerializerSettings> settingsCallback)
     {
         return this.WithSettingsCore(settingsCallback);
     }
 
+    [ExcludeFromCodeCoverage]
     ISerializer ISerializer.WithSettings(Action<SerializerSettings> settingsCallback)
     {
         return this.WithSettingsCore(settingsCallback);

src/FlatSharp.Runtime/IO/SpanWriterExtensions.cs

@@ -50,15 +50,10 @@ public static class SpanWriterExtensions
     {
         // Since we are copying bytes here, only LE is supported.
         FlatSharpInternal.AssertLittleEndian();
+        FlatSharpInternal.AssertWellAligned<TElement>(alignment);
 
         var size = Unsafe.SizeOf<TElement>();
 
-        // Inlining this method should allow this check to be elided as the alignment is a constant from the callsite.
-        if (size % alignment != 0)
-        {
-            throw new InvalidOperationException($"Type '{typeof(TElement).FullName}' does not support Unsafe Span serialization because the size ({size}) is not a multiple of the alignment ({alignment}).");
-        }
-
         int numberOfItems = buffer.Length;
         int vectorStartOffset = ctx.AllocateVector(
             itemAlignment: alignment,

src/FlatSharp.Runtime/SortedVectorHelpers.cs

@@ -58,8 +58,8 @@ public static class SortedVectorHelpers
         {
             using SimpleStringComparer cmp = new SimpleStringComparer(str);
 
-            return BinarySearchByFlatBufferKey<ListIndexable<TTable, string?>, TTable, string?, SimpleStringComparer>(
-                new ListIndexable<TTable, string?>(sortedVector),
+            return BinarySearchByFlatBufferKey<ListIndexable<TTable, string>, TTable, string, SimpleStringComparer>(
+                new ListIndexable<TTable, string>(sortedVector),
                 sortedVector,
                 cmp);
         }
@@ -86,8 +86,8 @@ public static class SortedVectorHelpers
         {
             using SimpleStringComparer cmp = new SimpleStringComparer(str);
 
-            return BinarySearchByFlatBufferKey<ReadOnlyListIndexable<TTable, string?>, TTable, string?, SimpleStringComparer>(
-                new ReadOnlyListIndexable<TTable, string?>(sortedVector),
+            return BinarySearchByFlatBufferKey<ReadOnlyListIndexable<TTable, string>, TTable, string, SimpleStringComparer>(
+                new ReadOnlyListIndexable<TTable, string>(sortedVector),
                 sortedVector,
                 cmp);
         }
@@ -235,6 +235,7 @@ private static void EnsureInitialized()
             }
         }
 
+        [ExcludeFromCodeCoverage]
         [MethodImpl(MethodImplOptions.NoInlining)]
         [DoesNotReturn]
         private static void ThrowNotInitialized()
@@ -319,8 +320,14 @@ public ReadOnlyMemory<byte> KeyAt(int index)
             // Follow soffset to start of vtable.
             int vtableStart = offset - buffer.ReadInt(offset);
 
-            // Offset within the table.
-            int tableOffset = buffer.ReadUShort(vtableStart + 4 + (2 * this.keyIndex));
+            ushort vtableLength = buffer.ReadUShort(vtableStart);
+            int tableOffset = 0;
+            int keyFieldOffset = 4 + (2 * this.keyIndex);
+
+            if (keyFieldOffset + sizeof(ushort) <= vtableLength)
+            {
+                tableOffset = buffer.ReadUShort(vtableStart + keyFieldOffset);
+            }
 
             if (tableOffset == 0)
             {
@@ -371,7 +378,7 @@ public void Dispose()
         }
     }
 
-    private struct SimpleStringComparer : ISimpleComparer<string?>, ISimpleComparer<ReadOnlyMemory<byte>>
+    private struct SimpleStringComparer : ISimpleComparer<string>, ISimpleComparer<ReadOnlyMemory<byte>>
     {
         private readonly byte[] pooledArray;
         private readonly int length;
@@ -385,12 +392,9 @@ public SimpleStringComparer(string right)
             this.length = enc.GetBytes(right, 0, right.Length, this.pooledArray, 0);
         }
 
-        public int CompareTo(string? left)
+        public int CompareTo(string left)
         {
-            if (left is null)
-            {
-                throw new InvalidOperationException("Sorted FlatBuffer vectors may not have null-valued keys.");
-            }
+            FlatSharpInternal.Assert(left is not null, "Sorted FlatBuffer vectors may not have null-valued keys.");
 
             var enc = SerializationHelpers.Encoding;
             int comp;

src/FlatSharp.sln

@@ -17,8 +17,6 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Benchmark", "Benchmarks\Ben
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "ExperimentalBenchmark", "Benchmarks\ExperimentalBenchmark\ExperimentalBenchmark.csproj", "{BD1950AE-7A04-4677-AD81-35F512D45112}"
 EndProject
-Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "FlatSharpTests", "Tests\FlatSharpTests\FlatSharpTests.csproj", "{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8}"
-EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "FlatSharpCompilerTests", "Tests\FlatSharpCompilerTests\FlatSharpCompilerTests.csproj", "{A72F5D45-6AAF-40F7-AB73-1F4E33CE9A9D}"
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Google.FlatBuffers", "Google.FlatBuffers\Google.FlatBuffers.csproj", "{D0C5D64D-68B1-467A-A097-A49B17A637E8}"
@@ -61,10 +59,6 @@ Global
 		{BD1950AE-7A04-4677-AD81-35F512D45112}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{BD1950AE-7A04-4677-AD81-35F512D45112}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{BD1950AE-7A04-4677-AD81-35F512D45112}.Release|Any CPU.Build.0 = Release|Any CPU
-		{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
-		{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8}.Debug|Any CPU.Build.0 = Debug|Any CPU
-		{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8}.Release|Any CPU.ActiveCfg = Release|Any CPU
-		{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8}.Release|Any CPU.Build.0 = Release|Any CPU
 		{A72F5D45-6AAF-40F7-AB73-1F4E33CE9A9D}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
 		{A72F5D45-6AAF-40F7-AB73-1F4E33CE9A9D}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{A72F5D45-6AAF-40F7-AB73-1F4E33CE9A9D}.Release|Any CPU.ActiveCfg = Release|Any CPU
@@ -100,7 +94,6 @@ Global
 	GlobalSection(NestedProjects) = preSolution
 		{09AC4B32-5EAF-4FB0-B2E3-925EF4F5DDAD} = {83478353-8C5A-41C2-84C2-F79488B43CB0}
 		{BD1950AE-7A04-4677-AD81-35F512D45112} = {83478353-8C5A-41C2-84C2-F79488B43CB0}
-		{2D7F2684-3C86-4D97-8E9B-824D5F6E8FB8} = {D1E90BAE-FC51-44DB-8215-1D9BB6059886}
 		{A72F5D45-6AAF-40F7-AB73-1F4E33CE9A9D} = {D1E90BAE-FC51-44DB-8215-1D9BB6059886}
 		{E6B5266D-A983-4A9C-BA2D-85AE1B7CEF07} = {D1E90BAE-FC51-44DB-8215-1D9BB6059886}
 		{F9C48012-AB11-4541-911C-BAF2B0B1158B} = {83478353-8C5A-41C2-84C2-F79488B43CB0}

src/FlatSharp/Serialization/CSharpHelpers.cs

@@ -34,6 +34,7 @@ internal static string GetGlobalCompilableTypeName(this Type t)
     internal static string GetCompilableTypeName(this Type t)
     {
         FlatSharpInternal.Assert(!string.IsNullOrEmpty(t.FullName), $"{t} has null/empty full name.");
+        FlatSharpInternal.Assert(!t.IsArray, "Not expecting an array");
 
         string name;
         if (t.IsGenericType)
@@ -46,10 +47,6 @@ internal static string GetCompilableTypeName(this Type t)
 
             name = $"{t.FullName.Split('`')[0]}<{string.Join(", ", parameters)}>";
         }
-        else if (t.IsArray)
-        {
-            name = $"{GetCompilableTypeName(t.GetElementType()!)}[]";
-        }
         else
         {
             name = t.FullName;

src/FlatSharp/Serialization/RoslynSerializerGenerator.cs

@@ -155,10 +155,10 @@ public RoslynSerializerGenerator(FlatBufferSerializerOptions options, TypeModelC
     internal (string text, string serializerTypeName) GenerateCSharpRecursive<TRoot>()
     {
         ITypeModel rootModel = this.typeModelContainer.CreateTypeModel(typeof(TRoot));
-        if (rootModel.SchemaType != FlatBufferSchemaType.Table)
-        {
-            throw new InvalidFlatBufferDefinitionException($"Can only compile [FlatBufferTable] elements as root types. Type '{typeof(TRoot).GetCompilableTypeName()}' is a {rootModel.SchemaType}.");
-        }
+
+        FlatSharpInternal.Assert(
+            rootModel.SchemaType == FlatBufferSchemaType.Table,
+            $"Can only compile [FlatBufferTable] elements as root types. Type '{typeof(TRoot).GetCompilableTypeName()}' is a {rootModel.SchemaType}.");
 
         IMethodNameResolver resolver = new DefaultMethodNameResolver();
 

src/FlatSharp/TypeModel/EnumTypeModel.cs

@@ -41,15 +41,13 @@ public override void Initialize()
         this.underlyingTypeModel = this.typeModelContainer.CreateTypeModel(underlyingType);
 
         var attribute = enumType.GetCustomAttribute<FlatBufferEnumAttribute>();
-        if (attribute == null)
-        {
-            throw new InvalidFlatBufferDefinitionException($"Enum '{CSharpHelpers.GetCompilableTypeName(enumType)}' is not tagged with a [FlatBufferEnum] attribute.");
-        }
+        FlatSharpInternal.Assert(
+            attribute is not null,
+            $"Enum '{CSharpHelpers.GetCompilableTypeName(enumType)}' is not tagged with a [FlatBufferEnum] attribute.");
 
-        if (attribute.DeclaredUnderlyingType != Enum.GetUnderlyingType(enumType))
-        {
-            throw new InvalidFlatBufferDefinitionException($"Enum '{CSharpHelpers.GetCompilableTypeName(enumType)}' declared underlying type '{attribute.DeclaredUnderlyingType}', but was actually '{CSharpHelpers.GetCompilableTypeName(Enum.GetUnderlyingType(enumType))}'");
-        }
+        FlatSharpInternal.Assert(
+            attribute.DeclaredUnderlyingType == Enum.GetUnderlyingType(enumType),
+            $"Enum '{CSharpHelpers.GetCompilableTypeName(enumType)}' declared underlying type '{attribute.DeclaredUnderlyingType}', but was actually '{CSharpHelpers.GetCompilableTypeName(Enum.GetUnderlyingType(enumType))}'");
     }
 
     public override bool IsParsingInvariant => true;
@@ -66,10 +64,6 @@ public override void Initialize()
 
     public override bool IsValidVectorMember => true;
 
-    public override bool IsValidUnionMember => false;
-
-    public override bool IsValidSortedVectorKey => false;
-
     public override bool SerializesInline => true;
 
     public override bool SerializeMethodRequiresContext => this.underlyingTypeModel.SerializeMethodRequiresContext;

src/FlatSharp/TypeModel/FlatSharpTypeModelProvider.cs

@@ -86,11 +86,11 @@ public bool TryCreateTypeModel(TypeModelContainer container, Type type, [NotNull
         var tableAttribute = type.GetCustomAttribute<FlatBufferTableAttribute>();
         var structAttribute = type.GetCustomAttribute<FlatBufferStructAttribute>();
 
-        if (tableAttribute is not null && structAttribute is not null)
-        {
-            throw new InvalidFlatBufferDefinitionException($"Type '{CSharpHelpers.GetCompilableTypeName(type)}' is declared as both [FlatBufferTable] and [FlatBufferStruct].");
-        }
-        else if (tableAttribute is not null)
+        FlatSharpInternal.Assert(
+            tableAttribute is null || structAttribute is null,
+            $"Type '{CSharpHelpers.GetCompilableTypeName(type)}' is declared as both [FlatBufferTable] and [FlatBufferStruct].");
+
+        if (tableAttribute is not null)
         {
             typeModel = new TableTypeModel(type, container);
             return true;

src/FlatSharp/TypeModel/ItemMemberModel.cs

@@ -48,10 +48,7 @@ public enum SetMethodKind
         this.IsRequired = attribute.Required;
         this.Attribute = attribute;
 
-        if (getMethod is null)
-        {
-            throw new InvalidFlatBufferDefinitionException($"Property {this.DeclaringType} on did not declare a getter.");
-        }
+        FlatSharpInternal.Assert(getMethod is not null, $"Property {this.DeclaringType} on did not declare a getter.");
 
         if (setMethod is not null)
         {
@@ -66,22 +63,19 @@ public enum SetMethodKind
     internal virtual void Validate()
     {
         MethodInfo getMethod = this.PropertyInfo.GetMethod!; // validated in .ctor.
-        var setMethod = this.PropertyInfo.SetMethod;
+        bool validAccessor = getMethod.IsPublic || !string.IsNullOrEmpty(this.CustomAccessor);
 
-        if (!getMethod.IsPublic && string.IsNullOrEmpty(this.CustomAccessor))
-        {
-            throw new InvalidFlatBufferDefinitionException($"Property {this.DeclaringType} must declare a public getter.");
-        }
+        FlatSharpInternal.Assert(
+            validAccessor,
+            $"Property {this.DeclaringType} must declare a public getter.");
 
-        if (!ValidateVirtualPropertyMethod(getMethod, false))
-        {
-            throw new InvalidFlatBufferDefinitionException($"Property {this.DeclaringType} did not declare a public/protected virtual getter.");
-        }
+        FlatSharpInternal.Assert(
+            ValidateVirtualPropertyMethod(getMethod, false),
+            $"Property {this.DeclaringType} did not declare a public/protected virtual getter.");
 
-        if (!ValidateVirtualPropertyMethod(setMethod, true))
-        {
-            throw new InvalidFlatBufferDefinitionException($"Property {this.DeclaringType} declared a set method, but it was not public/protected and virtual.");
-        }
+        FlatSharpInternal.Assert(
+            ValidateVirtualPropertyMethod(this.PropertyInfo.SetMethod, true),
+            $"Property {this.DeclaringType} declared a set method, but it was not public/protected and virtual.");
     }
 
     protected string DeclaringType
@@ -124,10 +118,7 @@ private static bool ValidateVirtualPropertyMethod(MethodInfo? method, bool allow
             return allowNull;
         }
 
-        if (!CanBeOverridden(method))
-        {
-            return false;
-        }
+        FlatSharpInternal.Assert(CanBeOverridden(method), "virtual method expected");
 
         return method.IsPublic || method.IsFamilyOrAssembly || method.IsFamily;
     }