GeezerVison is showing github.com — Fix getting a property whose override defines only the setter by piupiunn · Pull Request #28282 · PowerShell/PowerShell
Conversation
PR Summary
PowerShell can now get a .NET property whose override defines only the setter. The getter is inherited from the base property, as in C#.
Fix #25038
PR Context
For public override int Foo { set { ... } }, reflection returns a PropertyInfo for the override that has only the setter, although the getter is inherited. The .NET adapter used that PropertyInfo for getting too, so the property was treated as write-only:
$obj.Foo threw WriteOnlyProperty.
$obj.psobject.Properties['Foo'].Value and Select-Object Foo returned $null.
Get-Member showed int Foo {set;}.
PropertyCacheEntry still stores the override's PropertyInfo, so setting, attributes and the declaring type don't change. Only for getting, when the override has no getter, it now uses the property that introduced the virtual property (found with MethodInfo.GetBaseDefinition()). That getter is virtual, so a getter override in a type in between is still called. ParameterizedPropertyCacheEntry takes the inherited getter the same way, so $obj.Item(1) works too, and the setter keeps the override's parameters. Protected properties used inside a PowerShell class are covered as well, because they also get a PropertyCacheEntry.
Other visible changes for these properties: $obj.Item(1) now calls the matching overload instead of another overload that has a getter, Get-Member shows an indexer as the base getter declares it, and Select-Object *, Format-List * and ConvertTo-Json now output their values.
In-box properties that become readable:
Not changed:
- An override that defines only a non-public setter of a property with a public getter, for example
public override int Foo { protected set { ... } }, used outside a PowerShell class that derives from the type. Reflection doesn't return such a property as public, so it isn't in the property table.
- Completion tooltips for types inferred without an instance still show
{ set; }, because CompletionCompleters reads the override's PropertyInfo itself.
- The opposite case: an override that defines only the getter of a property that has a setter. It has the same cause, so such a property can't be set from PowerShell (for example
RSACryptoServiceProvider.KeySize), although C# can call the inherited setter. I left it out because the issue doesn't cover it, and it would make some properties settable that look read-only today.
I can handle the last two here or in separate PRs if you want them.
The new tests fail without the fix and pass with it.
PR Checklist
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
The change regresses indexer setters’ optional-argument defaults and params support, and breaks protected-property assignments involving generic base types.
2 open issues
What changed in this PR
Addresses #25038 by allowing PowerShell to read inherited getters when a .NET property override defines only the setter.
Changes:
- Resolves inherited getters when populating the .NET property cache.
- Applies the same lookup to non-public properties accessed inside PowerShell classes.
- Adds regression tests for ordinary, indexed, generic-base, and protected properties, including virtual dispatch.
| File |
Description |
test/powershell/engine/ETS/Adapter.Tests.ps1 |
Adds setter-only override regression tests. |
src/System.Management.Automation/engine/runtime/Binding/Binders.cs |
Uses inherited-getter lookup for non-public property binding. |
src/System.Management.Automation/engine/CoreAdapter.cs |
Adds inherited-getter resolution and integrates it into property caching. |
Review effort: Balanced
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
|
This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Reviewers
At least 1 approving review is required to merge this pull request.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|