Improve UseMemberBody and add new diagnostics#174
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #134 by improving [Projectable(UseMemberBody = ...)] support for method scenarios (including extension methods) and adding clearer generator diagnostics plus documentation/tests around the feature.
Changes:
- Add generator diagnostics EFP0010 (member not found) and EFP0011 (incompatible member) for
UseMemberBody. - Extend generator/test coverage for
UseMemberBodywith methods, properties, andExpression<Func<...>>-backed bodies (including extension methods), plus functional coverage. - Update README to document
UseMemberBodyusage patterns and diagnostics.
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.cs | New generator tests covering valid/invalid UseMemberBody scenarios. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.Method_UsesMethodBody_SameReturnType.verified.txt | Snapshot for generated output from redirected body case. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.Method_UsesExpressionPropertyBody_StaticExtension.verified.txt | Snapshot for generated output when backing an extension method via Expression<Func<...>> property. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.Method_UsesExpressionPropertyBody_InstanceMethod.verified.txt | Snapshot for generated output when backing an instance method via Expression<Func<...>> property. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.Property_UsesPropertyBody_SameType.verified.txt | Snapshot for generated output when redirecting a property body. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.StaticMethod_UsesStaticMethodBody.verified.txt | Snapshot for generated output when redirecting a static method body. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/ExtensionMethodTests.cs | Adds a generator test for extension methods backed by an expression property body. |
| tests/EntityFrameworkCore.Projectables.Generator.Tests/ExtensionMethodTests.ProjectableExtensionMethod_WithExpressionPropertyBody.verified.txt | Snapshot for new extension-method test output. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.cs | Adds functional test ensuring EF SQL translation works with expression-property-backed extension methods. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethod_WithExpressionPropertyBody.verified.txt | Functional snapshot (default TFM) for the new query translation. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethod_WithExpressionPropertyBody.DotNet9_0.verified.txt | Functional snapshot for .NET 9. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethod_WithExpressionPropertyBody.DotNet10_0.verified.txt | Functional snapshot for .NET 10. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethodAcceptingDbContext.verified.txt | Updates expected SQL projection due to model change (new Name column). |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethodAcceptingDbContext.DotNet9_0.verified.txt | Same snapshot update for .NET 9. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/ExtensionMethodTests.ExtensionMethodAcceptingDbContext.DotNet10_0.verified.txt | Same snapshot update for .NET 10. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/EntityExtensions.cs | Adds NameEquals projectable extension method using UseMemberBody and an expression-property body. |
| tests/EntityFrameworkCore.Projectables.FunctionalTests/ExtensionsMethods/Entity.cs | Adds nullable Name to the functional-test entity model. |
| src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs | Adjusts runtime UseMemberBody expression-property resolution logic for static methods. |
| src/EntityFrameworkCore.Projectables.Generator/ProjectableInterpreter.cs | Implements generator-side UseMemberBody candidate selection, expression-property handling for methods, and diagnostic emission. |
| src/EntityFrameworkCore.Projectables.Generator/Diagnostics.cs | Adds descriptors for EFP0010/EFP0011. |
| src/EntityFrameworkCore.Projectables.Generator/AnalyzerReleases.Shipped.md | Documents new diagnostics in shipped analyzer rules list. |
| README.md | Adds UseMemberBody documentation, examples, and diagnostic reference. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.cs
Show resolved
Hide resolved
tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.cs
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables.Generator/ProjectableInterpreter.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables.Generator/ProjectableInterpreter.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables.Generator/ProjectableInterpreter.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/EntityFrameworkCore.Projectables/Services/ProjectionExpressionResolver.cs
Outdated
Show resolved
Hide resolved
src/EntityFrameworkCore.Projectables.Generator/ProjectableInterpreter.MemberBodyResolver.cs
Show resolved
Hide resolved
tests/EntityFrameworkCore.Projectables.Generator.Tests/UseMemberBodyTests.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var returnType = declarationSyntaxRewriter.Visit(originalMethodDecl.ReturnType); | ||
| descriptor.ReturnTypeName = returnType.ToString(); | ||
| descriptor.ExpressionBody = (ExpressionSyntax)expressionSyntaxRewriter.Visit(innerBody); | ||
|
|
||
| foreach (var additionalParameter in | ||
| ((ParameterListSyntax)declarationSyntaxRewriter.Visit(originalMethodDecl.ParameterList)).Parameters) | ||
| { | ||
| descriptor.ParametersList = descriptor.ParametersList!.AddParameters(additionalParameter); | ||
| } |
There was a problem hiding this comment.
TryApplyExpressionPropertyBody extracts only the inner lambda body and then generates a new parameter list from the method signature. If the expression property's lambda uses different parameter identifiers (e.g. x => x.Value > 0 for an instance method where the generated parameter is @this), the generated code will not compile, and no EFP0011 diagnostic is produced. Consider either rewriting lambda-parameter identifiers to the generated parameters (by symbol), or validating the lambda parameter names and emitting a dedicated diagnostic when they don't match.
| SyntaxFactory.TypeParameter(tp.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat))); | ||
|
|
||
| // See https://github.com/dotnet/roslyn/blob/d7e010bbe5b1d37837417fc5e79ecb2fd9b7b487/src/VisualStudio/CSharp/Impl/ObjectBrowser/DescriptionBuilder.cs#L340 | ||
| if (tp.ConstraintTypes.IsDefaultOrEmpty) |
There was a problem hiding this comment.
SetupGenericTypeParameters skips emitting a constraint clause when tp.ConstraintTypes is empty (line 245), but Roslyn represents class, struct, notnull, and new() constraints via flags (HasReferenceTypeConstraint, etc.) even when ConstraintTypes is empty. This means type parameters constrained only by those flags won't have their constraints emitted into the generated class. Consider emitting a constraint clause whenever any constraint flag is present (or any constraint types exist).
| if (tp.ConstraintTypes.IsDefaultOrEmpty) | |
| var hasAnyConstraints = | |
| !tp.ConstraintTypes.IsDefaultOrEmpty || | |
| tp.HasReferenceTypeConstraint || | |
| tp.HasValueTypeConstraint || | |
| tp.HasNotNullConstraint || | |
| tp.HasConstructorConstraint; | |
| if (!hasAnyConstraints) |
| var exprProperty = declaringType.GetProperty(memberName, BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic); | ||
|
|
||
| static LambdaExpression? GetExpressionFromMemberBody(MemberInfo projectableMemberInfo, string memberName) | ||
| if (exprProperty?.GetValue(null) is not LambdaExpression lambda) |
There was a problem hiding this comment.
GetProperty(... BindingFlags.Static | BindingFlags.Instance ...) can return an instance property, but GetValue(null) will then throw TargetException instead of returning null. Either restrict lookup to static properties (and/or validate exprProperty.GetGetMethod(true)?.IsStatic) or safely handle instance properties to avoid a runtime crash when UseMemberBody points at a non-static expression property.
| if (exprProperty?.GetValue(null) is not LambdaExpression lambda) | |
| // Only static properties can be accessed with a null target; ignore instance properties to avoid TargetException. | |
| var getter = exprProperty?.GetGetMethod(true); | |
| if (getter is null || !getter.IsStatic || exprProperty.GetValue(null) is not LambdaExpression lambda) |
Updated [EntityFrameworkCore.Projectables](https://github.com/EFNext/EntityFrameworkCore.Projectables) from 5.0.2 to 6.0.0. <details> <summary>Release notes</summary> _Sourced from [EntityFrameworkCore.Projectables's releases](https://github.com/EFNext/EntityFrameworkCore.Projectables/releases)._ ## 6.0.0 ## What's Changed ### Major changes * Add support for projectable method overloads by @PhenX in EFNext/EntityFrameworkCore.Projectables#143 * Add C#14 extension members by @PhenX in EFNext/EntityFrameworkCore.Projectables#148 * Support explicitly implemented interface members and default interface properties by @rhodon-jargon in EFNext/EntityFrameworkCore.Projectables#135 * Support block-bodied members with [Projectable] attribute by @Copilot in EFNext/EntityFrameworkCore.Projectables#152 * Add ExpandEnumMethods to expand enum extension calls into ternary expressions by @Copilot in EFNext/EntityFrameworkCore.Projectables#150 * Add support for pattern matching in various cases by @PhenX in EFNext/EntityFrameworkCore.Projectables#158 * Add support for projectable constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#161 * AOT-compatible static projection registry + SyntaxFactory-based emission by @Copilot in EFNext/EntityFrameworkCore.Projectables#166 * Improve UseMemberBody and add new diagnostics by @PhenX in EFNext/EntityFrameworkCore.Projectables#174 * Add code fixes for EFP0001, EFP0002 and EFP0008, with tests by @PhenX in EFNext/EntityFrameworkCore.Projectables#181 * Add a code fixer and a code refactorer to transform factory methods into constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#183 * Improve source generator and analyzer responsiveness by @PhenX in EFNext/EntityFrameworkCore.Projectables#171 * Docs website by @PhenX in EFNext/EntityFrameworkCore.Projectables#194 * Implement global MSBuild defaults for [Projectable] options by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#191 ### Other changes * Check source generator output compilation by @PhenX in EFNext/EntityFrameworkCore.Projectables#156 * Fix GetImplementingProperty issue when using interfaces by @PhenX in EFNext/EntityFrameworkCore.Projectables#144 * Full qualification not needed anymore by @PhenX in EFNext/EntityFrameworkCore.Projectables#157 * Split generator tests in different classes by @PhenX in EFNext/EntityFrameworkCore.Projectables#162 * Refactor ExpressionSyntaxRewriter into focused partial class files by @Copilot in EFNext/EntityFrameworkCore.Projectables#163 * Update readme and add new test for constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#165 * Remove obsolete code by @PhenX in EFNext/EntityFrameworkCore.Projectables#167 * Add source generator self-benchmark covering all expression transformers by @Copilot in EFNext/EntityFrameworkCore.Projectables#168 * Fix null conditional rewrite generating invalid member access on nullable value types by @Copilot in EFNext/EntityFrameworkCore.Projectables#169 * Improve generator benchmark with more realistic scenario by @PhenX in EFNext/EntityFrameworkCore.Projectables#170 * Fix stale incremental generator cache when referenced types change in other source files by @Copilot in EFNext/EntityFrameworkCore.Projectables#172 * Add tests for properties with a getter and setter by @PhenX in EFNext/EntityFrameworkCore.Projectables#173 * UseMemberBody more strict with expressions by @PhenX in EFNext/EntityFrameworkCore.Projectables#175 * Add custom Copilot instructions by @PhenX in EFNext/EntityFrameworkCore.Projectables#177 * Reorganize generator project by @PhenX in EFNext/EntityFrameworkCore.Projectables#178 * Add closure resolution benchmark by @PhenX in EFNext/EntityFrameworkCore.Projectables#179 * Feature/optimize closures by @PhenX in EFNext/EntityFrameworkCore.Projectables#180 * Feature/optimize resolver by @PhenX in EFNext/EntityFrameworkCore.Projectables#182 * Update github project urls by @PhenX in EFNext/EntityFrameworkCore.Projectables#184 * Update deps by @PhenX in EFNext/EntityFrameworkCore.Projectables#185 * Remove obsolete verified files and name net8 files correctly by @PhenX in EFNext/EntityFrameworkCore.Projectables#186 * Remove all allocations when resolving and make it even faster by @PhenX in EFNext/EntityFrameworkCore.Projectables#189 * Add devcontainer configuration for C# (.NET) development by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#190 **Full Changelog**: EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0 Commits viewable in [compare view](EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Gunn <james@gunn.io>
Updated [EntityFrameworkCore.Projectables](https://github.com/EFNext/EntityFrameworkCore.Projectables) from 5.0.2 to 6.0.0. <details> <summary>Release notes</summary> _Sourced from [EntityFrameworkCore.Projectables's releases](https://github.com/EFNext/EntityFrameworkCore.Projectables/releases)._ ## 6.0.0 ## What's Changed ### Major changes * Add support for projectable method overloads by @PhenX in EFNext/EntityFrameworkCore.Projectables#143 * Add C#14 extension members by @PhenX in EFNext/EntityFrameworkCore.Projectables#148 * Support explicitly implemented interface members and default interface properties by @rhodon-jargon in EFNext/EntityFrameworkCore.Projectables#135 * Support block-bodied members with [Projectable] attribute by @Copilot in EFNext/EntityFrameworkCore.Projectables#152 * Add ExpandEnumMethods to expand enum extension calls into ternary expressions by @Copilot in EFNext/EntityFrameworkCore.Projectables#150 * Add support for pattern matching in various cases by @PhenX in EFNext/EntityFrameworkCore.Projectables#158 * Add support for projectable constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#161 * AOT-compatible static projection registry + SyntaxFactory-based emission by @Copilot in EFNext/EntityFrameworkCore.Projectables#166 * Improve UseMemberBody and add new diagnostics by @PhenX in EFNext/EntityFrameworkCore.Projectables#174 * Add code fixes for EFP0001, EFP0002 and EFP0008, with tests by @PhenX in EFNext/EntityFrameworkCore.Projectables#181 * Add a code fixer and a code refactorer to transform factory methods into constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#183 * Improve source generator and analyzer responsiveness by @PhenX in EFNext/EntityFrameworkCore.Projectables#171 * Docs website by @PhenX in EFNext/EntityFrameworkCore.Projectables#194 * Implement global MSBuild defaults for [Projectable] options by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#191 ### Other changes * Check source generator output compilation by @PhenX in EFNext/EntityFrameworkCore.Projectables#156 * Fix GetImplementingProperty issue when using interfaces by @PhenX in EFNext/EntityFrameworkCore.Projectables#144 * Full qualification not needed anymore by @PhenX in EFNext/EntityFrameworkCore.Projectables#157 * Split generator tests in different classes by @PhenX in EFNext/EntityFrameworkCore.Projectables#162 * Refactor ExpressionSyntaxRewriter into focused partial class files by @Copilot in EFNext/EntityFrameworkCore.Projectables#163 * Update readme and add new test for constructors by @PhenX in EFNext/EntityFrameworkCore.Projectables#165 * Remove obsolete code by @PhenX in EFNext/EntityFrameworkCore.Projectables#167 * Add source generator self-benchmark covering all expression transformers by @Copilot in EFNext/EntityFrameworkCore.Projectables#168 * Fix null conditional rewrite generating invalid member access on nullable value types by @Copilot in EFNext/EntityFrameworkCore.Projectables#169 * Improve generator benchmark with more realistic scenario by @PhenX in EFNext/EntityFrameworkCore.Projectables#170 * Fix stale incremental generator cache when referenced types change in other source files by @Copilot in EFNext/EntityFrameworkCore.Projectables#172 * Add tests for properties with a getter and setter by @PhenX in EFNext/EntityFrameworkCore.Projectables#173 * UseMemberBody more strict with expressions by @PhenX in EFNext/EntityFrameworkCore.Projectables#175 * Add custom Copilot instructions by @PhenX in EFNext/EntityFrameworkCore.Projectables#177 * Reorganize generator project by @PhenX in EFNext/EntityFrameworkCore.Projectables#178 * Add closure resolution benchmark by @PhenX in EFNext/EntityFrameworkCore.Projectables#179 * Feature/optimize closures by @PhenX in EFNext/EntityFrameworkCore.Projectables#180 * Feature/optimize resolver by @PhenX in EFNext/EntityFrameworkCore.Projectables#182 * Update github project urls by @PhenX in EFNext/EntityFrameworkCore.Projectables#184 * Update deps by @PhenX in EFNext/EntityFrameworkCore.Projectables#185 * Remove obsolete verified files and name net8 files correctly by @PhenX in EFNext/EntityFrameworkCore.Projectables#186 * Remove all allocations when resolving and make it even faster by @PhenX in EFNext/EntityFrameworkCore.Projectables#189 * Add devcontainer configuration for C# (.NET) development by @koenbeuk in EFNext/EntityFrameworkCore.Projectables#190 **Full Changelog**: EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0 Commits viewable in [compare view](EFNext/EntityFrameworkCore.Projectables@v5.0.2...v6.0.0). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: James Gunn <james@gunn.io>
Fixes #134