Address review comments

clockworklabs · Nov 12, 2024 · 28c430a · 28c430a
1 parent bc96dd2
commit 28c430a
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 31 deletions.
diff --git a/crates/bindings-csharp/BSATN.Runtime.Tests/BSATN.Runtime.Tests.csproj b/crates/bindings-csharp/BSATN.Runtime.Tests/BSATN.Runtime.Tests.csproj
@@ -11,8 +11,7 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <PackageReference Include="CSharpier.Core" Version="0.28.2" />
-    <PackageReference Include="Microsoft.Extensions.Logging" Version="8.0.0" />
+    <PackageReference Include="CsCheck" Version="4.1.0" />
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.0" />
     <PackageReference Include="xunit" Version="2.9.0" />
     <PackageReference Include="xunit.runner.visualstudio" Version="2.8.1" PrivateAssets="all" />

diff --git a/crates/bindings-csharp/BSATN.Runtime.Tests/Tests.cs b/crates/bindings-csharp/BSATN.Runtime.Tests/Tests.cs
@@ -1,5 +1,6 @@
 namespace SpacetimeDB;
 
+using CsCheck;
 using Xunit;
 
 public static class BSATNRuntimeTests
@@ -62,20 +63,29 @@ public static void AddressRoundtrips()
         Assert.Equal(addr5, addr);
     }
 
+    static readonly Gen<string> genHex = Gen.String[Gen.Char["0123456789abcdef"], 0, 128];
+
     [Fact]
     public static void AddressLengthCheck()
     {
-        for (var i = 0; i < 64; i++)
+        genHex.Sample(s =>
         {
-            if (i == 16)
+            if (s.Length == 32)
             {
-                continue;
+                return;
             }
-
-            var bytes = new byte[i];
-
-            Assert.ThrowsAny<Exception>(() => Address.From(bytes));
-        }
+            Assert.ThrowsAny<Exception>(() => Address.FromHexString(s));
+        });
+        Gen.Byte.Array[0, 64]
+            .Sample(arr =>
+            {
+                if (arr.Length == 16)
+                {
+                    return;
+                }
+                Assert.ThrowsAny<Exception>(() => Address.FromBigEndian(arr));
+                Assert.ThrowsAny<Exception>(() => Address.From(arr));
+            });
     }
 
     [Fact]
@@ -147,16 +157,23 @@ public static void IdentityRoundtrips()
     [Fact]
     public static void IdentityLengthCheck()
     {
-        for (var i = 0; i < 64; i++)
+        genHex.Sample(s =>
         {
-            if (i == 32)
+            if (s.Length == 64)
             {
-                continue;
+                return;
             }
-
-            var bytes = new byte[i];
-
-            Assert.ThrowsAny<Exception>(() => Identity.From(bytes));
-        }
+            Assert.ThrowsAny<Exception>(() => Identity.FromHexString(s));
+        });
+        Gen.Byte.Array[0, 64]
+            .Sample(arr =>
+            {
+                if (arr.Length == 32)
+                {
+                    return;
+                }
+                Assert.ThrowsAny<Exception>(() => Identity.FromBigEndian(arr));
+                Assert.ThrowsAny<Exception>(() => Identity.From(arr));
+            });
     }
 }
diff --git a/crates/bindings-csharp/BSATN.Runtime/Builtins.cs b/crates/bindings-csharp/BSATN.Runtime/Builtins.cs
@@ -59,17 +59,20 @@ public static T ReadBigEndian<T>(ReadOnlySpan<byte> source)
     static T Read<T>(ReadOnlySpan<byte> source, bool reverse)
         where T : struct
     {
+        Debug.Assert(
+            source.Length == Marshal.SizeOf<T>(),
+            $"Error while reading ${typeof(T).FullName}: expected source span to be {Marshal.SizeOf<T>()} bytes long, but was {source.Length} bytes."
+        );
+
+        var result = MemoryMarshal.Read<T>(source);
+
         if (reverse)
         {
-            Span<byte> reversed = stackalloc byte[source.Length];
-            source.CopyTo(reversed);
-            reversed.Reverse();
-            return MemoryMarshal.Read<T>(reversed);
-        }
-        else
-        {
-            return MemoryMarshal.Read<T>(source);
+            var resultSpan = MemoryMarshal.CreateSpan(ref result, 1);
+            MemoryMarshal.AsBytes(resultSpan).Reverse();
         }
+
+        return result;
     }
 
     /// <summary>
@@ -80,7 +83,7 @@ static T Read<T>(ReadOnlySpan<byte> source, bool reverse)
     /// <param name="source"></param>
     /// <returns></returns>
     public static byte[] AsBytesLittleEndian<T>(T source)
-        where T : struct => AsBytes<T>(source, !BitConverter.IsLittleEndian);
+        where T : struct => AsBytes(source, !BitConverter.IsLittleEndian);
 
     /// <summary>
     /// Convert the passed T to a big-endian byte array.
@@ -171,7 +174,6 @@ public readonly record struct Address
     /// <param name="bytes"></param>
     public static Address? From(byte[] bytes)
     {
-        Debug.Assert(bytes.Length == 16);
         var addr = new Address(Util.ReadLittleEndian<U128>(bytes));
         return addr == default ? null : addr;
     }
@@ -191,7 +193,6 @@ public readonly record struct Address
     /// <param name="bytes"></param>
     public static Address? FromBigEndian(byte[] bytes)
     {
-        Debug.Assert(bytes.Length == 16);
         var addr = new Address(Util.ReadBigEndian<U128>(bytes));
         return addr == default ? null : addr;
     }
@@ -245,7 +246,6 @@ public readonly record struct Identity
     /// <param name="bytes"></param>
     public Identity(byte[] bytes)
     {
-        Debug.Assert(bytes.Length == 32);
         value = Util.ReadLittleEndian<U256>(bytes);
     }
 
@@ -271,7 +271,6 @@ public Identity(byte[] bytes)
     /// <param name="bytes"></param>
     public static Identity FromBigEndian(byte[] bytes)
     {
-        Debug.Assert(bytes.Length == 32);
         return new Identity(Util.ReadBigEndian<U256>(bytes));
     }