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.
``` C#
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
// 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>);
}
}
}
```
Pilchie
All 113 comments
Is this something we can fix on the IDE side, or do we need changes/additions to compiler APIs.
Pilchie
on 29 Jan 2016
@Pilchie This was on your list of places where the compiler doesn't help much. I'm not sure what you would want of the compiler here. The compiler doesn't make up types that combine the members of existing types for the purposes of error recovery. Is that what you would want? @rchande Please feel free to suggest what we could do to help here.
gafter
on 2 Jun 2016
@Pilchie @gafter @rchande Could you please provide an update on this issue? It is marked as blocked but from reading the comments it seems there was some outstanding question on whether this is really a bug on Roslyn, or something that would need to be fixed on the way the IDE uses Roslyn to get the complete list of applicable members. If the latter, what would be the best way to get some movement on this?
Customers keep hitting this limitation, as you can see by the number of GitHub issues that reference this one (there is about twice as many questions on email and other websites about the same problem).
divega
on 8 Feb 2017
@gafter Given the code:
c#
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.
CyrusNajmabadi
on 8 Feb 2017
Hey guys,
Just wanted to let you know I'm hitting this issue. I'm using the latest VS 2017 Community RC.
colltoaction
on 13 Feb 2017
I am also feeling this issue. Also using the latest VS 2017 Community RC.
Shadetheartist
on 22 Feb 2017
This issue is also present in VSCode using the C# extension.
0xdeafcafe
on 21 Mar 2017
Same issue reported here: https://developercommunity.visualstudio.com/content/problem/50498/intellisense-not-working-for-ef-core-theninclude.html
divega
on 30 Apr 2017
Any updates? Please fix! :)
spottedmahn
on 30 Apr 2017
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.
gafter
on 1 May 2017
Reported again in https://developercommunity.visualstudio.com/content/problem/50498/intellisense-not-working-for-ef-core-theninclude.html
Pilchie
on 2 May 2017
@Pilchie That's what https://github.com/dotnet/roslyn/issues/8237#issuecomment-298243181 says.
gafter
on 2 May 2017
There is no mechanism right now for the semantic model to “merge” results from distinct (erroneous) trial bindings of a lambda...
@gafter then the solution that @CyrusNajmabadi described at https://github.com/dotnet/roslyn/issues/8237#issuecomment-278251000 does not apply?
divega
on 2 May 2017
@divega I'm saying that there is no mechanism to do that today. We could add such a mechanism, but that could be a lot of work.
gafter
on 3 May 2017
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).
gafter
on 3 May 2017
Hey guys - just want to say thanks for problem solving this for us in the community. Hopefully you guys can find a solution that is a reasonable number of hours to enchance It.
spottedmahn
on 3 May 2017
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.
dguisinger
on 3 May 2017
@gafter, @Pilchie, @CyrusNajmabadi and others:
TL;DR: Even getting the union of all possible members is not ideal for this API. However we believe a (hopefully) simpler solution would be sufficient to fix the EF Core experience: somehow change Intellisense to give preference to the other overload from the one it is using now.
Details
Today @smitpatel (one of the EF Core developers) came up with an insight on this issue that I think may create an opportunity for a stopgap solution. Hopefully this will save you two weeks :smile::
It turns out that from the perspective of the usability of this particular API, we are not really interested in Intellisense showing the union of all the possible members in the completion list. It should be sufficient for the vast majority of real-world EF Core scenarios if Intellisense gave preference to the "right" overload of ThenInclude().
E.g. currently given this query:
``` C#
var reg = registrations
.Include(r => r.Activities).ThenInclude(a => a.$$
The following overload is (seemingly) arbitrarily picked:
``` C#
public static IIncludableQueryable<TEntity, TProperty> ThenInclude<TEntity, TPreviousProperty, TProperty>(
this IIncludableQueryable<TEntity, TPreviousProperty> source,
Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath) where TEntity : class
... and hence Intellisense shows the members of ICollection<Activity> in the completion list.
We actually need this one to be picked:
``` C#
public static IIncludableQueryable
this IIncludableQueryable
Expression
... so that the members of `Activity` are displayed in the completion list.
I am not sure if it would be acceptable to plainly invert the choice in the code used by Intellisense or if we would need a way (e.g. an attribute) to designate the preferred overload. If the later, I think it could have other uses besides this scenario, like addressing the question in
http://stackoverflow.com/questions/1904876/can-visual-studios-c-sharp-intellisense-be-given-a-hint-to-display-a-certain-me.
There is of course a corner case in which an entity type happens to implement `ICollection<T>`, e.g.:
``` C#
public class Registration : ICollection<Foo> { ... }
...
var act = activities
.Include(r => r.Registration).ThenInclude(a => a.$$
From the perspective of everything that is possible to express in the language, I believe it is still valid to say that ideally Intellisense should show the union of all the possible members, but pragmatically I believe this scenario is kind of fringe and it should be ok if Intellisense always preferred the overload that takes an IIncludableQueryable<TEntity, ICollection<TPreviousProperty>>.
I would say even more: if you come up with a solution that displays the union of all the possible members in the completion list, the usability of this EF Core API is still going to be bad unless the members corresponding to our preferred overload are displayed at the top of the completion list.
cc @ajcvickers
divega
on 4 May 2017
@dguisinger good point about the documentation. I have crated an issue in our docs: https://github.com/aspnet/EntityFramework.Docs/issues/394.
divega
on 4 May 2017
It should be sufficient for the vast majority of real-world EF Core scenarios if Intellisense gave preference to the "right" overload of ThenInclude().
Do you have a suggestion on how the compiler would know that one particular overload was better than the other?
CyrusNajmabadi
on 4 May 2017
@CyrusNajmabadi, my knowledge of the inner workings of the compiler is very limited, but I suspect it might not be an option for the compiler to automatically know that a particular overload is better. E.g. I suspect rules similar to what the compiler uses to choose the most specialized overload in overload resolution wouldn't be applicable here, although the problem seems similar on the surface.
So, I had in mind these alternatives:
Just invert whatever order is being used today: I assume there is an order of preference today and that somehow the "first" overload is being picked according to that order. Then in theory, that order could just be inverted. In fact @smitpatel and I tried to test the hypothesis that we could influence the order by just changing reflection order or even the order in which the overloads were defined in the source code, but the things that we tried had no effect. The main downside of this
hackapproach is that it could break the Intellisense experience for an existing API for which Intellisense is doing the most desirable thing today. It would also not be guaranteed to be stable over time, e.g. it could stop working in the desired way if you change some implementation detail in the future.Define an attribute that can be used to designate preferred overloads: Maybe this doesn't qualify as making the compiler know which overload is better, but there is precedence of attributes influencing the behavior of Intellisense, e.g.
EditorBrowsableAttributeandObsoleteAttribute.
divega
on 4 May 2017
We select the overload with the "fewest" errors. I'll look to see if that heuristic can be refined to improve the situation for these APIs.
gafter
on 4 May 2017
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.
brunoalreis
on 19 Jun 2017
Hey guys,
Appreciate the great work. Am looking forward for this to be fixed. :)
akw123
on 24 Aug 2017
@divega - Can we follow-up with this just to make one overload preferable over other?
smitpatel
on 24 Aug 2017
@smitpatel thanks for the reminder.
@gafter, @jaredpar is there any new data on this? Last time @gafter mentioned he would look at tweaking the current heuristic. Also, it is not clear to me what "15.later" maps to in terms of a timeframe.
Thanks.
divega
on 25 Aug 2017
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 :)
louislewis2
on 12 Sep 2017
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.
SharePointRadi
on 23 Sep 2017
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!
961Group
on 8 Jun 2018
hi there, may I know any workaround solution?
darting
on 12 Jun 2018
@darting The workaround is to type your program in C#.
gafter
on 12 Jun 2018
Hi @gafter, may I know what you mean?
darting
on 13 Jun 2018
@gafter iam using Dot Net Core 2.0 and for sure C# and same bug ! its not the lang its the IDE i think...
961Group
on 13 Jun 2018
@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.
Suchiman
on 13 Jun 2018
@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
961Group
on 13 Jun 2018
@961Group PRs welcome :)
CyrusNajmabadi
on 13 Jun 2018
Hi @gafter - did you see @divega's comment? Did you ever get a chance to:
I'll look to see if that heuristic can be refined to improve the situation for these APIs.
spottedmahn
on 13 Jun 2018
@spottedmahn No, I did not yet.
gafter
on 13 Jun 2018
In EF core 2.1.1 with the latest VS, still having this issue as of today. I'm lucky I found this page fast. I could have lost a bunch of time on this issue!
ajean42
on 10 Jul 2018
Would it be an option to have two differently named methods instead of two overloads?
(Asked on https://github.com/aspnet/EntityFrameworkCore/issues/4117#issuecomment-404461872, too.)
GeroIwan
on 12 Jul 2018
This bug has been killing dreams since 2016
TechnikEmpire
on 11 Sep 2018
This bug has been killing dreams since 2016
Try using Jetbrains Rider, it actually works in that ide.
Shadetheartist
on 11 Sep 2018
@rchande @jinujoseph This seems to have a fair amount of interest (26 votes), but no milestone assigned. Is there an intent that this be looked at in any sort of timeframe?
CyrusNajmabadi
on 11 Sep 2018
@CyrusNajmabadi @jinujoseph IIRC this requires work from the compiler in order to be completed. I've unassigned myself (but left @gafter) and defer to Jinu in choosing a new IDE advocate for this compiler work 😄
rchande
on 11 Sep 2018
@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.
CyrusNajmabadi
on 11 Sep 2018
Maybe someone should just email the Jetbrains team and ask what they did to fix it in their ide?
Shadetheartist
on 11 Sep 2018
Another option as well might be to change how intellisense just works here when it comes to extension methods. Specifically, if we could find out there was a compiler error, we could try to fallback to an alternative approach where we got the method group for 'ThenInclude' ourselves. We coudl then attempt to Reduce that method group using ReduceExtensionMethod to get the reduced extensions with enough type information filled in.
IDE could then try to extract out the many potential parameters we could be arguments for. We could then apply our own heuristics here to pick a 'better' candidate. Or we could try some sort of 'merging' approach. This would alleviate the burden on the compiler here, and allow us to try to iterate on some solution at the IDE level that coudl result in a better experience.
--
In the above code, we woudl see that we had:
registrations.Include(r => r.Activities). We'd get the type of that, and the overload group for .ThenInclude. Calling ReduceExtensionMethod on each of the IMethodSymbols in that overload group would then give us several potential reduced method options. If we were then passing a lambda to either a delegate type (or Expression<DelegateType>) we'd then use those overloads to try to decide what to offer off of a, and would then show a merged list (i.e containing Task).
Note: from this point we should be good. I just tested this out, and completion once you have written Task is totally fine. So as long as we just do this for an identifier that binds to a lambda being passed to an overloaded method, then we should be able to give proper completion, and get the user back on track.
CyrusNajmabadi
on 11 Sep 2018
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.
CyrusNajmabadi
on 11 Sep 2018
@Shadetheartist Would you be interested in contributing a fix here? I think teh approach i outlined above would be viable.
CyrusNajmabadi
on 11 Sep 2018
Sorry, i have no knowledge of compiler programming :(
Shadetheartist
on 11 Sep 2018
@Shadetheartist This woudl not be in the compiler. This would be purely in the Roslyn IDE side of things.
CyrusNajmabadi
on 11 Sep 2018
No knowledge of that either :(((((((
Shadetheartist
on 11 Sep 2018
I'm happy to help walk someone through it :)
CyrusNajmabadi
on 11 Sep 2018
Doesn't Microsoft have paid developers work on this stuff? I appreciate things are open source but at a certain point it just kind of feels like working for free for a company that makes billions upon billions of dollars per year on closed source software, including software that makes use of projects like this.
TechnikEmpire
on 12 Sep 2018
@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.
jaredpar
on 12 Sep 2018
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.
CyrusNajmabadi
on 12 Sep 2018
If someone can point in the general direction that'd be great. Also, while I appreciate the arguments, this bug is responsible for issues using asp.net core with entity framework, a major technology being promoted and heavily invested in my Microsoft, as well as companies pouring money back in. So there's always the option of, ya know, spending those billions on more employees. :)
Anyway general direction please, no promises but I'll see if I can get a look in.
TechnikEmpire
on 12 Sep 2018
If someone can point in the general direction that'd be great.
Sure! I'll write something up and get back here soon :)
this bug is responsible for issues using asp.net core with entity framework, a major technology being promoted and heavily invested in my Microsoft, as well as companies pouring money back in.
All the stuff being worked on is important to many groups of people :) And then there's the mythical man month issue. You can't just keep throwing people at things. It doesn't actually scale.
Again, this is the virtue of being open source. If this is important to you (or the asp.net core teams, or entity framework teams, or whoever), then the people who care deeply can show how much they care by investing their own limited resources into things. :)
CyrusNajmabadi
on 12 Sep 2018
So, i think teh place you'd want to start is here:
CSharpRecommendationService.GetSymbolsOffOfBoundExpression
https://github.com/dotnet/roslyn/blob/1743628ce1b564c10e053ff145cf67daa9dd3c8e/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs#L434
This is the specific completion code that is invoked when you have <some expr><dot> (for C# at least). For now, let's just work on a C# fix. I don't know if VB has the same problem. If so, we'd probably port any fix here over to the equivalent version on the VB side.
This function will be passed in a few things of interest. As you can see, one of the first things checked is the "symbol that the expression on the left of the dot bound to:
https://github.com/dotnet/roslyn/blob/1743628ce1b564c10e053ff145cf67daa9dd3c8e/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs#L447
If this symbol is an IParameterSymbol (which we check for here: https://github.com/dotnet/roslyn/blob/1743628ce1b564c10e053ff145cf67daa9dd3c8e/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs#L503) then i would say we should now start writing our specialized handling (most likely with a helper function to keep thigns cleanly separated).
I would first check if this parameter was a lambda parameter symbol. You can do that by grabbing it's containing symbol, and seeing if htat is an IMethodSymbol whose kind is an AnonymousFunction. If so, i would then grab the lambda syntax from that IMethodSymbol, and check to see if it is an 'argument expression', and it had a containing invocation. i.e. you're trying to grab this code ...ThenInclude(...
Once you have the invocation, you can use the SemanticModel.GetMemberGroup API to say: "give me all the overloads of 'ThenInclude' that you can find". Those resultant methods may or may not already be in reduced-extension-method form. "Reduced extension method form" just means "the extension method has been transformed into instance-method form". If it's already in that form, that's great. If not, let me know and i'll let you know what to do.
Now that you have all these appropriate overloads of the method (in instance-method form), we'll then find the index of our lambda in its parameter list. This will tell us the type expected for that parameter. If this type is an Expression<...> unwrap the expression part and grab the inner type parameter. If not, just use the type found. If this is a delegate type, then get the parameter-list for its 'Invoke' method, and find the index of the original lambda parameter you had in that invoke-method list. That's going to be the 'parameter' symbol we'll want to now get completions for.
The result of this will be a list of viable parameters that we want completion for. We should only use the different parameters if htey actually have different types. Once you have this, we'd want to then extract this part of GetSymbolsOffOfBoundExpression:
https://github.com/dotnet/roslyn/blob/1743628ce1b564c10e053ff145cf67daa9dd3c8e/src/Workspaces/CSharp/Portable/Recommendations/CSharpRecommendationService.cs#L542-L554
And call that for each parameter symbol we found, merging all the results together.
--
Each of these steps is pretty basic, but likely will take some spelunking to learn the proper roslyn APIs for it. I'm happy to help out if you get stuck at any point. Cheers!
CyrusNajmabadi
on 12 Sep 2018
Also, here's the link to getting started with Roslyn:
CyrusNajmabadi
on 12 Sep 2018
Thanks for the direction. Also all companies should reduce to a single staff member immediately because the division of labor and Adam Smith is fake news and trillion dollars market values were not obtained by scaling. Lol
TechnikEmpire
on 12 Sep 2018
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 :)
CyrusNajmabadi
on 12 Sep 2018
Book seems irrelevant since it predates the dinosaurs. Anyway I'll apply the summary of this book, that my contribution will only slow down the solution, and withdraw! Looking forward to a faster fix to this issue by not increasing the number of contributors.
TechnikEmpire
on 12 Sep 2018
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. :)
CyrusNajmabadi
on 12 Sep 2018
and withdraw!
Pity. If anyone else is interested, the above instructions still stand, and i'm definitely happy to try to help out if you run into issues here. :) I think this would be great if the community was able to work through issues they run into, and i think that would help out for future contributions when other things are encountered in the future.
It will be a very helpful way to address what is obviously a very real problem when some subset of users wants priorities to be different than what the team itself feels is appropriate at any given time.
CyrusNajmabadi
on 12 Sep 2018
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.
TechnikEmpire
on 12 Sep 2018
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 :)
CyrusNajmabadi
on 12 Sep 2018
"What one programmer can do in one month, two programmers can do in two months." - Fred Brooks
Source: Twitter
👍 @CyrusNajmabadi 😊
spottedmahn
on 13 Sep 2018
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 😕
th0mk
on 9 Nov 2018
@th0mk There has not been. However, i have outlined an implementation strategy that someone could follow if they wanted to provide a PR here. Thanks!
CyrusNajmabadi
on 9 Nov 2018
Up
NickMaev
on 30 Nov 2018
I knew about this bug, then i forgot about it and here I am here again... it's been 3 years already -.-
BojidarStanchev
on 19 Feb 2019
@NickMaev @BojidarStanchev did you see https://github.com/dotnet/roslyn/issues/8237#issuecomment-437453414 ? Contributions would be welcome here if you are interested!
CyrusNajmabadi
on 19 Feb 2019
@NickMaev @BojidarStanchev did you see #8237 (comment) ? Contributions would be welcome here if you are interested!
I'm too tired in my free time to be able to do this. If I ever find the time I'll try to help!
BojidarStanchev
on 20 Feb 2019
one day -_-, why they don't fix this
trungngotdt
on 23 Feb 2019
@trungngotdt Because it's non trivial to fix. If you are interested in contributing, i've listed instructions on how a possible fix might work out.
CyrusNajmabadi
on 23 Feb 2019
Received another internal report of this.
dpoeschl
on 4 Mar 2019
having the same issue like: https://stackoverflow.com/questions/45658411/ef-core-second-level-theninclude-missworks
.Include().ThenInclude().ThenInclude()
wstaelens
on 8 Mar 2019
@CyrusNajmabadi I'm working on the issue. I'm interested how delegatescan be involved in the scenario. Do you have an idea for a sample in mind?
ivanbasov
on 20 Mar 2019
@ivanbasov the delegate repro would be similar to the OP, except intead of using Exprssion<Func<...>> you just used things like Func<...>. For example, hre:
Expression<Func<TPreviousProperty, TProperty>> navigationPropertyPath
We just want to make sure whatever work we do is not solely for the 'Func' case or solely for the Expression<> case. It should work for both.
CyrusNajmabadi
on 20 Mar 2019
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.
ivanbasov
on 20 Mar 2019
also reported here
jinujoseph
on 20 Mar 2019
That's awesome! Thanks @ivanbasov 👏👏
So when would this show up Visual Studio?
spottedmahn
on 29 Mar 2019
@spottedmahn given that the Milestone of the PR is 16.1.P2 i would expect it in Visual Studio 2019 Update 1 Preview 2
Suchiman
on 29 Mar 2019
oh, I see now, thanks! 🤦♂️
spottedmahn
on 29 Mar 2019
@spottedmahn given that the Milestone of the PR is
16.1.P2i would expect it in Visual Studio 2019 Update 1 Preview 2
Exactly!
ivanbasov
on 29 Mar 2019
When will this be available in VS2017? 🙏
wstaelens
on 1 Apr 2019
I'm working on EntityFramework Core 2.2. And still unable to use "ThenInclude" for a collection
ICollection <Order> Orders;
.Include(member => member.Orders)
.ThenInclude(ords => ords.Select(o => o.OrderSubCategory))
.ThenInclude(oc => oc.OrderCategory),
Tried the above code to include an object OrderSubCategory within a collection property Orders. This throws an exception. Does anyone have a work around for this ?
kvijayan-aliera
on 3 May 2019
@kvijayan-aliera do you need to do:
ICollection <Order> Orders;
.Include(member => member.Orders)
.ThenInclude(ords => ords.Select(o => o.OrderSubCategory))
.ThenInclude(ocGroup => ocGroup.SelectMany(oc => oc.OrderCategory) ),
?
Only just briefly looked at it, so i may be way off. worth a shot though?
JNotelddim
on 3 May 2019
@kvijayan-aliera do you need to do:
ICollection <Order> Orders; .Include(member => member.Orders) .ThenInclude(ords => ords.Select(o => o.OrderSubCategory)) .ThenInclude(ocGroup => ocGroup.SelectMany(oc => oc.OrderCategory) ),?
Only just briefly looked at it, so i may be way off. worth a shot though?
@JNotelddim Yup! Your way off. Unfortunately .Include and .ThenInclude doesn't allow us to use .Select as the navigation property path. You'll end up with an exception that says..
The ThenInclude property lambda expression '... in select ...}' is invalid. The expression should represent a property access: 't => t.MyProperty'. To target navigations declared on derived types, specify an explicitly typed lambda parameter of the target type, E.g. '(Derived d) => d.MyProperty'.
But thanks for taking a shot!
kvijayan-aliera
on 3 May 2019
EDIT:
sorry y'know what nevermind I think I'm getting this mixed up with something else. you can disregard this :P
@kvijayan-aliera maybe I'm misunderstanding what a navigation property path is -- but from what you're saying it seems like this shouldn't be allowed:
List<RateStructureComponent> components = rateStructure.RateStructureDimensions
.SelectMany(dimension => dimension.RateStrucutreDimensionColumns
.SelectMany(dimensionColumn => dimensionColumn.RateStructureDimensionColumnValues
.SelectMany(columnValue => columnValue.RateStructureValue.RateStructureComponents)
)
).Select(item => item.RateStructureComponent).Distinct().ToList();
I can't find an example of using both .Select() and .SelectMany() at the moment -- though I know I've used them both on their own and in conjunction with one another within .Inlude() and .ThenInclude() multiple times.
Did you try the snippit I sent?
I'm pretty sure the reason the first example of yours wouldn't work is because when you try and do oc => oc.Category you're trying to access a Category property of an object when that object is a collection.
JNotelddim
on 3 May 2019
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)
}
Jochii12
on 10 May 2019
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.
olavorn
on 29 Jun 2019
@olavorn Yes, that is the description of this bug report. It's already been discussed and confirmed. The issue has been fixed for a future Visual Studio release.
mqudsi
on 29 Jun 2019
I haven't tested it but it should be in VS 2019 Update 1, the current version
Suchiman
on 29 Jun 2019
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?
michalgruda1
on 10 Jul 2019
It can be solved like: instead of .ThenInclude((Collection
goldengrisha
on 30 Aug 2019
Confirmed that on VS 16.3.1 Intellisense properly proposes Task for the original code sample.
roji
on 27 Sep 2019
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.
julielerman
on 8 Dec 2019
@julielerman I can confirm that. I'm using Visual Studio 2019 16.4.1 with .Net Core 3.1. The intelisense still does not show when I do Include on a collection, and then ThenInclude on a single item.
guidevnet
on 12 Dec 2019
@julielerman @guidevnet Can you post a standalone repro (or a pointer to one)? I'm having trouble reproducing the problem locally. I've tried variations of this, but they all work as expected ("Toys" is offered in the ThenInclude):
```C#
using System.Linq;
using Microsoft.EntityFrameworkCore;
class C
{
void Test()
{
using (var context = new FamilyContext())
{
context.Dogs.Include(d => d.Puppies).ThenInclude(p => p.Toys);
}
}
}
internal class FamilyContext : DbContext
{
public IQueryable
}
internal class Dog
{
public IQueryable
}
public class Puppy
{
public IQueryable
}
public class Toy
{
}
```
dpoeschl
on 13 Dec 2019
Sorry, didn't mean to reopen. Let's get a repro and maybe start a new issue depending on if it's related.
dpoeschl
on 13 Dec 2019
It's because Toys is a collection. You should try something like this:
context.Dogs.Include(d => d.Puppies).ThenInclude(p => p.Toy);
Where Toy is a one-to-one item on one Puppy.
public class Puppy
{
public Toy Toy { get; internal set; }
}
@guidevnet Toy is still in the list for me in ThenInclude. For completeness, here's my updated code. When you try this exact code, you don't get Toy in the list in the ThenInclude?
```C#
using System.Linq;
using Microsoft.EntityFrameworkCore;
class C
{
void Test()
{
using (var context = new FamilyContext())
{
context.Dogs.Include(d => d.Puppies).ThenInclude(p => p.Toy);
}
}
}
internal class FamilyContext : DbContext
{
public IQueryable
}
internal class Dog
{
public IQueryable
}
public class Puppy
{
public Toy Toy { get; internal set; }
}
public class Toy
{
}
```
dpoeschl
on 13 Dec 2019
Nope...
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.
guidevnet
on 13 Dec 2019
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...
dpoeschl
on 13 Dec 2019
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.
ivanbasov
on 13 Dec 2019
the bug still exists in Visual Studio 2019 16.4.2 and .net core 3.1
haue
on 21 Dec 2019
Hi @haue, we believe the original issue here should be fixed. If you're still seeing that exact repro fail, please let me know. If you're seeing some other scenario fail, it could be related to one of the other bugs that came up recently. Could you file a new issue in that case so we can investigate it separately?
Thanks!
dpoeschl
on 24 Dec 2019
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.
ghost
on 9 Feb 2020
@C26000 Can you file a new issue on this with a full repro? I can't repro this myself. Thanks!
CyrusNajmabadi
on 9 Feb 2020
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.
jmoralesv
on 25 Feb 2020
@jmoralesv PRs welcome :)
CyrusNajmabadi
on 4 Mar 2020
Related issues
marler8997
·
3Comments
binki
·
3Comments
glennblock
·
3Comments
NikChao
·
3Comments
OndrejPetrzilka
·
3Comments
Most helpful comment
It can be solved like: instead of .ThenInclude((Collection o) => o.Property)" put ".ThenInclude((ConcreteType o) => o.Property)".