Skip to content

Simplified ExpressiveFor#38

Merged
koenbeuk merged 2 commits intomainfrom
feat/expressive-for-enhancements
Apr 17, 2026
Merged

Simplified ExpressiveFor#38
koenbeuk merged 2 commits intomainfrom
feat/expressive-for-enhancements

Conversation

@koenbeuk
Copy link
Copy Markdown
Collaborator

// Before
[ExpressiveFor(typeof(User), nameof(User.FullName))]
static string FullName(User u) => u.LastName + ", " + u.FirstName;

// After
[ExpressiveFor(nameof(FullName))]
private string FullNameExpression => LastName + ", " + FirstName;

Copilot AI review requested due to automatic review settings April 16, 2026 23:50
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 16, 2026

Comment thread src/ExpressiveSharp.Generator/ExpressiveGenerator.cs Fixed
Comment thread src/ExpressiveSharp.Generator/ExpressiveGenerator.cs Fixed
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends ExpressiveSharp’s [ExpressiveFor] mapping model to support a more ergonomic “co-located” form (single-argument attribute + instance/property stubs), and updates the generator, EF Core conventions, tests, and docs accordingly.

Changes:

  • Add [ExpressiveFor(string memberName)] shorthand (target defaults to the stub’s containing type) and allow [ExpressiveFor] to be applied to property stubs.
  • Update the generator to interpret method/property stubs (including instance stubs) and centralize signature matching via a shared matcher.
  • Update EF Core integration to ignore [ExpressiveFor] stubs/targets as unmapped, and add/refresh tests + documentation for the new forms.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/ExpressiveSharp.IntegrationTests/Scenarios/Store/Models/DisplayFormatter.cs Switch Wrap mapping to co-located instance-stub + single-arg [ExpressiveFor]; keep legacy external static mapping for Label.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.cs Adjust diagnostics expectation (retire EXP0016) and add test coverage for instance stubs, single-arg form, and property stubs.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.SingleArgForm_InstanceMethodTarget.verified.txt Snapshot for generated output of single-arg instance-method target.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.SingleArgForm_DefaultsToContainingType.verified.txt Snapshot for generated output of single-arg target defaulting to containing type.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.PropertyStub_InstanceProperty_SameType.verified.txt Snapshot for generated output of property-stub mapping.
tests/ExpressiveSharp.Generator.Tests/ExpressiveGenerator/ExpressiveForTests.InstanceStub_OnInstanceProperty_SameType.verified.txt Snapshot for generated output of instance method stub targeting instance property.
tests/ExpressiveSharp.EntityFrameworkCore.IntegrationTests/Tests/Sqlite/ExpressivePropertiesNotMappedConventionTests.cs New test suite for EF convention ignoring [Expressive], [ExpressiveFor] stubs, and [ExpressiveFor] targets.
src/ExpressiveSharp.Generator/Models/ExpressiveForAttributeData.cs Parse both (Type,string) and (string) constructors for [ExpressiveFor].
src/ExpressiveSharp.Generator/Interpretation/ExpressiveForSignatureMatcher.cs New shared matcher to keep interpreter and registry extraction aligned on signature rules.
src/ExpressiveSharp.Generator/Interpretation/ExpressiveForInterpreter.cs Support method/property stubs and instance stubs; synthesize @this parameter for instance stubs.
src/ExpressiveSharp.Generator/Infrastructure/Diagnostics.cs Retire EXP0016 (static-only stub requirement).
src/ExpressiveSharp.Generator/ExpressiveGenerator.cs Expand [ExpressiveFor] discovery to properties; adapt pipeline types from method-only to member-level; update registry extraction logic.
src/ExpressiveSharp.Generator/Comparers/ExpressiveForMemberCompilationEqualityComparer.cs Broaden comparer tuple to MemberDeclarationSyntax to support property stubs.
src/ExpressiveSharp.EntityFrameworkCore/Infrastructure/Internal/ExpressivePropertiesNotMappedConventionPlugin.cs Inject IExpressiveResolver into convention plugin to detect external mappings.
src/ExpressiveSharp.EntityFrameworkCore/Infrastructure/Internal/ExpressivePropertiesNotMappedConvention.cs Ignore [ExpressiveFor] stubs/targets (including external stubs via resolver) in EF model-building.
src/ExpressiveSharp.EntityFrameworkCore/Infrastructure/Internal/ExpressiveOptionsExtension.cs Register IExpressiveResolver and wire new convention constructor dependency.
src/ExpressiveSharp.Abstractions/Mapping/ExpressiveForAttribute.cs Allow attribute on properties; add single-arg constructor; make TargetType nullable.
docs/reference/projectable-properties.md Update comparison text to mention non-static/co-located stubs.
docs/reference/expressive-for.md Document new mapping rules and co-located forms; remove EXP0016 mention.
docs/reference/diagnostics.md Document EXP0016 retirement and remove its section/table entry.
docs/recipes/external-member-mapping.md Update guidance for new co-located and property-stub forms; remove static-only wording.
docs/guide/migration-from-projectables.md Update migration guidance/examples to prefer single-arg co-located stubs for same-type scenarios.
Comments suppressed due to low confidence (2)

src/ExpressiveSharp.Generator/ExpressiveGenerator.cs:516

  • In ExtractRegistryEntryForExternal, the property path sets isProperty = stubIsProperty ? true : ... without validating that the stub can actually target that property under the same signature rules enforced by ExpressiveForInterpreter (static vs instance, same-type requirement for instance stubs, etc.). To keep the registry extractor and interpreter from diverging, consider resolving the target property using ExpressiveForSignatureMatcher.MatchesPropertyFromPropertyStub / MatchesPropertyFromMethodStub (and returning null when no match).
            // Property stubs can only target properties; method stubs may target either.
            var isProperty = stubIsProperty
                ? true
                : targetType.GetMembers(memberName).OfType<IPropertySymbol>().Any();

            if (isProperty)
            {
                memberKind = ExpressionRegistryMemberType.Property;
                memberLookupName = memberName;
                // Properties have no parameter types in the registry

src/ExpressiveSharp.Abstractions/Mapping/ExpressiveForAttribute.cs:27

  • MemberName’s XML doc still says “on TargetType”, but TargetType is now nullable for the single-argument constructor (where the target defaults to the stub’s containing type). Consider updating this doc comment to avoid implying TargetType is always non-null.
    /// <summary>
    /// The name of the target member on <see cref="TargetType"/>.
    /// </summary>
    public string MemberName { get; }

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

Comment on lines 12 to +15
/// <para>For <b>instance properties</b>, the stub takes a single parameter (the receiver) and returns the property type.</para>
/// <para>For <b>static properties</b>, the stub is parameterless and returns the property type.</para>
/// </remarks>
[AttributeUsage(AttributeTargets.Method, Inherited = false, AllowMultiple = false)]
[AttributeUsage(AttributeTargets.Method | AttributeTargets.Property, Inherited = false, AllowMultiple = false)]
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The XML docs in the <remarks> still describe only the legacy static-method-stub shapes (receiver as first parameter, etc.). With this PR adding property stubs, instance stubs on the target type (implicit this), and the single-argument constructor (TargetType nullable), these docs are now inaccurate—please update the remarks (and summary if needed) to reflect the new supported stub shapes and matching rules.

Copilot uses AI. Check for mistakes.
.ForAttributeWithMetadataName(
attributeFullName,
predicate: static (s, _) => s is MethodDeclarationSyntax,
predicate: static (s, _) => s is MethodDeclarationSyntax or PropertyDeclarationSyntax,
Copy link

Copilot AI Apr 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ExpressiveForAttribute can now be applied to properties, which includes indexers, but this syntax-provider predicate only matches PropertyDeclarationSyntax (not IndexerDeclarationSyntax). As a result, [ExpressiveFor] on an indexer would be silently ignored (no diagnostic, no generated output). Consider including IndexerDeclarationSyntax here (and then rejecting indexers explicitly in the interpreter, since indexers can’t be expressive targets).

Suggested change
predicate: static (s, _) => s is MethodDeclarationSyntax or PropertyDeclarationSyntax,
predicate: static (s, _) => s is MethodDeclarationSyntax or PropertyDeclarationSyntax or IndexerDeclarationSyntax,

Copilot uses AI. Check for mistakes.
…iveForInterpreter

test: add comprehensive tests for ExpressiveFor signature matching scenarios
@koenbeuk koenbeuk merged commit ad1718a into main Apr 17, 2026
20 checks passed
@koenbeuk koenbeuk deleted the feat/expressive-for-enhancements branch April 17, 2026 01:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants