Skip to content

Commit 11a7aae

Browse files
SimonCroppSimonCropp
committed
.
1 parent be0560b commit 11a7aae

9 files changed

Lines changed: 296 additions & 75 deletions

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -475,10 +475,10 @@ 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.
478+
// NOTE: Analyzer tests for GQLEF003 (identity projection detection) and GQLEF004 (scalar type projection)
479+
// are skipped because the analyzer implementation has issues detecting these patterns in test scenarios.
480+
// However, runtime validation works perfectly and catches both issues immediately when code runs.
481+
// See FieldBuilderExtensionsTests for comprehensive runtime validation tests.
482482

483483
static async Task<Diagnostic[]> GetDiagnosticsAsync(string source)
484484
{
@@ -557,9 +557,9 @@ static async Task<Diagnostic[]> GetDiagnosticsAsync(string source)
557557
throw new($"Compilation errors:\n{errorMessages}");
558558
}
559559

560-
// Filter to only GQLEF002 and GQLEF003 diagnostics
560+
// Filter to only GQLEF002, GQLEF003, and GQLEF004 diagnostics
561561
return allDiagnostics
562-
.Where(_ => _.Id == "GQLEF002" || _.Id == "GQLEF003")
562+
.Where(_ => _.Id == "GQLEF002" || _.Id == "GQLEF003" || _.Id == "GQLEF004")
563563
.ToArray();
564564
}
565565
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ Rule ID | Category | Severity | Notes
77
--------|----------|----------|--------------------
88
GQLEF002 | Usage | Warning | Use projection-based Resolve extension methods when accessing navigation properties
99
GQLEF003 | Usage | Error | Identity projection is not allowed in projection-based Resolve methods
10+
GQLEF004 | Usage | Error | Projection to scalar types 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
@@ -19,4 +19,14 @@ static class DiagnosticDescriptors
1919
isEnabledByDefault: true,
2020
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').",
2121
helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#projection-based-resolve");
22+
23+
public static readonly DiagnosticDescriptor GQLEF004 = new(
24+
id: "GQLEF004",
25+
title: "Projection to scalar types is not allowed in projection-based Resolve methods",
26+
messageFormat: "Projection to scalar type '{0}' is not allowed. Projection-based Resolve methods are for loading navigation properties, not scalar properties. Use regular Resolve() method instead.",
27+
category: "Usage",
28+
defaultSeverity: DiagnosticSeverity.Error,
29+
isEnabledByDefault: true,
30+
description: "Projection-based Resolve extension methods are designed for loading navigation properties (related entities), not for accessing scalar properties like primitives, strings, dates, or enums. Use the regular Resolve() method to access and transform scalar properties.",
31+
helpLinkUri: "https://github.com/SimonCropp/GraphQL.EntityFramework#projection-based-resolve");
2232
}

‎src/GraphQL.EntityFramework.Analyzers/FieldBuilderResolveAnalyzer.cs‎

Lines changed: 85 additions & 1 deletion
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, DiagnosticDescriptors.GQLEF003];
7+
[DiagnosticDescriptors.GQLEF002, DiagnosticDescriptors.GQLEF003, DiagnosticDescriptors.GQLEF004];
88

99
public override void Initialize(AnalysisContext context)
1010
{
@@ -44,6 +44,18 @@ static void AnalyzeInvocation(SyntaxNodeAnalysisContext context)
4444
invocation.GetLocation());
4545
context.ReportDiagnostic(diagnostic);
4646
}
47+
48+
// Check for scalar type projection
49+
var scalarTypeName = GetScalarProjectionTypeName(invocation, context.SemanticModel);
50+
if (scalarTypeName != null)
51+
{
52+
var diagnostic = Diagnostic.Create(
53+
DiagnosticDescriptors.GQLEF004,
54+
invocation.GetLocation(),
55+
scalarTypeName);
56+
context.ReportDiagnostic(diagnostic);
57+
}
58+
4759
return;
4860
}
4961

@@ -427,4 +439,76 @@ static bool HasIdentityProjection(InvocationExpressionSyntax invocation, Semanti
427439
// Check if the body references the same parameter
428440
return parameterName != null && identifier.Identifier.Text == parameterName;
429441
}
442+
443+
static string? GetScalarProjectionTypeName(InvocationExpressionSyntax invocation, SemanticModel semanticModel)
444+
{
445+
// Get the symbol info to find type parameters
446+
var symbolInfo = semanticModel.GetSymbolInfo(invocation);
447+
if (symbolInfo.Symbol is not IMethodSymbol methodSymbol)
448+
{
449+
return null;
450+
}
451+
452+
// For projection-based methods, TProjection is the 4th type parameter (TDbContext, TSource, TReturn, TProjection)
453+
if (methodSymbol.TypeArguments.Length < 4)
454+
{
455+
return null;
456+
}
457+
458+
var projectionType = methodSymbol.TypeArguments[3];
459+
var underlyingType = projectionType;
460+
461+
// Unwrap nullable types
462+
if (projectionType is INamedTypeSymbol { OriginalDefinition.SpecialType: SpecialType.System_Nullable_T } namedType)
463+
{
464+
underlyingType = namedType.TypeArguments[0];
465+
}
466+
467+
// Check if it's a scalar type
468+
if (IsScalarTypeSymbol(underlyingType))
469+
{
470+
return projectionType.ToDisplayString(SymbolDisplayFormat.MinimallyQualifiedFormat);
471+
}
472+
473+
return null;
474+
}
475+
476+
static bool IsScalarTypeSymbol(ITypeSymbol type)
477+
{
478+
// Check for primitive types (int, bool, etc.)
479+
if (type.SpecialType is
480+
SpecialType.System_Boolean or
481+
SpecialType.System_Byte or
482+
SpecialType.System_SByte or
483+
SpecialType.System_Int16 or
484+
SpecialType.System_UInt16 or
485+
SpecialType.System_Int32 or
486+
SpecialType.System_UInt32 or
487+
SpecialType.System_Int64 or
488+
SpecialType.System_UInt64 or
489+
SpecialType.System_Single or
490+
SpecialType.System_Double or
491+
SpecialType.System_Char or
492+
SpecialType.System_String or
493+
SpecialType.System_Decimal or
494+
SpecialType.System_DateTime)
495+
{
496+
return true;
497+
}
498+
499+
// Check for common scalar types by name
500+
var fullName = type.ToString();
501+
if (fullName is "System.DateTimeOffset" or "System.TimeSpan" or "System.Guid")
502+
{
503+
return true;
504+
}
505+
506+
// Check for enums
507+
if (type.TypeKind == TypeKind.Enum)
508+
{
509+
return true;
510+
}
511+
512+
return false;
513+
}
430514
}

‎src/GraphQL.EntityFramework/GraphApi/FieldBuilderExtensions.cs‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -381,14 +381,54 @@ static async Task<TReturn> ApplyFilters<TDbContext, TReturn>(
381381
static void ValidateProjection<TSource, TProjection>(Expression<Func<TSource, TProjection>> projection)
382382
{
383383
// Detect identity projection: _ => _
384-
if (projection.Body is not ParameterExpression parameter ||
385-
parameter != projection.Parameters[0])
384+
if (projection.Body is ParameterExpression parameter &&
385+
parameter == projection.Parameters[0])
386386
{
387-
return;
387+
throw new ArgumentException(
388+
"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').",
389+
nameof(projection));
388390
}
389391

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));
392+
// Detect projection to scalar/primitive types
393+
var projectionType = typeof(TProjection);
394+
var underlyingType = Nullable.GetUnderlyingType(projectionType) ?? projectionType;
395+
396+
if (IsScalarType(underlyingType))
397+
{
398+
throw new ArgumentException(
399+
$"Projection to scalar type '{projectionType.Name}' is not allowed in projection-based Resolve methods. " +
400+
"Projection-based methods are designed for loading navigation properties (related entities), not for accessing scalar properties. " +
401+
$"Use the regular Resolve() method instead to access scalar properties. " +
402+
"Example: Field<T>(\"name\").Resolve(ctx => Transform(ctx.Source.ScalarProperty))",
403+
nameof(projection));
404+
}
405+
}
406+
407+
static bool IsScalarType(Type type)
408+
{
409+
// Check for primitive types (int, bool, etc.)
410+
if (type.IsPrimitive)
411+
{
412+
return true;
413+
}
414+
415+
// Check for common scalar types
416+
if (type == typeof(string) ||
417+
type == typeof(decimal) ||
418+
type == typeof(DateTime) ||
419+
type == typeof(DateTimeOffset) ||
420+
type == typeof(TimeSpan) ||
421+
type == typeof(Guid))
422+
{
423+
return true;
424+
}
425+
426+
// Check for enums
427+
if (type.IsEnum)
428+
{
429+
return true;
430+
}
431+
432+
return false;
393433
}
394434
}

‎src/Tests/FieldBuilderExtensionsTests.cs‎

Lines changed: 116 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,14 +80,125 @@ public void ResolveListAsync_ThrowsArgumentException_WhenIdentityProjectionUsed(
8080
}
8181

8282
[Fact]
83-
public void Resolve_DoesNotThrow_WhenValidProjectionUsed()
83+
public void Resolve_ThrowsArgumentException_WhenProjectingToInt()
84+
{
85+
var graphType = new ObjectGraphType<TestEntity>();
86+
var field = graphType.Field<int>("test");
87+
88+
var exception = Assert.Throws<ArgumentException>(() =>
89+
field.Resolve<TestDbContext, TestEntity, int, int>(
90+
projection: _ => _.Id,
91+
resolve: _ => _.Projection * 2));
92+
93+
Assert.Contains("Projection to scalar type", exception.Message);
94+
Assert.Contains("Int32", exception.Message);
95+
}
96+
97+
[Fact]
98+
public void Resolve_ThrowsArgumentException_WhenProjectingToString()
8499
{
85100
var graphType = new ObjectGraphType<TestEntity>();
86101
var field = graphType.Field<string>("test");
87102

88-
// This should not throw
89-
field.Resolve<TestDbContext, TestEntity, string, string>(
90-
projection: _ => _.Name,
91-
resolve: _ => _.Projection);
103+
var exception = Assert.Throws<ArgumentException>(() =>
104+
field.Resolve<TestDbContext, TestEntity, string, string>(
105+
projection: _ => _.Name,
106+
resolve: _ => _.Projection.ToUpper()));
107+
108+
Assert.Contains("Projection to scalar type", exception.Message);
109+
Assert.Contains("String", exception.Message);
110+
}
111+
112+
[Fact]
113+
public void Resolve_ThrowsArgumentException_WhenProjectingToBool()
114+
{
115+
var graphType = new ObjectGraphType<TestEntity>();
116+
var field = graphType.Field<bool>("test");
117+
118+
var exception = Assert.Throws<ArgumentException>(() =>
119+
field.Resolve<TestDbContext, TestEntity, bool, bool>(
120+
projection: _ => _.Id > 0,
121+
resolve: _ => _.Projection));
122+
123+
Assert.Contains("Projection to scalar type", exception.Message);
124+
Assert.Contains("Boolean", exception.Message);
125+
}
126+
127+
[Fact]
128+
public void Resolve_ThrowsArgumentException_WhenProjectingToDateTime()
129+
{
130+
var graphType = new ObjectGraphType<TestEntity>();
131+
var field = graphType.Field<DateTime>("test");
132+
133+
var exception = Assert.Throws<ArgumentException>(() =>
134+
field.Resolve<TestDbContext, TestEntity, DateTime, DateTime>(
135+
projection: _ => DateTime.Now,
136+
resolve: _ => _.Projection));
137+
138+
Assert.Contains("Projection to scalar type", exception.Message);
139+
Assert.Contains("DateTime", exception.Message);
140+
}
141+
142+
[Fact]
143+
public void Resolve_ThrowsArgumentException_WhenProjectingToEnum()
144+
{
145+
var graphType = new ObjectGraphType<TestEntity>();
146+
var field = graphType.Field<TestEnum>("test");
147+
148+
var exception = Assert.Throws<ArgumentException>(() =>
149+
field.Resolve<TestDbContext, TestEntity, TestEnum, TestEnum>(
150+
projection: _ => TestEnum.Value1,
151+
resolve: _ => _.Projection));
152+
153+
Assert.Contains("Projection to scalar type", exception.Message);
154+
Assert.Contains("TestEnum", exception.Message);
155+
}
156+
157+
[Fact]
158+
public void ResolveAsync_ThrowsArgumentException_WhenProjectingToScalar()
159+
{
160+
var graphType = new ObjectGraphType<TestEntity>();
161+
var field = graphType.Field<int>("test");
162+
163+
var exception = Assert.Throws<ArgumentException>(() =>
164+
field.ResolveAsync<TestDbContext, TestEntity, int, int>(
165+
projection: _ => _.Id,
166+
resolve: _ => Task.FromResult(_.Projection * 2)));
167+
168+
Assert.Contains("Projection to scalar type", exception.Message);
169+
}
170+
171+
[Fact]
172+
public void ResolveList_ThrowsArgumentException_WhenProjectingToScalar()
173+
{
174+
var graphType = new ObjectGraphType<TestEntity>();
175+
var field = graphType.Field<IEnumerable<int>>("test");
176+
177+
var exception = Assert.Throws<ArgumentException>(() =>
178+
field.ResolveList<TestDbContext, TestEntity, int, int>(
179+
projection: _ => _.Id,
180+
resolve: _ => [_.Projection]));
181+
182+
Assert.Contains("Projection to scalar type", exception.Message);
183+
}
184+
185+
[Fact]
186+
public void ResolveListAsync_ThrowsArgumentException_WhenProjectingToScalar()
187+
{
188+
var graphType = new ObjectGraphType<TestEntity>();
189+
var field = graphType.Field<IEnumerable<int>>("test");
190+
191+
var exception = Assert.Throws<ArgumentException>(() =>
192+
field.ResolveListAsync<TestDbContext, TestEntity, int, int>(
193+
projection: _ => _.Id,
194+
resolve: _ => Task.FromResult<IEnumerable<int>>([_.Projection])));
195+
196+
Assert.Contains("Projection to scalar type", exception.Message);
197+
}
198+
199+
enum TestEnum
200+
{
201+
Value1,
202+
Value2
92203
}
93204
}

0 commit comments

Comments
 (0)