Powershell: More specific method (indexer) overloads are not considered during overload resolution if they're part of an explicit interface implementation

[mklement0]

Please see #10688

---

If a given type implements the following method overloads:

• one with an [object]-typed parameter
• one with an [int]-typed parameter

the _more specifically typed_ [int] overload should take precedence over the [object]-typed one, if an [int] instance is passed on invocation, but that is not _guaranteed_ to be the case and currently depends on the _implementation details_ of the type at hand.

The problem is that a more specific overload that is part of an _explicit interface implementation_ isn't considered if a match was found as a _direct type member_. (#7633 is related, but doesn't solve this problem, because it only consults the explicit interfaces if no direct member is found.) See @SeeminglyScience's comment below.

Note: A real-world example are the two indexers (parameterized .Item property) overloads that Json.NET's [JObject] type exposes: one from the type directly, for _named_ indexing (property names), and one from the implementation of the IList[JToken] interface, for _numeric_ indexing - the latter is ignored.

The .Item() overloads are:

PS> ([Newtonsoft.Json.Linq.JObject]::Parse('{"foo": "bar"}') | Get-Member Item).Definition -split ', '
Newtonsoft.Json.Linq.JToken Item(System.Object key) {get;set;}
Newtonsoft.Json.Linq.JToken IList[JToken].Item(int index) {get;set;}

The only - impractical - workaround at the moment is to use _reflection_:

# Get the interface's parameterized [int]-parameter .Item() property that underlies the indexer.
$intIndexer = [System.Collections.Generic.IList[Newtonsoft.Json.Linq.JToken]].GetProperty('Item')

$obj = [Newtonsoft.Json.Linq.JObject]::Parse('{"foo": "bar"}')

# Call the indexer with an [int]
$intIndexer.GetValue($obj, 0).ToString()

Steps to reproduce

Add-Type -TypeDefinition @'

  // Interface with NUMERIC indexer.
  public interface IFoo {
    string this[int index] { get; }
  }

  // Class with [object] indexer and IMPLICIT IFoo implementation.
  public class FooImplicit : IFoo {
    public string this[object key] {
      get => $"item with KEY {key}";
    }
    public string this[int index] {
      get => $"item with INTEGER {index}";
    }
  }

  // Class with [object] indexer and EXPLICIT IFoo implementation.
  public class FooExplicit : IFoo {
    public string this[object key] {
      get => $"item with KEY {key}";
    }
    string IFoo.this[int index] {
      get => $"item with INTEGER {index}";
    }
  }
'@

# OK - direct-member [int] overload takes precedence.
[FooImplicit]::new()[1] | Should -Match 'INTEGER'

# BROKEN - [int] overload from explicit interface method implementation is
#          NOT used.
[FooExplicit]::new()[1] | Should -Match 'INTEGER'

# OK - explicit cast to interface [IFoo], 
# !! but note that the same approach does NOT work with [JObject]
([IFoo] [FooExplicit]::new())[1] | Should -Match 'INTEGER'

Expected behavior

All tests should pass.

Actual behavior

The 2nd test fails with:

Expected regular expression 'INTEGER' to match 'item with KEY 1', but it did not match.

That is, the more specific [int]-typed overload from the _explicit_ interface implementation was not called.

Environment data

PowerShell Core 7.0.0-preview.4

[SeeminglyScience]

I believe this is more of a general problem with method resolution. The first matching overload seems to be the one picked, making the indexer explicit just changes the order in which the Adapter retrieves them (explicitly implemented methods need to be retrieved separately from the initial Type.GetMethods call).

Another example that isn't indexer related is Marshal.SizeOf(Type).

```powershell
PS> [Runtime.InteropServices.Marshal]::SizeOf

OverloadDefinitions

-------------------

static int SizeOf(System.Object structure)

static int SizeOfT

static int SizeOf(type t)

static int SizeOfT

This picks SizeOf(Object) instead of SizeOf(Type)

PS> [Runtime.InteropServices.Marshal]::SizeOf([datetime])

Exception calling "SizeOf" with "1" argument(s): "Type 'System.RuntimeType' cannot be marshaled

as an unmanaged structure; no meaningful size or offset can be computed."

At line:1 char:1

+ [Runtime.InteropServices.Marshal]::SizeOf([datetime])

+ ~~~~~~~~~~~~~

+ CategoryInfo : NotSpecified: (:) [], MethodInvocationException

+ FullyQualifiedErrorId : ArgumentException

[lzybkr]

@SeeminglyScience - I thought the problem with Marshal.SizeOf is the generic. I believe PowerShell is trying to invoke Marshal.SizeOf<System.RuntimeType>([datetime]) and it shouldn't be trying that overload because RuntimeType is not a public type.

[mklement0]

Thanks, @SeeminglyScience and @lzybkr.

Does @SeeminglyScience's basic point still stand, however? I've updated the OP - please let me know if it now accurately describes the problem without incorrectly inferring causes.

[SeeminglyScience]

I thought the problem with Marshal.SizeOf is the generic.

@lzybkr Yep that's my bad, since this works:

Add-Type -TypeDefinition '
public static class OverloadTesting
{
    public static int SizeOf(object structure) { return 1; }

    public static int SizeOf(System.Type t) { return 2; }
}'

PS> [OverloadTesting]::([int])
# 2

I believe PowerShell is trying to invoke Marshal.SizeOf([datetime]) and it shouldn't be trying that overload because RuntimeType is not a public type.

Even if it fell back to TypeInfo it would still act the same right? What overload should win between a single generic argument and an exact type match. And can that even be safely changed? Probably off topic now though.

[SeeminglyScience]

Thanks, @SeeminglyScience and @lzybkr.

Does @SeeminglyScience's basic point still stand, however? I've updated the OP - please let me know if it now accurately describes the problem without incorrectly inferring causes.

They are similar problems, but I don't think they're strictly related like I initially thought. Fixing one probably won't fix the other.

[mklement0]

And do we have an issue for the related generics problem yet? If not, would you mind creating one, @SeeminglyScience?

[SeeminglyScience]

@mklement0 oh you know what, this really should have clicked for me immediately. I added support for indexers that are explicitly implemented interface methods, that didn't work before #7633 (it says ITuple in the title, but ended up being a general fix).

Looking for explicit interface implementations is only done as a fall back because it's pretty expensive (you need iterate and reflect on all declared interfaces).

[mklement0]

I was wondering about performance implications.

It's not clear to me from glancing at #7633: are you saying that it fixes this problem also, or that it doesn't, because if a matching direct-member indexer is present, the explicit interfaces aren't consulted?

[SeeminglyScience]

@mklement0 Yeah it doesn't fix this issue, but before if there was an object that only had an explicitly implemented indexer (like all Tuple's), it just wouldn't find an indexer. My PR made it so if it doesn't find any indexer, it then checks for explicitly implemented indexers.

Personally I think it's probably fine to always check so it can find the best "match", but part of me also thinks that this is the more "correct" route. I say that because in order for you to invoke that indexer in C# you'd have to do something like this:

JObject obj = GetJObjectSomehow();
((IList<JToken>)obj)[10];

The point of explicitly implementing something is typically to hide it except in specific scenarios. Typically it's more advisable to use the implicit implementation. Ideally you'd be able to do something like that in PowerShell:

([IList[JToken]]$obj)[10]

But unfortunately you can't really cast as something that isn't a concrete type. Well you can, but it won't actually effect anything since $obj technically already is IList[JToken] and PowerShell doesn't remember that you cast that to customize binding.

Also worth noting that JObject implements IDynamicMetaObjectProvider which means in most cases PowerShell is going to let JObject decide how it's bound. This problem does exist in PowerShell's binder as well, but PowerShell might also defer binding to JObject in this case.

[mklement0]

Good stuff, @SeeminglyScience, thanks.

I do see the point re requiring an explicit action to call the explicitly implemented interface method in general, and in C# it makes perfect sense, given that a simple cast will do that.

Unfortunately, as you point out, we don't have that option in PowerShell - and the workaround via reflection, as now shown in the OP, is quite cumbersome.

More generally, interfaces are not on the average PowerShell user's mind, if I were to guess, and seeing a method / property in Get-Member's output for a given type leads to a reasonable expectation that it can actually be called. The conditions under which they can't are far from obvious.

That said, perhaps this is too exotic a scenario to worry about - especially if the fix would come with an always-paid performance penalty - so perhaps the pragmatic solution is to simply _document_ the issue.

@lzybkr, any thoughts?

---

On a side note, @SeeminglyScience, since you mention IDynamicMetaObjectProvider:

I've noticed that, for example, [JObject] instances do not have the usual hidden properties such as .psobject and .pstypenames; e.g., [Newtonsoft.Json.Linq.JObject]::Parse('{"foo":1}').pstypenames yields $null.

Is this (a) due to delegating to IDynamicMetaObjectProvider, and (b) as designed?

[SeeminglyScience]

Is this (a) due to delegating to IDynamicMetaObjectProvider

Yeah

and (b) as designed?

Probably not

[lzybkr]

@SeeminglyScience Regarding

But unfortunately you can't really cast as something that isn't a concrete type

PSMethodInvocationConstraints is used for exactly this scenario - the cast is meaningless other than to guide overload resolution.

[SeeminglyScience]

Yeah I thought that was used exclusively in PowerShell classes as an analog to C#'s base.Method() syntax. Is it hooked up in other spots? Any time I've tried to do it it hasn't worked for me, but if it's supposed to work then I probably need to file some issues

[lzybkr]

@SeeminglyScience - I believe the invocation constraints are wired through everywhere they need to be. See here where explicit method impls are explicitly tested.

[mklement0]

Using the cast approach actually _does_ work with the repro code in the OP, so again there must be something special about the Json.NET types involved:

using namespace System.Collections.Generic
using namespace Newtonsoft.Json.Linq

# OK, due to the cast to [IFoo]
([IFoo] [FooExplicit]::new())[1] | Should -Match 'INTEGER'

# !! BROKEN when the same approach is applied to  [JObject] / [IList[JToken]]:
# !! returns $null.
([IList[JToken]] [JObject]::Parse('{"foo":1}'))[0] | Should -BeOfType ([JProperty])

I don't know if it's relevant, but using the name-based indexer, which works in principle, breaks the Should call, with an error that I've also seen in #10650 and #10652, Target type System.Collections.IEnumerator is not a value type or a non-abstract class. (Parameter 'targetType')

# !! Indexing works, but the result breaks the Should call.
([JObject]::Parse('{"foo":1}'))['foo'] | Should -BeOfType ([JProperty])

[SeeminglyScience]

@lzybkr oh neat! Now that I think about it, I've probably only tried that on non-IDispatch COM objects. Learn something new every day 🙂 Thanks!

@mklement0

Using the cast approach actually does work with the repro code in the OP, so again there must be something special about the Json.NET types involved:

Yeah probably IDMOP. That's also where the Target type System.Collections.IEnumerator is not a value type or a non-abstract class error comes from. If you look at the call stack, you can see their convert method being called.

[mklement0]

Thanks, @SeeminglyScience; please confirm my current understanding:

• If an overload match is found as a direct member, explicit interface implementations aren't consulted for more specific matches - this is by design, not least because changing this would be a performance concern.

• The solution is to explicitly cast to the interface in order to reliably target the overload of interest.

• The latter is currently broken in some situations, presumably relating to types that implement IDynamicMetaObjectProvider, so a fix is needed.

---

If the above is correct, it probably makes sense to close this issue and create a new, more focused one.

While I'm happy to take a stab at it, I wonder if it makes sense for you to do it, given that I am out of my depth here with respect to the implementation.

I also wonder, whether the following issues could / should be folded into the new one, as they all seem related; at the very least it's worth linking to them:

• #10650
• #10652
• #10668

[SeeminglyScience]

If an overload match is found as a direct member, explicit interface implementations aren't consulted for more specific matches - this is by design, not least because changing this would be a performance concern.

Just to be clear, I did purposefully add the check for explicit implementations only as a fallback, but truthfully I wasn't considering the scenario of calculating the best overload to use. It was more of a "if we already found one, we probably don't need to find more" type of thing. Would it be a performance hit? Yeah for sure. Would it be impactful? Ehhhh only benchmarks could tell. The binder is cached so I would think it would be pretty minimal.

What I'm trying to get at is that saying it's by design on my part would be generous. I think it mostly makes sense the way it is, or rather not particularly worth changing. But if you disagree I encourage you to switch it up in the binder and run some benchmarks.

The latter is currently broken in some situations, presumably relating to types that implement IDynamicMetaObjectProvider, so a fix is needed.

A fix is needed for sure, what's unclear is who needs to fix what. It seems bizarre to me that their convert binder wouldn't account for interfaces. Maybe PowerShell should swallow the exception depending on where it happens. Maybe it should approach conversions for IDMOP more delicately, but imo that's kind of the point of IDMOP, especially in PowerShell. My gut reaction is that the fix should be in Json.NET but I haven't looked closely enough at the PowerShell side to see if there's something better it could do there.

[mklement0]

Understood re design intent and thanks for clarifying, @SeeminglyScience.

Now that I understand the issue better and now that I know that casting to an interface _generally_ works, I'm not personally interested in pursuing this aspect further.

Instead I've created a docs issue at https://github.com/MicrosoftDocs/PowerShell-Docs/issues/4871

I've opened a new issue focused on fixing the broken interface casting at #10688

I'm closing this.