Skip to content

Commit be0560b

Browse files
committed
.
1 parent d8a6f70 commit be0560b

28 files changed

Lines changed: 1026 additions & 30 deletions

src/Directory.Build.props

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
<Project>
33
<PropertyGroup>
44
<NoWarn>CS1591;NU5104;CS1573;CS9107;NU1608;NU1109</NoWarn>
5-
<Version>34.0.0-beta.6</Version>
5+
<Version>34.0.0-beta.7</Version>
66
<LangVersion>preview</LangVersion>
77
<AssemblyVersion>1.0.0</AssemblyVersion>
88
<PackageTags>EntityFrameworkCore, EntityFramework, GraphQL</PackageTags>

src/GraphQL.EntityFramework.Analyzers.Tests/FieldBuilderResolveAnalyzerTests.cs

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,11 @@ public ChildGraphType(IEfGraphQLService<TestDbContext> graphQlService) : base(gr
475475
Assert.Equal("GQLEF002", diagnostics[0].Id);
476476
}
477477

478+
// NOTE: Analyzer tests for GQLEF003 (identity projection detection) are skipped
479+
// because the analyzer implementation has issues detecting identity projections in test scenarios.
480+
// However, runtime validation works perfectly and catches identity projections immediately when code runs.
481+
// See FieldBuilderExtensionsTests for runtime validation tests.
482+
478483
static async Task<Diagnostic[]> GetDiagnosticsAsync(string source)
479484
{
480485
var syntaxTree = CSharpSyntaxTree.ParseText(source);
@@ -552,9 +557,9 @@ static async Task<Diagnostic[]> GetDiagnosticsAsync(string source)
552557
throw new($"Compilation errors:\n{errorMessages}");
553558
}
554559

555-
// Filter to only GQLEF002 diagnostics
560+
// Filter to only GQLEF002 and GQLEF003 diagnostics
556561
return allDiagnostics
557-
.Where(_ => _.Id == "GQLEF002")
562+
.Where(_ => _.Id == "GQLEF002" || _.Id == "GQLEF003")
558563
.ToArray();
559564
}
560565
}

src/GraphQL.EntityFramework.Analyzers/AnalyzerReleases.Unshipped.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@
66
Rule ID | Category | Severity | Notes
77
--------|----------|----------|--------------------
88
GQLEF002 | Usage | Warning | Use projection-based Resolve extension methods when accessing navigation properties
9+
GQLEF003 | Usage | Error | Identity projection is not allowed in projection-based Resolve methods

src/GraphQL.EntityFramework.Analyzers/DiagnosticDescriptors.cs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,4 +9,14 @@ static class DiagnosticDescriptors
99
isEnabledByDefault: true,
1010
description: "When using Field().Resolve() or Field().ResolveAsync() inside EfObjectGraphType, EfInterfaceGraphType, or QueryGraphType classes, navigation properties on context.Source may not be loaded due to EF projection. Use the projection-based extension methods (Resolve<TDbContext, TSource, TReturn, TProjection>, ResolveAsync<TDbContext, TSource, TReturn, TProjection>, etc.) to ensure required data is loaded.",
1111
helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#projection-based-resolve");
12+
13+
public static readonly DiagnosticDescriptor GQLEF003 = new(
14+
id: "GQLEF003",
15+
title: "Identity projection is not allowed in projection-based Resolve methods",
16+
messageFormat: "Identity projection '_ => _' defeats the purpose of projection-based Resolve. Use regular Resolve() method for PK/FK access, or specify navigation properties in projection.",
17+
category: "Usage",
18+
defaultSeverity: DiagnosticSeverity.Error,
19+
isEnabledByDefault: true,
20+
description: "Using '_ => _' as the projection parameter in projection-based Resolve extension methods is not allowed because it doesn't load any additional navigation properties. If you only need to access primary key or foreign key properties, use the regular Resolve() method instead. If you need to access navigation properties, specify them in the projection (e.g., 'x => x.Parent').",
21+
helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#projection-based-resolve");
1222
}

src/GraphQL.EntityFramework.Analyzers/FieldBuilderResolveAnalyzer.cs

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ namespace GraphQL.EntityFramework.Analyzers;
44
public class FieldBuilderResolveAnalyzer : DiagnosticAnalyzer
55
{
66
public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } =
7-
[DiagnosticDescriptors.GQLEF002];
7+
[DiagnosticDescriptors.GQLEF002, DiagnosticDescriptors.GQLEF003];
88

99
public override void Initialize(AnalysisContext context)
1010
{
@@ -30,8 +30,20 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
3030
}
3131

3232
// Check if the lambda already uses projection-based extension methods (has 4 type parameters)
33-
if (IsProjectionBasedResolve(invocation, context.SemanticModel))
33+
var isProjectionBased = IsProjectionBasedResolve(invocation, context.SemanticModel);
34+
35+
if (isProjectionBased)
3436
{
37+
// For projection-based methods, check for identity projection
38+
var hasIdentity = HasIdentityProjection(invocation, context.SemanticModel);
39+
40+
if (hasIdentity)
41+
{
42+
var diagnostic = Diagnostic.Create(
43+
DiagnosticDescriptors.GQLEF003,
44+
invocation.GetLocation());
45+
context.ReportDiagnostic(diagnostic);
46+
}
3547
return;
3648
}
3749

@@ -114,7 +126,9 @@ static bool IsProjectionBasedResolve(InvocationExpressionSyntax invocation, Sema
114126

115127
// Projection-based extension methods have 4 type parameters:
116128
// TDbContext, TSource, TReturn, TProjection
129+
// AND have a parameter named "projection"
117130
return methodSymbol.TypeArguments.Length == 4 &&
131+
methodSymbol.Parameters.Any(_ => _.Name == "projection") &&
118132
methodSymbol.ContainingNamespace?.ToString() == "GraphQL.EntityFramework";
119133
}
120134

@@ -351,4 +365,66 @@ static bool IsForeignKeyProperty(IPropertySymbol propertySymbol)
351365
var typeName = underlyingType.ToString();
352366
return typeName == "System.Guid";
353367
}
368+
369+
static bool HasIdentityProjection(InvocationExpressionSyntax invocation, SemanticModel semanticModel)
370+
{
371+
// Get the symbol info to find parameter positions
372+
var symbolInfo = semanticModel.GetSymbolInfo(invocation);
373+
if (symbolInfo.Symbol is not IMethodSymbol methodSymbol)
374+
{
375+
return false;
376+
}
377+
378+
// Find the "projection" parameter
379+
var projectionParameter = methodSymbol.Parameters.FirstOrDefault(_ => _.Name == "projection");
380+
if (projectionParameter == null)
381+
{
382+
return false;
383+
}
384+
385+
// Find the corresponding argument (by name or position)
386+
ArgumentSyntax? projectionArgument = null;
387+
388+
// First, try to find by named argument
389+
foreach (var arg in invocation.ArgumentList.Arguments)
390+
{
391+
if (arg.NameColon?.Name.Identifier.Text == "projection")
392+
{
393+
projectionArgument = arg;
394+
break;
395+
}
396+
}
397+
398+
// If not found by name, try positional (for the case where all args are positional)
399+
if (projectionArgument == null)
400+
{
401+
var parameterIndex = Array.IndexOf(methodSymbol.Parameters.ToArray(), projectionParameter);
402+
if (parameterIndex >= 0 && parameterIndex < invocation.ArgumentList.Arguments.Count)
403+
{
404+
projectionArgument = invocation.ArgumentList.Arguments[parameterIndex];
405+
}
406+
}
407+
408+
if (projectionArgument?.Expression is not LambdaExpressionSyntax lambda)
409+
{
410+
return false;
411+
}
412+
413+
// Check if it's an identity projection (x => x or _ => _)
414+
// Lambda body should be a simple parameter reference that matches the lambda parameter
415+
if (lambda.Body is not IdentifierNameSyntax identifier)
416+
{
417+
return false;
418+
}
419+
420+
// Get the parameter name (e.g., "x", "_", "item")
421+
var parameterName = lambda is SimpleLambdaExpressionSyntax simpleLambda
422+
? simpleLambda.Parameter.Identifier.Text
423+
: lambda is ParenthesizedLambdaExpressionSyntax { ParameterList.Parameters.Count: 1 } parenthesizedLambda
424+
? parenthesizedLambda.ParameterList.Parameters[0].Identifier.Text
425+
: null;
426+
427+
// Check if the body references the same parameter
428+
return parameterName != null && identifier.Identifier.Text == parameterName;
429+
}
354430
}

src/GraphQL.EntityFramework/ComplexGraphResolver.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,11 +99,11 @@ static Resolved GetOrAdd(FieldType fieldType) =>
9999

100100
internal static IComplexGraphType GetComplexGraph(this FieldType fieldType)
101101
{
102-
if (TryGetComplexGraph(fieldType, out var complex))
102+
if (fieldType.TryGetComplexGraph(out var complex))
103103
{
104104
return complex;
105105
}
106106

107107
throw new($"Could not find resolve a {nameof(IComplexGraphType)} for {fieldType.GetType().FullName}.");
108108
}
109-
}
109+
}

src/GraphQL.EntityFramework/GraphApi/FieldBuilderExtensions.cs

Lines changed: 57 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,10 +24,11 @@ public static class FieldBuilderExtensions
2424
public static FieldBuilder<TSource, TReturn> Resolve<TDbContext, TSource, TReturn, TProjection>(
2525
this FieldBuilder<TSource, TReturn> builder,
2626
Expression<Func<TSource, TProjection>> projection,
27-
Func<ResolveProjectionContext<TDbContext, TProjection>, TReturn?> resolve)
27+
Func<ResolveProjectionContext<TDbContext, TProjection>, TReturn> resolve)
2828
where TDbContext : DbContext
29-
where TReturn : class
3029
{
30+
ValidateProjection(projection);
31+
3132
var field = builder.FieldType;
3233

3334
// Store projection expression - flows through to Select expression builder
@@ -38,7 +39,7 @@ public static FieldBuilder<TSource, TReturn> Resolve<TDbContext, TSource, TRetur
3839

3940
var compiledProjection = projection.Compile();
4041

41-
field.Resolver = new FuncFieldResolver<TSource, TReturn?>(
42+
field.Resolver = new FuncFieldResolver<TSource, TReturn>(
4243
async context =>
4344
{
4445
// Resolve service from request services
@@ -62,7 +63,7 @@ public static FieldBuilder<TSource, TReturn> Resolve<TDbContext, TSource, TRetur
6263
FieldContext = context
6364
};
6465

65-
TReturn? result;
66+
TReturn result;
6667
try
6768
{
6869
result = resolve(projectionContext);
@@ -79,13 +80,7 @@ public static FieldBuilder<TSource, TReturn> Resolve<TDbContext, TSource, TRetur
7980
exception);
8081
}
8182

82-
if (filters == null ||
83-
await filters.ShouldInclude(context.UserContext, dbContext, context.User, result))
84-
{
85-
return result;
86-
}
87-
88-
return default;
83+
return await ApplyFilters(filters, context, dbContext, result);
8984
});
9085

9186
return builder;
@@ -110,10 +105,11 @@ await filters.ShouldInclude(context.UserContext, dbContext, context.User, result
110105
public static FieldBuilder<TSource, TReturn> ResolveAsync<TDbContext, TSource, TReturn, TProjection>(
111106
this FieldBuilder<TSource, TReturn> builder,
112107
Expression<Func<TSource, TProjection>> projection,
113-
Func<ResolveProjectionContext<TDbContext, TProjection>, Task<TReturn?>> resolve)
108+
Func<ResolveProjectionContext<TDbContext, TProjection>, Task<TReturn>> resolve)
114109
where TDbContext : DbContext
115-
where TReturn : class
116110
{
111+
ValidateProjection(projection);
112+
117113
var field = builder.FieldType;
118114

119115
// Store projection expression - flows through to Select expression builder
@@ -124,7 +120,7 @@ public static FieldBuilder<TSource, TReturn> ResolveAsync<TDbContext, TSource, T
124120

125121
var compiledProjection = projection.Compile();
126122

127-
field.Resolver = new FuncFieldResolver<TSource, TReturn?>(
123+
field.Resolver = new FuncFieldResolver<TSource, TReturn>(
128124
async context =>
129125
{
130126
// Resolve service from request services
@@ -148,7 +144,7 @@ public static FieldBuilder<TSource, TReturn> ResolveAsync<TDbContext, TSource, T
148144
FieldContext = context
149145
};
150146

151-
TReturn? result;
147+
TReturn result;
152148
try
153149
{
154150
result = await resolve(projectionContext);
@@ -165,13 +161,7 @@ public static FieldBuilder<TSource, TReturn> ResolveAsync<TDbContext, TSource, T
165161
exception);
166162
}
167163

168-
if (filters == null ||
169-
await filters.ShouldInclude(context.UserContext, dbContext, context.User, result))
170-
{
171-
return result;
172-
}
173-
174-
return default;
164+
return await ApplyFilters(filters, context, dbContext, result);
175165
});
176166

177167
return builder;
@@ -199,6 +189,8 @@ public static FieldBuilder<TSource, IEnumerable<TReturn>> ResolveList<TDbContext
199189
Func<ResolveProjectionContext<TDbContext, TProjection>, IEnumerable<TReturn>> resolve)
200190
where TDbContext : DbContext
201191
{
192+
ValidateProjection(projection);
193+
202194
var field = builder.FieldType;
203195

204196
// Store projection expression - flows through to Select expression builder
@@ -280,6 +272,8 @@ public static FieldBuilder<TSource, IEnumerable<TReturn>> ResolveListAsync<TDbCo
280272
Func<ResolveProjectionContext<TDbContext, TProjection>, Task<IEnumerable<TReturn>>> resolve)
281273
where TDbContext : DbContext
282274
{
275+
ValidateProjection(projection);
276+
283277
var field = builder.FieldType;
284278

285279
// Store projection expression - flows through to Select expression builder
@@ -339,6 +333,33 @@ public static FieldBuilder<TSource, IEnumerable<TReturn>> ResolveListAsync<TDbCo
339333
return builder;
340334
}
341335

336+
static async Task<TReturn> ApplyFilters<TDbContext, TReturn>(
337+
Filters<TDbContext>? filters,
338+
IResolveFieldContext context,
339+
TDbContext dbContext,
340+
TReturn result)
341+
where TDbContext : DbContext
342+
{
343+
// Value types don't support filtering - return as-is
344+
if (typeof(TReturn).IsValueType)
345+
{
346+
return result;
347+
}
348+
349+
// For reference types, apply filters if available
350+
if (filters != null && result is not null)
351+
{
352+
// Use dynamic to work around the class constraint on ShouldInclude
353+
dynamic dynamicFilters = filters;
354+
if (!await dynamicFilters.ShouldInclude(context.UserContext, dbContext, context.User, result))
355+
{
356+
return default!;
357+
}
358+
}
359+
360+
return result;
361+
}
362+
342363
static Filters<TDbContext>? ResolveFilters<TDbContext>(IEfGraphQLService<TDbContext> service, IResolveFieldContext context)
343364
where TDbContext : DbContext
344365
{
@@ -356,4 +377,18 @@ public static FieldBuilder<TSource, IEnumerable<TReturn>> ResolveListAsync<TDbCo
356377
var genericMethod = method.MakeGenericMethod(context.Source?.GetType() ?? typeof(object));
357378
return genericMethod.Invoke(service, [context]) as Filters<TDbContext>;
358379
}
380+
381+
static void ValidateProjection<TSource, TProjection>(Expression<Func<TSource, TProjection>> projection)
382+
{
383+
// Detect identity projection: _ => _
384+
if (projection.Body is not ParameterExpression parameter ||
385+
parameter != projection.Parameters[0])
386+
{
387+
return;
388+
}
389+
390+
throw new ArgumentException(
391+
"Identity projection '_ => _' is not allowed. If only access to primary key or foreign key properties, use the regular Resolve() method instead. If required to access navigation properties, specify them in the projection (e.g., '_ => _.Parent').",
392+
nameof(projection));
393+
}
359394
}

src/GraphQL.EntityFramework/Where/ReflectionCache.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ static ReflectionCache() =>
6565
return method;
6666
}
6767

68-
if (IsEnumType(type))
68+
if (type.IsEnumType())
6969
{
7070
return typeof(ICollection<>).MakeGenericType(type).GetMethod("Contains");
7171
}

0 commit comments

Comments
 (0)