Skip to content

Fix SG evolver construction for required-member aggregates (marten#4542) #362

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jeremydmiller wants to merge 1 commit into from
Closed

Fix SG evolver construction for required-member aggregates (marten#4542) #362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
158 changes: 158 additions & 0 deletions src/JasperFx.Events.SourceGenerator.Tests/AggregateEvolverGeneratorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -733,4 +733,162 @@ public class UpdatedEvent { }
evolver.ShouldContain("case global::Test.CreatedEvent");
evolver.ShouldContain("case global::Test.UpdatedEvent");
}

[Fact]
public void required_members_with_primary_ctor_and_conventional_projection_compiles()
{
// marten#4542: a record aggregate with a primary constructor for the id
// PLUS required members, projected by a partial SingleStreamProjection
// with conventional Create/Apply. The projection's Create fully
// initialises the aggregate, so the generator must NOT emit a standalone
// fallback evolver whose `new T(data)` ignores the required members
// (CS9035). The record's primary ctor (string Id) is the id-construction
// parameter, not an event-shaped implicit Create handler.
var source = @"
using System;
using JasperFx.Events;
using JasperFx.Events.Aggregation;
using JasperFx.Events.Projections;

namespace Test;

public record DiagnostiekActiviteit(string Id)
{
public required Guid? SubContractorId { get; set; }
public required string Aanlevercode { get; set; }
public required int Prestatiecodelijst { get; set; }
}

public class CareReceived { public string ProvidedCareId { get; set; } public Guid? SubContractorId { get; set; } public string Prestatiecode { get; set; } public int Prestatiecodelijst { get; set; } }
public class CareImported { }

public abstract class SingleStreamProjection<TDoc, TId> : JasperFxSingleStreamProjectionBase<TDoc, TId, object, object>
where TDoc : notnull where TId : notnull
{
protected SingleStreamProjection() : base(AggregationScope.SingleStream) { }
}

public partial class DiagnostiekActiviteitProjection : SingleStreamProjection<DiagnostiekActiviteit, string>
{
public static DiagnostiekActiviteit Create(CareReceived e) => new(e.ProvidedCareId)
{
SubContractorId = e.SubContractorId,
Aanlevercode = e.Prestatiecode,
Prestatiecodelijst = e.Prestatiecodelijst,
};

public void Apply(CareImported e, DiagnostiekActiviteit a) { }
}
";
var diagnostics = CompileWithGenerator(source);

// CS9035 = required member not set in initializer; CS7036 = required
// constructor argument (the record's primary-ctor id) not supplied.
// The generator hits one or the other depending on which construction
// path it picks; both are the same root gap (primary ctor + required
// members). Filter to just these so the stub projection base's
// unrelated constraint errors don't mask the assertion.
var ctorErrors = diagnostics
.Where(d => d.Severity == DiagnosticSeverity.Error && d.Id is "CS9035" or "CS7036")
.Select(d => $"{d.Id}: {d.GetMessage()}")
.ToArray();

ctorErrors.ShouldBeEmpty();
}

[Fact]
public void required_members_with_primary_ctor_apply_only_synthesizes_via_uninitialized_object_not_default()
{
// marten#4542: a primary-constructor record (no accessible parameterless
// ctor) with required members, built purely via Apply (no Create). There
// is no legal `new T { Required = default! }` here — the object
// initializer needs a parameterless ctor the record doesn't have. The
// emitter must instead allocate via RuntimeHelpers.GetUninitializedObject
// and let Apply populate the members — never emit a `default!`
// initializer that wouldn't compile.
var source = @"
using System;
using JasperFx.Events;
using JasperFx.Events.Aggregation;
using JasperFx.Events.Projections;

namespace Test;

public record DiagnostiekActiviteit(string Id)
{
public required Guid? SubContractorId { get; set; }
public required string Aanlevercode { get; set; }
}

public class CareImported { public Guid? SubContractorId { get; set; } public string Aanlevercode { get; set; } }

public abstract class SingleStreamProjection<TDoc, TId> : JasperFxSingleStreamProjectionBase<TDoc, TId, object, object>
where TDoc : notnull where TId : notnull
{
protected SingleStreamProjection() : base(AggregationScope.SingleStream) { }
}

public partial class DiagnostiekActiviteitProjection : SingleStreamProjection<DiagnostiekActiviteit, string>
{
public void Apply(CareImported e, DiagnostiekActiviteit a) { }
}
";
var diagnostics = CompileWithGenerator(source);

diagnostics
.Where(d => d.Severity == DiagnosticSeverity.Error && d.Id is "CS9035" or "CS7036")
.Select(d => $"{d.Id}: {d.GetMessage()}")
.ShouldBeEmpty();

var (_, generatedSources) = RunGenerator(source);
var evolver = generatedSources.Single(s => s.Contains("DiagnostiekActiviteitProjection"));

// No accessible parameterless ctor → reflective allocation, never a
// `default!` object initializer.
evolver.ShouldContain("GetUninitializedObject(typeof(global::Test.DiagnostiekActiviteit))");
evolver.ShouldNotContain("default!");
}

[Fact]
public void required_members_on_plain_class_apply_only_synthesizes_via_default_initializers()
{
// marten#4542: a required-member PLAIN CLASS with an implicit public
// parameterless ctor — built purely via Apply, no Create (Marten's
// sample_external-account-link pattern). Unlike a primary-ctor record,
// `new T { Required = default! }` compiles here (an accessible
// parameterless ctor exists) and Apply overwrites the members, so the
// emitter keeps using the object initializer.
var source = @"
using System;
using JasperFx.Events;
using JasperFx.Events.Aggregation;
using JasperFx.Events.Projections;

namespace Test;

public class ExternalAccountLink
{
public required string Id { get; set; }
public required Guid CustomerId { get; set; }
}

public class CustomerLinked { public string ExternalAccountId { get; set; } public Guid CustomerId { get; set; } }

public abstract class SingleStreamProjection<TDoc, TId> : JasperFxSingleStreamProjectionBase<TDoc, TId, object, object>
where TDoc : notnull where TId : notnull
{
protected SingleStreamProjection() : base(AggregationScope.SingleStream) { }
}

public partial class ExternalAccountLinkProjection : SingleStreamProjection<ExternalAccountLink, string>
{
public void Apply(CustomerLinked e, ExternalAccountLink link) { link.Id = e.ExternalAccountId; link.CustomerId = e.CustomerId; }
}
";
var (_, generatedSources) = RunGenerator(source);

// The emitter synthesizes the plain class via the object initializer.
var evolver = generatedSources.Single(s => s.Contains("ExternalAccountLinkProjection"));
evolver.ShouldContain("new global::Test.ExternalAccountLink { Id = default!, CustomerId = default! }");
}
}
58 changes: 39 additions & 19 deletions src/JasperFx.Events.SourceGenerator/EvolverCodeEmitter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -713,24 +713,36 @@ private static bool IsQuerySession(ITypeSymbol type)
/// Constructor expression for the aggregate when the SG needs to build a
/// fresh instance in the Apply-only / null-snapshot branch. Three cases:
///
/// 1. Public parameterless ctor, no required members → `new T()`.
/// 2. Required members → `new T { Required = default!, ... }`. The C#
/// compiler accepts the construction even though required members are
/// about to be overwritten by Apply.
/// 3. No public parameterless ctor (e.g. `private T()` signalling
/// "construct me reflectively" or no parameterless ctor at all) →
/// `(T)RuntimeHelpers.GetUninitializedObject(typeof(T))`. Allocates
/// the instance without invoking any ctor or field initializers. Pre-#276
/// Marten used `Activator.CreateInstance(nonPublic: true)` for this
/// pattern; GetUninitializedObject is the AOT-clean equivalent.
/// Caveat: field initializers don't run — aggregates that rely on
/// them must move initialization into Apply / Create.
/// 1. Public parameterless ctor (or a struct) WITH required members →
/// `new T { R = default!, … }`. The C# compiler accepts the object
/// initializer (the required members are nominally satisfied) and Apply
/// immediately overwrites them. Only legal because there's an accessible
/// parameterless ctor for the initializer to call — `default!` is never
/// emitted without one (marten#4542).
/// 2. Public parameterless ctor (or a struct) WITHOUT required members →
/// `new T()`.
/// 3. No accessible parameterless ctor (e.g. `private T()` signalling
/// "construct me reflectively", or a primary-constructor record) →
/// `(T)RuntimeHelpers.GetUninitializedObject(typeof(T))`. Allocates the
/// instance without invoking any ctor, field initializer, or required-
/// member enforcement; Apply then populates it. Pre-#276 Marten used
/// `Activator.CreateInstance(nonPublic: true)` for this;
/// GetUninitializedObject is the AOT-clean equivalent. Caveat: field
/// initializers don't run — aggregates that rely on them must initialize
/// in Apply / Create.
///
/// See #297 + Marten 9.0 migration guide.
/// The marten#4542 bug was emitting case 1's `new T { R = default! }` for a
/// primary-constructor record (no parameterless ctor): the initializer needs
/// a parameterless ctor the record doesn't have (CS7036) and `new T(id)`
/// leaves the required members unset (CS9035). Guarding `default!` on
/// `hasPublicParameterless` routes those types to case 3 instead.
///
/// See #297, marten#4542 + Marten 9.0 migration guide.
/// </summary>
private static string BuildAggregateConstructorExpression(INamedTypeSymbol type)
{
var fqn = Fqn(type);

var requiredMembers = new List<string>();
for (INamedTypeSymbol? current = type;
current is not null && current.SpecialType != SpecialType.System_Object;
Expand All @@ -745,20 +757,28 @@ private static string BuildAggregateConstructorExpression(INamedTypeSymbol type)
}
}

var ctors = type.InstanceConstructors;
var hasPublicParameterless = ctors.Length == 0
|| ctors.Any(c => c.Parameters.Length == 0 && c.DeclaredAccessibility == Accessibility.Public);
// A struct always has a parameterless ctor; a class has one when it's
// declared public (or implicit, when no instance ctors are declared).
// A positional record (`record T(string Id)`) has only its primary +
// copy ctors, so this is false — that's the marten#4542 case.
var hasPublicParameterless = type.TypeKind == TypeKind.Struct
|| type.InstanceConstructors.Any(c =>
c.Parameters.Length == 0 && c.DeclaredAccessibility == Accessibility.Public);

if (requiredMembers.Count > 0)
// Only synthesize `new T { Required = default! }` when an accessible
// parameterless ctor exists for the object initializer to call. Without
// one (a primary-ctor record), fall through to GetUninitializedObject —
// never emit `default!` initializers that wouldn't compile (marten#4542).
if (hasPublicParameterless && requiredMembers.Count > 0)
{
var inits = string.Join(", ", requiredMembers.Select(n => $"{n} = default!"));
return $"new {fqn} {{ {inits} }}";
}

if (hasPublicParameterless) return $"new {fqn}()";

// Reflective-instantiation fallback for `private T()` / no-parameterless-ctor
// aggregates. Field initializers won't run; document in the migration guide.
// Reflective fallback for `private T()` / primary-ctor-record aggregates.
// Field initializers won't run; documented in the migration guide.
return $"({fqn})global::System.Runtime.CompilerServices.RuntimeHelpers.GetUninitializedObject(typeof({fqn}))";
}

Expand Down
Loading