15

Consider this code:

using System.Linq;

namespace ExtensionMethodIssue
{
    static class Program
    {
        static void Main(string[] args)
        {
            var a = new[] { 1 };
            var b = new[] { 1, 2 }.Where(a.Contains).ToList();
            var c = new[] { 1, 2 }.Where(i => a.Contains(i)).ToList();
        }
    }
}

The code compiles successfully. Then I'm adding the nuget package "itext7 7.0.4", and now the compilation fails because of:

//Error CS0122: 'KernelExtensions.Contains<TKey, TValue>(IDictionary<TKey, TValue>, TKey)' is inaccessible due to its protection level
var b = new[] { 1, 2, 3 }.Where(a.Contains).ToList();

// This is still ok.
var c = new[] { 1, 2, 3 }.Where(i => a.Contains(i)).ToList();

The reason is that the itext7 library has an internal class with extension methods in the global namespace (here it is).

internal static class KernelExtensions {
    public static bool Contains<TKey, TValue>(this IDictionary<TKey, TValue> dictionary, TKey key) {
        return dictionary.ContainsKey(key);
}
}

For some reason the compiler chooses an inaccessible extension method with an incompatible signature from the global namespace instead of the accessible extension method with a compatible signature from the LINQ namespace.

The question is: is this behavior expected in terms of the language specification or is it a bug in the compiler? And why does it fail only in the case with a "method group" and still work with i => a.Contains(i)?

Boann
  • 48,794
  • 16
  • 117
  • 146
Inok
  • 213
  • 2
  • 9
  • Method groups, the gift that keeps on giving. This is obviously a big, fat, stinking bug -- one that cannot be reproduced by adding a class of your own with that extension method signature in the project itself, so it's doubly painful (since the internals of another assembly should be the least of your concerns). – Jeroen Mostert Mar 29 '18 at 11:03
  • Oh, actually, it gets better -- the problem disappears if you declare your own `static class Foo { public static bool Contains(this IDictionary dictionary, TKey key) => throw new NotImplementedException(); };` (no, this method is not invoked, it just fixes overload resolution). Visibility or namespace of this class don't seem to matter. – Jeroen Mostert Mar 29 '18 at 11:13
  • Ah, no, addendum: if the method is accessible (`public` or `internal`), it's all good. If it's `private`, however, the compiler will complain that `Foo.Contains` is inaccessible. This may give a clue as to how things work cross-assembly, since the `internal` method of another assembly is likewise inaccessible -- I suspect a foolish consistency is at work here. The fact that `Contains` is considered at all isn't a bug, I think, due to how method groups work (types are not considered initially). Even `static bool Contains(this bool x)` will be considered. – Jeroen Mostert Mar 29 '18 at 11:21
  • 1
    Explicit type argument masks the issue because these two versions have different number of type arguments : `a.Contains` – Leonid Vasilev Mar 29 '18 at 11:46
  • 4
    I described your case in [C# compiler chooses inaccessible extension method over accessible one](https://github.com/dotnet/roslyn/issues/25813) Roslyn issue on GitHub. – Leonid Vasilev Mar 29 '18 at 12:55

1 Answers1

3

That is a compiler bug, an inaccessible function should not affect overload resolution.

As a general workaround use lambdas as an argument instead of a method groups because overload resolution seems to work fine with them. It is not obvious which is faster/more efficient in your particular scenario, micro-optimize as needed using relevant performance metrics.

In this particular case you can also use other extension methods like Enumerable.Intersect() if you are working with sets, and aren't concerned about duplicates, Enumerable.Join() or a simple loop.

For more information check:

Leonid Vasilev
  • 11,910
  • 4
  • 36
  • 50