From 6a7026182864f56fdf1a8e3f0370363a1037dec6 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 14 Mar 2018 16:36:54 -0700 Subject: [PATCH 1/3] [Feature] Fix 'PropertyOnlyAdapter' to allow calling base methods --- .../engine/CoreAdapter.cs | 36 +++++++++++++-- .../engine/runtime/Binding/Binders.cs | 13 +++--- test/powershell/engine/ETS/Adapter.Tests.ps1 | 46 +++++++++++++++++++ 3 files changed, 86 insertions(+), 9 deletions(-) diff --git a/src/System.Management.Automation/engine/CoreAdapter.cs b/src/System.Management.Automation/engine/CoreAdapter.cs index 4fbb5f662df..293429fb841 100644 --- a/src/System.Management.Automation/engine/CoreAdapter.cs +++ b/src/System.Management.Automation/engine/CoreAdapter.cs @@ -37,6 +37,19 @@ namespace System.Management.Automation /// internal abstract class Adapter { + /// + /// The member type that a callsite binder operates on. + /// + /// + /// An adapter can decide whether the binder can optimize the operations + /// on an object member based on the type of the member. + /// + internal enum SiteBinderOperatesOn + { + Property, + Method, + } + /// /// tracer for this and derivate classes /// @@ -46,7 +59,7 @@ internal abstract class Adapter #region member - internal virtual bool SiteBinderCanOptimize { get { return false; } } + internal virtual bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) { return false; } protected static IEnumerable GetDotNetTypeNameHierarchy(Type type) { @@ -3519,7 +3532,7 @@ private static bool PropertyIsStatic(PSProperty property) #region member - internal override bool SiteBinderCanOptimize { get { return true; } } + internal override bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) { return true; } private static ConcurrentDictionary s_typeToTypeNameDictionary = new ConcurrentDictionary(); @@ -4545,7 +4558,24 @@ protected override PSMemberInfoInternalCollection GetMembers(object obj) /// internal abstract class PropertyOnlyAdapter : DotNetAdapter { - internal override bool SiteBinderCanOptimize { get { return false; } } + /// + /// For a PropertyOnlyAdapter, the property may come from various sources, + /// but methods, including parameterized properties, still come from DotNetAdapter. + /// So, the binder can optimize on method calls for objects that map to a + /// custom PropertyOnlyAdapter. + /// + internal override bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) + { + switch (opType) + { + case SiteBinderOperatesOn.Property: + return false; + case SiteBinderOperatesOn.Method: + return true; + default: + throw new InvalidOperationException("Should be unreachable. Update this code if new members are added to 'SiteBinderOperation'"); + } + } protected override ConsolidatedString GetInternedTypeNameHierarchy(object obj) { diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index d35d139ed40..b3156a0e00a 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -5053,7 +5053,7 @@ public override DynamicMetaObject FallbackGetMember(DynamicMetaObject target, Dy bool canOptimize; Type aliasConversionType; - memberInfo = GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType); + memberInfo = GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Property); if (!canOptimize) { @@ -5415,8 +5415,8 @@ private PSMemberInfo ResolveAlias(PSAliasProperty alias, DynamicMetaObject targe return null; } - PSMemberInfo result = binder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, aliases, aliasRestrictions); - + PSMemberInfo result = binder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, + Adapter.SiteBinderOperatesOn.Property, aliases, aliasRestrictions); return result; } @@ -5424,6 +5424,7 @@ internal PSMemberInfo GetPSMemberInfo(DynamicMetaObject target, out BindingRestrictions restrictions, out bool canOptimize, out Type aliasConversionType, + Adapter.SiteBinderOperatesOn binderOperatesOn, HashSet aliases = null, List aliasRestrictions = null) { @@ -5493,7 +5494,7 @@ internal PSMemberInfo GetPSMemberInfo(DynamicMetaObject target, var adapterSet = PSObject.GetMappedAdapter(value, typeTable); if (memberInfo == null) { - canOptimize = adapterSet.OriginalAdapter.SiteBinderCanOptimize; + canOptimize = adapterSet.OriginalAdapter.CanSiteBinderOptimize(binderOperatesOn); // Don't bother looking for the member if we're not going to use it. if (canOptimize) { @@ -5969,7 +5970,7 @@ public override DynamicMetaObject FallbackSetMember(DynamicMetaObject target, Dy BindingRestrictions restrictions; bool canOptimize; Type aliasConversionType; - memberInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType); + memberInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Property); restrictions = restrictions.Merge(value.PSGetTypeRestriction()); @@ -6464,7 +6465,7 @@ public override DynamicMetaObject FallbackInvokeMember(DynamicMetaObject target, BindingRestrictions restrictions; bool canOptimize; Type aliasConversionType; - var methodInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType) as PSMethodInfo; + var methodInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Method) as PSMethodInfo; restrictions = args.Aggregate(restrictions, (current, arg) => current.Merge(arg.PSGetMethodArgumentRestriction())); // If the process has ever used ConstrainedLanguage, then we need to add the language mode diff --git a/test/powershell/engine/ETS/Adapter.Tests.ps1 b/test/powershell/engine/ETS/Adapter.Tests.ps1 index 5dc7794ef54..2293ee00865 100644 --- a/test/powershell/engine/ETS/Adapter.Tests.ps1 +++ b/test/powershell/engine/ETS/Adapter.Tests.ps1 @@ -315,3 +315,49 @@ Describe "DataRow and DataRowView Adapter tests" -tags "CI" { } } } + +Describe "Base method call on object mapped to PropertyOnlyAdapter should work" -tags "CI" { + It "Base method call on object of a subclass of 'XmlDocument' -- Add-Type" { + $code =@' +namespace BaseMethodCallTest.OnXmlDocument { + public class Foo : System.Xml.XmlDocument { + public string MyName { get; set; } + public override void LoadXml(string content) { + MyName = content; + } + } +} +'@ + try { + $null = [BaseMethodCallTest.OnXmlDocument.Foo] + } catch { + Add-Type -TypeDefinition $code + } + + $foo = [BaseMethodCallTest.OnXmlDocument.Foo]::new() + $foo.LoadXml('bar') + $foo.MyName | Should -Be 'bar' + $foo.ChildNodes.Count | Should -Be 0 + + ([System.Xml.XmlDocument]$foo).LoadXml('bar') + $foo.test | Should -Be 'bar' + $foo.ChildNodes.Count | Should -Be 1 + } + + It "Base method call on object of a subclass of 'XmlDocument' -- PowerShell Class" { + class XmlDocChild : System.Xml.XmlDocument { + [string] $MyName + [void] LoadXml([string]$content) { + $this.MyName = $content + # Try to call the base type's .LoadXml() method. + ([System.Xml.XmlDocument] $this).LoadXml($content) + } + } + + $child = [XmlDocChild]::new() + $child.LoadXml('bar') + $child.MyName | Should -Be 'bar' + $child.test | Should -Be 'bar' + $child.ChildNodes.Count | Should -Be 1 + } +} From 3905a8fa584aaf18caf5093856d926a3f552e816 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 14 Mar 2018 19:06:41 -0700 Subject: [PATCH 2/3] Use 'System.Reflection.MemberTypes' instead of 'SiteBinderOperatesOn' --- .../engine/CoreAdapter.cs | 27 +++++-------------- .../engine/runtime/Binding/Binders.cs | 12 ++++----- 2 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/System.Management.Automation/engine/CoreAdapter.cs b/src/System.Management.Automation/engine/CoreAdapter.cs index 293429fb841..1092fa290f3 100644 --- a/src/System.Management.Automation/engine/CoreAdapter.cs +++ b/src/System.Management.Automation/engine/CoreAdapter.cs @@ -37,19 +37,6 @@ namespace System.Management.Automation /// internal abstract class Adapter { - /// - /// The member type that a callsite binder operates on. - /// - /// - /// An adapter can decide whether the binder can optimize the operations - /// on an object member based on the type of the member. - /// - internal enum SiteBinderOperatesOn - { - Property, - Method, - } - /// /// tracer for this and derivate classes /// @@ -59,7 +46,7 @@ internal enum SiteBinderOperatesOn #region member - internal virtual bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) { return false; } + internal virtual bool CanSiteBinderOptimize(MemberTypes typeToOperateOn) { return false; } protected static IEnumerable GetDotNetTypeNameHierarchy(Type type) { @@ -3532,7 +3519,7 @@ private static bool PropertyIsStatic(PSProperty property) #region member - internal override bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) { return true; } + internal override bool CanSiteBinderOptimize(MemberTypes typeToOperateOn) { return true; } private static ConcurrentDictionary s_typeToTypeNameDictionary = new ConcurrentDictionary(); @@ -4564,16 +4551,16 @@ internal abstract class PropertyOnlyAdapter : DotNetAdapter /// So, the binder can optimize on method calls for objects that map to a /// custom PropertyOnlyAdapter. /// - internal override bool CanSiteBinderOptimize(SiteBinderOperatesOn opType) + internal override bool CanSiteBinderOptimize(MemberTypes typeToOperateOn) { - switch (opType) + switch (typeToOperateOn) { - case SiteBinderOperatesOn.Property: + case MemberTypes.Property: return false; - case SiteBinderOperatesOn.Method: + case MemberTypes.Method: return true; default: - throw new InvalidOperationException("Should be unreachable. Update this code if new members are added to 'SiteBinderOperation'"); + throw new InvalidOperationException("Should be unreachable. Update code if other member types need to be handled here."); } } diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index b3156a0e00a..3d376e52556 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -5053,7 +5053,7 @@ public override DynamicMetaObject FallbackGetMember(DynamicMetaObject target, Dy bool canOptimize; Type aliasConversionType; - memberInfo = GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Property); + memberInfo = GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, MemberTypes.Property); if (!canOptimize) { @@ -5416,7 +5416,7 @@ private PSMemberInfo ResolveAlias(PSAliasProperty alias, DynamicMetaObject targe } PSMemberInfo result = binder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, - Adapter.SiteBinderOperatesOn.Property, aliases, aliasRestrictions); + MemberTypes.Property, aliases, aliasRestrictions); return result; } @@ -5424,7 +5424,7 @@ internal PSMemberInfo GetPSMemberInfo(DynamicMetaObject target, out BindingRestrictions restrictions, out bool canOptimize, out Type aliasConversionType, - Adapter.SiteBinderOperatesOn binderOperatesOn, + MemberTypes memberTypeToOperateOn, HashSet aliases = null, List aliasRestrictions = null) { @@ -5494,7 +5494,7 @@ internal PSMemberInfo GetPSMemberInfo(DynamicMetaObject target, var adapterSet = PSObject.GetMappedAdapter(value, typeTable); if (memberInfo == null) { - canOptimize = adapterSet.OriginalAdapter.CanSiteBinderOptimize(binderOperatesOn); + canOptimize = adapterSet.OriginalAdapter.CanSiteBinderOptimize(memberTypeToOperateOn); // Don't bother looking for the member if we're not going to use it. if (canOptimize) { @@ -5970,7 +5970,7 @@ public override DynamicMetaObject FallbackSetMember(DynamicMetaObject target, Dy BindingRestrictions restrictions; bool canOptimize; Type aliasConversionType; - memberInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Property); + memberInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, MemberTypes.Property); restrictions = restrictions.Merge(value.PSGetTypeRestriction()); @@ -6465,7 +6465,7 @@ public override DynamicMetaObject FallbackInvokeMember(DynamicMetaObject target, BindingRestrictions restrictions; bool canOptimize; Type aliasConversionType; - var methodInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, Adapter.SiteBinderOperatesOn.Method) as PSMethodInfo; + var methodInfo = _getMemberBinder.GetPSMemberInfo(target, out restrictions, out canOptimize, out aliasConversionType, MemberTypes.Method) as PSMethodInfo; restrictions = args.Aggregate(restrictions, (current, arg) => current.Merge(arg.PSGetMethodArgumentRestriction())); // If the process has ever used ConstrainedLanguage, then we need to add the language mode From acb5a33064c4a4e3be8801b0f5d809357e9b7840 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 15 Mar 2018 08:41:01 -0700 Subject: [PATCH 3/3] Update test to use '-BeExactly' for string comparison --- test/powershell/engine/ETS/Adapter.Tests.ps1 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/powershell/engine/ETS/Adapter.Tests.ps1 b/test/powershell/engine/ETS/Adapter.Tests.ps1 index 2293ee00865..ba4b1a9852c 100644 --- a/test/powershell/engine/ETS/Adapter.Tests.ps1 +++ b/test/powershell/engine/ETS/Adapter.Tests.ps1 @@ -336,11 +336,11 @@ namespace BaseMethodCallTest.OnXmlDocument { $foo = [BaseMethodCallTest.OnXmlDocument.Foo]::new() $foo.LoadXml('bar') - $foo.MyName | Should -Be 'bar' + $foo.MyName | Should -BeExactly 'bar' $foo.ChildNodes.Count | Should -Be 0 ([System.Xml.XmlDocument]$foo).LoadXml('bar') - $foo.test | Should -Be 'bar' + $foo.test | Should -BeExactly 'bar' $foo.ChildNodes.Count | Should -Be 1 } @@ -356,8 +356,8 @@ namespace BaseMethodCallTest.OnXmlDocument { $child = [XmlDocChild]::new() $child.LoadXml('bar') - $child.MyName | Should -Be 'bar' - $child.test | Should -Be 'bar' + $child.MyName | Should -BeExactly 'bar' + $child.test | Should -BeExactly 'bar' $child.ChildNodes.Count | Should -Be 1 } }