Improve overload resolution to avoid picking non-applicable overload in case with type inference and generics

[divega]

I suspect this may be one of the cases described in #98 but since I am not sure, I prefer to give the language team a chance to decide. Regardless, this may be a useful scenario to consider supporting.

Background

While exploring alternatives to enable support for eager loading with properties defined in derived types in Entity Frameowrk Core, we came up with an API that can be used in the following way (note the lambda in the call to ThenInclude):

var query = context.Levels.Include(l => l.Items).ThenInclude((Container c) => c.Items);

In this query, Level.Items is a collection of Item and Container derives from Item. Container in turn defines a collection property Items which can contain nested Item instances.

We tested separately two new overloads of ThenInclude:

• one that allows addressing properties declared on derived types of the collection element of a collection navigation property:

        // ThenInclude-collection-inheritance
        public static IIncludableQueryable<TEntity, TProperty> 
            ThenInclude<TEntity, TPreviousProperty, TDerived, TProperty>(
                this IIncludableQueryable<TEntity, IEnumerable<TPreviousProperty>> source,
                Expression<Func<TDerived, TProperty>> navigationPropertyPath)
                where TEntity : class
                where TDerived : TPreviousProperty =>
                    default(IIncludableQueryable<TEntity, TProperty>);

• one for derived types on reference navigation properties:

       // ThenInclude-reference-inheritance
       public static IIncludableQueryable<TEntity, TProperty> 
           ThenInclude<TEntity, TPreviousProperty, TDerived, TProperty>(
               this IIncludableQueryable<TEntity, TPreviousProperty> source,
               Expression<Func<TDerived, TProperty>> navigationPropertyPath)
               where TEntity : class
               where TDerived : TPreviousProperty =>
                   default(IIncludableQueryable<TEntity, TProperty>);

When we tested these two new overloads separately they worked correctly. We liked the pattern because type inferences allows customers to write a very simple lambda and skip specifying all the generic arguments of the method.

However, when we did more complete testing, we found that if both overloads are declared, resolution for the collection overload fails (see dotnet/efcore#10441 for the original issue) . This seems to be caused by a limitation in overload resolution.

Compiler error

The code produces de following compiler error:

CS0311 The type 'ThenIncludeOverloadResolutionBug.Container' cannot be used as type parameter 'TDerived' in the generic type or method 'EntityFrameworkQuerybleExtensions.ThenInclude<TEntity, TPreviousProperty, TDerived, TProperty>(IIncludableQueryable<TEntity, TPreviousProperty>, Expression<Func<TDerived, TProperty>>)'. There is no implicit reference conversion from 'ThenIncludeOverloadResolutionBug.Container' to 'System.Collections.Generic.ICollection<ThenIncludeOverloadResolutionBug.Item>'.

As the error indicates, the compiler is resolving to the overload labeled "ThenInclude-reference-inheritance" which is actually not applicable in this case. If you comment out or rename that overload, then the compiler is able to resolve the correct overload (labeled "ThenInclude-collection-inheritance").

Repro code

This is a self-contained repro which includes a mock implementations of the APIs we tried on EF Core. We have already decided not to use this API pattern given the compiler has this limitation, but we wanted to give the language team a chance to decide if this is something that should be addressed. It seems like it should affect other fluent APIs with similar design to Include/ThenInclude in EF Core.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;

namespace ThenIncludeOverloadResolutionBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var levels = new List<Level>().AsQueryable();

            var query = levels
                .Include(l => l.Items)
                .ThenInclude((Container c) => c.Items); // error here 
        }
    }

    public class Level
    {
        public virtual int Id { get; set; }
        public virtual int GameId { get; set; }
        public virtual ICollection<Item> Items { get; set; } = new HashSet<Item>();
    }

    public class Item
    {
        public virtual int Id { get; set; }
        public virtual int GameId { get; set; }
        public virtual Level Level { get; set; }
    }

    public class Container : Item
    {
        public virtual ICollection<Item> Items { get; set; } = new HashSet<Item>();
    }


    public interface IIncludableQueryable<out TEntity, out TProperty> : IQueryable<TEntity>
    {
    }

    public static class EntityFrameworkQuerybleExtensions
    {
        // Include
        public static IIncludableQueryable<TEntity, TProperty> Include<TEntity, TProperty>(
            this IQueryable<TEntity> source,
            Expression<Func<TEntity, TProperty>> navigationPropertyPath)
            where TEntity : class =>
                default(IIncludableQueryable<TEntity, TProperty>);

        // Include-inheritance
        public static IIncludableQueryable<TEntity, TProperty> 
            Include<TEntity, TDerived, TProperty>(
                this IQueryable<TEntity> source,
                Expression<Func<TDerived, TProperty>> navigationPropertyPath)
                where TEntity : class
                where TDerived : TEntity =>
                    default(IIncludableQueryable<TEntity, TProperty>);

        // ThenInclude-reference
        public static IIncludableQueryable<TEntity, TProperty> 
            ThenInclude<TEntity, TPreviousProperty, TProperty>(
                this IIncludableQueryable<TEntity, TPreviousProperty> source,
                Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath)
                where TEntity : class =>
                    default(IIncludableQueryable<TEntity, TProperty>);

        // ThenInclude-collection
        public static IIncludableQueryable<TEntity, TProperty> 
            ThenInclude<TEntity, TPreviousProperty, TProperty>(
                this IIncludableQueryable<TEntity, IEnumerable<TPreviousProperty>> source,
                Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath)
                where TEntity : class =>
                    default(IIncludableQueryable<TEntity, TProperty>);

        // ThenInclude-reference-inheritance
        public static IIncludableQueryable<TEntity, TProperty> 
            ThenInclude<TEntity, TPreviousProperty, TDerived, TProperty>(
                this IIncludableQueryable<TEntity, TPreviousProperty> source,
                Expression<Func<TDerived, TProperty>> navigationPropertyPath)
                where TEntity : class
                where TDerived : TPreviousProperty =>
                    default(IIncludableQueryable<TEntity, TProperty>);

        // ThenInclude-collection-inheritance
        public static IIncludableQueryable<TEntity, TProperty> 
            ThenInclude<TEntity, TPreviousProperty, TDerived, TProperty>(
                this IIncludableQueryable<TEntity, IEnumerable<TPreviousProperty>> source,
                Expression<Func<TDerived, TProperty>> navigationPropertyPath)
                where TEntity : class
                where TDerived : TPreviousProperty =>
                    default(IIncludableQueryable<TEntity, TProperty>);

    }
}

[divega]

Also looks very similar to #1121.

[gafter]

Yes, this appears to be an example of #98. However, once #98 is implemented this use case should be tested.