Skip to content

Commit 0d2a45b

Browse files
kazimuthrekhoff
andauthored
Fix fields named 'read' (#2525)
# Description of Changes Fixes https://github.com/orgs/clockworklabs/projects/22?pane=issue&itemId=102392974&issue=clockworklabs%7Ccom.clockworklabs.spacetimedbsdk%7C276 by renaming `internal` `static` serializer fields so that they do not overlap with user-provided names. Note, however, that some field names still will not work: `ReadFields`, `WriteFields`, `Equals`, and `GetHashCode`. This would require a separate fix since the error would happen in a different place. In this case we would need to change the name of the generated member to something like `ReadFields_` or `Equals_`, which I'm not sure is a good idea. # API and ABI breaking changes N/A # Expected complexity level and risk 0 # Testing SDK tests and `dotnet-verify` tests are passing. --------- Signed-off-by: rekhoff <[email protected]> Co-authored-by: rekhoff <[email protected]>
1 parent 434894f commit 0d2a45b

32 files changed

+718
-594
lines changed

crates/bindings-csharp/BSATN.Codegen/Type.cs

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,14 @@
11
namespace SpacetimeDB.Codegen;
22

3+
// Generate code to implement serialization to the BSATN format (https://spacetimedb.com/docs/bsatn).
4+
// C# doesn't support static methods in interfaces, so instead we declare a zero-sized `struct` type that implements
5+
// the serialization interface (IReadWrite) for us.
6+
//
7+
// See BSATN.Runtime for the support code referenced by code generation,
8+
// and see Codegen.Tests/fixtures/*/snapshots for examples of generated code.
9+
// Also, if you set <EmitCompilerGeneratedFiles>true</EmitCompilerGeneratedFiles> in a csproj,
10+
// you can find the generated code in obj/Debug/*/generated/SpacetimeDB.BSATN.Codegen.
11+
312
using System.Collections.Immutable;
413
using Microsoft.CodeAnalysis;
514
using Microsoft.CodeAnalysis.CSharp;
@@ -27,6 +36,14 @@ namespace SpacetimeDB.Codegen;
2736
/// <param name="BSATNName">The name of the BSATN struct for the type.</param>
2837
public abstract record TypeUse(string Name, string BSATNName)
2938
{
39+
internal static string BSATN_FIELD_SUFFIX = "RW";
40+
41+
/// <summary>
42+
/// The name of the static field containing an IReadWrite in the IReadWrite struct associated with this type.
43+
/// We make sure this is different from the field name so that collisions cannot occur.
44+
/// </summary>
45+
public static string BsatnFieldSuffix => $"{BSATN_FIELD_SUFFIX}";
46+
3047
/// <summary>
3148
/// Parse a type use for a member.
3249
/// </summary>
@@ -330,14 +347,20 @@ IEnumerable<MemberDeclaration> members
330347
var visStr = SyntaxFacts.GetText(visibility);
331348
return string.Join(
332349
"\n ",
333-
members.Select(m => $"{visStr} static readonly {m.Type.BSATNName} {m.Name} = new();")
350+
members.Select(m =>
351+
$"{visStr} static readonly {m.Type.BSATNName} {m.Name}{TypeUse.BsatnFieldSuffix} = new();"
352+
)
334353
);
335354
}
336355

337356
public static string GenerateDefs(IEnumerable<MemberDeclaration> members) =>
338357
string.Join(
339358
",\n ",
340-
members.Select(m => $"new(nameof({m.Name}), {m.Name}.GetAlgebraicType(registrar))")
359+
// we can't use nameof(m.Type.BsatnFieldName) because the bsatn field name differs from the logical name
360+
// assigned in the type.
361+
members.Select(m =>
362+
$"new(\"{m.Name}\", {m.Name}{TypeUse.BsatnFieldSuffix}.GetAlgebraicType(registrar))"
363+
)
341364
);
342365
}
343366

@@ -450,8 +473,6 @@ public Scope.Extensions ToExtensions()
450473
var extensions = new Scope.Extensions(Scope, FullName);
451474

452475
var bsatnDecls = Members.Cast<MemberDeclaration>();
453-
var fieldNames = bsatnDecls.Select(m => m.Name);
454-
var fieldNamesAndIds = fieldNames.Select((name, i) => (name, i));
455476

456477
extensions.BaseTypes.Add($"System.IEquatable<{ShortName}>");
457478

@@ -480,8 +501,8 @@ public override string ToString() =>
480501
return reader.ReadByte() switch {
481502
{{string.Join(
482503
"\n ",
483-
fieldNames.Select((name, i) =>
484-
$"{i} => new {name}({name}.Read(reader)),"
504+
bsatnDecls.Select((m, i) =>
505+
$"{i} => new {m.Name}({m.Name}{TypeUse.BsatnFieldSuffix}.Read(reader)),"
485506
)
486507
)}}
487508
_ => throw new System.InvalidOperationException("Invalid tag value, this state should be unreachable.")
@@ -492,12 +513,12 @@ public override string ToString() =>
492513
switch (value) {
493514
{{string.Join(
494515
"\n",
495-
fieldNames.Select((name, i) => $"""
496-
case {name}(var inner):
497-
writer.Write((byte){i});
498-
{name}.Write(writer, inner);
499-
break;
500-
"""))}}
516+
bsatnDecls.Select((m, i) => $"""
517+
case {m.Name}(var inner):
518+
writer.Write((byte){i});
519+
{m.Name}{TypeUse.BsatnFieldSuffix}.Write(writer, inner);
520+
break;
521+
"""))}}
501522
}
502523
""";
503524

@@ -530,14 +551,14 @@ public override string ToString() =>
530551
public void ReadFields(System.IO.BinaryReader reader) {
531552
{{string.Join(
532553
"\n",
533-
fieldNames.Select(name => $" {name} = BSATN.{name}.Read(reader);")
554+
bsatnDecls.Select(m => $" {m.Name} = BSATN.{m.Name}{TypeUse.BsatnFieldSuffix}.Read(reader);")
534555
)}}
535556
}
536557
537558
public void WriteFields(System.IO.BinaryWriter writer) {
538559
{{string.Join(
539560
"\n",
540-
fieldNames.Select(name => $" BSATN.{name}.Write(writer, {name});")
561+
bsatnDecls.Select(m => $" BSATN.{m.Name}{TypeUse.BsatnFieldSuffix}.Write(writer, {m.Name});")
541562
)}}
542563
}
543564
@@ -557,7 +578,7 @@ object SpacetimeDB.BSATN.IStructuralReadWrite.GetSerializer() {
557578
public override string ToString() =>
558579
$"{{ShortName}} {{start}} {{string.Join(
559580
", ",
560-
fieldNames.Select(name => $$"""{{name}} = {SpacetimeDB.BSATN.StringUtil.GenericToString({{name}})}""")
581+
bsatnDecls.Select(m => $$"""{{m.Name}} = {SpacetimeDB.BSATN.StringUtil.GenericToString({{m.Name}})}""")
561582
)}} {{end}}";
562583
"""
563584
);

crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomClass.verified.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,14 @@ partial struct CustomClass : System.IEquatable<CustomClass>, SpacetimeDB.BSATN.I
66
{
77
public void ReadFields(System.IO.BinaryReader reader)
88
{
9-
IntField = BSATN.IntField.Read(reader);
10-
StringField = BSATN.StringField.Read(reader);
9+
IntField = BSATN.IntFieldRW.Read(reader);
10+
StringField = BSATN.StringFieldRW.Read(reader);
1111
}
1212

1313
public void WriteFields(System.IO.BinaryWriter writer)
1414
{
15-
BSATN.IntField.Write(writer, IntField);
16-
BSATN.StringField.Write(writer, StringField);
15+
BSATN.IntFieldRW.Write(writer, IntField);
16+
BSATN.StringFieldRW.Write(writer, StringField);
1717
}
1818

1919
object SpacetimeDB.BSATN.IStructuralReadWrite.GetSerializer()
@@ -26,8 +26,8 @@ public override string ToString() =>
2626

2727
public readonly partial struct BSATN : SpacetimeDB.BSATN.IReadWrite<CustomClass>
2828
{
29-
internal static readonly SpacetimeDB.BSATN.I32 IntField = new();
30-
internal static readonly SpacetimeDB.BSATN.String StringField = new();
29+
internal static readonly SpacetimeDB.BSATN.I32 IntFieldRW = new();
30+
internal static readonly SpacetimeDB.BSATN.String StringFieldRW = new();
3131

3232
public CustomClass Read(System.IO.BinaryReader reader)
3333
{
@@ -47,8 +47,8 @@ SpacetimeDB.BSATN.ITypeRegistrar registrar
4747
registrar.RegisterType<CustomClass>(_ => new SpacetimeDB.BSATN.AlgebraicType.Product(
4848
new SpacetimeDB.BSATN.AggregateElement[]
4949
{
50-
new(nameof(IntField), IntField.GetAlgebraicType(registrar)),
51-
new(nameof(StringField), StringField.GetAlgebraicType(registrar))
50+
new("IntField", IntFieldRW.GetAlgebraicType(registrar)),
51+
new("StringField", StringFieldRW.GetAlgebraicType(registrar))
5252
}
5353
));
5454

crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomStruct.verified.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -8,14 +8,14 @@ partial struct CustomStruct
88
{
99
public void ReadFields(System.IO.BinaryReader reader)
1010
{
11-
IntField = BSATN.IntField.Read(reader);
12-
StringField = BSATN.StringField.Read(reader);
11+
IntField = BSATN.IntFieldRW.Read(reader);
12+
StringField = BSATN.StringFieldRW.Read(reader);
1313
}
1414

1515
public void WriteFields(System.IO.BinaryWriter writer)
1616
{
17-
BSATN.IntField.Write(writer, IntField);
18-
BSATN.StringField.Write(writer, StringField);
17+
BSATN.IntFieldRW.Write(writer, IntField);
18+
BSATN.StringFieldRW.Write(writer, StringField);
1919
}
2020

2121
object SpacetimeDB.BSATN.IStructuralReadWrite.GetSerializer()
@@ -28,8 +28,8 @@ public override string ToString() =>
2828

2929
public readonly partial struct BSATN : SpacetimeDB.BSATN.IReadWrite<CustomStruct>
3030
{
31-
internal static readonly SpacetimeDB.BSATN.I32 IntField = new();
32-
internal static readonly SpacetimeDB.BSATN.String StringField = new();
31+
internal static readonly SpacetimeDB.BSATN.I32 IntFieldRW = new();
32+
internal static readonly SpacetimeDB.BSATN.String StringFieldRW = new();
3333

3434
public CustomStruct Read(System.IO.BinaryReader reader)
3535
{
@@ -49,8 +49,8 @@ SpacetimeDB.BSATN.ITypeRegistrar registrar
4949
registrar.RegisterType<CustomStruct>(_ => new SpacetimeDB.BSATN.AlgebraicType.Product(
5050
new SpacetimeDB.BSATN.AggregateElement[]
5151
{
52-
new(nameof(IntField), IntField.GetAlgebraicType(registrar)),
53-
new(nameof(StringField), StringField.GetAlgebraicType(registrar))
52+
new("IntField", IntFieldRW.GetAlgebraicType(registrar)),
53+
new("StringField", StringFieldRW.GetAlgebraicType(registrar))
5454
}
5555
));
5656

crates/bindings-csharp/Codegen.Tests/fixtures/client/snapshots/Type#CustomTaggedEnum.verified.cs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,15 @@ public override string ToString() =>
1818

1919
public readonly partial struct BSATN : SpacetimeDB.BSATN.IReadWrite<CustomTaggedEnum>
2020
{
21-
internal static readonly SpacetimeDB.BSATN.I32 IntVariant = new();
22-
internal static readonly SpacetimeDB.BSATN.String StringVariant = new();
21+
internal static readonly SpacetimeDB.BSATN.I32 IntVariantRW = new();
22+
internal static readonly SpacetimeDB.BSATN.String StringVariantRW = new();
2323

2424
public CustomTaggedEnum Read(System.IO.BinaryReader reader)
2525
{
2626
return reader.ReadByte() switch
2727
{
28-
0 => new IntVariant(IntVariant.Read(reader)),
29-
1 => new StringVariant(StringVariant.Read(reader)),
28+
0 => new IntVariant(IntVariantRW.Read(reader)),
29+
1 => new StringVariant(StringVariantRW.Read(reader)),
3030
_
3131
=> throw new System.InvalidOperationException(
3232
"Invalid tag value, this state should be unreachable."
@@ -40,11 +40,11 @@ public void Write(System.IO.BinaryWriter writer, CustomTaggedEnum value)
4040
{
4141
case IntVariant(var inner):
4242
writer.Write((byte)0);
43-
IntVariant.Write(writer, inner);
43+
IntVariantRW.Write(writer, inner);
4444
break;
4545
case StringVariant(var inner):
4646
writer.Write((byte)1);
47-
StringVariant.Write(writer, inner);
47+
StringVariantRW.Write(writer, inner);
4848
break;
4949
}
5050
}
@@ -55,8 +55,8 @@ SpacetimeDB.BSATN.ITypeRegistrar registrar
5555
registrar.RegisterType<CustomTaggedEnum>(_ => new SpacetimeDB.BSATN.AlgebraicType.Sum(
5656
new SpacetimeDB.BSATN.AggregateElement[]
5757
{
58-
new(nameof(IntVariant), IntVariant.GetAlgebraicType(registrar)),
59-
new(nameof(StringVariant), StringVariant.GetAlgebraicType(registrar))
58+
new("IntVariant", IntVariantRW.GetAlgebraicType(registrar)),
59+
new("StringVariant", StringVariantRW.GetAlgebraicType(registrar))
6060
}
6161
));
6262

0 commit comments

Comments
 (0)