Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Disallow Generic DataLoader when using the source generator #7381

Merged
merged 1 commit into from
Aug 19, 2024
Merged

Disallow Generic DataLoader when using the source generator #7381

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 11 additions & 13 deletions ...Chocolate/AspNetCore/test/AspNetCore.Tests/Subscriptions/Apollo/WebSocketProtocolTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,9 @@

namespace HotChocolate.AspNetCore.Subscriptions.Apollo;

public class WebSocketProtocolTests : SubscriptionTestBase
public class WebSocketProtocolTests(TestServerFactory serverFactory)
: SubscriptionTestBase(serverFactory)
{
public WebSocketProtocolTests(TestServerFactory serverFactory)
: base(serverFactory)
{
}

[Fact]
public Task Send_Connect_AcceptAndKeepAlive()
=> TryTest(async ct =>
Expand All @@ -32,7 +28,7 @@ public Task Send_Connect_AcceptAndKeepAlive()
// assert
var message = await webSocket.ReceiveServerMessageAsync(ct);
Assert.NotNull(message);
Assert.Equal("connection_ack", message!["type"]);
Assert.Equal("connection_ack", message["type"]);
});

[Fact]
Expand Down Expand Up @@ -98,7 +94,7 @@ public Task Send_Connect_With_Auth_Accept()
// assert
var message = await webSocket.ReceiveServerMessageAsync(ct);
Assert.NotNull(message);
Assert.Equal("connection_ack", message![MessageProperties.Type]);
Assert.Equal("connection_ack", message[MessageProperties.Type]);
});

[Fact]
Expand Down Expand Up @@ -141,7 +137,7 @@ public Task Send_Connect_AcceptAndKeepAlive_Explicit_Route()
// assert
var message = await webSocket.ReceiveServerMessageAsync(ct);
Assert.NotNull(message);
Assert.Equal("connection_ack", message!["type"]);
Assert.Equal("connection_ack", message["type"]);
});

[Fact]
Expand All @@ -161,7 +157,7 @@ public Task Send_Connect_AcceptAndKeepAlive_Explicit_Route_Explicit_Path()
// assert
var message = await webSocket.ReceiveServerMessageAsync(ct);
Assert.NotNull(message);
Assert.Equal("connection_ack", message!["type"]);
Assert.Equal("connection_ack", message["type"]);
});

[Fact]
Expand Down Expand Up @@ -228,15 +224,17 @@ public Task Send_Start_ReceiveDataOnMutation()
await testServer.SendPostRequestAsync(
new ClientQueryRequest
{
Query = @"
Query =
"""
mutation {
createReview(episode: NEW_HOPE review: {
commentary: ""foo""
commentary: "foo"
stars: 5
}) {
stars
}
}",
}
"""
});

var message = await WaitForMessage(webSocket, "data", ct);
Expand Down
13 changes: 11 additions & 2 deletions src/HotChocolate/Core/src/Types.Analyzers/Errors.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static class Errors

public static readonly DiagnosticDescriptor TooManyNodeResolverArguments =
new(
id: "HCXXXX",
id: "HC0083",
title: "Too Many Arguments.",
messageFormat: "A node resolver can only have a single field argument called `id`.",
category: "TypeSystem",
Expand All @@ -70,10 +70,19 @@ public static class Errors

public static readonly DiagnosticDescriptor InvalidNodeResolverArgumentName =
new(
id: "HCXXXX",
id: "HC0084",
title: "Invalid Argument Name.",
messageFormat: "A node resolver can only have a single field argument called `id`.",
category: "TypeSystem",
DiagnosticSeverity.Error,
isEnabledByDefault: true);

public static readonly DiagnosticDescriptor DataLoaderCannotBeGeneric =
new(
id: "HC0085",
title: "DataLoader Cannot Be Generic.",
messageFormat: "The DataLoader source generator cannot generate generic DataLoaders.",
category: "DataLoader",
DiagnosticSeverity.Error,
isEnabledByDefault: true);
}
18 changes: 15 additions & 3 deletions src/HotChocolate/Core/src/Types.Analyzers/Generators/DataLoaderGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ private static void WriteDataLoader(

if (dataLoader.MethodSymbol.Parameters.Length == 0)
{
context.ReportDiagnostic(
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.KeyParameterMissing,
Location.Create(
Expand All @@ -43,11 +43,12 @@ private static void WriteDataLoader(
continue;
}

if (dataLoader.MethodSymbol.DeclaredAccessibility is not Accessibility.Public and
if (dataLoader.MethodSymbol.DeclaredAccessibility is
not Accessibility.Public and
not Accessibility.Internal and
not Accessibility.ProtectedAndInternal)
{
context.ReportDiagnostic(
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.MethodAccessModifierInvalid,
Location.Create(
Expand All @@ -56,6 +57,17 @@ not Accessibility.Internal and
continue;
}

if (dataLoader.MethodSymbol.IsGenericMethod)
{
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.DataLoaderCannotBeGeneric,
Location.Create(
dataLoader.MethodSyntax.SyntaxTree,
dataLoader.MethodSyntax.Modifiers.Span)));
continue;
}

dataLoaders.Add(dataLoader);
}

Expand Down
5 changes: 5 additions & 0 deletions src/HotChocolate/Core/src/Types.Analyzers/Generators/DataLoaderModuleGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ public void Generate(

foreach (var syntaxInfo in syntaxInfos)
{
if(syntaxInfo.Diagnostics.Length > 0)
{
continue;
}

switch (syntaxInfo)
{
case RegisterDataLoaderInfo dataLoader:
Expand Down
5 changes: 5 additions & 0 deletions src/HotChocolate/Core/src/Types.Analyzers/Generators/TypeModuleSyntaxGenerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ private static void WriteConfiguration(

foreach (var syntaxInfo in syntaxInfos)
{
if(syntaxInfo.Diagnostics.Length > 0)
{
continue;
}

switch (syntaxInfo)
{
case TypeInfo type:
Expand Down
37 changes: 36 additions & 1 deletion src/HotChocolate/Core/src/Types.Analyzers/Inspectors/DataLoaderInspector.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,46 @@ public bool TryHandle(
if (fullName.Equals(WellKnownAttributes.DataLoaderAttribute, Ordinal) &&
context.SemanticModel.GetDeclaredSymbol(methodSyntax) is { } methodSymbol)
{
syntaxInfo = new DataLoaderInfo(
var dataLoader = new DataLoaderInfo(
attributeSyntax,
attributeSymbol,
methodSymbol,
methodSyntax);

if (dataLoader.MethodSymbol.Parameters.Length == 0)
{
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.KeyParameterMissing,
Location.Create(
dataLoader.MethodSyntax.SyntaxTree,
dataLoader.MethodSyntax.ParameterList.Span)));
}

if (dataLoader.MethodSymbol.DeclaredAccessibility is
not Accessibility.Public and
not Accessibility.Internal and
not Accessibility.ProtectedAndInternal)
{
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.MethodAccessModifierInvalid,
Location.Create(
dataLoader.MethodSyntax.SyntaxTree,
dataLoader.MethodSyntax.Modifiers.Span)));
}

if (dataLoader.MethodSymbol.IsGenericMethod)
{
dataLoader.AddDiagnostic(
Diagnostic.Create(
Errors.DataLoaderCannotBeGeneric,
Location.Create(
dataLoader.MethodSyntax.SyntaxTree,
dataLoader.MethodSyntax.Modifiers.Span)));
}

syntaxInfo = dataLoader;
return true;
}
}
Expand Down
23 changes: 23 additions & 0 deletions src/HotChocolate/Core/test/Types.Analyzers.Tests/DataLoaderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,4 +70,27 @@ public static async Task<Entity> GetEntityByIdAsync(
}
""").MatchMarkdownAsync();
}

[Fact]
public async Task GenerateSource_GenericBatchDataLoader_MatchesSnapshot()
{
await TestHelper.GetGeneratedSourceSnapshot(
"""
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;
using HotChocolate;
using GreenDonut;

namespace TestNamespace;

internal static class TestClass
{
[DataLoader]
public static async Task<IReadOnlyDictionary<int, T>> GetEntityByIdAsync<T>(
IReadOnlyList<int> entityIds,
CancellationToken cancellationToken) { }
}
""").MatchMarkdownAsync();
}
}
17 changes: 17 additions & 0 deletions ...hots__/DataLoaderTests.GenerateSource_GenericBatchDataLoader_MatchesSnapshot.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
# GenerateSource_GenericBatchDataLoader_MatchesSnapshot

```json
[
{
"Id": "HCXXXX",
"Title": "DataLoader Cannot Be Generic.",
"Severity": "Error",
"WarningLevel": 0,
"Location": ": (11,4)-(11,23)",
"MessageFormat": "The DataLoader source generator cannot generate generic DataLoaders.",
"Message": "The DataLoader source generator cannot generate generic DataLoaders.",
"Category": "DataLoader",
"CustomTags": []
}
]
```
Loading