Introduce ExpressiveFor and ExpressiveForConstructor attributes by koenbeuk · Pull Request #4 · EFNext/ExpressiveSharp

koenbeuk · 2026-03-26T02:46:34Z

… for expression tree mapping

Copilot

Pull request overview

Adds support for mapping external methods/properties/constructors to generated expression trees via new [ExpressiveFor] / [ExpressiveForConstructor] attributes, enabling providers like EF Core to translate otherwise-nontranslatable calls after ExpandExpressives().

Changes:

Introduce [ExpressiveFor] and [ExpressiveForConstructor] attributes and generator support (interpretation, emission, registry entries).
Extend runtime resolution/replacement to locate external mappings across loaded registries.
Add generator + integration tests demonstrating external method usage (EF Core + expression-compile runners).

Reviewed changes

Copilot reviewed 26 out of 26 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/ExpressiveSharp.Tests/Services/ExpressiveReplacerTests.cs	Updates test resolver stub to support external-expression lookup.
tests/ExpressiveSharp.IntegrationTests/Scenarios/Store/Models/PricingUtils.cs	Adds “external” utility methods plus `[ExpressiveFor]` mapping stubs.
tests/ExpressiveSharp.IntegrationTests/Scenarios/Common/Tests/ExpressiveForMappingTests.cs	Scenario tests exercising expansion of mapped external calls.
tests/ExpressiveSharp.IntegrationTests.ExpressionCompile/Tests/Common/ExpressiveForMappingTests.cs	Runs the scenario tests against expression compilation runner.
tests/ExpressiveSharp.IntegrationTests.EntityFrameworkCore/Tests/Common/ExpressiveForMappingTests.cs	Runs the scenario tests against EF Core SQLite runner.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.SingleProperty_RegistryContainsEntry.verified.txt	Updates expected registry emission to locate generated expression classes for external targets.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.SingleMethod_RegistryContainsEntry.verified.txt	Same as above.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.MultipleExpressives_AllRegistered.verified.txt	Same as above.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/RegistryTests.MethodOverloads_BothRegistered.verified.txt	Same as above.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.cs	Adds generator tests for `[ExpressiveFor]` scenarios + diagnostics.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.StaticProperty.verified.txt	Verified output for static-property mapping.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.StaticMethod.verified.txt	Verified output for static-method mapping.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.OverloadDisambiguation.verified.txt	Verified output for overload disambiguation.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.InstanceProperty.verified.txt	Verified output for instance-property mapping.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.InstanceMethod.verified.txt	Verified output for instance-method mapping.
src/ExpressiveSharp/Services/IExpressiveResolver.cs	Adds `FindExternalExpression(MemberInfo)` API.
src/ExpressiveSharp/Services/ExpressiveResolver.cs	Implements external lookup via scanning loaded registries and ambiguity detection.
src/ExpressiveSharp/Services/ExpressiveReplacer.cs	Uses resolver external lookup when member isn’t `[Expressive]`.
src/ExpressiveSharp/Mapping/ExpressiveForAttribute.cs	New attribute for external method/property mapping.
src/ExpressiveSharp/Mapping/ExpressiveForConstructorAttribute.cs	New attribute for external constructor mapping.
src/ExpressiveSharp.Generator/Registry/ExpressionRegistryEmitter.cs	Updates registry helper to find generated expression classes even when target member’s assembly differs.
src/ExpressiveSharp.Generator/Models/ExpressiveForAttributeData.cs	Adds immutable-ish snapshot model for `[ExpressiveFor*]` attribute args.
src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs	Resolves/validates external targets and builds descriptors from stub bodies.
src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs	Adds diagnostics for `[ExpressiveFor*]` validation.
src/ExpressiveSharp.Generator/ExpressiveGenerator.cs	Adds incremental pipelines for `[ExpressiveFor]` + `[ExpressiveForConstructor]` and merges registry entries.
src/ExpressiveSharp.Generator/Comparers/ExpressiveForMemberCompilationEqualityComparer.cs	Adds comparer to support incremental caching for the new pipelines.

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

Copilot · 2026-03-26T02:51:51Z

src/ExpressiveSharp.Generator/Models/ExpressiveForAttributeData.cs

+    public bool? AllowBlockBody { get; }
+
+    public IReadOnlyList<string> TransformerTypeNames { get; }
+
+    public ExpressiveForAttributeData(AttributeData attribute, ExpressiveForMemberKind memberKind)
+    {
+        MemberKind = memberKind;
+        bool? allowBlockBody = null;
+        var transformerTypeNames = new List<string>();
+
+        // Extract target type from first constructor argument
+        if (attribute.ConstructorArguments.Length > 0 &&
+            attribute.ConstructorArguments[0].Value is INamedTypeSymbol targetTypeSymbol)
+        {
+            TargetTypeFullName = targetTypeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat);
+            TargetTypeMetadataName = GetMetadataName(targetTypeSymbol);
+        }
+        else
+        {
+            TargetTypeFullName = "";
+            TargetTypeMetadataName = null;
+        }
+
+        // Extract member name from second constructor argument (only for ExpressiveFor, not ExpressiveForConstructor)
+        if (memberKind != ExpressiveForMemberKind.Constructor &&
+            attribute.ConstructorArguments.Length > 1 &&
+            attribute.ConstructorArguments[1].Value is string memberName)
+        {
+            MemberName = memberName;
+        }
+
+        // Extract named arguments
+        foreach (var namedArgument in attribute.NamedArguments)
+        {
+            var key = namedArgument.Key;
+            var value = namedArgument.Value;
+            switch (key)
+            {
+                case "AllowBlockBody":
+                    allowBlockBody = value.Value is true;
+                    break;
+                case "Transformers":
+                    if (value.Kind == TypedConstantKind.Array)
+                    {
+                        foreach (var element in value.Values)
+                        {
+                            if (element.Value is INamedTypeSymbol typeSymbol)
+                            {
+                                transformerTypeNames.Add(
+                                    typeSymbol.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat));
+                            }
+                        }
+                    }
+                    break;
+            }
+        }
+
+        AllowBlockBody = allowBlockBody;
+        TransformerTypeNames = transformerTypeNames;
+    }


TransformerTypeNames is stored as an IReadOnlyList<string> but the constructor assigns a mutable List<string>. Since this is a record struct intended for incremental generator caching, using a mutable reference type also makes equality/hashcode depend on list reference identity rather than contents, defeating caching. Consider storing an ImmutableArray<string>/string[] (or calling ToImmutableArray()/ToArray()) to keep the data immutable and value-comparable.

Copilot · 2026-03-26T02:51:51Z

tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.cs

+    public void TargetTypeNotFound_EXP0014()
+    {
+        var compilation = CreateCompilation(
+            """
+            using ExpressiveSharp.Mapping;
+
+            namespace Foo {
+                static class Mappings {
+                    [ExpressiveFor(typeof(System.Math), "NonExistentMethod")]
+                    static int Nope(int value) => value;
+                }
+            }
+            """);
+        var result = RunExpressiveGenerator(compilation);
+
+        Assert.AreEqual(1, result.Diagnostics.Length);
+        Assert.AreEqual("EXP0015", result.Diagnostics[0].Id);
+    }


The test name TargetTypeNotFound_EXP0014 doesn't match what the test actually exercises (a missing member on an existing type) and it asserts EXP0015. Either rename the test to reflect EXP0015/member-not-found, or adjust the scenario so it can actually trigger EXP0014 (target type not resolved).

Copilot · 2026-03-26T02:51:52Z

src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs

+
+    public readonly static DiagnosticDescriptor ExpressiveForDuplicateMapping = new DiagnosticDescriptor(
+        id: "EXP0020",
+        title: "Duplicate [ExpressiveFor] mapping",
+        messageFormat: "Duplicate [ExpressiveFor] mapping for member '{0}' on type '{1}'; only one stub per target member is allowed",
+        category: "Design",
+        DiagnosticSeverity.Error,
+        isEnabledByDefault: true);


ExpressiveForDuplicateMapping (EXP0020) is declared here, but there doesn't appear to be any reporting site for it in the generator. If duplicate [ExpressiveFor] stubs are meant to be an error, add detection/reporting (e.g., group by resolved target member signature) or remove the descriptor until it's implemented to avoid dead/unused diagnostics.

Suggested change

public readonly static DiagnosticDescriptor ExpressiveForDuplicateMapping = new DiagnosticDescriptor(

id: "EXP0020",

title: "Duplicate [ExpressiveFor] mapping",

messageFormat: "Duplicate [ExpressiveFor] mapping for member '{0}' on type '{1}'; only one stub per target member is allowed",

category: "Design",

DiagnosticSeverity.Error,

isEnabledByDefault: true);

Copilot · 2026-03-26T02:51:52Z

src/ExpressiveSharp.Generator/ExpressiveGenerator.cs

+                // Find the matching target method to get its parameter types (not the stub's)
+                var targetMethod = targetType.GetMembers(memberName).OfType<IMethodSymbol>()
+                    .Where(m => m.MethodKind is not (MethodKind.PropertyGet or MethodKind.PropertySet))
+                    .FirstOrDefault(m =>
+                    {
+                        var expectedParamCount = m.IsStatic ? m.Parameters.Length : m.Parameters.Length + 1;
+                        if (stubSymbol.Parameters.Length != expectedParamCount)
+                            return false;
+
+                        var offset = m.IsStatic ? 0 : 1;
+                        for (var i = 0; i < m.Parameters.Length; i++)
+                        {
+                            if (!SymbolEqualityComparer.Default.Equals(
+                                m.Parameters[i].Type, stubSymbol.Parameters[i + offset].Type))
+                                return false;
+                        }
+                        return true;
+                    });


In ExtractRegistryEntryForExternal, the method-matching predicate compares target method parameter types to the stub parameters (with an offset), but it does not validate the receiver parameter type for instance methods. This can cause the registry entry to be generated for an invalid stub whose first parameter is not the target type. Add an explicit check for !m.IsStatic that stubSymbol.Parameters[0].Type equals targetType so registry extraction matches the intended stub rules.

Copilot · 2026-03-26T02:51:52Z

src/ExpressiveSharp/Services/ExpressiveResolver.cs

+        private static void EnsureAllRegistriesLoaded()
+        {
+            if (_allRegistriesScanned) return;
+            _allRegistriesScanned = true;
+


EnsureAllRegistriesLoaded sets _allRegistriesScanned = true before the scan completes. A concurrent call can observe the flag and skip scanning while _assemblyRegistries is still incomplete, causing FindExternalExpression to miss mappings nondeterministically. Set the flag only after the scan finishes and guard the scan with a lock/LazyInitializer/Interlocked so other threads either participate safely or wait for completion.

Copilot · 2026-03-26T02:51:52Z

src/ExpressiveSharp/Services/ExpressiveResolver.cs

+                // Skip system/framework assemblies for performance
+                var name = assembly.GetName().Name;
+                if (name is null ||
+                    name.StartsWith("System", StringComparison.Ordinal) ||
+                    name.StartsWith("Microsoft", StringComparison.Ordinal) ||
+                    name.StartsWith("mscorlib", StringComparison.Ordinal) ||
+                    name.StartsWith("netstandard", StringComparison.Ordinal) ||
+                    assembly.IsDynamic)
+                    continue;
+


The assembly-scan filter skips any assembly whose name starts with System/Microsoft/etc. This can prevent discovery of registries in non-framework assemblies that happen to use those prefixes (e.g., internal apps or packages named Microsoft.*). Prefer a positive check (e.g., assembly.GetType("ExpressiveSharp.Generated.ExpressionRegistry") is not null) rather than excluding by name prefix, or document that such assemblies must call EnsureRegistryLoaded explicitly.

Suggested change

// Skip system/framework assemblies for performance

var name = assembly.GetName().Name;

if (name is null ||

name.StartsWith("System", StringComparison.Ordinal) ||

name.StartsWith("Microsoft", StringComparison.Ordinal) ||

name.StartsWith("mscorlib", StringComparison.Ordinal) ||

name.StartsWith("netstandard", StringComparison.Ordinal) ||

assembly.IsDynamic)

continue;

// Skip dynamic assemblies; they cannot contain the generated registry type.

if (assembly.IsDynamic)

continue;

// Only process assemblies that actually contain the generated registry type.

// This avoids relying on assembly-name prefixes (e.g., "System", "Microsoft")

// and ensures registries in similarly named non-framework assemblies are discovered.

if (assembly.GetType("ExpressiveSharp.Generated.ExpressionRegistry") == null)

continue;

Copilot · 2026-03-26T02:51:52Z

src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs

+            // Instance property: stub should have 1 param (the instance)
+            // Static property: stub should have 0 params
+            if (property.IsStatic && stubSymbol.Parameters.Length == 0)
+                return property;
+            if (!property.IsStatic && stubSymbol.Parameters.Length == 1)
+                return property;


FindTargetProperty matches instance properties solely by stub parameter count (1) without validating that the receiver parameter type matches targetType. This allows invalid stubs (e.g., static int P(object o)) to bind and later produce incorrect expression signatures at runtime. Also consider excluding indexers (property.Parameters.Length > 0), since the current logic can incorrectly treat an indexer as a parameterless/receiver-only property mapping.

Suggested change

// Instance property: stub should have 1 param (the instance)

// Static property: stub should have 0 params

if (property.IsStatic && stubSymbol.Parameters.Length == 0)

return property;

if (!property.IsStatic && stubSymbol.Parameters.Length == 1)

return property;

// Exclude indexers (properties with parameters)

if (property.Parameters.Length > 0)

continue;

// Static property: stub should have 0 params

if (property.IsStatic)

{

if (stubSymbol.Parameters.Length == 0)

return property;

continue;

}

// Instance property: stub should have 1 param (the instance),

// and its type must match the target type.

if (stubSymbol.Parameters.Length != 1)

continue;

var receiverParamType = stubSymbol.Parameters[0].Type;

if (SymbolEqualityComparer.Default.Equals(receiverParamType, targetType))

return property;

Copilot · 2026-03-26T02:51:53Z

src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs

+            // For instance methods: first stub param = this, rest = method params
+            // For static methods: all stub params = method params
+            var expectedStubParamCount = method.IsStatic
+                ? method.Parameters.Length
+                : method.Parameters.Length + 1;
+
+            if (stubSymbol.Parameters.Length != expectedStubParamCount)
+                continue;
+
+            // Check parameter types match
+            var offset = method.IsStatic ? 0 : 1;
+            var match = true;
+            for (var i = 0; i < method.Parameters.Length; i++)
+            {
+                var targetParamType = method.Parameters[i].Type;
+                var stubParamType = stubSymbol.Parameters[i + offset].Type;
+                if (!SymbolEqualityComparer.Default.Equals(targetParamType, stubParamType))
+                {
+                    match = false;
+                    break;
+                }
+            }
+
+            if (match)
+                return method;
+        }


FindTargetMethod validates stub parameter types against the target method parameters, but it never validates the receiver type for instance methods (stub parameter 0). This can incorrectly accept a stub whose first parameter is not targetType, producing mismatched lambdas and replacements. Add a check for !method.IsStatic that stubSymbol.Parameters[0].Type equals targetType (and mirror the same rule in the registry-entry extraction path).

github-actions

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'ExpressiveSharp Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.

Benchmark suite	Current: `be03a1a`	Previous: `a944078`	Ratio
`ExpressiveSharp.Benchmarks.EFCoreQueryOverheadBenchmarks.WithExpressives_Method`	`26694.59842936198` ns (`± 8891.111034721811`)	`21373.67755126953` ns (`± 7361.186582722646`)	`1.25`
`ExpressiveSharp.Benchmarks.ExpressionReplacerBenchmarks.Replace_DeepChain`	`14258.631693522135` ns (`± 7761.665773405919`)	`7669.970245361328` ns (`± 188.8991716430791`)	`1.86`
`ExpressiveSharp.Benchmarks.TransformerBenchmarks.ExpandExpressives_FullPipeline`	`15015.31532796224` ns (`± 7936.4594859497665`)	`7716.534535725911` ns (`± 196.44370128311223`)	`1.95`
`ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator(ExpressiveCount: 1)`	`2582576.4622395835` ns (`± 1179280.1215652542`)	`1935933.0729166667` ns (`± 318306.0797608759`)	`1.33`
`ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_NoiseChange(ExpressiveCount: 1)`	`2156488.19921875` ns (`± 127032.2481198264`)	`1301776.9869791667` ns (`± 211884.62758293742`)	`1.66`
`ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_Incremental_ExpressiveChange(ExpressiveCount: 1)`	`1265257.66796875` ns (`± 142005.56824997542`)	`910163.3170572916` ns (`± 58371.300269983905`)	`1.39`
`ExpressiveSharp.Benchmarks.GeneratorBenchmarks.RunGenerator_Incremental_ExpressiveChange(ExpressiveCount: 100)`	`88837938` ns (`± 21025701.90452524`)	`72219961.58333333` ns (`± 22393867.14749421`)	`1.23`

This comment was automatically generated by workflow using github-action-benchmark.

feat: Introduce ExpressiveFor and ExpressiveForConstructor attributes…

be03a1a

… for expression tree mapping

Copilot AI review requested due to automatic review settings March 26, 2026 02:46

Copilot started reviewing on behalf of koenbeuk March 26, 2026 02:47 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

github-actions bot reviewed Mar 26, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce ExpressiveFor and ExpressiveForConstructor attributes#4

Introduce ExpressiveFor and ExpressiveForConstructor attributes#4
koenbeuk wants to merge 1 commit intomainfrom
feat/proxied-expressives

koenbeuk commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

github-actions bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

koenbeuk commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions bot left a comment

Choose a reason for hiding this comment

⚠️ Performance Alert ⚠️

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants