diff --git a/Directory.Build.props b/Directory.Build.props index 7d24fb2..e787834 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -1,7 +1,7 @@ - 2.37.2 + 2.37.3 13 1570;1571;1572;1573;1574;1587;1591;1701;1702;1711;1735;0618 Jeremy D. Miller;Jaedyn Tonee diff --git a/src/JasperFx.Events.SourceGenerator.Tests/AggregateEvolverGeneratorTests.cs b/src/JasperFx.Events.SourceGenerator.Tests/AggregateEvolverGeneratorTests.cs index ba75e82..1c7ece7 100644 --- a/src/JasperFx.Events.SourceGenerator.Tests/AggregateEvolverGeneratorTests.cs +++ b/src/JasperFx.Events.SourceGenerator.Tests/AggregateEvolverGeneratorTests.cs @@ -1411,4 +1411,113 @@ public partial class CartProjection : SingleStreamProjection allGenerated.ShouldContain("return s.Apply(data);"); allGenerated.ShouldNotContain("GetUninitializedObject(typeof(global::Test.Cart)).Apply("); } + + // Published-type discovery from an explicit ApplyAsync override (marten#4166). The document type + // is read off the bound method symbol, so the explicit and inferred spellings of the same call + // must produce the same registration -- see the remarks on + // AggregateAnalyzer.DiscoverDocumentTypesFromMethodBodies. + private const string EventProjectionPreamble = @" +using System; +using System.Threading; +using System.Threading.Tasks; +using JasperFx.Events; +using JasperFx.Events.Projections; + +namespace Test; + +public interface ITestQuerySession { } + +public interface ITestOperations : ITestQuerySession, IStorageOperations +{ + void Store(T entity) where T : notnull; +} + +public abstract class TestEventProjection : JasperFxEventProjectionBase +{ + protected override void storeEntity(ITestOperations ops, T entity) => ops.Store(entity); +} + +public class AuditRecord { public Guid Id { get; set; } } +public class AuditableEvent { } +"; + + [Fact] + public void registers_published_type_from_an_explicit_store_type_argument() + { + var (_, generatedSources) = RunGenerator(EventProjectionPreamble + @" +public partial class ExplicitStoreProjection : TestEventProjection +{ + public override ValueTask ApplyAsync(ITestOperations operations, IEvent e, CancellationToken cancellation) + { + operations.Store(new AuditRecord()); + return default; + } +} +"); + + string.Join("\n", generatedSources) + .ShouldContain("RegisterPublishedType(typeof(global::Test.AuditRecord));"); + } + + [Fact] + public void registers_published_type_when_the_store_type_argument_is_inferred() + { + // The trap this closes: discovery used to be syntactic, so this spelling compiled and + // registered nothing at all, while the explicit one above worked. + var (_, generatedSources) = RunGenerator(EventProjectionPreamble + @" +public partial class InferredStoreProjection : TestEventProjection +{ + public override ValueTask ApplyAsync(ITestOperations operations, IEvent e, CancellationToken cancellation) + { + operations.Store(new AuditRecord()); + return default; + } +} +"); + + string.Join("\n", generatedSources) + .ShouldContain("RegisterPublishedType(typeof(global::Test.AuditRecord));"); + } + + [Fact] + public void reports_a_diagnostic_when_the_document_type_cannot_be_registered() + { + var (diagnostics, generatedSources) = RunGenerator(EventProjectionPreamble + @" +public partial class ObjectStoreProjection : TestEventProjection +{ + public override ValueTask ApplyAsync(ITestOperations operations, IEvent e, CancellationToken cancellation) + { + operations.Store(new AuditRecord()); + return default; + } +} +"); + + diagnostics.ShouldContain(d => d.Id == "JFXEVT005"); + string.Join("\n", generatedSources).ShouldNotContain("RegisterPublishedType"); + } + + [Fact] + public void ignores_a_store_call_that_is_not_on_the_projection_session() + { + // A same-named method on an unrelated object is not a document operation, and must produce + // neither a registration nor a diagnostic. + var (diagnostics, generatedSources) = RunGenerator(EventProjectionPreamble + @" +public class SomethingElse { public void Store(T thing) { } } + +public partial class UnrelatedStoreProjection : TestEventProjection +{ + private readonly SomethingElse _other = new(); + + public override ValueTask ApplyAsync(ITestOperations operations, IEvent e, CancellationToken cancellation) + { + _other.Store(new AuditRecord()); + return default; + } +} +"); + + diagnostics.ShouldNotContain(d => d.Id == "JFXEVT005"); + string.Join("\n", generatedSources).ShouldNotContain("RegisterPublishedType"); + } } diff --git a/src/JasperFx.Events.SourceGenerator/AggregateAnalyzer.cs b/src/JasperFx.Events.SourceGenerator/AggregateAnalyzer.cs index b4a7039..e60189d 100644 --- a/src/JasperFx.Events.SourceGenerator/AggregateAnalyzer.cs +++ b/src/JasperFx.Events.SourceGenerator/AggregateAnalyzer.cs @@ -120,12 +120,37 @@ internal sealed class CandidateInfo /// See https://github.com/JasperFx/marten/issues/4166 /// public List DiscoveredPublishedTypes { get; set; } = new(); + + /// + /// Document operations that bound to the projection's own session but whose document type is + /// not registrable (object, dynamic, an open type parameter). Reported as JFXEVT005 rather than + /// silently skipped, because nothing at the call site tells the user it was skipped. + /// + public List UnresolvedDocumentOperations { get; set; } = new(); // SelfAggregatingEvolve-specific public EvolveMethodInfo? EvolveMethod { get; set; } // Natural key discovery public bool HasNaturalKey { get; set; } } +/// +/// A document operation that bound to an EventProjection's own session but whose document type +/// could not be turned into a published-type registration. +/// +internal sealed class UnresolvedDocumentOperation +{ + public UnresolvedDocumentOperation(string methodName, string documentTypeDisplay, Location location) + { + MethodName = methodName; + DocumentTypeDisplay = documentTypeDisplay; + Location = location; + } + + public string MethodName { get; } + public string DocumentTypeDisplay { get; } + public Location Location { get; } +} + internal static class AggregateAnalyzer { private const string AggregationProjectionBaseFullName = @@ -161,7 +186,8 @@ internal static class AggregateAnalyzer var eventProjBaseInfo = FindEventProjectionBase(classSymbol); if (eventProjBaseInfo != null) { - return AnalyzeEventProjectionSubclass(classDecl, classSymbol, eventProjBaseInfo.Value, ct); + return AnalyzeEventProjectionSubclass(classDecl, classSymbol, eventProjBaseInfo.Value, + context.SemanticModel, ct); } // Not a projection subclass - check if self-aggregating @@ -895,6 +921,7 @@ public static (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionTyp TypeDeclarationSyntax classDecl, INamedTypeSymbol classSymbol, (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionType) baseInfo, + SemanticModel? semanticModel, CancellationToken ct) { var isPartial = classDecl.Modifiers.Any(SyntaxKind.PartialKeyword); @@ -907,8 +934,12 @@ public static (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionTyp // See https://github.com/JasperFx/marten/issues/4166 if (!isPartial) return null; - var discoveredTypes = DiscoverDocumentTypesFromMethodBodies(classDecl, classSymbol); - if (discoveredTypes.Count == 0) return null; + var unresolved = new List(); + var discoveredTypes = DiscoverDocumentTypesFromMethodBodies(classDecl, classSymbol, + baseInfo.operationsType, semanticModel, unresolved); + + // A class with nothing registrable AND nothing to warn about is not a candidate at all. + if (discoveredTypes.Count == 0 && unresolved.Count == 0) return null; return new CandidateInfo { @@ -919,6 +950,7 @@ public static (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionTyp OperationsType = baseInfo.operationsType, QuerySessionType = baseInfo.querySessionType, DiscoveredPublishedTypes = discoveredTypes, + UnresolvedDocumentOperations = unresolved, HasExistingParameterlessConstructor = HasExplicitParameterlessConstructor(classSymbol) }; } @@ -944,6 +976,13 @@ public static (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionTyp }; } + /// + /// Operation methods on a document session that carry the document type as their single + /// generic argument, whether it was written explicitly or inferred from the argument. + /// + private static readonly HashSet DocumentOperationMethodNames = new() + { "Store", "Insert", "Delete", "Update", "HardDelete" }; + /// /// Scans method bodies in an EventProjection class for calls to Store<T>, Insert<T>, /// Delete<T>, Update<T> on document operations to discover published document types. @@ -951,53 +990,75 @@ public static (INamedTypeSymbol operationsType, INamedTypeSymbol querySessionTyp /// methods with document types that are not otherwise registered. /// See https://github.com/JasperFx/marten/issues/4166 /// + /// + /// Resolution is SEMANTIC, deliberately. This used to match only GenericNameSyntax, which + /// meant ops.Store<Doc>(x) registered the published type and the equally valid + /// ops.Store(x) compiled and registered nothing at all — a silent trap, since the store + /// then provisions that document's storage on demand and only the schema-ahead-of-time and + /// known-document-type surfaces come up short. Binding the invocation gives the same answer for + /// both spellings, because the type argument is on the symbol whether or not it was written down. + /// The syntactic path stays as a fallback for code that does not bind (mid-edit, broken trees). + /// private static List DiscoverDocumentTypesFromMethodBodies( TypeDeclarationSyntax classDecl, - INamedTypeSymbol classSymbol) + INamedTypeSymbol classSymbol, + INamedTypeSymbol operationsType, + SemanticModel? semanticModel, + List? unresolved = null) { var documentTypes = new List(); var typeNames = new HashSet(); + var seen = new HashSet(); - // Operation methods that accept a document type as a generic argument - var operationMethodNames = new HashSet - { "Store", "Insert", "Delete", "Update", "HardDelete" }; - - // Scan all method bodies in the class for generic invocations foreach (var node in classDecl.DescendantNodes()) { - if (node is InvocationExpressionSyntax invocation) - { - // Look for patterns like: operations.Store(...) or ops.Insert(...) - var methodName = invocation.Expression switch - { - MemberAccessExpressionSyntax memberAccess => memberAccess.Name, - _ => null - }; + if (node is not InvocationExpressionSyntax invocation) continue; + if (invocation.Expression is not MemberAccessExpressionSyntax memberAccess) continue; + + var simpleName = memberAccess.Name; + if (!DocumentOperationMethodNames.Contains(simpleName.Identifier.ValueText)) continue; - if (methodName is GenericNameSyntax genericName && - operationMethodNames.Contains(genericName.Identifier.ValueText) && - genericName.TypeArgumentList.Arguments.Count == 1) + if (semanticModel != null && + TryResolveDocumentOperation(semanticModel, invocation, memberAccess, operationsType, + out var resolvedType, out var isOnOperations)) + { + if (IsRegistrableDocumentType(resolvedType)) { - var typeArg = genericName.TypeArgumentList.Arguments[0]; - var typeName = ExtractTypeNameFromTypeSyntax(typeArg); - if (typeName != null) + if (seen.Add(resolvedType!.ToDisplayString())) { - typeNames.Add(typeName); + documentTypes.Add(resolvedType); } } + else if (isOnOperations && unresolved != null) + { + // Bound to the operations session but the document type is not something we can + // register -- object, dynamic, an open type parameter. Nothing to emit, and the + // user cannot tell from the call site, so it is worth a diagnostic. + unresolved.Add(new UnresolvedDocumentOperation( + simpleName.Identifier.ValueText, + resolvedType?.ToDisplayString() ?? "?", + simpleName.GetLocation())); + } + + continue; } - // Also detect: new SomeType(...) passed to operations.Insert(new SomeType(...)) - // This is handled by looking for non-generic overloads like Insert(entity) where - // the entity is a new expression. But this is harder to detect without semantic analysis. - // For now, focus on the generic overload pattern which is the most common. + // Fallback: unbindable code. Only the explicit spelling is recoverable from syntax alone. + if (simpleName is GenericNameSyntax genericName && + genericName.TypeArgumentList.Arguments.Count == 1) + { + var typeName = ExtractTypeNameFromTypeSyntax(genericName.TypeArgumentList.Arguments[0]); + if (typeName != null) + { + typeNames.Add(typeName); + } + } } - // Resolve type names to symbols foreach (var tn in typeNames) { var resolved = FindTypeByName(tn, classSymbol); - if (resolved != null && !IsFrameworkType(resolved)) + if (resolved != null && !IsFrameworkType(resolved) && seen.Add(resolved.ToDisplayString())) { documentTypes.Add(resolved); } @@ -1006,6 +1067,88 @@ private static List DiscoverDocumentTypesFromMethodBodies( return documentTypes; } + /// + /// Bind a candidate document operation call and pull the document type off the method symbol. + /// + /// + /// True when the receiver really is the projection's operations session, which is what separates + /// "we could not register this document type" from "this was somebody else's Store method". + /// + private static bool TryResolveDocumentOperation( + SemanticModel semanticModel, + InvocationExpressionSyntax invocation, + MemberAccessExpressionSyntax memberAccess, + INamedTypeSymbol operationsType, + out ITypeSymbol? documentType, + out bool isOnOperations) + { + documentType = null; + isOnOperations = false; + + var symbol = semanticModel.GetSymbolInfo(invocation).Symbol + ?? semanticModel.GetSymbolInfo(invocation).CandidateSymbols.FirstOrDefault(); + + if (symbol is not IMethodSymbol method) return false; + + var receiverType = semanticModel.GetTypeInfo(memberAccess.Expression).Type; + isOnOperations = receiverType != null && IsOrImplements(receiverType, operationsType); + + // Extension methods hang the session off the first parameter rather than the receiver. + if (!isOnOperations && method.IsExtensionMethod && method.ReducedFrom == null && + method.Parameters.Length > 0) + { + isOnOperations = IsOrImplements(method.Parameters[0].Type, operationsType); + } + + if (!isOnOperations) return true; + + if (method.TypeArguments.Length == 1) + { + var candidate = method.TypeArguments[0]; + // Kept even when it is not registrable, so the diagnostic can name what was written. + documentType = candidate; + } + + return true; + } + + /// + /// Can this type be named in a RegisterPublishedType(typeof(...)) call and mean something? + /// + /// + /// SpecialType is the check that matters and cannot stand in + /// for it: object and string render through ToDisplayString() as their C# + /// keywords, not as System.Object / System.String, so a name-prefix test lets them + /// through and the projection ends up registering object as a published document type. + /// + private static bool IsRegistrableDocumentType(ITypeSymbol? type) + { + if (type == null) return false; + if (type.TypeKind is TypeKind.TypeParameter or TypeKind.Dynamic or TypeKind.Error) return false; + if (type.SpecialType != SpecialType.None) return false; + + return !IsFrameworkType(type); + } + + private static bool IsOrImplements(ITypeSymbol type, INamedTypeSymbol target) + { + if (SymbolEqualityComparer.Default.Equals(type, target)) return true; + + foreach (var iface in type.AllInterfaces) + { + if (SymbolEqualityComparer.Default.Equals(iface, target)) return true; + } + + var baseType = type.BaseType; + while (baseType != null) + { + if (SymbolEqualityComparer.Default.Equals(baseType, target)) return true; + baseType = baseType.BaseType; + } + + return false; + } + private static List DiscoverEventProjectionMethods( INamedTypeSymbol classSymbol, INamedTypeSymbol operationsType) diff --git a/src/JasperFx.Events.SourceGenerator/AggregateEvolverGenerator.cs b/src/JasperFx.Events.SourceGenerator/AggregateEvolverGenerator.cs index 1a4c043..f0d7d79 100644 --- a/src/JasperFx.Events.SourceGenerator/AggregateEvolverGenerator.cs +++ b/src/JasperFx.Events.SourceGenerator/AggregateEvolverGenerator.cs @@ -482,6 +482,16 @@ private static void EmitEventProjection(SourceProductionContext context, Candida private static void EmitEventProjectionTypeRegistration(SourceProductionContext context, CandidateInfo info) { + foreach (var unresolved in info.UnresolvedDocumentOperations) + { + context.ReportDiagnostic(Diagnostic.Create( + DiagnosticDescriptors.UnregistrableDocumentOperation, + unresolved.Location, + info.ClassSymbol.Name, + unresolved.MethodName, + unresolved.DocumentTypeDisplay)); + } + if (info.DiscoveredPublishedTypes.Count == 0) return; var source = EvolverCodeEmitter.EmitEventProjectionTypeRegistrationPartial(info); diff --git a/src/JasperFx.Events.SourceGenerator/DiagnosticDescriptors.cs b/src/JasperFx.Events.SourceGenerator/DiagnosticDescriptors.cs index 755c11a..8cdcddf 100644 --- a/src/JasperFx.Events.SourceGenerator/DiagnosticDescriptors.cs +++ b/src/JasperFx.Events.SourceGenerator/DiagnosticDescriptors.cs @@ -28,6 +28,21 @@ internal static class DiagnosticDescriptors defaultSeverity: DiagnosticSeverity.Info, isEnabledByDefault: true); + /// + /// An EventProjection's explicit ApplyAsync writes a document whose type the generator cannot + /// name, so no published-type registration is emitted for it. Silent otherwise: the store will + /// still provision that document's storage on demand, and only the ahead-of-time surfaces + /// (schema creation, known document types, rebuild teardown) come up short. + /// + public static readonly DiagnosticDescriptor UnregistrableDocumentOperation = new( + id: "JFXEVT005", + title: "Document type cannot be registered from this operation", + messageFormat: + "'{0}' calls {1} on its session with document type '{2}', which cannot be registered as a published type; call it with a concrete document type so the projection registers it", + category: "JasperFx.Events", + defaultSeverity: DiagnosticSeverity.Info, + isEnabledByDefault: true); + public static readonly DiagnosticDescriptor HasLambdaRegistrations = new( id: "JFXEVT004", title: "Has lambda registrations",