Skip to content

Refactor Generators: extract SourceEmitting namespace, decompose ExecutionRuntime, add 59 tests#90

Merged
dex3r merged 6 commits intofeature/metehod_templatefrom
copilot/refactor-generators-code
Mar 25, 2026
Merged

Refactor Generators: extract SourceEmitting namespace, decompose ExecutionRuntime, add 59 tests#90
dex3r merged 6 commits intofeature/metehod_templatefrom
copilot/refactor-generators-code

Conversation

Copy link
Contributor

Copilot AI commented Mar 25, 2026

The IncrementalGenerators namespace had monolithic classes mixing Roslyn concerns with string emission, significant duplication between execution paths, and no unit tests for source output logic.

New SourceEmitting namespace

Extracts all C# source string-building into small, focused, Roslyn-free classes testable with plain data records:

  • CSharpLiteralFormatter — value/key → C# literal (string"string", booltrue, enums → Type.Value)
  • CSharpAccessibilityKeyword / CSharpTypeKeyword — enum → keyword mappers (ToKeyword(accessibility, defaultToPrivate) single method with parameter instead of separate methods)
  • PartialMethodSourceEmitter — emits complete partial method source files from PartialMethodEmitData
  • DummyImplementationEmitter — emits dummy partial implementations for execution compilations
  • RoslynSymbolDataMapper — thin bridge: INamedTypeSymbol/IMethodSymbol → emit data records
// Before: Roslyn types threaded through string-building methods
AppendNamespaceAndTypeHeader(builder, containingType, partialMethod);

// After: data extraction separated from emission
PartialMethodEmitData emitData = RoslynSymbolDataMapper.ToPartialMethodEmitData(containingType, partialMethod);
return PartialMethodSourceEmitter.Emit(emitData, returnValueLiteral);

Decomposed GeneratesMethodExecutionRuntime

The 466-line class with duplicated compile→load→execute logic is split into:

  • GeneratorAssemblyExecutor — shared compilation, AssemblyLoadContext setup, type/method resolution
  • LoadedAssemblyContext (IDisposable) — owns the isolated ALC lifecycle (own file)
  • BodyGenerationDataExtractor — reflection-based FluentBodyResult extraction
  • AbstractionsAssemblyResolver — resolves the abstractions assembly from compilation references (file-based and in-memory)
  • DataGeneratorsFactorySetup — wires up the DataGeneratorsFactory to Generate.CurrentGenerator
  • DiagnosticMessageHelper — testable helper for joining error diagnostics into strings
  • GeneratesMethodExecutionRuntime — now a thin orchestrator (~170 lines) delegating to the above

Tests & docs

  • 59 new unit tests across 6 test files covering all SourceEmitting classes and DiagnosticMessageHelper
  • /// <summary> on all internal/public methods
  • All 218 test executions (net8.0 + net10.0) pass
Original prompt

This section details on the original issue you should resolve

<issue_title>Refactor Generators code</issue_title>
<issue_description>Code under namespace EasySourceGenerators.Generators.IncrementalGenerators is currently a mess. Let's refactor it to be readable, maintanable and testable.

Besides general good coding rules, focus on:

  • Don't store global state, pass necessary data as parameters
  • Create small classes with small methods, that only do one thing
  • make sure the classes are unit-testable
  • if needed, create abstraction layer for Roslyn Generators and other parts. minimize using Roslyn Generators code to a small subset of classes that need them, making most of the code logic unit-testable without running any actuall generators
  • extract all parts of code that emit code as strings (like "{" or "private") into separate namespace with separate classes. create unit tests for them.
  • create documentation ///<summary> comments for internal/public methods when name is not enough</issue_description>

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


📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI and others added 2 commits March 25, 2026 12:55
…or ExecutionRuntime

- Create CSharpLiteralFormatter for value/key literal formatting
- Create CSharpAccessibilityKeyword for accessibility mapping
- Create CSharpTypeKeyword for type kind mapping
- Create PartialMethodEmitData record and PartialMethodSourceEmitter
- Create DummyImplementationEmitter with data records
- Create RoslynSymbolDataMapper to bridge Roslyn types to emitter records
- Create GeneratorAssemblyExecutor for shared compile/load logic
- Create BodyGenerationDataExtractor for reflection-based data extraction
- Refactor GeneratesMethodExecutionRuntime to use new classes
- Refactor GeneratesMethodPatternSourceBuilder to delegate to SourceEmitting
- Add documentation comments to all internal/public methods

Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/cb205769-20ee-4e1e-9b44-3ef890163968
- CSharpLiteralFormatterTests: 16 tests for value/key literal formatting
- CSharpAccessibilityKeywordTests: 10 tests for From and FromOrEmpty
- CSharpTypeKeywordTests: 5 tests for type kind mapping
- PartialMethodSourceEmitterTests: 12 tests for partial method source emission
- DummyImplementationEmitterTests: 11 tests for dummy implementation generation

Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/cb205769-20ee-4e1e-9b44-3ef890163968
…n DummyMethodData

Use consistent MethodModifiers pattern ("partial" / "static partial") in
DummyMethodData, matching PartialMethodEmitData approach and eliminating
the fragile trailing-space convention.

Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/cb205769-20ee-4e1e-9b44-3ef890163968
Copilot AI changed the title [WIP] Refactor Generators code for readability and maintainability Refactor Generators: extract SourceEmitting namespace, decompose ExecutionRuntime, add 54 tests Mar 25, 2026
Copilot AI requested a review from dex3r March 25, 2026 13:03
/// Handles the compilation, assembly loading, and method invocation steps
/// required to execute generator methods at compile time in an isolated context.
/// </summary>
internal static class GeneratorAssemblyExecutor
Copy link
Owner

Choose a reason for hiding this comment

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

Move into separate file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved LoadedAssemblyContext to its own file LoadedAssemblyContext.cs in 174a3e2.

Comment on lines +73 to +75
string errors = string.Join("; ", emitResult.Diagnostics
.Where(diagnostic => diagnostic.Severity == DiagnosticSeverity.Error)
.Select(diagnostic => diagnostic.GetMessage()));
Copy link
Owner

Choose a reason for hiding this comment

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

Error joining to string should be a separate, testable helper method (in helpers class).

Also, it is too big, so it probably does too many things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted DiagnosticMessageHelper.JoinErrorDiagnostics() as a testable helper with 5 unit tests. Also extracted AbstractionsAssemblyResolver and DataGeneratorsFactorySetup from the class to reduce its scope. Done in 174a3e2.

/// for <see cref="Accessibility.Private"/> and unrecognized values.
/// Used in contexts where the <c>private</c> keyword is implicit (e.g., dummy implementations).
/// </summary>
internal static string FromOrEmpty(Accessibility accessibility)
Copy link
Owner

Choose a reason for hiding this comment

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

This method should be a switch in From method, not an entire new method.

Also, the name "From" sucks, this doesnt tell me anything:
CSharpAccessibilityKeyword.From.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merged into a single method CSharpAccessibilityKeyword.ToKeyword(accessibility, defaultToPrivate: true). The defaultToPrivate parameter controls whether unrecognized/private values return "private" (default) or "". Done in 174a3e2.


return null;
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

This class is still too big, meaning it probably does too many things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extracted AbstractionsAssemblyResolver.Resolve(), DataGeneratorsFactorySetup.Setup(), and InvokeStaticMethod() private helper. GeneratesMethodExecutionRuntime is now a thin orchestrator — ~170 lines down from ~276. Done in 174a3e2.

Copilot AI and others added 2 commits March 25, 2026 13:15
…ityKeyword

- Move LoadedAssemblyContext into its own file
- Extract DiagnosticMessageHelper for testable error-joining
- Extract AbstractionsAssemblyResolver from GeneratesMethodExecutionRuntime
- Extract DataGeneratorsFactorySetup from GeneratesMethodExecutionRuntime
- Merge FromOrEmpty into ToKeyword with defaultToPrivate parameter
- Add DiagnosticMessageHelperTests (5 tests)
- Update CSharpAccessibilityKeywordTests for renamed API (12 tests)

Co-authored-by: dex3r <3155725+dex3r@users.noreply.github.com>
Agent-Logs-Url: https://github.com/dex3r/EasySourceGenerators/sessions/7c930db8-e3f7-42d9-98cd-41cf86cdc442
Copilot AI changed the title Refactor Generators: extract SourceEmitting namespace, decompose ExecutionRuntime, add 54 tests Refactor Generators: extract SourceEmitting namespace, decompose ExecutionRuntime, add 59 tests Mar 25, 2026
Copilot AI requested a review from dex3r March 25, 2026 13:19
@dex3r dex3r marked this pull request as ready for review March 25, 2026 13:37
Copilot AI review requested due to automatic review settings March 25, 2026 13:37
@dex3r dex3r merged commit fa858eb into feature/metehod_template Mar 25, 2026
2 checks passed
@dex3r dex3r deleted the copilot/refactor-generators-code branch March 25, 2026 13:37
Copy link
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 refactors generator implementation code by separating Roslyn symbol handling from C# source string emission, decomposing the compile→load→execute runtime into smaller components, and adding unit tests for the new emitters/helpers.

Changes:

  • Introduce SourceEmitting namespace with Roslyn-free emitters/formatters plus a Roslyn→data mapper.
  • Split the previous monolithic execution runtime into focused helpers (assembly compile/load, abstractions resolution, factory wiring, reflection extraction, diagnostic formatting).
  • Add new unit tests covering source emission utilities and diagnostic formatting.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
EasySourceGenerators.Generators/SourceEmitting/RoslynSymbolDataMapper.cs Maps Roslyn symbols to plain emit-data records (bridge layer).
EasySourceGenerators.Generators/SourceEmitting/PartialMethodSourceEmitter.cs Emits partial method implementation source from PartialMethodEmitData.
EasySourceGenerators.Generators/SourceEmitting/PartialMethodEmitData.cs Data record for partial method emission.
EasySourceGenerators.Generators/SourceEmitting/DummyImplementationEmitter.cs Emits dummy partial implementations for execution compilations.
EasySourceGenerators.Generators/SourceEmitting/CSharpTypeKeyword.cs Maps TypeKind to declaration keyword.
EasySourceGenerators.Generators/SourceEmitting/CSharpLiteralFormatter.cs Formats values/keys as C# literals for emission.
EasySourceGenerators.Generators/SourceEmitting/CSharpAccessibilityKeyword.cs Maps Accessibility to C# keywords.
EasySourceGenerators.Generators/IncrementalGenerators/LoadedAssemblyContext.cs Encapsulates collectible ALC + loaded assembly lifecycle.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratorAssemblyExecutor.cs Builds execution compilation, emits/loads assembly in isolated ALC, provides reflection helpers.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodPatternSourceBuilder.cs Refactors simple pattern source generation to delegate to SourceEmitting.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodGenerator.cs Adds docs; generator entry remains the same.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodGenerationTargetCollector.cs Adds docs; target collection/validation remains the same.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodGenerationPipeline.cs Adds docs; pipeline now calls refactored runtime/emitter components.
EasySourceGenerators.Generators/IncrementalGenerators/GeneratesMethodExecutionRuntime.cs Becomes a thin orchestrator delegating to new helper classes.
EasySourceGenerators.Generators/IncrementalGenerators/DiagnosticMessageHelper.cs Extracted helper for joining error diagnostics.
EasySourceGenerators.Generators/IncrementalGenerators/DataGeneratorsFactorySetup.cs Wires DataGeneratorsFactory into Generate.CurrentGenerator in execution context.
EasySourceGenerators.Generators/IncrementalGenerators/BodyGenerationDataExtractor.cs Extracts fluent body return data via reflection.
EasySourceGenerators.Generators/IncrementalGenerators/AbstractionsAssemblyResolver.cs Resolves abstractions assembly from compilation references (file/in-memory).
EasySourceGenerators.GeneratorTests/PartialMethodSourceEmitterTests.cs Tests for PartialMethodSourceEmitter.
EasySourceGenerators.GeneratorTests/DummyImplementationEmitterTests.cs Tests for DummyImplementationEmitter.
EasySourceGenerators.GeneratorTests/DiagnosticMessageHelperTests.cs Tests for DiagnosticMessageHelper.
EasySourceGenerators.GeneratorTests/CSharpTypeKeywordTests.cs Tests for CSharpTypeKeyword.
EasySourceGenerators.GeneratorTests/CSharpLiteralFormatterTests.cs Tests for CSharpLiteralFormatter.
EasySourceGenerators.GeneratorTests/CSharpAccessibilityKeywordTests.cs Tests for CSharpAccessibilityKeyword.

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


if (assemblyName.Name != null && compilationReferenceBytes.TryGetValue(assemblyName.Name, out byte[]? bytes))
{
Assembly loaded = context.LoadFromStream(new MemoryStream(bytes));
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

In the AssemblyLoadContext.Resolving handler, assemblies are loaded from new MemoryStream(bytes) without disposing the stream. Per AssemblyLoadContext.LoadFromStream guidance, the caller should own/dispose the stream; leaking streams here can add avoidable memory pressure when many resolutions occur. Consider wrapping the MemoryStream in a using (or reusing a pooled stream) before calling LoadFromStream.

Suggested change
Assembly loaded = context.LoadFromStream(new MemoryStream(bytes));
using MemoryStream assemblyStream = new(bytes);
Assembly loaded = context.LoadFromStream(assemblyStream);

Copilot uses AI. Check for mistakes.
if (context.CompilationReferenceBytes.TryGetValue(Consts.AbstractionsAssemblyName,
out byte[]? abstractionBytes))
{
Assembly assembly = context.LoadContext.LoadFromStream(new MemoryStream(abstractionBytes));
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

LoadFromCompilationReference loads the abstractions assembly via new MemoryStream(abstractionBytes) but never disposes the stream. Disposing the stream after LoadFromStream avoids unnecessary allocations/retained buffers during generator execution. Consider using a using around the MemoryStream.

Suggested change
Assembly assembly = context.LoadContext.LoadFromStream(new MemoryStream(abstractionBytes));
using var stream = new MemoryStream(abstractionBytes);
Assembly assembly = context.LoadContext.LoadFromStream(stream);

Copilot uses AI. Check for mistakes.
Comment on lines +39 to +52
internal static string FormatKeyAsLiteral(object key, TypeKind? typeKind, string? typeDisplayString)
{
if (typeKind == TypeKind.Enum)
{
return $"{typeDisplayString}.{key}";
}

return key switch
{
bool b => b ? "true" : "false",
// SyntaxFactory.Literal handles escaping and quoting (e.g. "hello" → "\"hello\"")
string s => SyntaxFactory.Literal(s).Text,
_ => key.ToString()!
};
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

FormatKeyAsLiteral falls back to key.ToString() for non-bool/non-string cases. This can emit invalid C# for common key types (e.g., char becomes A instead of 'A'), and floating-point/decimal formatting can become culture-dependent (comma vs dot) leading to non-compilable generated code on some locales. Consider adding explicit handling for char (and other primitives) via SyntaxFactory.Literal(...) / invariant formatting, and guard against typeDisplayString being null in the enum branch.

Copilot uses AI. Check for mistakes.
Comment on lines +31 to +34
PropertyInfo? currentGeneratorProperty = generatorStaticType.GetProperty(
Consts.CurrentGeneratorPropertyName, BindingFlags.NonPublic | BindingFlags.Static);
currentGeneratorProperty?.SetValue(null, dataGeneratorsFactory);

Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

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

Setup claims to return an error when required properties can't be found, but it currently silently succeeds if Generate.CurrentGenerator isn't found (currentGeneratorProperty is null) or if Activator.CreateInstance returns null. This can lead to confusing downstream failures when executing fluent generators. Consider validating that the property exists and is settable, and return a descriptive error if wiring fails.

Suggested change
PropertyInfo? currentGeneratorProperty = generatorStaticType.GetProperty(
Consts.CurrentGeneratorPropertyName, BindingFlags.NonPublic | BindingFlags.Static);
currentGeneratorProperty?.SetValue(null, dataGeneratorsFactory);
if (dataGeneratorsFactory is null)
{
return
$"Could not create an instance of {Consts.DataGeneratorsFactoryTypeFullName}.";
}
PropertyInfo? currentGeneratorProperty = generatorStaticType.GetProperty(
Consts.CurrentGeneratorPropertyName, BindingFlags.NonPublic | BindingFlags.Static);
if (currentGeneratorProperty is null)
{
return
$"Could not find static property {Consts.CurrentGeneratorPropertyName} on {Consts.GenerateTypeFullName}.";
}
if (!currentGeneratorProperty.CanWrite)
{
return
$"Property {Consts.GenerateTypeFullName}.{Consts.CurrentGeneratorPropertyName} is not writable.";
}
if (!currentGeneratorProperty.PropertyType.IsAssignableFrom(dataGeneratorsFactoryType))
{
return
$"Property {Consts.GenerateTypeFullName}.{Consts.CurrentGeneratorPropertyName} is of type {currentGeneratorProperty.PropertyType.FullName}, which is not assignable from {Consts.DataGeneratorsFactoryTypeFullName}.";
}
currentGeneratorProperty.SetValue(null, dataGeneratorsFactory);

Copilot uses AI. Check for mistakes.
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.

3 participants