Skip to content

Commit 31b9af3

Browse files
stablegenius49rekhoff
stablegenius49
and
rekhoff
authored
fix(csharp-codegen): escape C# reserved keywords in generated identifiers (#4535)
## Summary This PR fixes an issue where C# reserved keywords (like `params`, `class`, `event`, etc.) used as field or parameter names in SpacetimeDB types would cause compilation errors in the generated code. ## Problem When a user defines a table or reducer with a field named using a C# reserved keyword: ```csharp [SpacetimeDB.Table] public partial struct MyTable { public int @params; // User escapes it in their code public string @Class; } ``` The codegen would generate invalid C# like: ```csharp // Generated code (broken) public int params; // Error: keyword used as identifier public void Read(BinaryReader reader) { params = ...; // Error } ``` ## Solution 1. Added an `Identifier` property to `MemberDeclaration` in the codegen that automatically detects C# reserved keywords using `SyntaxFacts.GetKeywordKind()` and prefixes them with `@` when needed. 2. Updated all code generation sites to use `Identifier` instead of `Name` when generating: - Field declarations - Property accessors - Constructor parameters - BSATN serialization code - Index accessors - Reducer/procedure parameters 3. Added a regression test that verifies tables, reducers, and procedures with keyword field names compile successfully. ## Test Plan - Added `CSharpKeywordIdentifiersAreEscapedInGeneratedCode` test that creates a table with `@class` and `@params` fields, plus a reducer and procedure with keyword parameters - Existing tests continue to pass (verified locally with `FormerlyForbiddenFieldNames` fixture which already tests edge cases like `Read`, `Write`, `GetAlgebraicType`) Fixes #4529 --------- Co-authored-by: Stable Genius <259448942+stablegenius49@users.noreply.github.com> Co-authored-by: Ryan <r.ekhoff@clockworklabs.io>
1 parent 6300236 commit 31b9af3

4 files changed

Lines changed: 188 additions & 69 deletions

File tree

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

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -422,6 +422,8 @@ public MemberDeclaration(ISymbol member, ITypeSymbol type, DiagReporter diag)
422422
public MemberDeclaration(IFieldSymbol field, DiagReporter diag)
423423
: this(field, field.Type, diag) { }
424424

425+
public string Identifier => EscapeIdentifier(Name);
426+
425427
public static string GenerateBsatnFields(
426428
Accessibility visibility,
427429
IEnumerable<MemberDeclaration> members
@@ -431,7 +433,7 @@ IEnumerable<MemberDeclaration> members
431433
return string.Join(
432434
"\n ",
433435
members.Select(m =>
434-
$"{visStr} static readonly {m.Type.ToBSATNString()} {m.Name}{TypeUse.BsatnFieldSuffix} = new();"
436+
$"{visStr} static readonly {m.Type.ToBSATNString()} {m.Identifier}{TypeUse.BsatnFieldSuffix} = new();"
435437
)
436438
);
437439
}
@@ -442,7 +444,7 @@ public static string GenerateDefs(IEnumerable<MemberDeclaration> members) =>
442444
// we can't use nameof(m.Type.BsatnFieldName) because the bsatn field name differs from the logical name
443445
// assigned in the type.
444446
members.Select(m =>
445-
$"new(\"{m.Name}\", {m.Name}{TypeUse.BsatnFieldSuffix}.GetAlgebraicType(registrar))"
447+
$"new(\"{m.Name}\", {m.Identifier}{TypeUse.BsatnFieldSuffix}.GetAlgebraicType(registrar))"
446448
)
447449
);
448450
}
@@ -462,6 +464,12 @@ public abstract record BaseTypeDeclaration<M>
462464
public readonly TypeKind Kind;
463465
public readonly EquatableArray<M> Members;
464466

467+
/// <summary>
468+
/// Returns the escaped version of ShortName for use in generated C# code where the type name
469+
/// appears as an identifier (e.g., in IEquatable&lt;T&gt; or as a base type reference).
470+
/// </summary>
471+
public string ShortNameIdentifier => EscapeIdentifier(ShortName);
472+
465473
protected abstract M ConvertMember(int index, IFieldSymbol field, DiagReporter diag);
466474

467475
public BaseTypeDeclaration(GeneratorAttributeSyntaxContext context, DiagReporter diag)
@@ -557,7 +565,7 @@ public Scope.Extensions ToExtensions()
557565

558566
var bsatnDecls = Members.Cast<MemberDeclaration>();
559567

560-
extensions.BaseTypes.Add($"System.IEquatable<{ShortName}>");
568+
extensions.BaseTypes.Add($"System.IEquatable<{ShortNameIdentifier}>");
561569

562570
if (Kind is TypeKind.Sum)
563571
{
@@ -569,10 +577,10 @@ public Scope.Extensions ToExtensions()
569577
// To avoid this, we append an underscore to the field name.
570578
// In most cases the field name shouldn't matter anyway as you'll idiomatically use pattern matching to extract the value.
571579
$$"""
572-
public sealed record {{m.Name}}({{m.Type.Name}} {{m.Name}}_) : {{ShortName}}
580+
public sealed record {{m.Identifier}}({{m.Type.Name}} {{m.Identifier}}_) : {{ShortNameIdentifier}}
573581
{
574582
public override string ToString() =>
575-
$"{{m.Name}}({ SpacetimeDB.BSATN.StringUtil.GenericToString({{m.Name}}_) })";
583+
$"{{m.Name}}({ SpacetimeDB.BSATN.StringUtil.GenericToString({{m.Identifier}}_) })";
576584
}
577585
578586
"""
@@ -585,7 +593,7 @@ public override string ToString() =>
585593
{{string.Join(
586594
"\n ",
587595
bsatnDecls.Select((m, i) =>
588-
$"{i} => new {m.Name}({m.Name}{TypeUse.BsatnFieldSuffix}.Read(reader)),"
596+
$"{i} => new {m.Identifier}({m.Identifier}{TypeUse.BsatnFieldSuffix}.Read(reader)),"
589597
)
590598
)}}
591599
_ => throw new System.InvalidOperationException("Invalid tag value, this state should be unreachable.")
@@ -597,9 +605,9 @@ public override string ToString() =>
597605
{{string.Join(
598606
"\n",
599607
bsatnDecls.Select((m, i) => $"""
600-
case {m.Name}(var inner):
608+
case {m.Identifier}(var inner):
601609
writer.Write((byte){i});
602-
{m.Name}{TypeUse.BsatnFieldSuffix}.Write(writer, inner);
610+
{m.Identifier}{TypeUse.BsatnFieldSuffix}.Write(writer, inner);
603611
break;
604612
"""))}}
605613
}
@@ -615,7 +623,7 @@ public override string ToString() =>
615623
var hashName = $"___hash{member.Name}";
616624
617625
return $"""
618-
case {member.Name}(var inner):
626+
case {member.Identifier}(var inner):
619627
{member.Type.GetHashCodeStatement("inner", hashName)}
620628
return {hashName};
621629
""";
@@ -634,14 +642,14 @@ public override string ToString() =>
634642
public void ReadFields(System.IO.BinaryReader reader) {
635643
{{string.Join(
636644
"\n",
637-
bsatnDecls.Select(m => $" {m.Name} = BSATN.{m.Name}{TypeUse.BsatnFieldSuffix}.Read(reader);")
645+
bsatnDecls.Select(m => $" {m.Identifier} = BSATN.{m.Identifier}{TypeUse.BsatnFieldSuffix}.Read(reader);")
638646
)}}
639647
}
640648
641649
public void WriteFields(System.IO.BinaryWriter writer) {
642650
{{string.Join(
643651
"\n",
644-
bsatnDecls.Select(m => $" BSATN.{m.Name}{TypeUse.BsatnFieldSuffix}.Write(writer, {m.Name});")
652+
bsatnDecls.Select(m => $" BSATN.{m.Identifier}{TypeUse.BsatnFieldSuffix}.Write(writer, {m.Identifier});")
645653
)}}
646654
}
647655
@@ -661,7 +669,7 @@ object SpacetimeDB.BSATN.IStructuralReadWrite.GetSerializer() {
661669
public override string ToString() =>
662670
$"{{ShortName}} {{start}} {{string.Join(
663671
", ",
664-
bsatnDecls.Select(m => $$"""{{m.Name}} = {SpacetimeDB.BSATN.StringUtil.GenericToString({{m.Name}})}""")
672+
bsatnDecls.Select(m => $$"""{{m.Name}} = {SpacetimeDB.BSATN.StringUtil.GenericToString({{m.Identifier}})}""")
665673
)}} {{end}}";
666674
"""
667675
);
@@ -680,7 +688,7 @@ public override string ToString() =>
680688
var declHashName = (MemberDeclaration decl) => $"___hash{decl.Name}";
681689

682690
getHashCode = $$"""
683-
{{string.Join("\n", bsatnDecls.Select(decl => decl.Type.GetHashCodeStatement(decl.Name, declHashName(decl))))}}
691+
{{string.Join("\n", bsatnDecls.Select(decl => decl.Type.GetHashCodeStatement(decl.Identifier, declHashName(decl))))}}
684692
return {{JoinOrValue(
685693
" ^\n ",
686694
bsatnDecls.Select(declHashName),
@@ -735,7 +743,7 @@ public override int GetHashCode()
735743
public bool Equals({{fullNameMaybeRef}} that)
736744
{
737745
{{(Scope.IsStruct ? "" : "if (((object?)that) == null) { return false; }\n ")}}
738-
{{string.Join("\n", bsatnDecls.Select(decl => decl.Type.EqualsStatement($"this.{decl.Name}", $"that.{decl.Name}", declEqualsName(decl))))}}
746+
{{string.Join("\n", bsatnDecls.Select(decl => decl.Type.EqualsStatement($"this.{decl.Identifier}", $"that.{decl.Identifier}", declEqualsName(decl))))}}
739747
return {{JoinOrValue(
740748
" &&\n ",
741749
bsatnDecls.Select(declEqualsName),

‎crates/bindings-csharp/BSATN.Codegen/Utils.cs‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,26 @@ public readonly record struct EquatableArray<T>(ImmutableArray<T> Array) : IEnum
3333
.AddMemberOptions(SymbolDisplayMemberOptions.IncludeContainingType)
3434
.AddMiscellaneousOptions(
3535
SymbolDisplayMiscellaneousOptions.IncludeNullableReferenceTypeModifier
36+
| SymbolDisplayMiscellaneousOptions.EscapeKeywordIdentifiers
3637
);
3738

3839
public static string SymbolToName(ISymbol symbol)
3940
{
4041
return symbol.ToDisplayString(SymbolFormat);
4142
}
4243

44+
public static string EscapeIdentifier(string name)
45+
{
46+
if (name.Length > 0 && name[0] == '@')
47+
{
48+
return name;
49+
}
50+
51+
var kind = SyntaxFacts.GetKeywordKind(name);
52+
var contextualKind = SyntaxFacts.GetContextualKeywordKind(name);
53+
return kind != SyntaxKind.None || contextualKind != SyntaxKind.None ? $"@{name}" : name;
54+
}
55+
4356
public static void RegisterSourceOutputs(
4457
this IncrementalValuesProvider<Scope.Extensions> methods,
4558
IncrementalGeneratorInitializationContext context

‎crates/bindings-csharp/Codegen.Tests/Tests.cs‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,87 @@ public static async Task SettingsAndExplicitNames()
256256
AssertGeneratedCodeDoesNotUseInternalBound(compilationAfterGen);
257257
}
258258

259+
[Fact]
260+
public static async Task CSharpKeywordIdentifiersAreEscapedInGeneratedCode()
261+
{
262+
var fixture = await Fixture.Compile("server");
263+
264+
const string source = """
265+
using SpacetimeDB;
266+
267+
[SpacetimeDB.Table]
268+
public partial struct KeywordTable
269+
{
270+
[SpacetimeDB.PrimaryKey]
271+
public ulong @class;
272+
273+
public int @params;
274+
}
275+
276+
[SpacetimeDB.Table(Accessor = "event")]
277+
public partial struct AccessorKeywordTable
278+
{
279+
[SpacetimeDB.PrimaryKey]
280+
[SpacetimeDB.Index.BTree(Accessor = "params")]
281+
public int Id;
282+
}
283+
284+
[SpacetimeDB.Table]
285+
public partial struct @class
286+
{
287+
[SpacetimeDB.PrimaryKey]
288+
public int Id;
289+
}
290+
291+
public static partial class KeywordApis
292+
{
293+
[SpacetimeDB.Reducer]
294+
public static void KeywordReducer(ReducerContext ctx, int @params, string @class)
295+
{
296+
_ = @params;
297+
_ = @class;
298+
}
299+
300+
[SpacetimeDB.Reducer]
301+
public static void @class(ReducerContext ctx)
302+
{
303+
}
304+
305+
[SpacetimeDB.Procedure]
306+
public static int KeywordProcedure(ProcedureContext ctx, int @params, int @class)
307+
{
308+
return @params + @class;
309+
}
310+
311+
[SpacetimeDB.Procedure]
312+
public static void @params(ProcedureContext ctx)
313+
{
314+
}
315+
}
316+
""";
317+
318+
var parseOptions = new CSharpParseOptions(fixture.SampleCompilation.LanguageVersion);
319+
var tree = CSharpSyntaxTree.ParseText(source, parseOptions, path: "KeywordNames.cs");
320+
var compilation = fixture.SampleCompilation.AddSyntaxTrees(tree);
321+
322+
var driver = CSharpGeneratorDriver.Create(
323+
[
324+
new SpacetimeDB.Codegen.Type().AsSourceGenerator(),
325+
new SpacetimeDB.Codegen.Module().AsSourceGenerator(),
326+
],
327+
driverOptions: new(
328+
disabledOutputs: IncrementalGeneratorOutputKind.None,
329+
trackIncrementalGeneratorSteps: true
330+
),
331+
parseOptions: parseOptions
332+
);
333+
334+
var runResult = driver.RunGenerators(compilation).GetRunResult();
335+
var compilationAfterGen = compilation.AddSyntaxTrees(runResult.GeneratedTrees);
336+
337+
Assert.Empty(GetCompilationErrors(compilationAfterGen));
338+
}
339+
259340
[Fact]
260341
public static async Task TestDiagnostics()
261342
{

0 commit comments

Comments
 (0)