Skip to content

include non-public accessors when resolving implementing properties - #90

Open
koenbeuk wants to merge 1 commit into
mainfrom
fix/nonpublic-interface-accessors
Open

include non-public accessors when resolving implementing properties#90
koenbeuk wants to merge 1 commit into
mainfrom
fix/nonpublic-interface-accessors

Conversation

@koenbeuk

@koenbeuk koenbeuk commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Copilot AI review requested due to automatic review settings August 2, 2026 01:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 updates TypeExtensions.GetImplementingProperty to handle interface properties whose accessors are non-public (e.g., internal interface accessors), and adds a regression test to cover that scenario in the runtime test suite.

Changes:

  • Include non-public accessors when selecting an interface property accessor for interface-map resolution.
  • Add a test case for an interface property with a non-public accessor implemented explicitly.
  • (Needed) Adjust accessor-kind detection so non-public getters don’t get misclassified as setters during implementing-property lookup.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/ExpressiveSharp/Extensions/TypeExtensions.cs Switches to GetAccessors(true) for interface properties; currently needs a follow-up fix to correctly identify getter vs setter for non-public accessors.
tests/ExpressiveSharp.Tests/Extensions/TypeExtensionsTests.cs Adds a regression test for non-public interface accessors; currently needs a stronger assertion to ensure the concrete implementing property is returned.
Suppressed comments (1)

src/ExpressiveSharp/Extensions/TypeExtensions.cs:129

  • GetImplementingProperty now selects a non-public accessor via GetAccessors(true)[0], but the later propertyInfo.GetMethod == accessor check only works for public getters (it will be null for non-public get accessors). For interface properties with non-public getters this makes the code treat the accessor as a setter, causing the implementing property lookup to fail and returning the original interface PropertyInfo. Use GetGetMethod(true)/GetSetMethod(true) to both pick the accessor and determine whether it is a getter vs setter.
        var accessor = propertyInfo.GetAccessors(true)[0];

        var implementingAccessor = derivedType.GetImplementingMethod(accessor);
        if (implementingAccessor == accessor)
        {

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

Comment on lines +131 to +134
var result = typeof(HiddenImpl).GetConcreteProperty(interfaceProperty);

Assert.IsNotNull(result);
}
@codecov

codecov Bot commented Aug 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

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