Skip to content

Move MethodBuilder and RecordingGeneratorsFactory to Generators#45

Merged
dex3r merged 3 commits into
mainfrom
copilot/move-methodbuilders-and-recordinggeneratorsfactory
Mar 4, 2026
Merged

Move MethodBuilder and RecordingGeneratorsFactory to Generators#45
dex3r merged 3 commits into
mainfrom
copilot/move-methodbuilders-and-recordinggeneratorsfactory

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 3, 2026

Abstractions contained concrete implementation classes (MethodBuilder, RecordingGeneratorsFactory, SwitchBodyRecord, and related Recording* types) that belong in Generators.

Changes

  • Moved MethodBuilder.cs and RecordingGeneratorsFactory.cs from AbstractionsGenerators, namespace updated to MattSourceGenHelpers.Generators
  • Explicit using directives added to both files — required because they are injected as raw source text into execution compilations (which don't inherit the project's implicit usings)
  • Embedded both files as resources in Generators.csproj so they can be read and injected at runtime
  • BuildExecutionCompilation now injects MethodBuilder.cs and RecordingGeneratorsFactory.cs as extra syntax trees into the ephemeral execution compilation, making the recording types available in the correct AssemblyLoadContext without relying on Assembly.Location (which is empty when loaded as a Roslyn analyzer)
  • ExecuteFluentGeneratorMethod now resolves RecordingGeneratorsFactory from the execution assembly instead of from the Abstractions assembly
  • Consts gains GeneratorsNamespace/GeneratorsAssemblyName constants; RecordingGeneratorsFactoryTypeFullName updated accordingly
Original prompt

This section details on the original issue you should resolve

<issue_title>Move MethodBuilders and RecordingGeneratorsFactorys to Generators</issue_title>
<issue_description>Concrete classes from files MethodBuilder and RecordingGeneratorsFactory shoulnt be part of Abstractions, they should be moved to Genrators project.

  • move the classes/files
  • adjust namespaces
  • make sure tests still pass</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

…o Generators

Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com>
Copilot AI changed the title [WIP] Move MethodBuilders and RecordingGeneratorsFactory to Generators Move MethodBuilder and RecordingGeneratorsFactory to Generators Mar 3, 2026
Copilot finished work on behalf of dex3r March 3, 2026 17:32
@dex3r
Copy link
Copy Markdown
Owner

dex3r commented Mar 4, 2026

Embedded both files as resources in Generators.csproj so they can be read and injected at runtime

Why do they need to be added as resources? Can't they just be compiled into Generators and thats it? @copilot

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 4, 2026

Embedded both files as resources in Generators.csproj so they can be read and injected at runtime

Why do they need to be added as resources? Can't they just be compiled into Generators and thats ...

They need to be injected as source into the ephemeral execution compilation rather than just compiled into Generators as-is, because of a .NET AssemblyLoadContext type identity constraint:

  1. Generator method execution happens inside an isolated AssemblyLoadContext("__GeneratorExec") — every assembly loaded there gets its own copy of every type.
  2. Generate.CurrentGenerator (type IGeneratorsFactory) lives in that context's copy of the Abstractions assembly.
  3. The RecordingGeneratorsFactory instance assigned to CurrentGenerator must implement the same IGeneratorsFactory from the same Abstractions copy. If RecordingGeneratorsFactory came from a separately-loaded Generators assembly, it would reference a different load of IGeneratorsFactory and PropertyInfo.SetValue would throw a type-mismatch error at runtime.
  4. Assembly.Location is empty when an assembly is loaded as a Roslyn analyzer, so loading Generators from disk into the execution context isn't possible (this was the original "Could not determine Generators assembly path" error).

Injecting the source code of RecordingGeneratorsFactory and MethodBuilder into the execution compilation ensures they're compiled against the exact same IGeneratorsFactory reference as everything else in that compilation. Embedding them as resources in the Generators DLL is how that source code is carried along at runtime.

Copilot finished work on behalf of dex3r March 4, 2026 18:03
@dex3r dex3r marked this pull request as ready for review March 4, 2026 18:05
Copilot AI review requested due to automatic review settings March 4, 2026 18:05
@dex3r dex3r merged commit 4a17a91 into main Mar 4, 2026
3 checks passed
@dex3r dex3r deleted the copilot/move-methodbuilders-and-recordinggeneratorsfactory branch March 4, 2026 18:06
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 relocates concrete “recording/execution” helper implementations out of Abstractions into the Generators project, and updates the execution-compilation path to inject these helpers as embedded source so they load in the correct AssemblyLoadContext when running as an analyzer.

Changes:

  • Moved MethodBuilder and RecordingGeneratorsFactory into MattSourceGenHelpers.Generators and added explicit using directives for source-injection scenarios.
  • Embedded both .cs files as resources in MattSourceGenHelpers.Generators.csproj and injected them as extra syntax trees into the ephemeral execution compilation.
  • Updated runtime type resolution and constants to locate RecordingGeneratorsFactory in the compiled execution assembly / Generators namespace.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
MattSourceGenHelpers.Generators/RecordingGeneratorsFactory.cs Moves recording types into Generators namespace and adds explicit usings for injection compilation.
MattSourceGenHelpers.Generators/MethodBuilder.cs Moves MethodBuilder into Generators namespace and adds explicit using for injected compilation.
MattSourceGenHelpers.Generators/MattSourceGenHelpers.Generators.csproj Embeds MethodBuilder.cs and RecordingGeneratorsFactory.cs as manifest resources.
MattSourceGenHelpers.Generators/GeneratesMethodExecutionRuntime.cs Injects embedded helper sources into execution compilation and resolves factory type from execution assembly.
MattSourceGenHelpers.Generators/Consts.cs Adds Generators namespace/assembly constants and updates recording factory full name.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 137 to 142
Type? generatorStaticType = abstractionsAssembly.GetType(Consts.GenerateTypeFullName);
Type? recordingFactoryType = abstractionsAssembly.GetType(Consts.RecordingGeneratorsFactoryTypeFullName);
Type? recordingFactoryType = assembly.GetType(Consts.RecordingGeneratorsFactoryTypeFullName);
if (generatorStaticType == null || recordingFactoryType == null)
{
return (null, $"Could not find {Consts.GenerateTypeFullName} or {Consts.RecordingGeneratorsFactoryTypeFullName} types in Abstractions assembly");
return (null, $"Could not find {Consts.GenerateTypeFullName} or {Consts.RecordingGeneratorsFactoryTypeFullName} types in compiled assembly");
}
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

The failure message here is misleading: GenerateTypeFullName is looked up from abstractionsAssembly, while RecordingGeneratorsFactoryTypeFullName is looked up from the compiled execution assembly. Consider splitting the null checks and reporting which type was missing from which assembly (or include both assembly names) to make troubleshooting accurate.

Copilot uses AI. Check for mistakes.
Comment on lines 341 to +343
string dummySource = BuildDummyImplementation(allPartialMethods);
string methodBuilderSource = ReadEmbeddedResource($"{Consts.GeneratorsAssemblyName}.MethodBuilder.cs");
string recordingFactorySource = ReadEmbeddedResource($"{Consts.GeneratorsAssemblyName}.RecordingGeneratorsFactory.cs");
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

BuildExecutionCompilation reads the embedded .cs resources on every call, which can add noticeable overhead in analyzer execution (multiple invocations per compilation). Consider caching the resource text (or even the parsed SyntaxTrees) in static fields so this work is done once per analyzer load.

Copilot uses AI. Check for mistakes.
{
using Stream? stream = typeof(GeneratesMethodExecutionRuntime).Assembly.GetManifestResourceStream(resourceName);
if (stream == null)
throw new InvalidOperationException($"Embedded resource '{resourceName}' not found in {Consts.GeneratorsAssemblyName} assembly");
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

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

Throwing InvalidOperationException when the embedded resource is missing will crash the analyzer/generator (often surfacing as a hard failure in the IDE/build). Prefer returning a structured error (consistent with the other Execute* methods) and/or reporting a diagnostic so the generator fails gracefully instead of throwing.

Suggested change
throw new InvalidOperationException($"Embedded resource '{resourceName}' not found in {Consts.GeneratorsAssemblyName} assembly");
{
// Fail gracefully when the embedded resource cannot be found instead of crashing the generator.
return string.Empty;
}

Copilot uses AI. Check for mistakes.
@codecov-commenter
Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...pers.Generators/GeneratesMethodExecutionRuntime.cs 80.00% 2 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Move MethodBuilders and RecordingGeneratorsFactorys to Generators

4 participants