Visual Studio 15 RC - Provide intellisense for when the type at the left-hand side is an interface type

[iam3yal]

It seems like Visual Studio cannot provide meaningful intellisense for when the type at the left-hand side is an interface type and you want to create an instance of a class that implements this interface.

Example:

private IList<int> _numbers = new List<int>();

When typing new and then a space I'd expect to see List<int>() but currently, Visual Studio offers nothing and when you type new L it will just auto complete it to List as opposed to ReSharper which will happily provide this and it will even put it at the top of the list.

[CyrusNajmabadi]

Good suggestion. PRs welcome.

[stefannikolei]

@CyrusNajmabadi Could you give me a hint where to start here?

[CyrusNajmabadi]

The line that suppresses completion is here: https://github.com/dotnet/roslyn/blob/master/src/Features/Core/Portable/Completion/Providers/AbstractObjectCreationCompletionProvider.cs#L100

You can use SymbolFinder.FindImplementationsAsync to attempt to find implementations of the interface type.

[stefannikolei]

Gonna look into that in the coming days. Thanks for the tip

[iam3yal]

@CyrusNajmabadi, @reaction1989 I no longer have VS2015 on my dev machine and I wonder whether it's a requirement? I followed the docs and it seems like it is, even though I managed to build it successfully but when I ran it I got loads of exceptions.

Another question, is it possible to control the ordering of items that go into the intellisense? what I'm thinking is once a list of implementations that matches a given interface is compiled the second priority should be matching it by convention like I pointed out in my post List<int>() should be first.

p.s. It might be a good idea to documents some parts of Roslyn for new contributors or people that are less familiar with the internals but maybe it exists and I missed it, I checked docs.microsoft.com, maybe the community can help with that. nvm, found it but might be a good idea to add it to docs.microsoft.com.

[CyrusNajmabadi]

Hrmm... I'm not sure waht is required to build roslyn actually. I use VS2017RC myself. Maybe @Pilchie knows.

Another question, is it possible to control the ordering of items that go into the intellisense?

Yes. CompletionItem has SortText for this very reason. It's why, for example, @class will be properly ordered among all the other 'c' items, versus appearing at the top or bottom of the list.

[CyrusNajmabadi]

what I'm thinking is once a list of implementations that matches a given interface is compiled the second priority should be matching it by convention like I pointed out in my post List() should be first.

Sure. Though i'm not sure how you'll codify 'convention'.

[iam3yal]

Sure. Though i'm not sure how you'll codify 'convention'.

Well, what I'm thinking is to say given a class that implement IX and the class name is X to be above all other candidates when typing new and a space; otherwise, match by whatever letter the user type.

[CyrusNajmabadi]

Ah. Sure, that would be reasonable.

[Pilchie]

Another question, is it possible to control the ordering of items that go into the intellisense?

Yes, it's possible as @CyrusNajmabadi says, but I've very hesitant to change the ordering to be something other than alphabetical. So far, we've just relied on selecting things, not actually changing the order.

[Pilchie]

As for building, the latest VS2017 RC should be sufficient. What exceptions are you seeing?

[iam3yal]

@Pilchie

Yes, it's possible as @CyrusNajmabadi says, but I've very hesitant to change the ordering to be something other than alphabetical. So far, we've just relied on selecting things, not actually changing the order.

Well, R# does this so I thought it would make sense to do it in the same way.

As for building, the latest VS2017 RC should be sufficient. What exceptions are you seeing?

I'll compile it tomorrow and let you know. :)

[CyrusNajmabadi]

@eyalsk

This is for using "new" in an interface context. In that regard, i think we should simply produce a list with potential candidates, sorting them as we always have. We'd simply attempt to preselect the most likely one. i.e. the one matching your heuristic (if such an item exists), or the first one in the list otherwise.

I don't see the need to actually change sorting order.

[iam3yal]

@Pilchie nvm I selected the wrong startup project. :)

[iam3yal]

@CyrusNajmabadi

I'm not too familiar with the API and the Roslyn codebase, I'm still learning it as I go so I hope you don't mind but I have few questions:

Currently, what I'm trying to do is focus on a specific case where I have IList<int> and I want the intellisense to preselect List<int>.

I've modified GetPreselectedSymbolsWorker and I get List<T> selected at the right-hand side when I type new and a space.

This is the code I wrote:

if (type.IsInterfaceType())
            {
                var implementations = SymbolFinder.FindImplementationsAsync(type, context.Workspace.CurrentSolution);

                ISymbol selection = null;

                implementations.SafeContinueWith(
                    task => selection = task.Result.FirstOrDefault(),
                    cancellationToken,
                    TaskContinuationOptions.ExecuteSynchronously,
                    TaskScheduler.Default);

                return Task.FromResult(ImmutableArray.Create(selection));
            }

Now, I want to substitute the generic type argument on the right-hand side with the same type argument that exists at the left-hand side, what would be the right thing to do here? do I need to build a new syntax tree?

The second question is ATM the intellisense is populated with other symbols but based on what we discussed it should show only the relevant types so where is the most optimal place to implement this?

[CyrusNajmabadi]

A few things:

1. You should not need SafeContinueWith. Just use standard C# 'await' expressions.
2. You should not be using the workspace.CurrentSolution. You should use the Solution that exists when you were called (if you don't have that, then you'll have to pass it (or the appropriate Document) in somehow).
3. I would limit the search not to the whole solution, but just to the project you're in. We don't want suggestions for things you can't even reference. And we don't want this to take too long.
4. To create generic types, you can use INamedTypeSymbol.Construct(...)

Cheers!

[CyrusNajmabadi]

We also have two options here as to the completion experience. We can move this code into an an 'exclusive' provider. In which case, we won't show any other items. Or, we can just choose to preselect one of these items.

I'm fine with either.

[iam3yal]

@CyrusNajmabadi

You should not need SafeContinueWith. Just use standard C# 'await' expressions.

How do you mean? ATM GetPreselectedSymbolsWorker returns a Task<...> but it isn't async, are we speaking about different overloads? I've modified the one that exists in AbstractObjectCreationCompletionProvider. :)

You should not be using the workspace.CurrentSolution. You should use the Solution that exists when you were called (if you don't have that, then you'll have to pass it (or the appropriate Document) in somehow).

Yeah, it seems like I don't have this information or I don't know how to get it atm, any pointers?

I would limit the search not to the whole solution, but just to the project you're in. We don't want suggestions for things you can't even reference. And we don't want this to take too long.

Sure, so basically if I understand you correctly, I need to find the project where the document currently lives and then pass it to the extended overload of SymbolFinder.FindImplementationsAsync?

To create generic types, you can use INamedTypeSymbol.Construct(...)

Thank you! got it to work.

We can move this code into an an 'exclusive' provider. In which case, we won't show any other items.

Can you please elaborate a bit on this point?

Thanks for helping me.

[CyrusNajmabadi]

but it isn't async

Make it async :)

[CyrusNajmabadi]

Yeah, it seems like I don't have this information or I don't know how to get it atm, any pointers?

We'll have to add it to the SyntaxContext. That shouldn't be too difficult to do. If you have trouble, i can try to help with that.

[CyrusNajmabadi]

Sure, so basically if I understand you correctly, I need to find the project where the document currently lives and then pass it to the extended overload of SymbolFinder.FindImplementationsAsync

Yes. Note: once you have you the document, getting hte project is just "document.Project".

[CyrusNajmabadi]

Can you please elaborate a bit on this point?

So, completion has the concept of the CompletionContext. It's hwere the data eventually gets stored in (like what the completion items are). CompletionContext has an IsExclusive property. If set to true, then only these completoin items will be shown, and no others will be added.

Right now, the CompletionContext is not being passed all the way to where you are. But if you do that, then you can make it so we'll only show the implementations here and nothing else.

[iam3yal]

@CyrusNajmabadi

Sorry for late reply!

Make it async :)

I'm guessing I shouldn't rename it, right? :)

We'll have to add it to the SyntaxContext. That shouldn't be too difficult to do. If you have trouble, i can try to help with that.

I'll see if I can manage that myself and let you know if I need help once I'll get to it.

Yes. Note: once you have you the document, getting hte project is just "document.Project".

Okay, thanks.

So, completion has the concept of the CompletionContext. It's hwere the data eventually gets stored in (like what the completion items are). CompletionContext has an IsExclusive property. If set to true, then only these completoin items will be shown, and no others will be added.

Right now, the CompletionContext is not being passed all the way to where you are. But if you do that, then you can make it so we'll only show the implementations here and nothing else.

Thank you again.

[iam3yal]

I've worked on this a bit more and these are the results I'm getting for preselection:

IList<int> generic_list = new List<int>();
IDictionary<int, string> generic_dic = new Dictionary<int, string>();
IList list = new ArrayList();
IEnumerable enumerable = new string(new[] { 'x' });
ICollection collection = new Queue();
IDictionary dictionary = new Hashtable();

That's what I've done so far:

if (type.IsInterfaceType())
{
    var typeArity = type.GetArity();

    var implementations = (await SymbolFinder
                            .FindImplementationsAsync(type, context.Workspace.CurrentSolution).ConfigureAwait(false))
                            .Where(impl => !impl.IsAbstract && impl.GetArity() == typeArity);

    var implType = implementations
                    .FirstOrDefault(impl => type.Name.Contains(impl.Name)) ?? implementations.FirstOrDefault();

    if (implType != null)
    {
        var arguments = type.GetTypeArguments();

        if (arguments.Length != 0)
        {
            implType = ((INamedTypeSymbol)implType).Construct(arguments.ToArray());
        }

        return await Task.FromResult(ImmutableArray.Create(implType)).ConfigureAwait(false);
    }
    else
    {
        return await SpecializedTasks.EmptyImmutableArray<ISymbol>().ConfigureAwait(false);
    }
}

I have few questions:

1. What's the difference between GetTypeArguments and GetAllTypeArguments?

2. I'm guessing GetPreselectedSymbolsWorker doesn't have tests related to it so how do I test the functionality I've implemented? what would be the right approach to take here?

3. Is there really a need to repeat return await SpecializedTasks.EmptyImmutableArray<ISymbol>().ConfigureAwait(false); many times throughout the function?

[CyrusNajmabadi]

a few things:

1. We should return all the potential implementations. We'd just want to prioritize the one that matches your heuristic.
2. You're in an async method. You can just return ImmutableArray<...>.Empty, or ImmutableArray.Create(implType)

What's the difference between GetTypeArguments and GetAllTypeArguments?

if you have A<int>.B<string> then GetTypeArguments is "string", GetAllTypeArguments is "[int, string]".

so how do I test the functionality I've implemented?

Look for CompletionCommandHandlerTests

[CyrusNajmabadi]

I'm guessing I shouldn't rename it, right? :)

I'm fine either way. It should have Async in the name. but it's no big deal.

[CyrusNajmabadi]

My general philosophy is that we should always be improving and cleaning up code. But i'm not going to insist on that. Esp. as this is your first PR :)

[iam3yal]

@CyrusNajmabadi Thank you very much, I'll look into it later today! ;)

[iam3yal]

@CyrusNajmabadi

We should return all the potential implementations. We'd just want to prioritize the one that matches your heuristic.

So I should return multiple implementations out of GetPreselectedSymbolsWorker? I think I've tried it on my first attempt and got an exception this is what led me to the conclusion that I might need to return a single item.

My general philosophy is that we should always be improving and cleaning up code. But i'm not going to insist on that. Esp. as this is your first PR :)

Thank you. :)

[iam3yal]

@CyrusNajmabadi

When I'm doing something like this return implementations.ToImmutableArray(); inside GetPreselectedSymbolsWorker I get an exception, as I pointed out in my last post so can you please clarify the part where we should return all potential implementations?

[CyrusNajmabadi]

Yup. I'll take a look after lunch!

[iam3yal]

@CyrusNajmabadi Thank you, meanwhile bon appetit! :)

[CyrusNajmabadi]

BTW, just making a PR would make this easier. Thanks!

[CyrusNajmabadi]

Hrmm... I can't see why you'd be getting an exception. Can you provide more info? Alternatively, if you just make a PR with a branch, i'll be able to just try it out myself to see what the issue would be.

Please base your PR off the post-dev15 branch. Thanks!

[iam3yal]

@CyrusNajmabadi Okay, I'll do that asap, thanks! 😉

[iam3yal]

@CyrusNajmabadi I've sent the PR #16701

Actually, I've got it wrong, forgot to use the right branch!

[iam3yal]

@CyrusNajmabadi I will have some more time soon and I'm willing to take another shot at this given that someone else is not working on it and the infrastructure is improved since then because previously we had few blockers that prevented my progress on this, so I wonder what do you think? waiting for your response.

[CyrusNajmabadi]

Catch me up. What were the blockers?

[iam3yal]

@CyrusNajmabadi You can start reading from here as we were discussing this in the PR.

I just want this feature so bad and next month I'll be free again so I want to get back to it but don't want to end up being blocked by the infrastructure because before I ended up doing major refactoring and it went quite deep so after we discussed this you said the following:

We really need to do a lot of cleanup work in the core completion-impl, so that it isn't so painful to do this.

So I wonder if any work was done in this area. :)

[CyrusNajmabadi]

I don't think any work was done here :) reading through, i think the suggestoins i gave last time still apply. Namely:

https://github.com/dotnet/roslyn/pull/17250#issuecomment-289324969

So, inside AbstractObjectCreationCompletionProvider there are two overridden functions: "GetSymbolsWorker" and "GetPreselectedSymbolsWorker". I recommend updating GetPreselectedSymbolsWorker to just return a single 'best' item when you have an interface, while GetSymbolsWorker returns all the non-best items.

To save on perf, you can use a ConditionalWeakTable (keyed on SyntaxContext) to store the data you compute in one so that it can be immediately retrieved from the other. i.e. I could see you doing:

ConditionalWeakTable<SyntaxContext, (int position, ISymbol preselectedSymbol, ImmutableArray<ISymbol> regularSymbols)>

[iam3yal]

@CyrusNajmabadi Okay, thank you, I'll look into it and see what I can do within this area.