roslyn: Completion missing members of lambda parameter in fault tolerance case

In the code below, Task should be shown in completion from one of the overloads of ThenInclude but it doesn’t.

This case is interesting because there are two applicable overloads. The ideal thing here would be to merge their individual members for the completion lists, but we don’t do that – we just pick one overload as the “best so far”, and it’s the one with ICollection<TPreviousProperty>, not the one with just TPreviousProperty.

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

namespace ThenIncludeIntellisenseBug
{
    class Program
    {
        static void Main(string[] args)
        {
            var registrations = new List<Registration>().AsQueryable();

            // type "a => a." and only ICollection<T> and Enumerable members appear
            var reg = registrations
              .Include(r => r.Activities).ThenInclude(a => a.$$
        }
    }

    internal class Registration
    {
        public ICollection<Activity> Activities { get; set; }
    }

    public class Activity
    {
        public Task Task { get; set; }
    }

    public class Task
    {
        public string Name { get; set; }
    }

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

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

        public static IIncludableQueryable<TEntity, TProperty> ThenInclude<TEntity, TPreviousProperty, TProperty>(
            this IIncludableQueryable<TEntity, ICollection<TPreviousProperty>> source,
            Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath) where TEntity : class
        {
            return default(IIncludableQueryable<TEntity, TProperty>);
        }

        public static IIncludableQueryable<TEntity, TProperty> ThenInclude<TEntity, TPreviousProperty, TProperty>(
            this IIncludableQueryable<TEntity, TPreviousProperty> source,
            Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath) where TEntity : class
        {
            return default(IIncludableQueryable<TEntity, TProperty>);
        }
    }
}

About this issue

  • Original URL
  • State: closed
  • Created 8 years ago
  • Reactions: 62
  • Comments: 113 (52 by maintainers)

Commits related to this issue

Most upvoted comments

It can be solved like: instead of .ThenInclude((Collection<ConcreteType> o) => o.Property)" put “.ThenInclude((ConcreteType o) => o.Property)”.

Running VS 2019 16.2.0 Preview 3.0 and issue still here. Is it planned for future preview release of 16.2, or needs to wait for final 16.2?

This still happens on Visual Studio 2019, version 16.4, with .NET Core 3.1 and EF Core 3.1. It’s incredible that I wasted a lot of time in a bug that still exists on Intellisense more than 4 years ago. Unbelievable.

Are there any updates on this issue? Ran into this today, cost me about an half an hour before I found it was just an autocomplete issue 😕

An issue for me too, intellisense doesn’t work for .Include(p => p.AccessIdentifiers).ThenInclude(p=>p.Identity)

Looking forward to a fix as it lost me quite some time.

Just found this issue. Just wanted to say I am also getting this problem. Using VS 2017 CE 15.3.3.

Also looking forward to a fix 😃

image

Same issue on a many to many relationship using VS2017 and .net core 2.2. Although when I type it as it should be, it compiles normally and the error goes away.

the same problem for me, I have the latest update for visual studio and i had this issue intellisense not working perfectly before I knew this is an intellisense problem i lost alot of time figuring out what the issue!

Greetings,

Today i’m hitting this issue. Do you have any news? Hope you find a simple solution to fix it. Thanks and keep the good work.

Doesn’t Microsoft have paid developers work on this stuff?

There are limited resources, and a near infinitely long backlog 😃 Everyone on the team has a full workload continuously. Basically, the same as every other project out there. The nice thing about being open source is that instead of being in the positoin where that means that you are beholden to that, you now have the power to contribute yourself if this really is that important to you. I’ve done this numerous times to roslyn for things that were not as important to the team as the stuff they were working on themselves.

As i mentioned already, i’m happy to help out another contributor here. My plate is currently full, so i can’t take it on myself. If you’re interested, i’d def work with you on this. If this that important to you then this would fast track getting things fixed 😃

If it isn’t that high a priority for you… well… then it’s easy to understand why it might not be for anyone on the Roslyn team either 😉 As i said above, when i encounter this myself, i try to contribute fixes/features. They’re happy to take them, and will work with you to facilitate that. This means i get the benefit of the features they are prioritizing, while also being able to address somewhat less important things for them that still matter a lot for me. That seems like a great position to be in, and far better than if this was not open source.

@gafter Given the code:

var reg = registrations
              .Include(r => r.Activities).ThenInclude(a => a.$$

When the IDE asks “what is the SymbolInfo for ‘a’?” I think it would be good if we could get a SymbolInfo with no Symbol, but two candidate symbols. One would be The parameter called 'a' with type 'ICollection<TPreviousProperty>' and the other would be The parameter called 'a' with type 'TPreviousProperty'

The IDE could then get the members of each and union them in a single list to show users.

@jmoralesv PRs welcome 😃

The problem still exists in Visual Studio 2019 and EF Core 3.1: For example I have the following entity clases:

class Cliente {
     public List<ContactoCliente> Contactos { get; set; }
}

class ContactoCliente {
   public Contacto Contacto { get; set; }
}

Then autocompletion of cc.Contacto fails in the following statement:

ctx.Clientes.Include(c => c.Contactos).ThenInclude(cc => cc.Contacto)

I have tested it and actually it has a very inconsistent behaviour. The first time you try to type Contacto it doesn’t show up. But after the first time that you type it manually and you delete it, it starts appearing in the autocompletion list.

That’s awesome! Thanks @ivanbasov 👏👏

So when would this show up Visual Studio?

This really does need to get fixed, I’m glad I found the bug reference after only 30 minutes of trying to figure out why it wasn’t giving me the options I expected and wasn’t matching EFCore documentation. This bug is likely a huge source of wasted time and frustration for anyone trying to use Include().ThenInclude()… or at least someone should update the ThenInclude documentation to indicate that there is a intellisense bug to stop people like me from pulling their hair out.

@C26000 Can you file a new issue on this with a full repro? I can’t repro this myself. Thanks!

Okay, this does reproduce for me using 16.4.1. But it doesn’t reproduce for me running our latest Roslyn bits. I’m not sure what might have fixed this or when it was fixed (or if the fix is available in the latest preview build of VS). @ivanbasov in case he knows what fixed this / etc.

EDIT: While this case behaves incorrectly in 16.4.1 for me, the repro in the original post here from Kevin behaves as expected, so there must be two different things going on…

Cases provided originally by Kevin are covered with unit tests. So, I do not expect them to fail.

The latest fix we made was the following one: https://github.com/dotnet/roslyn/issues/39468 Its meaning: we expected the semantic model to provide all reasonable candidates but found that it actually filters some of them. So, we decided to get more raw results from the semantic model and make filtering on our side.

Seen in 16.3.1 and 16.4: Code completion still does not offer up properties in ThenInclude. e.g., Author not an option in the docs example:

var blogs = context.Blogs
       .Include(blog => blog.Posts)
           .ThenInclude(post => post.Author)
       .ToList();

When entered manually, compiles with no errors or underlining and runs correctly.

Thank you, @CyrusNajmabadi ! I have a support of scenarios with Expression<> and without it. I could not find delegates there. Let me then start sharing my code to see what is missed. Will provide a draft PR tomorrow.

@NickMaev @BojidarStanchev did you see https://github.com/dotnet/roslyn/issues/8237#issuecomment-437453414 ? Contributions would be welcome here if you are interested!

Maybe you should stop preaching to people if you want help lol

I don’t personally care here. I don’t use these systems 😃 So the absence of this functionality here doesn’t bother me. I’m just trying to help out the best i can with the information i have available. I’m also trying to help explain why it’s not sufficient to just hire more people.

I was interested in helping out but I’m allergic to high horses

talking about engineering issues relevant here is not a high horse. It’s attempting to just help get clarity on the very real and important problems that make such solutions not necessarily suitable.

Teams contain finite resources, and there are very real and relevant issues involved with adding to that. In other words, while the first impression of finite resources bein ga problem is “just convert money to resources” it very often does not work to actually solve the problem at hand.

On the other hand, i’ve laid out an approach that can actually help out here. And i’m totally willing to work more toward trying to alleviate the problem for you. If you are interested let me know. At thsi point i’m not sure this metadiscussion is actually helpful toward solving the actual completion bug. So if you’d like to discuss that further, i’d recommend going over to gitter.im where we could discuss things privately. Otherwise, it would be good to keep things on topic here

Thanks! And i hope we’ll be able to work together on this 😃

Maybe you should stop preaching to people if you want help lol. Your argument is literally “this is open source so that more people can contribute for free because Microsoft shouldn’t have to hire more people because adding more people to the project doesn’t help so please everyone on Earth jump in and help because it’s open source.”

I was interested in helping out but I’m allergic to high horses and condescending motor mouth.

@Shadetheartist Would you be interested in contributing a fix here? I think teh approach i outlined above would be viable.

If anyone here would be interested in trying out the above potential solution, let me know. I can try to help guide you toward the right places in the code to look at for making this happen.

@961Group i think gafter meant that the workaround is to write it yourself, its only the autocompletion that is broken so the workaround is just to type it yourself.

Hey guys,

Appreciate the great work. Am looking forward for this to be fixed. 😃

the bug still exists in Visual Studio 2019 16.4.2 and .net core 3.1

Okay, this does reproduce for me using 16.4.1. But it doesn’t reproduce for me running our latest Roslyn bits. I’m not sure what might have fixed this or when it was fixed (or if the fix is available in the latest preview build of VS). @ivanbasov in case he knows what fixed this / etc.

EDIT: While this case behaves incorrectly in 16.4.1 for me, the repro in the original post here from Kevin behaves as expected, so there must be two different things going on…

Nope…

image

You can see that only methods are shown; the property filter icon is grayed out. This project was created using .Net Core 3.1 and Visual Studio 2019 16.4.1.

Another print showing all the properties inside the IQueryable object.

image

Sorry, didn’t mean to reopen. Let’s get a repro and maybe start a new issue depending on if it’s related.

Hallo ,

I also having a issue about thenInclude … i have this model with related data the collection is empty while im viewing.

public class Parent
{
public string test1 { get; set; }
public Testing Testing { get; set; }
}

public class Testing 
{
public ICollection<ListofData> ListofData { get; set; }
}

public class ListofData
{
 /// some code
}

then in my repo

public async Task<Parent> getParent(int id)
{
return await context.Parent.Include(x => x.Testing).ThenInclude(x => x.ListofData).FirstOrDefaultAsync(id)

}

@spottedmahn given that the Milestone of the PR is 16.1.P2 i would expect it in Visual Studio 2019 Update 1 Preview 2

Received another internal report of this.

I knew about this bug, then i forgot about it and here I am here again… it’s been 3 years already -.-

Book seems irrelevant since it predates the dinosaurs.

The book is relevant because nothing has changed here 😃 These aspects of engineering remain just as relevant today, and understanding them is super helpful for dealing with any software project.

Anyway I’ll apply the summary of this book, that my contribution will only slow down the solution, and withdraw!

That’s not at all the summary of the book. And this is why it would be valuable to actually read it. 😃

https://www.amazon.com/Mythical-Man-Month-Software-Engineering-Anniversary/dp/0201835959

It’s good reading. Basically, there usually is a sweet spot for a project, given its size, complexity, etc. Not going over that spot does not imply that one should go under 😃

@TechnikEmpire

Doesn’t Microsoft have paid developers work on this stuff?

Yes but even paid staff is a finite resource. Roslyn is a big, complicated code base with a number of competing priorities. We get to as much work as we can but even so we can’t fix every single issue that is filed.

@gafter you mentioned this:

For example, if we expose two different parameters as @CyrusNajmabadi suggests, each would have to have a corresponding lambda (even though there is only one lambda in the source).

Is that actually problematic though? For example, when the compiler is binding (for example, to determine which lambda would be best, and which will have the least errors), doesn’t it have to make a lambda for each potential candidate? So aren’t there multiple lambdas during binding to begin with?

In terms of consumption, it doesn’t feel strange to me that we might get many ‘candidate’ parameter symbols, each with a different potential different Lamdba symbol (even if each of those potential candidate lambda symbols pointed back to the same place in teh code). In effect, it would simply be the compiler saying: here’s all the potential things i considered for this chunk of code. I couldn’t decide which was best, but maybe you coudl find this information useful.

@961Group PRs welcome 😃

@Suchiman yes, i know but to knew it i lost a day rereading my project couple of times to see if any relationship design error. at the end, it was autocompletion problem… they should fix it

Hi @gafter, may I know what you mean?

@darting The workaround is to type your program in C#.

@dguisinger good point about the documentation. I have crated an issue in our docs: https://github.com/aspnet/EntityFramework.Docs/issues/394.

Fixing this would probably require a few weeks of effort.

There is no mechanism right now for the semantic model to “merge” results from distinct (erroneous) trial bindings of a lambda. There is no mechanism for binding an expression with the type of a variable being “either this or that”. The GetTypeInfo API, which the IDE currently uses to get the type from which its completion list is populated, has no mechanism for returning more than one type. This would require some effort in the compiler and in the IDE code and possibly some new APIs. It also risks degrading the IDE experience for the cases that we already handle well (because instead of returning what we think is the “best” result, we’d return all possible results). If we make the completion story better, we are likely to degrade the quality of data we provide when you hover over the parameter (e.g. its type).

The most likely way we’d handle this would be for an unbound lambda’s error-recovery binding to use a new kind of error type for the parameters that merge the members of all the possible types from the parameters of the delegates the lambda could bind to. Error types today don’t have any members, but these new types would have lots of members merged from different types. In order to maintain the invariant that a types’s members have a ContainingType that is the type from which the member was fetched, we may have to invent a number of other kinds of error symbols.