Add: Support fully-registered pre/post images#9
Conversation
…utes, we now generate a Pre or Post image wrapper with all attributes that have a logical name spacified. Add: Warning to avoid image registrations without attributes
There was a problem hiding this comment.
Pull request overview
Adds support for generating image wrapper classes when WithPreImage() / WithPostImage() are invoked without specifying attributes (full entity images), and introduces a new analyzer warning (XPC3005) to discourage this pattern due to performance overhead.
Changes:
- Update registration parsing to treat parameterless
WithPreImage()/WithPostImage()as “include all mapped entity attributes”. - Add new diagnostic descriptor + documentation for XPC3005 (full entity image registrations).
- Add a new Roslyn analyzer + unit tests covering wrapper generation and diagnostic reporting for parameterless image registrations.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| XrmPluginCore.SourceGenerator/rules/XPC3005.md | Adds rule documentation explaining the performance implications and how to fix/suppress. |
| XrmPluginCore.SourceGenerator/Parsers/RegistrationParser.cs | Populates image attribute metadata from all mapped entity properties when no attributes are specified. |
| XrmPluginCore.SourceGenerator/DiagnosticDescriptors.cs | Introduces XPC3005 diagnostic descriptor and help link. |
| XrmPluginCore.SourceGenerator/Analyzers/FullEntityImageAnalyzer.cs | New analyzer that reports XPC3005 for parameterless WithPreImage() / WithPostImage(). |
| XrmPluginCore.SourceGenerator/AnalyzerReleases.Unshipped.md | Registers XPC3005 in analyzer release notes. |
| XrmPluginCore.SourceGenerator.Tests/Helpers/TestFixtures.cs | Adds new plugin fixtures for full-entity pre/post image scenarios. |
| XrmPluginCore.SourceGenerator.Tests/GenerationTests/WrapperClassGenerationTests.cs | Adds generation tests validating wrappers + action wrapper behavior for full-entity images. |
| XrmPluginCore.SourceGenerator.Tests/DiagnosticTests/DiagnosticReportingTests.cs | Adds analyzer tests asserting XPC3005 reports only when no-arg image methods are used. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| context.ReportDiagnostic(Diagnostic.Create( | ||
| DiagnosticDescriptors.FullEntityImage, | ||
| invocation.GetLocation(), | ||
| methodName)); | ||
| } |
There was a problem hiding this comment.
FullEntityImageAnalyzer will report XPC3005 for any method named WithPreImage/WithPostImage anywhere in the codebase (including unrelated user-defined APIs), because it only checks the identifier text and does not verify the invocation is part of an XrmPluginCore RegisterStep fluent chain or that the target symbol is the expected method. This can create false-positive warnings in non-plugin code. Consider restricting analysis to invocations chained from a RegisterStep call (similar to ImageWithoutMethodReferenceAnalyzer) and/or using the semantic model to confirm the invoked IMethodSymbol belongs to the intended XrmPluginCore registration API type/namespace.
| context.ReportDiagnostic(Diagnostic.Create( | |
| DiagnosticDescriptors.FullEntityImage, | |
| invocation.GetLocation(), | |
| methodName)); | |
| } | |
| // Restrict matches to the XrmPluginCore registration fluent chain to avoid | |
| // reporting on unrelated user-defined APIs with the same method name. | |
| if (!IsRegisterStepChain(memberAccess.Expression)) | |
| return; | |
| context.ReportDiagnostic(Diagnostic.Create( | |
| DiagnosticDescriptors.FullEntityImage, | |
| invocation.GetLocation(), | |
| methodName)); | |
| } | |
| private static bool IsRegisterStepChain(ExpressionSyntax expression) | |
| { | |
| switch (expression) | |
| { | |
| case InvocationExpressionSyntax invocation: | |
| return IsRegisterStepInvocation(invocation) || IsRegisterStepChain(invocation.Expression); | |
| case MemberAccessExpressionSyntax memberAccess: | |
| return IsRegisterStepChain(memberAccess.Expression); | |
| default: | |
| return false; | |
| } | |
| } | |
| private static bool IsRegisterStepInvocation(InvocationExpressionSyntax invocation) | |
| { | |
| return invocation.Expression switch | |
| { | |
| MemberAccessExpressionSyntax memberAccess => | |
| memberAccess.Name.Identifier.Text == Constants.RegisterStepMethodName, | |
| IdentifierNameSyntax identifier => | |
| identifier.Identifier.Text == Constants.RegisterStepMethodName, | |
| _ => false | |
| }; | |
| } |
| .OfType<IPropertySymbol>() | ||
| .Where(p => p.GetAttributes().Any(a => a.AttributeClass?.Name == Constants.LogicalNameAttributeName)) | ||
| .Select(p => GetAttributeMetadata(p.Name, entityType)) | ||
| .Where(a => a != null); |
There was a problem hiding this comment.
GetAllEntityAttributes returns entity members in Roslyn’s default order, and later generation iterates image.Attributes as-is. This can make generated source unstable between compilations (and produces noisy diffs) if member enumeration order changes. Consider applying a deterministic ordering (e.g., order properties by Name and/or order the resulting AttributeMetadata by LogicalName) when building the full-entity attribute list.
| .Where(a => a != null); | |
| .Where(a => a != null) | |
| .OrderBy(a => a.LogicalName, global::System.StringComparer.Ordinal) | |
| .ThenBy(a => a.PropertyName, global::System.StringComparer.Ordinal); |
| // For WithPreImage/WithPostImage with no attributes specified, capture all entity attributes | ||
| if (!imageMetadata.Attributes.Any() && | ||
| (methodName == Constants.WithPreImageMethodName || methodName == Constants.WithPostImageMethodName)) | ||
| { | ||
| imageMetadata.Attributes.AddRange(GetAllEntityAttributes(entityType)); | ||
| } |
There was a problem hiding this comment.
The comment says this branch will "capture all entity attributes", but GetAllEntityAttributes only includes properties decorated with AttributeLogicalNameAttribute. To avoid confusion for future maintainers (and align with the actual behavior), consider rewording the comment to reflect that it captures all mapped entity properties/attributes (i.e., those with logical name metadata).
Add: When specifying a Pre or Post image without filtering the attributes, we now generate a Pre or Post image wrapper with all attributes that have a logical name spacified.
Add: Warning to avoid image registrations without attributes