From 5c2faaf02e26fb014c0c566ee138def3acc3c6a7 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 30 Aug 2018 13:54:26 -0400 Subject: [PATCH] Enable indexing operations on objects that implement `ITuple` (#7633) This change enables index operations on objects that implement `ITuple` and other interfaces that have `DefaultMemberAttribute` declared, including slicing and negative indexing. --- .../engine/runtime/Binding/Binders.cs | 72 +++++++++++++++---- .../Language/Scripting/Indexer.Tests.ps1 | 16 +++++ 2 files changed, 73 insertions(+), 15 deletions(-) diff --git a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs index ea33af93482..67e2fb8b899 100644 --- a/src/System.Management.Automation/engine/runtime/Binding/Binders.cs +++ b/src/System.Management.Automation/engine/runtime/Binding/Binders.cs @@ -3928,6 +3928,8 @@ public override DynamicMetaObject FallbackGetIndex(DynamicMetaObject target, Dyn return GetIndexArray(target, indexes, errorSuggestion).WriteToDebugLog(this); } + var defaultMember = target.LimitType.GetCustomAttributes(true).FirstOrDefault(); + PropertyInfo lengthProperty = null; foreach (var i in target.LimitType.GetInterfaces()) { if (i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IDictionary<,>)) @@ -3938,12 +3940,23 @@ public override DynamicMetaObject FallbackGetIndex(DynamicMetaObject target, Dyn return result.WriteToDebugLog(this); } } + + // If the type explicitly implements an indexer specified by an interface + // then the DefaultMemberAttribute will not carry over to the implementation. + // This check will catch those cases. + if (defaultMember == null) + { + defaultMember = i.GetCustomAttributes(inherit: false).FirstOrDefault(); + if (defaultMember != null) + { + lengthProperty = i.GetProperty("Count") ?? i.GetProperty("Length"); + } + } } - var defaultMember = target.LimitType.GetCustomAttributes(true).FirstOrDefault(); if (defaultMember != null) { - return InvokeIndexer(target, indexes, errorSuggestion, defaultMember.MemberName).WriteToDebugLog(this); + return InvokeIndexer(target, indexes, errorSuggestion, defaultMember.MemberName, lengthProperty).WriteToDebugLog(this); } return errorSuggestion ?? CannotIndexTarget(target, indexes).WriteToDebugLog(this); @@ -4027,9 +4040,29 @@ private DynamicMetaObject GetIndexDictionary(DynamicMetaObject target, bindingRestrictions); } - internal static bool CanIndexFromEndWithNegativeIndex(DynamicMetaObject target) - { - var limitType = target.LimitType; + internal static bool CanIndexFromEndWithNegativeIndex( + DynamicMetaObject target, + MethodInfo indexer, + ParameterInfo[] getterParams) + { + // PowerShell supports negative indexing for types that meet the following criteria: + // - Indexer method accepts one parameter that is typed as int + // - The int parameter is not a type argument from a constructed generic type + // (this is to exclude indexers for types that could use a negative index as + // a valid key like System.Linq.ILookup) + // - Declares a "Count" or "Length" property + // - Does not inherit from IDictionary<> as that is handled earlier in the binder + // For those types, generate special code to check for negative indices, otherwise just generate + // the call. Before we test for the above criteria explicitly, we will determine if the + // target is of a type known to be compatible. This is done to avoid the call to Module.ResolveMethod + // when possible. + + if (getterParams.Length != 1 || getterParams[0].ParameterType != typeof(int)) + { + return false; + } + + Type limitType = target.LimitType; if (limitType.IsArray || limitType == typeof(string) || limitType == typeof(StringBuilder)) { return true; @@ -4046,7 +4079,16 @@ internal static bool CanIndexFromEndWithNegativeIndex(DynamicMetaObject target) } // target implements IList? - return limitType.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>)); + if (limitType.GetInterfaces().Any(i => i.IsGenericType && i.GetGenericTypeDefinition() == typeof(IList<>))) + { + return true; + } + + // Get the base method definition of the indexer to determine if the int + // parameter is a generic type parameter. Module.ResolveMethod is used + // because the indexer could be a method from a constructed generic type. + MethodBase baseMethod = indexer.Module.ResolveMethod(indexer.MetadataToken); + return !baseMethod.GetParameters()[0].ParameterType.IsGenericParameter; } private DynamicMetaObject IndexWithNegativeChecks( @@ -4055,8 +4097,6 @@ private DynamicMetaObject IndexWithNegativeChecks( PropertyInfo lengthProperty, Func generateIndexOperation) { - Diagnostics.Assert(CanIndexFromEndWithNegativeIndex(target), "Unexpected target type to index from end with negative value"); - // Generate: // try { // len = obj.Length @@ -4197,7 +4237,8 @@ private DynamicMetaObject GetIndexMultiDimensionArray(DynamicMetaObject target, private DynamicMetaObject InvokeIndexer(DynamicMetaObject target, DynamicMetaObject[] indexes, DynamicMetaObject errorSuggestion, - string methodName) + string methodName, + PropertyInfo lengthProperty) { MethodInfo getter = PSInvokeMemberBinder.FindBestMethod(target, indexes, "get_" + methodName, false, _constraints); @@ -4258,13 +4299,14 @@ private DynamicMetaObject InvokeIndexer(DynamicMetaObject target, target.CombineRestrictions(indexes)); } - if (getterParams.Length == 1 && getterParams[0].ParameterType == typeof(int) && CanIndexFromEndWithNegativeIndex(target)) + if (CanIndexFromEndWithNegativeIndex(target, getter, getterParams)) { - // PowerShell supports negative indexing for some types (specifically, types implementing IList or IList). - // For those types, generate special code to check for negative indices, otherwise just generate - // the call. - PropertyInfo lengthProperty = target.LimitType.GetProperty("Count") ?? - target.LimitType.GetProperty("Length"); // for string + if (lengthProperty == null) + { + // Count is declared by most supported types, Length will catch some edge cases like strings. + lengthProperty = target.LimitType.GetProperty("Count") ?? + target.LimitType.GetProperty("Length"); + } if (lengthProperty != null) { diff --git a/test/powershell/Language/Scripting/Indexer.Tests.ps1 b/test/powershell/Language/Scripting/Indexer.Tests.ps1 index ded67dbdd01..bfc6656bb4a 100644 --- a/test/powershell/Language/Scripting/Indexer.Tests.ps1 +++ b/test/powershell/Language/Scripting/Indexer.Tests.ps1 @@ -24,4 +24,20 @@ Describe 'Tests for indexers' -Tags "CI" { $service = Get-WmiObject -List -Amended Win32_Service $service.Properties["Hello There"] | Should -BeNullOrEmpty } + + It 'ITuple implementations can be indexed' { + $tuple = [Tuple]::Create(10, 'Hello') + $tuple[0] | Should -Be 10 + $tuple[1] | Should -BeExactly 'Hello' + } + + It 'ITuple objects can be spliced' { + $tuple = [Tuple]::Create(10, 'Hello') + $tuple[0..1] | Should -Be @(10, 'Hello') + } + + It 'Index of -1 should return the last item for ITuple objects' { + $tuple = [Tuple]::Create(10, 'Hello') + $tuple[-1] | Should -BeExactly 'Hello' + } }