Skip to content

Make Select/ForEach/Where see dynamic properties #6898

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
daxian-dbw merged 2 commits into from
Jun 13, 2018
Merged

Make Select/ForEach/Where see dynamic properties #6898

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f527155
Make Select/ForEach/Where see dynamic properties
jazzdelightsme Apr 27, 2018
460beb8
[feature] CR feedback: remove unnecessary Import-Module
jazzdelightsme Jun 4, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 27 additions & 8 deletions src/System.Management.Automation/FormatAndOutput/common/Utilities/Mshexpression.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,11 @@
using System;
using System.Management.Automation;
using System.Management.Automation.Internal;
using System.Management.Automation.Language;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Runtime.CompilerServices;

namespace Microsoft.PowerShell.Commands.Internal.Format
{
Expand Down Expand Up @@ -123,13 +125,20 @@ internal List<MshExpression> ResolveNames(PSObject target, bool expand)
{
// get the members first: this will expand the globbing on each parameter
members = target.Members.Match(_stringValue,
PSMemberTypes.Properties | PSMemberTypes.PropertySet);
PSMemberTypes.Properties | PSMemberTypes.PropertySet | PSMemberTypes.Dynamic);
}
else
{
// we have no globbing: try an exact match, because this is quicker.
PSMemberInfo x = target.Members[_stringValue];

if ((x == null) && (target.BaseObject is System.Dynamic.IDynamicMetaObjectProvider))
{
// We could check if GetDynamicMemberNames includes the name... but
Copy link
Member

@daxian-dbw daxian-dbw Jun 12, 2018

Choose a reason for hiding this comment

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

About this comment, we have already checked GetDynamicMemberNames when reaching here (target.Members includes the dynamic members returned from GetDynamicMemberNames). So maybe we can change comment to: GetDynamicMemberNames does not return this name, but GetDynamicMemberNames is only a hint ...

Copy link
Contributor Author

@jazzdelightsme jazzdelightsme Jun 12, 2018

Choose a reason for hiding this comment

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

Your comment is not quite correct. (Maybe it should be correct and there is another bug; I'm not sure what the intended behavior is.)

If you iterate through .Members you will indeed see the dynamic members. However, if you do .Members[propName], where propName is the name of one of those dynamic members... you will get null back.

Sample output:

PS C:\src\PowerShell> $myObj.PSObject.Members | where Name -eq FooProp


MemberType      : Dynamic
Value           :
TypeNameOfValue : dynamic
Name            : FooProp
IsInstance      : True



PS C:\src\PowerShell> $myObj.PSObject.Members['FooProp']
PS C:\src\PowerShell> $myObj.PSObject.Members['FooProp'] -eq $null
True
PS C:\src\PowerShell>

I'm not sure if that's a bug or By Design. Because you can't do much with PSDynamicMembers (for instance, the getter/setter just throws), maybe it is intentionally excluded when indexing directly in. But it does seem odd.

Copy link
Member

@daxian-dbw daxian-dbw Jun 13, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I see. That does seem odd. I will look into it tomorrow. Thanks for bringing it up!

Copy link
Member

@daxian-dbw daxian-dbw Jun 15, 2018

Choose a reason for hiding this comment

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

This is because DotNetAdapter.GetMember<T>(object obj, string memberName) doesn't return dynamic members. I think it makes sense to not return dynamic member from this method, and submit #7087 to clarify it in the code comments.

// GetDynamicMemberNames is only a hint, not a contract, so we'd want
// to attempt the binding whether it's in there or not.
x = new PSDynamicMember(_stringValue);
}
List<PSMemberInfo> temp = new List<PSMemberInfo>();
if (x != null)
{
Expand Down Expand Up @@ -165,10 +174,14 @@ internal List<MshExpression> ResolveNames(PSObject target, bool expand)
}
}
}
continue;
}
// it can be a property
if (member is PSPropertyInfo)
else if (member is PSPropertyInfo)
{
temporaryMemberList.Add(member);
}
// it can be a dynamic member
else if (member is PSDynamicMember)
{
temporaryMemberList.Add(member);
}
Expand Down Expand Up @@ -225,11 +238,13 @@ internal List<MshExpressionResult> GetValues(PSObject target, bool expand, bool

#region Private Members

private CallSite<Func<CallSite, object, object>> _getValueDynamicSite;

private MshExpressionResult GetValue(PSObject target, bool eatExceptions)
{
try
{
object result;
object result = null;

if (Script != null)
{
Expand All @@ -243,12 +258,16 @@ private MshExpressionResult GetValue(PSObject target, bool eatExceptions)
}
else
{
PSMemberInfo member = target.Properties[_stringValue];
if (member == null)
if (_getValueDynamicSite == null)
{
return new MshExpressionResult(null, this, null);
_getValueDynamicSite =
CallSite<Func<CallSite, object, object>>.Create(
PSGetMemberBinder.Get(
_stringValue,
classScope: (Type) null,
@static: false));
}
result = member.Value;
result = _getValueDynamicSite.Target.Invoke(_getValueDynamicSite, target);
}

return new MshExpressionResult(result, this, null);
Expand Down
Loading