Skip to content

Commit d87e494

Browse files
[C#] Improve error messages for Views (#4435)
This is the implementation of a fix for #4425 # Description of Changes * Clarified C# generator diagnostics for view return types: 1. Updated the comments around `IQuery<T>` handling to describe the return value as `T?`, matching C# semantics. 2. Adjusted the validation comment to say views must return `List<T>` or nullable `T` instead of “Vec/Option”. * Synced the diagnostics fixture comments with the new terminology so STDB0024 examples talk about `List<T>`/`T?`. * Checked current documentation for anything C# related to “Vec/Option” and confirmed everything now references `List<T>`/`T?`. * Regenerated/verified tests and snapshots. # API and ABI breaking changes None # Expected complexity level and risk 1 - Changes are documentation and diagnostic-comment only. # Testing - [X] CLI rebuilt, local `dotnet test` pass and error output tests validated. --------- Signed-off-by: Ryan <r.ekhoff@clockworklabs.io> Co-authored-by: joshua-spacetime <josh@clockworklabs.io>
1 parent e8a2d33 commit d87e494

4 files changed

Lines changed: 82 additions & 46 deletions

File tree

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

Lines changed: 65 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ namespace SpacetimeDB.Codegen.Tests;
44
using System.Runtime.CompilerServices;
55
using Microsoft.CodeAnalysis;
66
using Microsoft.CodeAnalysis.CSharp;
7+
using Microsoft.CodeAnalysis.CSharp.Syntax;
78
using Microsoft.CodeAnalysis.MSBuild;
89
using Microsoft.CodeAnalysis.Text;
910

@@ -15,8 +16,19 @@ public static class GeneratorSnapshotTests
1516

1617
record struct StepOutput(string Key, IncrementalStepRunReason Reason, object Value);
1718

18-
class Fixture(string projectDir, CSharpCompilation sampleCompilation)
19+
class Fixture
1920
{
21+
private readonly string projectDir;
22+
private readonly CSharpCompilation sampleCompilation;
23+
24+
public Fixture(string projectDir, CSharpCompilation sampleCompilation)
25+
{
26+
this.projectDir = projectDir;
27+
this.sampleCompilation = sampleCompilation;
28+
}
29+
30+
public CSharpCompilation SampleCompilation => sampleCompilation;
31+
2032
public static async Task<Fixture> Compile(string name)
2133
{
2234
var projectDir = Path.Combine(GetProjectDir(), "fixtures", name);
@@ -63,6 +75,12 @@ IIncrementalGenerator generator
6375
return genResult.GeneratedTrees;
6476
}
6577

78+
public GeneratorDriverRunResult RunGeneratorAndGetResult(IIncrementalGenerator generator)
79+
{
80+
var driver = CreateDriver(generator, sampleCompilation.LanguageVersion);
81+
return driver.RunGenerators(sampleCompilation).GetRunResult();
82+
}
83+
6684
public async Task<CSharpCompilation> RunAndCheckGenerators(
6785
params IIncrementalGenerator[] generators
6886
) =>
@@ -257,4 +275,50 @@ public static async Task TestDiagnostics()
257275
AssertRuntimeDoesNotDefineLocal(compilationAfterGen);
258276
AssertGeneratedCodeDoesNotUseInternalBound(compilationAfterGen);
259277
}
278+
279+
[Fact]
280+
public static async Task ViewInvalidReturnHighlightsReturnType()
281+
{
282+
var fixture = await Fixture.Compile("diag");
283+
284+
var runResult = fixture.RunGeneratorAndGetResult(new SpacetimeDB.Codegen.Module());
285+
286+
var method = fixture
287+
.SampleCompilation.SyntaxTrees.Select(tree => new
288+
{
289+
Tree = tree,
290+
Root = tree.GetRoot(),
291+
})
292+
.SelectMany(entry =>
293+
entry
294+
.Root.DescendantNodes()
295+
.OfType<MethodDeclarationSyntax>()
296+
.Select(method => new
297+
{
298+
entry.Tree,
299+
entry.Root,
300+
Method = method,
301+
})
302+
)
303+
.Single(entry => entry.Method.Identifier.Text == "ViewDefIEnumerableReturnFromIter");
304+
305+
var returnTypeSpan = method.Method.ReturnType.Span;
306+
var diagnostics = runResult
307+
.Results.SelectMany(result => result.Diagnostics)
308+
.Where(d => d.Id == "STDB0024")
309+
.ToList();
310+
var diagnostic = diagnostics.FirstOrDefault(d =>
311+
d.GetMessage().Contains("ViewDefIEnumerableReturnFromIter")
312+
&& d.Location.SourceTree == method.Tree
313+
);
314+
315+
Assert.NotNull(diagnostic);
316+
317+
Assert.Equal(returnTypeSpan, diagnostic!.Location.SourceSpan);
318+
319+
var returnTypeText = method
320+
.Root.ToFullString()
321+
.Substring(returnTypeSpan.Start, returnTypeSpan.Length);
322+
Assert.Contains("IEnumerable", returnTypeText);
323+
}
260324
}

crates/bindings-csharp/Codegen.Tests/fixtures/diag/snapshots/Module.verified.txt

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -330,25 +330,17 @@ public partial struct TestScheduleIssues
330330
}
331331
},
332332
{/*
333-
// Invalid: Wrong return type is not List<T> or T?
334333
[SpacetimeDB.View(Accessor = "view_def_wrong_return", Public = true)]
335-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
336334
public static Player ViewDefWrongReturn(ViewContext ctx)
337-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
335+
^^^^^^
338336
{
339-
^^^^^
340-
return new Player { Identity = new() };
341-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
342-
}
343-
^^^^^
344-
345337
*/
346-
Message: View 'ViewDefWrongReturn' must return Vec<T> or Option<T>.,
338+
Message: View 'ViewDefWrongReturn' must return T?, List<T>, or IQuery<T>,
347339
Severity: Error,
348340
Descriptor: {
349341
Id: STDB0024,
350-
Title: Views must return Vec<T> or Option<T>,
351-
MessageFormat: View '{0}' must return Vec<T> or Option<T>.,
342+
Title: Views must return T?, List<T>, or IQuery<T>,
343+
MessageFormat: View '{0}' must return T?, List<T>, or IQuery<T>,
352344
Category: SpacetimeDB,
353345
DefaultSeverity: Error,
354346
IsEnabledByDefault: true
@@ -372,25 +364,17 @@ public partial struct TestScheduleIssues
372364
}
373365
},
374366
{/*
375-
// Invalid: IEnumerable<T> return type (from Iter()) is not List<T> or T?
376367
[SpacetimeDB.View(Accessor = "view_def_ienumerable_return_from_iter", Public = true)]
377-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
378368
public static IEnumerable<Player> ViewDefIEnumerableReturnFromIter(ViewContext ctx)
379-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
369+
^^^^^^^^^^^^^^^^^^^
380370
{
381-
^^^^^
382-
return ctx.Db.Player.Iter();
383-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
384-
}
385-
^^^^^
386-
387371
*/
388-
Message: View 'ViewDefIEnumerableReturnFromIter' must return Vec<T> or Option<T>.,
372+
Message: View 'ViewDefIEnumerableReturnFromIter' must return T?, List<T>, or IQuery<T>,
389373
Severity: Error,
390374
Descriptor: {
391375
Id: STDB0024,
392-
Title: Views must return Vec<T> or Option<T>,
393-
MessageFormat: View '{0}' must return Vec<T> or Option<T>.,
376+
Title: Views must return T?, List<T>, or IQuery<T>,
377+
MessageFormat: View '{0}' must return T?, List<T>, or IQuery<T>,
394378
Category: SpacetimeDB,
395379
DefaultSeverity: Error,
396380
IsEnabledByDefault: true
@@ -414,29 +398,17 @@ public partial struct TestScheduleIssues
414398
}
415399
},
416400
{/*
417-
// Invalid: IEnumerable<T> return type (from Filter()) is not List<T> or T?
418401
[SpacetimeDB.View(Accessor = "view_def_ienumerable_return_from_filter", Public = true)]
419-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
420402
public static IEnumerable<TestScheduleIssues> ViewDefIEnumerableReturnFromFilter(
421-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
403+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
422404
ViewContext ctx
423-
^^^^^^^^^^^^^^^^^^^^^^^
424-
)
425-
^^^^^
426-
{
427-
^^^^^
428-
return ctx.Db.TestIndexIssues.TestUnexpectedColumns.Filter(0);
429-
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
430-
}
431-
^^^^^
432-
433405
*/
434-
Message: View 'ViewDefIEnumerableReturnFromFilter' must return Vec<T> or Option<T>.,
406+
Message: View 'ViewDefIEnumerableReturnFromFilter' must return T?, List<T>, or IQuery<T>,
435407
Severity: Error,
436408
Descriptor: {
437409
Id: STDB0024,
438-
Title: Views must return Vec<T> or Option<T>,
439-
MessageFormat: View '{0}' must return Vec<T> or Option<T>.,
410+
Title: Views must return T?, List<T>, or IQuery<T>,
411+
MessageFormat: View '{0}' must return T?, List<T>, or IQuery<T>,
440412
Category: SpacetimeDB,
441413
DefaultSeverity: Error,
442414
IsEnabledByDefault: true

crates/bindings-csharp/Codegen/Diag.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,9 +215,9 @@ string typeName
215215
public static readonly ErrorDescriptor<MethodDeclarationSyntax> ViewInvalidReturn =
216216
new(
217217
group,
218-
"Views must return Vec<T> or Option<T>",
219-
method => $"View '{method.Identifier}' must return Vec<T> or Option<T>.",
220-
method => method
218+
"Views must return T?, List<T>, or IQuery<T>",
219+
method => $"View '{method.Identifier}' must return T?, List<T>, or IQuery<T>",
220+
method => method.ReturnType
221221
);
222222

223223
// TODO: Remove once Views support Private: Views must be Public currently

crates/bindings-csharp/Codegen/Module.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1189,7 +1189,7 @@ method.ReturnType is INamedTypeSymbol
11891189
? "SpacetimeDB.BSATN.ValueOption"
11901190
: "SpacetimeDB.BSATN.RefOption";
11911191
var opt = $"{optType}<{rowType.Name}, {rowType.BSATNName}>";
1192-
// Match Rust semantics: Query<T> is described as Option<T>.
1192+
// Match Rust semantics: Query<T> is described as a nullable row (T?).
11931193
ReturnType = new ReferenceUse(opt, opt);
11941194
}
11951195
else
@@ -1210,7 +1210,7 @@ method.ReturnType is INamedTypeSymbol
12101210
diag.Report(ErrorDescriptor.ViewContextParam, methodSyntax);
12111211
}
12121212

1213-
// Validate return type: must be Option<T> or Vec<T>
1213+
// Validate return type: must be List<T> or T?
12141214
if (
12151215
!ReturnType.BSATNName.Contains("SpacetimeDB.BSATN.ValueOption")
12161216
&& !ReturnType.BSATNName.Contains("SpacetimeDB.BSATN.RefOption")

0 commit comments

Comments
 (0)