Skip to content

[Analyzer] Source-gen non-static partial root type classes#9740

Open
glen-84 wants to merge 2 commits into
mainfrom
gai/source-gen-non-static-root-type-paging
Open

[Analyzer] Source-gen non-static partial root type classes#9740
glen-84 wants to merge 2 commits into
mainfrom
gai/source-gen-non-static-root-type-paging

Conversation

@glen-84
Copy link
Copy Markdown
Member

@glen-84 glen-84 commented May 19, 2026

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:

HotChocolate.SchemaException : The name `BookConnection` was already registered by another type. (HotChocolate.Types.Pagination.ConnectionType)

Cause: TypeAttributeInspector only routed static partial root classes through the deep source-gen path. Non-static or non-partial classes fell through to a runtime-extension shortcut emitted as builder.AddTypeExtension<T>() in the generated module file. The runtime path produced an independent ConnectionType<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 — any partial class with [QueryType]/[MutationType]/[SubscriptionType] now defers to ObjectTypeInspector (was: only static partial). Non-partial classes still fall through to the runtime-extension shortcut.
  • ObjectTypeInspectorObjectTypeStaticKeywordMissing (HC0081) is skipped for operation root types. The static requirement still applies to [ObjectType<T>] extensions.
  • Models/RootTypeInfo — captures IsStatic from the class declaration so the file builder can emit the matching partial modifier.
  • FileBuilders/RootTypeFileBuilder — overrides WriteBeginClass to emit partial class Foo (not static partial class Foo) for non-static roots, since C# rejects partial declarations that differ in modifiers.
  • FileBuilders/TypeFileBuilderBase — introduces protected 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 of WriteBatchResolver (the last two passing "contexts[0]").
  • FileBuilders/RootTypeFileBuilder — overrides GetInstanceReceiver to emit {ctx}.Resolver<T>(). This mirrors what DefaultResolverCompiler.CreateResolverOwner and BatchResolverCompiler.BuildResolverOwner already do at runtime when source != resolver: obtain the resolver instance via IResolverContext.Resolver<T>(), which resolves from DI or activates and caches an instance via ActivatorUtilities.CreateInstance. The root field's parent value is not an instance of the resolver class, so Parent<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: FixableDiagnosticIds was wired to "HC0089", which is not a Roslyn diagnostic in this repo (only a runtime ErrorCodes constant 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.cs with five cases:

  • StaticPartialQueryType_NoDiagnostic — control.
  • NonStaticPartialQueryType_NoDiagnostic — the new supported path; snapshot pins the generated receiver to context.Resolver<Query>().
  • StaticNonPartialQueryType_RaisesInfo — existing case still covered.
  • NonStaticNonPartialQueryType_RaisesInfo — the literal repro shape; HC0091 fires.
  • NonStaticPartialQueryType_InstanceBatchResolver_UsesResolverReceiver — pins batch receiver to contexts[0].Resolver<Query>().

Integration tests (Types.Analyzers.Integration.Tests)

New file IssueNonStaticConnection.cs adds Author, Publisher, and a [QueryType] public partial class NonStaticPagedQuery with a [GraphQLIgnore] GUID InstanceId and three [UsePaging] instance methods. These types feed the existing Schema_Snapshot / Schema_Snapshot_Without_ConnectionName_Inference tests (snapshots refreshed) to verify a single shared BookConnection.

New runtime test Resolves_Instance_Method_On_NonStatic_QueryType executes { someBooks { nodes { title } } }, asserts no "errors" in the JSON, and matches title against ^[0-9a-f]{32}$ (the per-instance GUID). This catches the null-ref that would have surfaced from a stale Parent<T>() receiver; verified by reverting the override locally before merging.

Snapshot refreshes

Two ObjectTypeXmlDocInferenceTests snapshots (receiver Parent<Query>()Resolver<Query>()) and the two Schema_Snapshot* snapshots in the integration project (single shared BookConnection).

Caveat for AOT / trimming

For non-static partial roots, the generated Resolver<T>() call lands in ResolverProvider.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 same Resolver<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 so GetService<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/103
  • HotChocolate.Core.slnx build — 0 warnings, 0 errors

Copilot AI review requested due to automatic review settings May 19, 2026 10:58
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/HotChocolate/Core/src/Types.Analyzers/Inspectors/ObjectTypeInspector.cs Outdated
Comment thread src/HotChocolate/Core/src/Types.Analyzers/FileBuilders/TypeFileBuilderBase.cs Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

@glen-84 glen-84 requested a review from michaelstaib May 19, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants