[Analyzer] Source-gen non-static partial root type classes#9740
Open
glen-84 wants to merge 2 commits into
Open
[Analyzer] Source-gen non-static partial root type classes#9740glen-84 wants to merge 2 commits into
glen-84 wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the HotChocolate Types analyzer/source-generator pipeline to support non-static, partial root operation types (e.g. [QueryType] partial class Query) while keeping the “missing partial” diagnostic behavior consistent for both static and non-static root types.
Changes:
- Extend the root-type “partial keyword missing” analyzer/code-fix to apply to all root types (not just static ones) and align the code-fix with diagnostic HC0091.
- Update source generation to correctly invoke instance root resolvers via
context.Resolver<T>()(including batch resolvers) and generate non-static partial root type shells when needed. - Add unit + integration coverage and update snapshots to validate diagnostics and generated code paths.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/HotChocolate/Core/test/Types.Analyzers.Tests/RootTypePartialAnalyzerTests.cs | Adds tests covering static/non-static + partial/non-partial root types and validates instance batch resolver receiver generation. |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/RootTypePartialAnalyzerTests.StaticPartialQueryType_NoDiagnostic.md | Snapshot for static partial root type (no diagnostic, static invocation). |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/RootTypePartialAnalyzerTests.NonStaticPartialQueryType_NoDiagnostic.md | Snapshot for non-static partial root type (no diagnostic, Resolver<T>() invocation). |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/RootTypePartialAnalyzerTests.StaticNonPartialQueryType_RaisesInfo.md | Snapshot asserting HC0091 diagnostic for static non-partial root type. |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/RootTypePartialAnalyzerTests.NonStaticNonPartialQueryType_RaisesInfo.md | Snapshot asserting HC0091 diagnostic for non-static non-partial root type. |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/RootTypePartialAnalyzerTests.NonStaticPartialQueryType_InstanceBatchResolver_UsesResolverReceiver.md | Snapshot verifying batch resolvers call instance root resolvers via contexts[0].Resolver<T>(). |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/ObjectTypeXmlDocInferenceTests.When_description_has_paramref_tag_then_it_is_converted.snap | Snapshot update due to changed receiver generation affecting diagnostic spans. |
| src/HotChocolate/Core/test/Types.Analyzers.Tests/snapshots/ObjectTypeXmlDocInferenceTests.When_description_has_generic_tags_then_it_is_converted.snap | Snapshot update due to changed receiver generation affecting diagnostic spans. |
| src/HotChocolate/Core/test/Types.Analyzers.Integration.Tests/IssueNonStaticConnection.cs | Adds an integration test schema with a non-static [QueryType] and paging fields to validate instance resolver execution. |
| src/HotChocolate/Core/test/Types.Analyzers.Integration.Tests/IntegrationTests.cs | Adds an integration assertion that an instance method on a non-static query type is actually invoked on an instance. |
| src/HotChocolate/Core/test/Types.Analyzers.Integration.Tests/snapshots/IntegrationTests.Schema_Snapshot.snap | Schema snapshot updates for the added paging/query/object types. |
| src/HotChocolate/Core/test/Types.Analyzers.Integration.Tests/snapshots/IntegrationTests.Schema_Snapshot_Without_ConnectionName_Inference.snap | Schema snapshot updates for the added paging/query/object types (without connection name inference). |
| src/HotChocolate/Core/src/Types.Analyzers/RootTypePartialCodeFixProvider.cs | Fixes code-fix to target HC0091. |
| src/HotChocolate/Core/src/Types.Analyzers/RootTypePartialAnalyzer.cs | Removes the “static-only” restriction so HC0091 applies to all root types missing partial. |
| src/HotChocolate/Core/src/Types.Analyzers/Models/RootTypeInfo.cs | Tracks whether a root type declaration is static to drive codegen decisions. |
| src/HotChocolate/Core/src/Types.Analyzers/Inspectors/TypeAttributeInspector.cs | Treats partial root operation types as source-gen candidates regardless of static-ness. |
| src/HotChocolate/Core/src/Types.Analyzers/Inspectors/ObjectTypeInspector.cs | Allows non-static operation root types while still requiring static for object type extensions. |
| src/HotChocolate/Core/src/Types.Analyzers/FileBuilders/TypeFileBuilderBase.cs | Centralizes instance-receiver expression building and uses it for resolver/property/batch generation. |
| src/HotChocolate/Core/src/Types.Analyzers/FileBuilders/RootTypeFileBuilder.cs | Overrides instance receiver to use Resolver<T>() and emits non-static partial root type shells when needed. |
| src/HotChocolate/Core/src/Types.Analyzers/Errors.cs | Updates HC0091 message to no longer mention “static” root types. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Source-gen non-static partial root type classes
Problem
A non-static
[QueryType]class with[UsePaging]instance methods caused a runtime schema-build failure when its element type was also paged from a source-generated[ObjectType<T>]extension:Cause:
TypeAttributeInspectoronly routedstatic partialroot classes through the deep source-gen path. Non-static or non-partial classes fell through to a runtime-extension shortcut emitted asbuilder.AddTypeExtension<T>()in the generated module file. The runtime path produced an independentConnectionType<Book>instance that collided by name with the source-generated one from sibling[ObjectType<>]extensions.Follow-up to #9734, which fixed the static-only variant of the same collision.
Changes
Source generator
TypeAttributeInspector— anypartialclass with[QueryType]/[MutationType]/[SubscriptionType]now defers toObjectTypeInspector(was: onlystatic partial). Non-partial classes still fall through to the runtime-extension shortcut.ObjectTypeInspector—ObjectTypeStaticKeywordMissing(HC0081) is skipped for operation root types. Thestaticrequirement still applies to[ObjectType<T>]extensions.Models/RootTypeInfo— capturesIsStaticfrom the class declaration so the file builder can emit the matching partial modifier.FileBuilders/RootTypeFileBuilder— overridesWriteBeginClassto emitpartial class Foo(notstatic partial class Foo) for non-static roots, since C# rejects partial declarations that differ in modifiers.FileBuilders/TypeFileBuilderBase— introducesprotected virtual string GetInstanceReceiver(string typeName, string contextExpression = "context")defaulting to{ctx}.Parent<T>(). All five non-static invocation sites route through it: async resolver, sync resolver, pure resolver, property resolver, and both async + sync paths ofWriteBatchResolver(the last two passing"contexts[0]").FileBuilders/RootTypeFileBuilder— overridesGetInstanceReceiverto emit{ctx}.Resolver<T>(). This mirrors whatDefaultResolverCompiler.CreateResolverOwnerandBatchResolverCompiler.BuildResolverOwneralready do at runtime whensource != resolver: obtain the resolver instance viaIResolverContext.Resolver<T>(), which resolves from DI or activates and caches an instance viaActivatorUtilities.CreateInstance. The root field's parent value is not an instance of the resolver class, soParent<T>()would yield null and the instance call would fault.Diagnostic
RootTypePartialAnalyzer+Errors.cs— HC0091 now fires for any non-partial[QueryType]/[MutationType]/[SubscriptionType]class (was: static-only). Message generalized to"A root type class should be declared as partial to allow source generation".RootTypePartialCodeFixProvider— drive-by:FixableDiagnosticIdswas wired to"HC0089", which is not a Roslyn diagnostic in this repo (only a runtimeErrorCodesconstant uses that string). The "Add partial keyword" code fix has therefore never been offered since the provider was introduced in 850ee8d. Corrected to"HC0091".Tests
Analyzer unit tests (
Types.Analyzers.Tests)New file
RootTypePartialAnalyzerTests.cswith five cases:StaticPartialQueryType_NoDiagnostic— control.NonStaticPartialQueryType_NoDiagnostic— the new supported path; snapshot pins the generated receiver tocontext.Resolver<Query>().StaticNonPartialQueryType_RaisesInfo— existing case still covered.NonStaticNonPartialQueryType_RaisesInfo— the literal repro shape; HC0091 fires.NonStaticPartialQueryType_InstanceBatchResolver_UsesResolverReceiver— pins batch receiver tocontexts[0].Resolver<Query>().Integration tests (
Types.Analyzers.Integration.Tests)New file
IssueNonStaticConnection.csaddsAuthor,Publisher, and a[QueryType] public partial class NonStaticPagedQuerywith a[GraphQLIgnore]GUIDInstanceIdand three[UsePaging]instance methods. These types feed the existingSchema_Snapshot/Schema_Snapshot_Without_ConnectionName_Inferencetests (snapshots refreshed) to verify a single sharedBookConnection.New runtime test
Resolves_Instance_Method_On_NonStatic_QueryTypeexecutes{ someBooks { nodes { title } } }, asserts no"errors"in the JSON, and matchestitleagainst^[0-9a-f]{32}$(the per-instance GUID). This catches the null-ref that would have surfaced from a staleParent<T>()receiver; verified by reverting the override locally before merging.Snapshot refreshes
Two
ObjectTypeXmlDocInferenceTestssnapshots (receiverParent<Query>()→Resolver<Query>()) and the twoSchema_Snapshot*snapshots in the integration project (single sharedBookConnection).Caveat for AOT / trimming
For non-static partial roots, the generated
Resolver<T>()call lands inResolverProvider.GetResolver<T>which is already annotated with class-level[UnconditionalSuppressMessage("AOT", "IL2067")]. The activator fallback (ActivatorUtilities.CreateInstance) is AOT-incompatible unless the resolver class is registered in DI.This is not a new AOT surface: pre-fix,
builder.AddTypeExtension<NonStaticPagedQuery>()was emitted in the source-generated module file and the runtime resolver compiler reached the sameResolver<T>()→ activator path through expression trees. The fix moves that call from runtime expression-tree compilation into emitted C#; same destination, more visible route.For NativeAOT targets, the existing recommendation stands: keep root types
static partial, or register the resolver class in DI soGetService<T>()succeeds and the activator is not invoked. Worth a line in the v16 migration notes.Test results
Types.Analyzers.Tests— 330/330 (+5 new)Types.Analyzers.Integration.Tests— 12/12 (+1 new runtime test + new schema coverage)Types.CursorPagination.Tests— 103/103HotChocolate.Core.slnxbuild — 0 warnings, 0 errors