Merge pull request #37290 from genlu/FullyQualifyInImport

Fully qualify unimported type name when in import directive

src/EditorFeatures/CSharpTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.cs

@@ -30,10 +30,14 @@ internal override CompletionProvider CreateCompletionProvider()
 
         private bool? ShowImportCompletionItemsOptionValue { get; set; } = true;
 
+        // -1 would disable timebox, whereas 0 means always timeout.
+        private int TimeoutInMilliseconds { get; set; } = -1;
+
         protected override void SetWorkspaceOptions(TestWorkspace workspace)
         {
             workspace.Options = workspace.Options
-                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, ShowImportCompletionItemsOptionValue);
+                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, ShowImportCompletionItemsOptionValue)
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForImportCompletion, TimeoutInMilliseconds);
         }
 
         protected override ExportProvider GetExportProvider()
@@ -1178,6 +1182,104 @@ class Program
             await VerifyTypeImportItemIsAbsentAsync(markup, "MyVBClass", inlineDescription: "Foo");
         }
 
+        [InlineData(SourceCodeKind.Regular)]
+        [InlineData(SourceCodeKind.Script)]
+        [WpfTheory, Trait(Traits.Feature, Traits.Features.Completion)]
+        [WorkItem(37038, "https://github.com/dotnet/roslyn/issues/37038")]
+        public async Task CommitTypeInUsingStaticContextShouldUseFullyQualifiedName(SourceCodeKind kind)
+        {
+            var file1 = @"
+namespace Foo
+{
+    public class MyClass { }
+}";
+
+            var file2 = @"
+using static $$";
+
+            var expectedCodeAfterCommit = @"
+using static Foo.MyClass$$";
+
+            var markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.CSharp);
+            await VerifyCustomCommitProviderAsync(markup, "MyClass", expectedCodeAfterCommit, sourceCodeKind: kind);
+        }
+
+        [InlineData(SourceCodeKind.Regular)]
+        [InlineData(SourceCodeKind.Script)]
+        [WpfTheory, Trait(Traits.Feature, Traits.Features.Completion)]
+        [WorkItem(37038, "https://github.com/dotnet/roslyn/issues/37038")]
+        public async Task CommitGenericTypeParameterInUsingAliasContextShouldUseFullyQualifiedName(SourceCodeKind kind)
+        {
+            var file1 = @"
+namespace Foo
+{
+    public class MyClass { }
+}";
+
+            var file2 = @"
+using CollectionOfStringBuilders = System.Collections.Generic.List<$$>";
+
+            var expectedCodeAfterCommit = @"
+using CollectionOfStringBuilders = System.Collections.Generic.List<Foo.MyClass$$>";
+
+            var markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.CSharp);
+            await VerifyCustomCommitProviderAsync(markup, "MyClass", expectedCodeAfterCommit, sourceCodeKind: kind);
+        }
+
+        [InlineData(SourceCodeKind.Regular)]
+        [InlineData(SourceCodeKind.Script)]
+        [WpfTheory, Trait(Traits.Feature, Traits.Features.Completion)]
+        [WorkItem(37038, "https://github.com/dotnet/roslyn/issues/37038")]
+        public async Task CommitGenericTypeParameterInUsingAliasContextShouldUseFullyQualifiedName2(SourceCodeKind kind)
+        {
+            var file1 = @"
+namespace Foo.Bar
+{
+    public class MyClass { }
+}";
+
+            var file2 = @"
+namespace Foo
+{
+    using CollectionOfStringBuilders = System.Collections.Generic.List<$$>
+}";
+
+            // Completion is not fully qualified
+            var expectedCodeAfterCommit = @"
+namespace Foo
+{
+    using CollectionOfStringBuilders = System.Collections.Generic.List<Foo.Bar.MyClass$$>
+}";
+
+            var markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.CSharp);
+            await VerifyCustomCommitProviderAsync(markup, "MyClass", expectedCodeAfterCommit, sourceCodeKind: kind);
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.Completion)]
+        [WorkItem(36624, "https://github.com/dotnet/roslyn/issues/36624")]
+        public async Task DoNotShowImportItemsIfTimeout()
+        {
+            // Set timeout to 0 so it always timeout
+            TimeoutInMilliseconds = 0;
+
+            var file1 = $@"
+namespace NS1
+{{
+    public class Bar
+    {{}}
+}}";
+            var file2 = @"
+namespace NS2
+{
+    class C
+    {
+         $$
+    }
+}";
+
+            var markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.CSharp);
+            await VerifyTypeImportItemIsAbsentAsync(markup, "Bar", inlineDescription: "NS1");
+        }
 
         private static void AssertRelativeOrder(List<string> expectedTypesInRelativeOrder, ImmutableArray<CompletionItem> allCompletionItems)
         {

src/EditorFeatures/Test2/IntelliSense/CSharpCompletionCommandHandlerTests.vb

@@ -4,6 +4,7 @@ Imports System.Collections.Immutable
 Imports System.Globalization
 Imports System.Threading
 Imports Microsoft.CodeAnalysis.Completion
+Imports Microsoft.CodeAnalysis.Completion.Providers
 Imports Microsoft.CodeAnalysis.CSharp
 Imports Microsoft.CodeAnalysis.Editor.CSharp.Formatting
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Extensions
@@ -5265,8 +5266,10 @@ namespace NS2
     public class Bar { }
 }
 "
-
-                state.Workspace.Options = state.Workspace.Options.WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True)
+                ' Enable import completion and disable timebox.
+                state.Workspace.Options = state.Workspace.Options _
+                    .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp, True) _
+                    .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForImportCompletion, -1)
 
                 state.SendInvokeCompletionList()
                 Await state.AssertSelectedCompletionItem(displayText:="Bar", isHardSelected:=True, inlineDescription:="NS2")

src/EditorFeatures/VisualBasicTest/Completion/CompletionProviders/TypeImportCompletionProviderTests.vb

 ' Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
 
 Imports Microsoft.CodeAnalysis.Completion
+Imports Microsoft.CodeAnalysis.Completion.Providers
 Imports Microsoft.CodeAnalysis.Editor.UnitTests
 Imports Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces
 Imports Microsoft.CodeAnalysis.Experiments
@@ -19,8 +20,13 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.Completion.Complet
 
         Private Property ShowImportCompletionItemsOptionValue As Boolean = True
 
+        ' -1 would disable timebox, whereas 0 means always timeout.
+        Private Property TimeoutInMilliseconds As Integer = -1
+
         Protected Overrides Sub SetWorkspaceOptions(workspace As TestWorkspace)
-            workspace.Options = workspace.Options.WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue)
+            workspace.Options = workspace.Options _
+                .WithChangedOption(CompletionOptions.ShowItemsFromUnimportedNamespaces, LanguageNames.VisualBasic, ShowImportCompletionItemsOptionValue) _
+                .WithChangedOption(CompletionServiceOptions.TimeoutInMillisecondsForImportCompletion, TimeoutInMilliseconds)
         End Sub
 
         Protected Overrides Function GetExportProvider() As ExportProvider
@@ -164,5 +170,45 @@ End Class]]></Text>.Value
             Dim markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.VisualBasic)
             Await VerifyItemExistsAsync(markup, "MyGenericClass", glyph:=Glyph.ClassPublic, inlineDescription:="Foo", displayTextSuffix:="(Of ...)", expectedDescriptionOrNull:="Class Foo.MyGenericClass(Of T)")
         End Function
+
+        <InlineData(SourceCodeKind.Regular)>
+        <InlineData(SourceCodeKind.Script)>
+        <WpfTheory, Trait(Traits.Feature, Traits.Features.Completion)>
+        <WorkItem(37038, "https://github.com/dotnet/roslyn/issues/37038")>
+        Public Async Function CommitTypeInImportAliasContextShouldUseFullyQualifiedName(kind As SourceCodeKind) As Task
+
+            Dim file1 = <Text>
+Namespace Foo
+    Public Class Bar
+    End Class
+End Namespace</Text>.Value
+
+            Dim file2 = "Imports BarAlias = $$"
+
+            Dim expectedCodeAfterCommit = "Imports BarAlias = Foo.Bar$$"
+
+            Dim markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.VisualBasic)
+            Await VerifyCustomCommitProviderAsync(markup, "Bar", expectedCodeAfterCommit, sourceCodeKind:=kind)
+        End Function
+
+        <InlineData(SourceCodeKind.Regular)>
+        <InlineData(SourceCodeKind.Script)>
+        <WpfTheory, Trait(Traits.Feature, Traits.Features.Completion)>
+        <WorkItem(37038, "https://github.com/dotnet/roslyn/issues/37038")>
+        Public Async Function CommitGenericTypeParameterInImportAliasContextShouldUseFullyQualifiedName(kind As SourceCodeKind) As Task
+
+            Dim file1 = <Text>
+Namespace Foo
+    Public Class Bar
+    End Class
+End Namespace</Text>.Value
+
+            Dim file2 = "Imports BarAlias = System.Collections.Generic.List(Of $$)"
+
+            Dim expectedCodeAfterCommit = "Imports BarAlias = System.Collections.Generic.List(Of Foo.Bar$$)"
+
+            Dim markup = CreateMarkupForSingleProject(file2, file1, LanguageNames.VisualBasic)
+            Await VerifyCustomCommitProviderAsync(markup, "Bar", expectedCodeAfterCommit, sourceCodeKind:=kind)
+        End Function
     End Class
 End Namespace

src/Features/CSharp/Portable/Completion/CompletionProviders/ImportCompletion/TypeImportCompletionProvider.cs

@@ -6,6 +6,7 @@
 using Microsoft.CodeAnalysis.Completion.Providers;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Extensions.ContextQuery;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery;
@@ -36,5 +37,12 @@ protected override async Task<SyntaxContext> CreateContextAsync(Document documen
             var semanticModel = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
             return CSharpSyntaxContext.CreateContext(document.Project.Solution.Workspace, semanticModel, position, cancellationToken);
         }
+
+        protected override async Task<bool> IsInImportsDirectiveAsync(Document document, int position, CancellationToken cancellationToken)
+        {
+            var syntaxTree = await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(false);
+            var leftToken = syntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken, includeDirectives: true);
+            return leftToken.GetAncestor<UsingDirectiveSyntax>() != null;
+        }
     }
 }

src/Features/CSharp/Portable/Completion/CompletionProviders/SymbolCompletionProvider.cs

@@ -79,7 +79,7 @@ protected override (string displayText, string suffix, string insertionText) Get
 
         protected override CompletionItemRules GetCompletionItemRules(List<ISymbol> symbols, SyntaxContext context, bool preselect)
         {
-            cachedRules.TryGetValue(ValueTuple.Create(context.IsInImportsDirective, preselect, context.IsPossibleTupleContext), out var rule);
+            cachedRules.TryGetValue(ValueTuple.Create(((CSharpSyntaxContext)context).IsLeftSideOfImportAliasDirective, preselect, context.IsPossibleTupleContext), out var rule);
 
             return rule ?? CompletionItemRules.Default;
         }

src/Features/Core/Portable/Completion/CompletionServiceOptions.cs

+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the Apache License, Version 2.0.  See License.txt in the project root for license information.
+
+using Microsoft.CodeAnalysis.Options;
+
+namespace Microsoft.CodeAnalysis.Completion
+{
+    internal static class CompletionServiceOptions
+    {
+        /// <summary>
+        /// Timeout value used for time-boxing import completion.
+        /// Telemetry shows that the average processing time with cache warmed up for 99th percentile is ~700ms,
+        /// Therefore we set the timeout to 1s to ensure it only applies to the case that cache is cold.
+        /// </summary>
+        public static readonly Option<int> TimeoutInMillisecondsForImportCompletion
+            = new Option<int>(nameof(CompletionServiceOptions), nameof(TimeoutInMillisecondsForImportCompletion), defaultValue: 1000);
+    }
+}

src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/AbstractTypeImportCompletionProvider.cs

@@ -35,9 +35,7 @@ internal abstract partial class AbstractTypeImportCompletionProvider : CommonCom
             SemanticModel semanticModel,
             CancellationToken cancellationToken);
 
-        // Telemetry shows that the average processing time with cache warmed up for 99th percentile is ~700ms.
-        // Therefore we set the timeout to 1s to ensure it only applies to the case that cache is cold.
-        internal int TimeoutInMilliseconds => 1000;
+        protected abstract Task<bool> IsInImportsDirectiveAsync(Document document, int position, CancellationToken cancellationToken);
 
         public override async Task ProvideCompletionsAsync(CompletionContext completionContext)
         {
@@ -109,8 +107,10 @@ private async Task AddCompletionItemsAsync(CompletionContext completionContext,
             // We want to timebox the operation that might need to traverse all the type symbols and populate the cache, 
             // the idea is not to block completion for too long (likely to happen the first time import completion is triggered).
             // The trade-off is we might not provide unimported types until the cache is warmed up.
+            var timeoutInMilliseconds = completionContext.Options.GetOption(CompletionServiceOptions.TimeoutInMillisecondsForImportCompletion);
             var combinedTask = Task.WhenAll(tasksToGetCompletionItems.ToImmutableAndFree());
-            if (await Task.WhenAny(combinedTask, Task.Delay(TimeoutInMilliseconds, cancellationToken)).ConfigureAwait(false) == combinedTask)
+
+            if (timeoutInMilliseconds != 0 && await Task.WhenAny(combinedTask, Task.Delay(timeoutInMilliseconds, cancellationToken)).ConfigureAwait(false) == combinedTask)
             {
                 // No timeout. We now have all completion items ready. 
                 var completionItemsToAdd = await combinedTask.ConfigureAwait(false);
@@ -209,7 +209,7 @@ internal override async Task<CompletionChange> GetChangeAsync(Document document,
             var containingNamespace = TypeImportCompletionItem.GetContainingNamespace(completionItem);
             Debug.Assert(containingNamespace != null);
 
-            if (ShouldCompleteWithFullyQualifyTypeName(document))
+            if (await ShouldCompleteWithFullyQualifyTypeName().ConfigureAwait(false))
             {
                 var fullyQualifiedName = $"{containingNamespace}.{completionItem.DisplayText}";
                 var change = new TextChange(completionListSpan, fullyQualifiedName);
@@ -261,7 +261,7 @@ internal override async Task<CompletionChange> GetChangeAsync(Document document,
                 return CompletionChange.Create(Utilities.Collapse(newText, builder.ToImmutableAndFree()));
             }
 
-            static bool ShouldCompleteWithFullyQualifyTypeName(Document document)
+            async Task<bool> ShouldCompleteWithFullyQualifyTypeName()
             {
                 var workspace = document.Project.Solution.Workspace;
 
@@ -285,7 +285,28 @@ static bool ShouldCompleteWithFullyQualifyTypeName(Document document)
                     return true;
                 }
 
-                return false;
+                // We might need to qualify unimported types to use them in an import directive, because they only affect members of the containing
+                // import container (e.g. namespace/class/etc. declarations).
+                //
+                // For example, `List` and `StringBuilder` both need to be fully qualified below: 
+                // 
+                //      using CollectionOfStringBuilders = System.Collections.Generic.List<System.Text.StringBuilder>;
+                //
+                // However, if we are typing in an C# using directive that is inside a nested import container (i.e. inside a namespace declaration block), 
+                // then we can add an using in the outer import container instead (this is not allowed in VB). 
+                //
+                // For example:
+                //
+                //      using System.Collections.Generic;
+                //      using System.Text;
+                //
+                //      namespace Foo
+                //      {
+                //          using CollectionOfStringBuilders = List<StringBuilder>;
+                //      }
+                //
+                // Here we will always choose to qualify the unimported type, just to be consistent and keeps things simple.
+                return await IsInImportsDirectiveAsync(document, completionListSpan.Start, cancellationToken).ConfigureAwait(false);
             }
         }
 

src/Features/VisualBasic/Portable/Completion/CompletionProviders/ImportCompletionProvider/TypeImportCompletionProvider.vb

@@ -8,6 +8,7 @@ Imports Microsoft.CodeAnalysis.PooledObjects
 Imports Microsoft.CodeAnalysis.Shared.Extensions.ContextQuery
 Imports Microsoft.CodeAnalysis.Text
 Imports Microsoft.CodeAnalysis.VisualBasic.Extensions.ContextQuery
+Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.Completion.Providers
 
@@ -42,5 +43,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Completion.Providers
 
             Return builder.ToImmutableAndFree()
         End Function
+
+        Protected Overrides Async Function IsInImportsDirectiveAsync(document As Document, position As Integer, cancellationToken As CancellationToken) As Task(Of Boolean)
+            Dim syntaxTree = Await document.GetSyntaxTreeAsync(cancellationToken).ConfigureAwait(False)
+            Dim leftToken = SyntaxTree.FindTokenOnLeftOfPosition(position, cancellationToken, includeDirectives:=True, includeDocumentationComments:=True)
+            Return leftToken.GetAncestor(Of ImportsStatementSyntax)() IsNot Nothing
+        End Function
     End Class
 End Namespace

src/Workspaces/CSharp/Portable/Extensions/ContextQuery/CSharpSyntaxContext.cs

@@ -48,6 +48,7 @@ internal sealed class CSharpSyntaxContext : SyntaxContext
         public readonly bool IsCrefContext;
         public readonly bool IsCatchFilterContext;
         public readonly bool IsDestructorTypeContext;
+        public readonly bool IsLeftSideOfImportAliasDirective;
 
         private CSharpSyntaxContext(
             Workspace workspace,
@@ -74,6 +75,7 @@ internal sealed class CSharpSyntaxContext : SyntaxContext
             bool isNameOfContext,
             bool isInQuery,
             bool isInImportsDirective,
+            bool isLeftSideOfImportAliasDirective,
             bool isLabelContext,
             bool isTypeArgumentOfConstraintContext,
             bool isRightOfDotOrArrowOrColonColon,
@@ -139,6 +141,7 @@ internal sealed class CSharpSyntaxContext : SyntaxContext
             this.IsCrefContext = isCrefContext;
             this.IsCatchFilterContext = isCatchFilterContext;
             this.IsDestructorTypeContext = isDestructorTypeContext;
+            this.IsLeftSideOfImportAliasDirective = isLeftSideOfImportAliasDirective;
         }
 
         public static CSharpSyntaxContext CreateContext(Workspace workspace, SemanticModel semanticModel, int position, CancellationToken cancellationToken)
@@ -227,7 +230,8 @@ private static CSharpSyntaxContext CreateContextWorker(Workspace workspace, Sema
                 isEnumTypeMemberAccessContext: syntaxTree.IsEnumTypeMemberAccessContext(semanticModel, position, cancellationToken),
                 isNameOfContext: syntaxTree.IsNameOfContext(position, semanticModel, cancellationToken),
                 isInQuery: leftToken.GetAncestor<QueryExpressionSyntax>() != null,
-                isInImportsDirective: IsLeftSideOfUsingAliasDirective(leftToken, cancellationToken),
+                isInImportsDirective: leftToken.GetAncestor<UsingDirectiveSyntax>() != null,
+                isLeftSideOfImportAliasDirective: IsLeftSideOfUsingAliasDirective(leftToken, cancellationToken),
                 isLabelContext: syntaxTree.IsLabelContext(position, cancellationToken),
                 isTypeArgumentOfConstraintContext: syntaxTree.IsTypeArgumentOfConstraintClause(position, cancellationToken),
                 isRightOfDotOrArrowOrColonColon: syntaxTree.IsRightOfDotOrArrowOrColonColon(position, targetToken, cancellationToken),