Changes based on review by Cyrus Najmabadi, + IgnoreCase in...

Changes based on review by Cyrus Najmabadi, + IgnoreCase in VisualBasicAddImportsService.MakeSafeToAddNamespaces

src/EditorFeatures/Test/MetadataAsSource/MetadataAsSourceTests.cs

@@ -2049,7 +2049,6 @@ void M()
 // {CodeAnalysisResources.InMemoryAssembly}
 #endregion
 
-using System.Runtime.CompilerServices;
 
 public class [|TestType|]<[NullableAttribute(1)]
 T> where T : notnull
@@ -2092,7 +2091,6 @@ void M()
 // {CodeAnalysisResources.InMemoryAssembly}
 #endregion
 
-using System.Runtime.CompilerServices;
 
 public class TestType
 {{
@@ -2131,7 +2129,6 @@ void M([|D|]&lt;int&gt; lambda)
 // {CodeAnalysisResources.InMemoryAssembly}
 #endregion
 
-using System.Runtime.CompilerServices;
 
 public delegate void [|D|]<[NullableAttribute(1)]
 T>() where T : notnull;";

src/Features/CSharp/Portable/AddImport/CSharpAddImportFeatureService.cs

@@ -376,7 +376,7 @@ private string GetUsingDirectiveString(INamespaceOrTypeSymbol namespaceOrTypeSym
 
                 var addImportService = document.GetLanguageService<IAddImportsService>();
                 var newRoot = addImportService.AddImports(
-                    semanticModel.Compilation, root, contextNode, newImports, placeSystemNamespaceFirst);
+                    semanticModel.Compilation, root, contextNode, newImports, placeSystemNamespaceFirst, cancellationToken);
                 return (CompilationUnitSyntax)newRoot;
             }
             finally
@@ -397,7 +397,7 @@ private string GetUsingDirectiveString(INamespaceOrTypeSymbol namespaceOrTypeSym
             var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
             var service = document.GetLanguageService<IAddImportsService>();
             var newRoot = service.AddImport(
-                compilation, root, contextNode, usingDirective, placeSystemNamespaceFirst);
+                compilation, root, contextNode, usingDirective, placeSystemNamespaceFirst, cancellationToken);
 
             return document.WithSyntaxRoot(newRoot);
         }

src/Features/Core/Portable/AliasAmbiguousType/AbstractAliasAmbiguousTypeCodeFixProvider.cs

@@ -52,7 +52,7 @@ public sealed override async Task RegisterCodeFixesAsync(CodeFixContext context)
                     codeActionsBuilder.Add(new MyCodeAction(codeActionPreviewText, c =>
                         {
                             var aliasDirective = syntaxGenerator.AliasImportDeclaration(typeName, symbol);
-                            var newRoot = addImportService.AddImport(compilation, root, diagnosticNode, aliasDirective, placeSystemNamespaceFirst);
+                            var newRoot = addImportService.AddImport(compilation, root, diagnosticNode, aliasDirective, placeSystemNamespaceFirst, cancellationToken);
                             return Task.FromResult(document.WithSyntaxRoot(newRoot));
                         }));
                 }

src/Features/Core/Portable/CodeRefactorings/SyncNamespace/AbstractChangeNamespaceService.cs

@@ -791,7 +791,7 @@ await FixReferencesAsync(document, changeNamespaceService, addImportService, ref
 
                 var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
                 var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
-                root = addImportService.AddImports(compilation, root, contextLocation, imports, placeSystemNamespaceFirst);
+                root = addImportService.AddImports(compilation, root, contextLocation, imports, placeSystemNamespaceFirst, cancellationToken);
                 document = document.WithSyntaxRoot(root);
             }
 

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

@@ -230,7 +230,7 @@ internal override async Task<CompletionChange> GetChangeAsync(Document document,
                 var compilation = await document.Project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
                 var importNode = CreateImport(document, containingNamespace);
 
-                var rootWithImport = addImportService.AddImport(compilation, root, addImportContextNode, importNode, placeSystemNamespaceFirst);
+                var rootWithImport = addImportService.AddImport(compilation, root, addImportContextNode, importNode, placeSystemNamespaceFirst, cancellationToken);
                 var documentWithImport = document.WithSyntaxRoot(rootWithImport);
                 var formattedDocumentWithImport = await Formatter.FormatAsync(documentWithImport, Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false);
 

src/Features/VisualBasic/Portable/AddImport/VisualBasicAddImportFeatureService.vb

@@ -292,7 +292,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.AddImport
             Dim importService = document.GetLanguageService(Of IAddImportsService)
 
             Dim root = Await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(False)
-            Dim newRoot = importService.AddImport(compilation, root, contextNode, importsStatement, placeSystemNamespaceFirst)
+            Dim newRoot = importService.AddImport(compilation, root, contextNode, importsStatement, placeSystemNamespaceFirst, cancellationToken)
             newRoot = newRoot.WithAdditionalAnnotations(CaseCorrector.Annotation, Formatter.Annotation)
             Dim newDocument = document.WithSyntaxRoot(newRoot)
 

src/VisualStudio/CSharp/Impl/Snippets/SnippetExpansionClient.cs

@@ -111,7 +111,7 @@ public override int GetExpansionFunction(IXMLDOMNode xmlFunctionNode, string bst
 
             var addImportService = document.GetLanguageService<IAddImportsService>();
             var compilation = document.Project.GetCompilationAsync(cancellationToken).WaitAndGetResult(cancellationToken);
-            var newRoot = addImportService.AddImports(compilation, root, contextLocation, newUsingDirectives, placeSystemNamespaceFirst);
+            var newRoot = addImportService.AddImports(compilation, root, contextLocation, newUsingDirectives, placeSystemNamespaceFirst, cancellationToken);
 
             var newDocument = document.WithSyntaxRoot(newRoot);
 

src/Workspaces/CSharp/Portable/AddImports/CSharpAddImportsService.cs

@@ -39,12 +39,13 @@ protected override bool IsStaticUsing(UsingDirectiveSyntax usingOrAlias)
             SyntaxNode staticUsingContainer,
             SyntaxNode aliasContainer,
             bool placeSystemNamespaceFirst,
-            SyntaxNode root)
+            SyntaxNode root,
+            CancellationToken cancellationToken)
         {
             var rewriter = new Rewriter(
                 externAliases, usingDirectives, staticUsingDirectives,
                 aliasDirectives, externContainer, usingContainer,
-                staticUsingContainer, aliasContainer, placeSystemNamespaceFirst);
+                staticUsingContainer, aliasContainer, placeSystemNamespaceFirst, cancellationToken);
 
             var newRoot = rewriter.Visit(root);
             return newRoot;
@@ -73,6 +74,7 @@ protected override SyntaxList<ExternAliasDirectiveSyntax> GetExterns(SyntaxNode
         private class Rewriter : CSharpSyntaxRewriter
         {
             private readonly bool _placeSystemNamespaceFirst;
+            private readonly CancellationToken _cancellationToken;
             private readonly SyntaxNode _externContainer;
             private readonly SyntaxNode _usingContainer;
             private readonly SyntaxNode _aliasContainer;
@@ -92,7 +94,8 @@ private class Rewriter : CSharpSyntaxRewriter
                 SyntaxNode usingContainer,
                 SyntaxNode aliasContainer,
                 SyntaxNode staticUsingContainer,
-                bool placeSystemNamespaceFirst)
+                bool placeSystemNamespaceFirst,
+                CancellationToken cancellationToken)
             {
                 _externAliases = externAliases;
                 _usingDirectives = usingDirectives;
@@ -103,6 +106,7 @@ private class Rewriter : CSharpSyntaxRewriter
                 _aliasContainer = aliasContainer;
                 _staticUsingContainer = staticUsingContainer;
                 _placeSystemNamespaceFirst = placeSystemNamespaceFirst;
+                _cancellationToken = cancellationToken;
             }
 
             public override SyntaxNode VisitNamespaceDeclaration(NamespaceDeclarationSyntax node)
@@ -110,7 +114,9 @@ public override SyntaxNode VisitNamespaceDeclaration(NamespaceDeclarationSyntax
                 // recurse downwards so we visit inner namespaces first.
                 var rewritten = (NamespaceDeclarationSyntax)base.VisitNamespaceDeclaration(node);
 
-                if (!node.CanAddUsingDirectives(CancellationToken.None))
+                _cancellationToken.ThrowIfCancellationRequested();
+
+                if (!node.CanAddUsingDirectives(_cancellationToken))
                 {
                     return rewritten;
                 }

src/Workspaces/CSharp/Portable/Editing/CSharpImportAdder.cs

@@ -33,16 +33,6 @@ protected override INamespaceSymbol GetExplicitNamespaceSymbol(SyntaxNode node,
             return null;
         }
 
-        protected override INamespaceSymbol GetContainedNamespace(SyntaxNode node, SemanticModel model)
-        {
-            var namespaceSyntax = node.AncestorsAndSelf().OfType<NamespaceDeclarationSyntax>().FirstOrDefault();
-
-            if (namespaceSyntax is null)
-                return null;
-
-            return model.GetDeclaredSymbol(namespaceSyntax);
-        }
-
         protected override SyntaxNode MakeSafeToAddNamespaces(SyntaxNode root, IEnumerable<INamespaceOrTypeSymbol> namespaceMembers, IEnumerable<IMethodSymbol> extensionMethods, SemanticModel model, Workspace workspace, CancellationToken cancellationToken)
         {
             var rewriter = new Rewriter(namespaceMembers, extensionMethods, model, workspace, cancellationToken);
@@ -70,10 +60,18 @@ private INamespaceSymbol GetExplicitNamespaceSymbol(ExpressionSyntax fullName, E
 
         private class Rewriter : CSharpSyntaxRewriter
         {
-            private Workspace _workspace;
-            private CancellationToken _cancellationToken;
             private readonly SemanticModel _model;
+            private readonly Workspace _workspace;
+            private readonly CancellationToken _cancellationToken;
+
+            /// <summary>
+            /// A hashset containing the short names of all namespace members 
+            /// </summary>
             private readonly HashSet<string> _namespaceMembers;
+
+            /// <summary>
+            /// A hashset containing the short names of all extension methods
+            /// </summary>
             private readonly HashSet<string> _extensionMethods;
 
             public Rewriter(
@@ -94,8 +92,8 @@ private class Rewriter : CSharpSyntaxRewriter
 
             public override SyntaxNode VisitIdentifierName(IdentifierNameSyntax node)
             {
-                //Only visit leading trivia, as we only care about xml doc comments
-                var leadingTrivia = VisitList(node.GetLeadingTrivia());
+                // We only care about xml doc comments
+                var leadingTrivia = CanHaveDocComments(node) ? VisitList(node.GetLeadingTrivia()) : node.GetLeadingTrivia();
 
                 if (_namespaceMembers.Contains(node.Identifier.Text))
                 {
@@ -108,12 +106,12 @@ public override SyntaxNode VisitIdentifierName(IdentifierNameSyntax node)
 
             public override SyntaxNode VisitGenericName(GenericNameSyntax node)
             {
-                //Only visit leading trivia, as we only care about xml doc comments
-                var leadingTrivia = VisitList(node.GetLeadingTrivia());
+                // We only care about xml doc comments
+                var leadingTrivia = CanHaveDocComments(node) ? VisitList(node.GetLeadingTrivia()) : node.GetLeadingTrivia();
 
                 if (_namespaceMembers.Contains(node.Identifier.Text))
                 {
-                    // no need to visit type argument list as simplifier will expand everything
+                    // No need to visit type argument list as simplifier will expand everything
                     var expanded = Simplifier.Expand<SyntaxNode>(node, _model, _workspace, cancellationToken: _cancellationToken);
                     return expanded.WithLeadingTrivia(leadingTrivia);
                 }
@@ -125,6 +123,7 @@ public override SyntaxNode VisitGenericName(GenericNameSyntax node)
             public override SyntaxNode VisitQualifiedName(QualifiedNameSyntax node)
             {
                 var left = (NameSyntax)base.Visit(node.Left);
+                // We don't recurse on the right, as if B is a member of the imported namespace, A.B is still not ambiguous
                 var right = node.Right;
                 if (right is GenericNameSyntax genericName)
                 {
@@ -141,7 +140,7 @@ public override SyntaxNode VisitInvocationExpression(InvocationExpressionSyntax
                 {
                     if (_extensionMethods.Contains(memberAccess.Name.Identifier.Text))
                     {
-                        // no need to visit this as simplifier will expand everything
+                        // No need to visit this as simplifier will expand everything
                         return Simplifier.Expand<SyntaxNode>(node, _model, _workspace, cancellationToken: _cancellationToken);
                     }
                 }
@@ -155,14 +154,45 @@ public override SyntaxNode VisitMemberAccessExpression(MemberAccessExpressionSyn
 
                 if (_extensionMethods.Contains(node.Name.Identifier.Text))
                 {
-                    // we will not visit this if the parent node is expanded, since we just expand the entire parent node.
-                    // therefore, since this is visited, we haven't expanded, and so we should warn
-                    node = node.WithAdditionalAnnotations(WarningAnnotation.Create(
-                        "Adding imports will bring an extension method into scope with the same name as " + node.Name.Identifier.Text));
+                    // If an extension method is used as a delegate rather than invoked directly,
+                    // there is no semantically valid transformation that will fully qualify the extension method. 
+                    // For example `Func<int> f = x.M;` is not the same as Func<int> f = () => Extensions.M(x);`
+                    // since one captures x by value, and the other by reference.
+                    //
+                    // We will not visit this node if the parent node was an InvocationExpression, 
+                    // since we would have expanded the parent node entirely, rather than visiting it.
+                    // Therefore it's possible that this is an extension method being used as a delegate so we warn.
+                    node = node.WithAdditionalAnnotations(WarningAnnotation.Create(string.Format(
+                        WorkspacesResources.Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access,
+                        node.Name.Identifier.Text)));
                 }
 
                 return node;
             }
+
+            private bool CanHaveDocComments(NameSyntax node)
+            {
+                // a node can only have doc comments in its leading trivia if it's the first node in a member declaration syntax.
+
+                SyntaxNode current = node;
+                while (current.Parent != null)
+                {
+                    var parent = current.Parent;
+                    if (parent is NameSyntax && parent.ChildNodes().First() == current)
+                    {
+                        current = parent;
+                        continue;
+                    }
+
+                    if (parent is MemberDeclarationSyntax && parent.ChildNodes().First() == current)
+                    {
+                        return true;
+                    }
+
+                    return false;
+                }
+                return false;
+            }
         }
     }
 }

src/Workspaces/CSharpTest/CodeGeneration/AddImportsTests.cs

@@ -43,11 +43,10 @@ private async Task<Document> GetDocument(string code, bool withAnnotations)
                     (o, c) =>
                     {
                         var symbol = model.GetSymbolInfo(o).Symbol;
-                        if (symbol != null)
-                            return c.WithAdditionalAnnotations(SymbolAnnotation.Create(symbol), Simplifier.Annotation);
-                        return c;
-                    }
-                    );
+                        return symbol != null
+                            ? c.WithAdditionalAnnotations(SymbolAnnotation.Create(symbol), Simplifier.Annotation) 
+                            : c;
+                    });
                 doc = doc.WithSyntaxRoot(root);
             }
             return doc;
@@ -638,6 +637,79 @@ class C
 }", safe, useSymbolAnnotations);
         }
 
+        [Theory, MemberData(nameof(TestAllData))]
+        public async Task TestImportNameNotSimplfied(bool safe, bool useSymbolAnnotations)
+        {
+            await TestAsync(
+@"namespace System
+{
+    using System.Threading;
+
+    class C
+    {
+        private System.Collections.Generic.List<int> F;
+    }
+}",
+
+@"namespace System
+{
+    using System.Collections.Generic;
+    using System.Threading;
+
+    class C
+    {
+        private System.Collections.Generic.List<int> F;
+    }
+}",
+
+@"namespace System
+{
+    using System.Collections.Generic;
+    using System.Threading;
+
+    class C
+    {
+        private List<int> F;
+    }
+}", safe, useSymbolAnnotations);
+        }
+
+        [Theory, InlineData(false, true)]
+        public async Task TestUnnecessaryImportAddedAndRemoved(bool safe, bool useSymbolAnnotations)
+        {
+            await TestAsync(
+@"using List = System.Collections.Generic.List<int>;
+
+namespace System
+{
+    class C
+    {
+        private List F;
+    }
+}",
+
+@"using System.Collections.Generic;
+using List = System.Collections.Generic.List<int>;
+
+namespace System
+{
+    class C
+    {
+        private List F;
+    }
+}",
+
+@"using List = System.Collections.Generic.List<int>;
+
+namespace System
+{
+    class C
+    {
+        private List F;
+    }
+}", safe, useSymbolAnnotations);
+        }
+
         [Theory, MemberData(nameof(TestAllData))]
         public async Task TestImportAddedToStartOfDocumentIfNoNestedImports(bool safe, bool useSymbolAnnotations)
         {

src/Workspaces/Core/Portable/AddImports/AbstractAddImportsService.cs

@@ -4,6 +4,7 @@
 using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Linq;
+using System.Threading;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.AddImports
@@ -108,7 +109,8 @@ public SyntaxNode GetImportContainer(SyntaxNode root, SyntaxNode contextLocation
             SyntaxNode root,
             SyntaxNode contextLocation,
             IEnumerable<SyntaxNode> newImports,
-            bool placeSystemNamespaceFirst)
+            bool placeSystemNamespaceFirst,
+            CancellationToken cancellationToken)
         {
             contextLocation ??= root;
 
@@ -128,7 +130,7 @@ public SyntaxNode GetImportContainer(SyntaxNode root, SyntaxNode contextLocation
                 externAliases, usingDirectives, staticUsingDirectives,
                 aliasDirectives, externContainer, usingContainer,
                 staticUsingContainer, aliasContainer,
-                placeSystemNamespaceFirst, root);
+                placeSystemNamespaceFirst, root, cancellationToken);
 
             return newRoot;
         }
@@ -136,7 +138,8 @@ public SyntaxNode GetImportContainer(SyntaxNode root, SyntaxNode contextLocation
         protected abstract SyntaxNode Rewrite(
             TExternSyntax[] externAliases, TUsingOrAliasSyntax[] usingDirectives, TUsingOrAliasSyntax[] staticUsingDirectives,
             TUsingOrAliasSyntax[] aliasDirectives, SyntaxNode externContainer, SyntaxNode usingContainer,
-            SyntaxNode staticUsingContainer, SyntaxNode aliasContainer, bool placeSystemNamespaceFirst, SyntaxNode root);
+            SyntaxNode staticUsingContainer, SyntaxNode aliasContainer, bool placeSystemNamespaceFirst, SyntaxNode root, 
+            CancellationToken cancellationToken);
 
         private void GetContainers(SyntaxNode root, SyntaxNode contextLocation, out SyntaxNode externContainer, out SyntaxNode usingContainer, out SyntaxNode staticUsingContainer, out SyntaxNode aliasContainer)
         {

src/Workspaces/Core/Portable/AddImports/IAddImportsService.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 System.Collections.Generic;
+using System.Threading;
 using Microsoft.CodeAnalysis.Host;
 using Roslyn.Utilities;
 
@@ -23,17 +24,17 @@ internal interface IAddImportsService : ILanguageService
 
         SyntaxNode AddImports(
             Compilation compilation, SyntaxNode root, SyntaxNode contextLocation,
-            IEnumerable<SyntaxNode> newImports, bool placeSystemNamespaceFirst);
+            IEnumerable<SyntaxNode> newImports, bool placeSystemNamespaceFirst, CancellationToken cancellationToken);
     }
 
     internal static class IAddImportServiceExtensions
     {
         public static SyntaxNode AddImport(
             this IAddImportsService service, Compilation compilation, SyntaxNode root,
-            SyntaxNode contextLocation, SyntaxNode newImport, bool placeSystemNamespaceFirst)
+            SyntaxNode contextLocation, SyntaxNode newImport, bool placeSystemNamespaceFirst, CancellationToken cancellationToken)
         {
             return service.AddImports(compilation, root, contextLocation,
-                SpecializedCollections.SingletonEnumerable(newImport), placeSystemNamespaceFirst);
+                SpecializedCollections.SingletonEnumerable(newImport), placeSystemNamespaceFirst, cancellationToken);
         }
     }
 }

src/Workspaces/Core/Portable/Editing/ImportAdder.cs

@@ -4,6 +4,7 @@
 using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.CodeActions;
 using Microsoft.CodeAnalysis.Options;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
@@ -13,8 +14,8 @@ namespace Microsoft.CodeAnalysis.Editing
     public static class ImportAdder
     {
         /// <summary>
-        /// Mark a sub-tree with this annotation in order for imports to be added for symbol annotations attached to nodes in the sub-tree,
-        /// once a code action is complete.
+        /// The annotation <see cref="CodeAction.CleanupDocumentAsync"/> uses to identify sub trees to look for symbol annotations on.
+        /// It will then add import directives for these symbol annotations.
         /// </summary>
         public static SyntaxAnnotation Annotation = new SyntaxAnnotation();
 
@@ -23,7 +24,7 @@ public static class ImportAdder
         /// </summary>
         public static Task<Document> AddImportsAsync(Document document, OptionSet options = null, CancellationToken cancellationToken = default)
         {
-            return AddImportsFromSyntaxesAsync(document, false, options, cancellationToken);
+            return AddImportsFromSyntaxesAsync(document, safe: false, options, cancellationToken);
         }
 
         /// <summary>
@@ -31,7 +32,7 @@ public static Task<Document> AddImportsAsync(Document document, OptionSet option
         /// </summary>
         public static Task<Document> AddImportsAsync(Document document, TextSpan span, OptionSet options = null, CancellationToken cancellationToken = default)
         {
-            return AddImportsFromSyntaxesAsync(document, new[] { span }, false, options, cancellationToken);
+            return AddImportsFromSyntaxesAsync(document, new[] { span }, safe: false, options, cancellationToken);
         }
 
         /// <summary>
@@ -39,7 +40,7 @@ public static Task<Document> AddImportsAsync(Document document, TextSpan span, O
         /// </summary>
         public static Task<Document> AddImportsAsync(Document document, SyntaxAnnotation annotation, OptionSet options = null, CancellationToken cancellationToken = default)
         {
-            return AddImportsFromSyntaxesAsync(document, annotation, false, options, cancellationToken);
+            return AddImportsFromSyntaxesAsync(document, annotation, safe: false, options, cancellationToken);
         }
 
         /// <summary>
@@ -47,7 +48,7 @@ public static Task<Document> AddImportsAsync(Document document, SyntaxAnnotation
         /// </summary>
         public static Task<Document> AddImportsAsync(Document document, IEnumerable<TextSpan> spans, OptionSet options = null, CancellationToken cancellationToken = default)
         {
-            return AddImportsFromSyntaxesAsync(document, spans, false, options, cancellationToken);
+            return AddImportsFromSyntaxesAsync(document, spans, safe: false, options, cancellationToken);
         }
 
 #pragma warning disable RS0026 // Do not add multiple public overloads with optional parameters

src/Workspaces/Core/Portable/Editing/ImportAdderService.cs

@@ -2,6 +2,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.ComponentModel;
 using System.Linq;
 using System.Threading;
@@ -10,6 +11,7 @@
 using Microsoft.CodeAnalysis.Formatting;
 using Microsoft.CodeAnalysis.Host;
 using Microsoft.CodeAnalysis.Options;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Collections;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Simplification;
@@ -44,10 +46,7 @@ public enum Strategy
             // Create a simple interval tree for simplification spans.
             var spansTree = new SimpleIntervalTree<TextSpan>(TextSpanIntervalIntrospector.Instance, spans);
 
-            bool isInSpan(SyntaxNode node) =>
-                spansTree.HasIntervalThatOverlapsWith(node.FullSpan.Start, node.FullSpan.Length);
-
-            var nodes = root.DescendantNodesAndSelf().Where(isInSpan);
+            var nodes = root.DescendantNodesAndSelf().Where(IsInSpan);
             var (importDirectivesToAdd, namespaceSymbols, context) = strategy switch
             {
                 Strategy.AddImportsFromSymbolAnnotations
@@ -57,36 +56,62 @@ public enum Strategy
                 _ => throw new InvalidEnumArgumentException(nameof(strategy), (int)strategy, typeof(Strategy)),
             };
 
-            if (importDirectivesToAdd.Count is 0)
+            if (importDirectivesToAdd.Length == 0)
             {
                 return document.WithSyntaxRoot(root); //keep any added simplifier annotations
             }
 
             if (safe)
             {
+                // Mark the context with an annotation. 
+                // This will allow us to find it after we have called MakeSafeToAddNamespaces.
                 var annotation = new SyntaxAnnotation();
-                root = root.ReplaceNode(context, context.WithAdditionalAnnotations(annotation));
-                document = document.WithSyntaxRoot(root);
+                document = document.WithSyntaxRoot(root.ReplaceNode(context, context.WithAdditionalAnnotations(annotation)));
+                root = await document.GetSyntaxRootAsync().ConfigureAwait(false);
+                model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+
+                // Make Safe to add namespaces
+                document = document.WithSyntaxRoot(
+                   MakeSafeToAddNamespaces(root, namespaceSymbols, model, document.Project.Solution.Workspace, cancellationToken));
+                document = document.WithSyntaxRoot(root.ReplaceNode(context, context.WithAdditionalAnnotations(annotation)));
                 root = await document.GetSyntaxRootAsync().ConfigureAwait(false);
                 model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
-                root = MakeSafeToAddNamespaces(root, namespaceSymbols, model, document.Project.Solution.Workspace, cancellationToken);
+
+                // Find the context. It might be null if we have removed the context in the process of complexifying the tree.
                 context = root.DescendantNodesAndSelf().FirstOrDefault(x => x.HasAnnotation(annotation)) ?? root;
             }
 
             var placeSystemNamespaceFirst = options.GetOption(GenerationOptions.PlaceSystemNamespaceFirst, document.Project.Language);
 
-            root = addImportsService.AddImports(model.Compilation, root, context, importDirectivesToAdd, placeSystemNamespaceFirst);
+            root = addImportsService.AddImports(model.Compilation, root, context, importDirectivesToAdd, placeSystemNamespaceFirst, cancellationToken);
 
             return document.WithSyntaxRoot(root);
+
+            bool IsInSpan(SyntaxNode node) =>
+                spansTree.HasIntervalThatOverlapsWith(node.FullSpan.Start, node.FullSpan.Length);
         }
 
         protected abstract INamespaceSymbol GetExplicitNamespaceSymbol(SyntaxNode node, SemanticModel model);
 
+        private SyntaxNode MakeSafeToAddNamespaces(
+            SyntaxNode root,
+            IEnumerable<INamespaceSymbol> namespaceSymbols,
+            SemanticModel model,
+            Workspace workspace,
+            CancellationToken cancellationToken)
+        {
+            var namespaceMembers = namespaceSymbols.SelectMany(x => x.GetMembers());
+            var extensionMethods =
+                namespaceMembers.OfType<INamedTypeSymbol>().Where(t => t.MightContainExtensionMethods)
+                .SelectMany(x => x.GetMembers().OfType<IMethodSymbol>().Where(x => x.IsExtensionMethod));
+
+            return MakeSafeToAddNamespaces(root, namespaceMembers, extensionMethods, model, workspace, cancellationToken);
+        }
+
         /// <summary>
-        /// Gets the namespace <paramref name="node"/> is contained in, or null otherwise
+        /// Fully qualifies parts of the document that may change meaning if namespaces are added, 
+        /// and marks them with <see cref="Simplifier.Annotation"/> so they can be reduced later.
         /// </summary>
-        protected abstract INamespaceSymbol GetContainedNamespace(SyntaxNode node, SemanticModel model);
-
         protected abstract SyntaxNode MakeSafeToAddNamespaces(
             SyntaxNode root,
             IEnumerable<INamespaceOrTypeSymbol> namespaceMembers,
@@ -97,12 +122,19 @@ public enum Strategy
 
         private SyntaxNode GenerateNamespaceImportDeclaration(INamespaceSymbol namespaceSymbol, SyntaxGenerator generator)
         {
+            // We add Simplifier.Annotation so that the import can be removed if it turns out to be unnecessary.
+            // This can happen for a number of reasons (we replace the type with var, inbuilt type, alias, etc.)
             return generator
                 .NamespaceImportDeclaration(namespaceSymbol.ToDisplayString(SymbolDisplayFormats.NameFormat))
                 .WithAdditionalAnnotations(Simplifier.Annotation, Formatter.Annotation);
         }
 
-        private (List<SyntaxNode> imports, IEnumerable<INamespaceSymbol> namespaceSymbols, SyntaxNode context) GetImportDirectivesFromSyntaxesAsync(
+        /// <summary>
+        /// 
+        /// </summary>
+        /// <param name="root">ref as we add simplifier annotations to nodes with explicit namespaces</param>
+        /// <returns></returns>
+        private (ImmutableArray<SyntaxNode> imports, IEnumerable<INamespaceSymbol> namespaceSymbols, SyntaxNode context) GetImportDirectivesFromSyntaxesAsync(
                 IEnumerable<SyntaxNode> syntaxNodes,
                 ref SyntaxNode root,
                 SemanticModel model,
@@ -111,13 +143,13 @@ private SyntaxNode GenerateNamespaceImportDeclaration(INamespaceSymbol namespace
                 CancellationToken cancellationToken
             )
         {
-            var importsToAdd = new List<SyntaxNode>();
+            var importsToAdd = ArrayBuilder<SyntaxNode>.GetInstance();
 
             var nodesWithExplicitNamespaces = syntaxNodes
                 .Select(n => (syntaxnode: n, namespaceSymbol: GetExplicitNamespaceSymbol(n, model)))
                 .Where(x => x.namespaceSymbol != null);
 
-            var nodesToSimplify = new List<SyntaxNode>();
+            var nodesToSimplify = ArrayBuilder<SyntaxNode>.GetInstance();
 
             var addedSymbols = new HashSet<INamespaceSymbol>();
 
@@ -139,7 +171,7 @@ CancellationToken cancellationToken
                     continue;
                 }
 
-                if (IsInsideNamespace(node, namespaceSymbol, model))
+                if (IsInsideNamespace(node, namespaceSymbol, model, cancellationToken))
                 {
                     continue;
                 }
@@ -148,9 +180,10 @@ CancellationToken cancellationToken
                 importsToAdd.Add(namespaceSyntax);
             }
 
-            if (nodesToSimplify.Count is 0)
+            if (nodesToSimplify.Count == 0)
             {
-                return (importsToAdd, addedSymbols, null);
+                nodesToSimplify.Free();
+                return (importsToAdd.ToImmutableAndFree(), addedSymbols, null);
             }
 
             var annotation = new SyntaxAnnotation();
@@ -162,10 +195,11 @@ CancellationToken cancellationToken
             var first = root.DescendantNodesAndSelf().First(x => x.HasAnnotation(annotation));
             var last = root.DescendantNodesAndSelf().Last(x => x.HasAnnotation(annotation));
 
-            return (importsToAdd, addedSymbols, first.GetCommonRoot(last));
+            nodesToSimplify.Free();
+            return (importsToAdd.ToImmutableAndFree(), addedSymbols, first.GetCommonRoot(last));
         }
 
-        private (List<SyntaxNode> imports, IEnumerable<INamespaceSymbol> namespaceSymbols, SyntaxNode context) GetImportDirectivesFromAnnotatedNodesAsync(
+        private (ImmutableArray<SyntaxNode> imports, IEnumerable<INamespaceSymbol> namespaceSymbols, SyntaxNode context) GetImportDirectivesFromAnnotatedNodesAsync(
             IEnumerable<SyntaxNode> syntaxNodes,
             SyntaxNode root,
             SemanticModel model,
@@ -175,7 +209,7 @@ CancellationToken cancellationToken
         {
             SyntaxNode first = null;
             SyntaxNode last = null;
-            var importsToAdd = new List<SyntaxNode>();
+            var importsToAdd = ArrayBuilder<SyntaxNode>.GetInstance();
 
             var annotatedNodes = syntaxNodes.Where(x => x.HasAnnotations(SymbolAnnotation.Kind));
             var addedSymbols = new HashSet<INamespaceSymbol>();
@@ -196,7 +230,7 @@ CancellationToken cancellationToken
                     foreach (var namedType in SymbolAnnotation.GetSymbols(annotation, model.Compilation).OfType<INamedTypeSymbol>())
                     {
                         cancellationToken.ThrowIfCancellationRequested();
-                        if (IsBuiltIn(namedType))
+                        if (namedType.IsSpecialType())
                         {
                             continue;
                         }
@@ -222,7 +256,7 @@ CancellationToken cancellationToken
                             continue;
                         }
 
-                        if(IsInsideNamespace(annotatedNode, namespaceSymbol, model))
+                        if(IsInsideNamespace(annotatedNode, namespaceSymbol, model, cancellationToken))
                         {
                             continue;
                         }
@@ -233,43 +267,20 @@ CancellationToken cancellationToken
                 }
             }
 
-            return (importsToAdd, addedSymbols, first?.GetCommonRoot(last));
-        }
-
-        private bool IsBuiltIn(INamedTypeSymbol type)
-        {
-            switch (type.OriginalDefinition.SpecialType)
-            {
-                case SpecialType.System_Object:
-                case SpecialType.System_Void:
-                case SpecialType.System_Boolean:
-                case SpecialType.System_Char:
-                case SpecialType.System_SByte:
-                case SpecialType.System_Byte:
-                case SpecialType.System_Int16:
-                case SpecialType.System_UInt16:
-                case SpecialType.System_Int32:
-                case SpecialType.System_UInt32:
-                case SpecialType.System_Int64:
-                case SpecialType.System_UInt64:
-                case SpecialType.System_Decimal:
-                case SpecialType.System_Single:
-                case SpecialType.System_Double:
-                case SpecialType.System_String:
-                case SpecialType.System_Nullable_T:
-                    return true;
-            }
+            // we don't add simplifier annotations here, 
+            // since whatever added the symbol annotation probably also added simplifier annotations,
+            // and if not they probably didn't for a reason
 
-            return false;
+            return (importsToAdd.ToImmutableAndFree(), addedSymbols, first?.GetCommonRoot(last));
         }
 
         /// <summary>
         /// Checks if the namespace declaration <paramref name="node"/> is contained inside,
         /// or any of its ancestor namespaces are the same as <paramref name="symbol"/>
         /// </summary>
-        private bool IsInsideNamespace(SyntaxNode node, INamespaceSymbol symbol, SemanticModel model)
+        private bool IsInsideNamespace(SyntaxNode node, INamespaceSymbol symbol, SemanticModel model, CancellationToken cancellationToken)
         {
-            var containedNamespace = GetContainedNamespace(node, model);
+            var containedNamespace = model.GetEnclosingNamespace(node.SpanStart, cancellationToken);
 
             while (containedNamespace != null)
             {
@@ -280,20 +291,5 @@ private bool IsInsideNamespace(SyntaxNode node, INamespaceSymbol symbol, Semanti
 
             return false;
         }
-
-        private SyntaxNode MakeSafeToAddNamespaces(
-            SyntaxNode root,
-            IEnumerable<INamespaceSymbol> namespaceSymbols,
-            SemanticModel model,
-            Workspace workspace,
-            CancellationToken cancellationToken)
-        {
-            var namespaceMembers = namespaceSymbols.SelectMany(x => x.GetMembers());
-            var extensionMethods =
-                namespaceMembers.OfType<ITypeSymbol>().Where(x => x.IsStatic || x.IsModuleType())
-                .SelectMany(x => x.GetMembers().OfType<IMethodSymbol>().Where(x => x.IsExtensionMethod));
-
-            return MakeSafeToAddNamespaces(root, namespaceMembers, extensionMethods, model, workspace, cancellationToken);
-        }
     }
 }

src/Workspaces/Core/Portable/WorkspacesResources.Designer.cs

@@ -3843,6 +3843,16 @@ internal class WorkspacesResources {
             }
         }
         
+        /// <summary>
+        ///   Looks up a localized string similar to Adding imports will bring an extension method into scope with the same name as &apos;{0}&apos;.
+        /// </summary>
+        internal static string Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access {
+            get {
+                return ResourceManager.GetString("Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_na" +
+                        "me_as_member_access", resourceCulture);
+            }
+        }
+        
         /// <summary>
         ///   Looks up a localized string similar to Workspace is not empty..
         /// </summary>

src/Workspaces/Core/Portable/WorkspacesResources.resx

@@ -1517,4 +1517,7 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
   <data name="Symbol_specifications" xml:space="preserve">
     <value>Symbol specifications</value>
   </data>
+  <data name="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access" xml:space="preserve">
+    <value>Adding imports will bring an extension method into scope with the same name as '{0}'</value>
+  </data>
 </root>
\ No newline at end of file

src/Workspaces/Core/Portable/xlf/WorkspacesResources.cs.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Soubory jazyka Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Pracovní prostor není platný.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.de.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Visual Basic-Dateien</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Arbeitsbereich ist nicht leer.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.es.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Archivos de Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">El área de trabajo no está vacía.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.fr.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Fichiers Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">L'espace de travail n'est pas vide.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.it.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">File Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">L'area di lavoro non è vuota.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ja.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Visual Basic ファイル</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">ワークスペースが空ではありません。</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ko.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Visual Basic 파일</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">작업 영역이 비어 있지 않습니다.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.pl.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Pliki języka Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Obszar roboczy nie jest pusty.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.pt-BR.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Arquivos do Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Workspace não está vazio.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.ru.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Файлы Visual Basic</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Рабочая область не пуста.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.tr.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Visual Basic dosyaları</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">Çalışma alanı boş değil.</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.zh-Hans.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">visual basic 文件</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">工作区不为空。</target>

src/Workspaces/Core/Portable/xlf/WorkspacesResources.zh-Hant.xlf

@@ -1352,6 +1352,11 @@ Zero-width positive lookbehind assertions are typically used at the beginning of
         <target state="translated">Visual Basic 檔案</target>
         <note />
       </trans-unit>
+      <trans-unit id="Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access">
+        <source>Adding imports will bring an extension method into scope with the same name as '{0}'</source>
+        <target state="new">Adding imports will bring an extension method into scope with the same name as '{0}'</target>
+        <note />
+      </trans-unit>
       <trans-unit id="Workspace_is_not_empty">
         <source>Workspace is not empty.</source>
         <target state="translated">工作區不是空的。</target>

src/Workspaces/VisualBasic/Portable/AddImports/VisualBasicAddImportsService.vb

@@ -68,11 +68,12 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.AddImports
                 staticUsingContainer As SyntaxNode,
                 aliasContainer As SyntaxNode,
                 placeSystemNamespaceFirst As Boolean,
-                root As SyntaxNode) As SyntaxNode
+                root As SyntaxNode,
+                cancellationToken As CancellationToken) As SyntaxNode
 
             Dim compilationUnit = DirectCast(root, CompilationUnitSyntax)
 
-            If Not compilationUnit.CanAddImportsStatements(CancellationToken.None) Then
+            If Not compilationUnit.CanAddImportsStatements(cancellationToken) Then
                 Return compilationUnit
             End If
 

src/Workspaces/VisualBasic/Portable/Editing/VisualBasicImportAdder.vb

@@ -32,16 +32,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
             Return Nothing
         End Function
 
-        Protected Overrides Function GetContainedNamespace(node As SyntaxNode, model As SemanticModel) As INamespaceSymbol
-            Dim namespaceSyntax = node.AncestorsAndSelf().OfType(Of NamespaceBlockSyntax).FirstOrDefault()
-
-            If namespaceSyntax Is Nothing Then
-                Return Nothing
-            End If
-
-            Return model.GetDeclaredSymbol(namespaceSyntax)
-        End Function
-
         Protected Overrides Function MakeSafeToAddNamespaces(root As SyntaxNode, namespaceMembers As IEnumerable(Of INamespaceOrTypeSymbol), extensionMethods As IEnumerable(Of IMethodSymbol), model As SemanticModel, workspace As Workspace, cancellationToken As CancellationToken) As SyntaxNode
             Dim Rewriter = New Rewriter(namespaceMembers, extensionMethods, model, workspace, cancellationToken)
 
@@ -67,8 +57,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
         Private Class Rewriter
             Inherits VisualBasicSyntaxRewriter
 
-            Private _workspace As Workspace
-            Private _cancellationToken As CancellationToken
+            Private ReadOnly _workspace As Workspace
+            Private ReadOnly _cancellationToken As CancellationToken
             Private ReadOnly _model As SemanticModel
             Private ReadOnly _namespaceMembers As HashSet(Of String)
             Private ReadOnly _extensionMethods As HashSet(Of String)
@@ -77,8 +67,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
                 _model = model
                 _workspace = workspace
                 _cancellationToken = cancellationToken
-                _namespaceMembers = New HashSet(Of String)(namespaceMembers.[Select](Function(x) x.Name))
-                _extensionMethods = New HashSet(Of String)(extensionMethods.[Select](Function(x) x.Name))
+                _namespaceMembers = New HashSet(Of String)(namespaceMembers.[Select](Function(x) x.Name), CaseInsensitiveComparison.Comparer)
+                _extensionMethods = New HashSet(Of String)(extensionMethods.[Select](Function(x) x.Name), CaseInsensitiveComparison.Comparer)
             End Sub
 
             Public Overrides ReadOnly Property VisitIntoStructuredTrivia As Boolean
@@ -106,6 +96,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
 
             Public Overrides Function VisitQualifiedName(node As QualifiedNameSyntax) As SyntaxNode
                 Dim left = CType(MyBase.Visit(node.Left), NameSyntax)
+                ' We don't recurse on the right, as if B is a member of the imported namespace, A.B is still not ambiguous
                 Dim right = node.Right
 
                 If TypeOf right Is GenericNameSyntax Then
@@ -121,7 +112,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
                 If TypeOf node.Expression Is MemberAccessExpressionSyntax Then
                     Dim memberAccess = DirectCast(node.Expression, MemberAccessExpressionSyntax)
                     If _extensionMethods.Contains(memberAccess.Name.Identifier.Text) Then
-                        ' no need to visit this as simplifier will expand everything
+                        ' No need to visit this as simplifier will expand everything
                         Return Simplifier.Expand(Of SyntaxNode)(node, _model, _workspace, cancellationToken:=_cancellationToken)
                     End If
                 End If
@@ -133,10 +124,17 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.Editing
                 node = DirectCast(MyBase.VisitMemberAccessExpression(node), MemberAccessExpressionSyntax)
 
                 If _extensionMethods.Contains(node.Name.Identifier.Text) Then
-                    ' we will not visit this if the parent node is expanded, since we just expand the entire parent node.
-                    ' therefore, since this is visited, we haven't expanded, and so we should warn
-                    node = node.WithAdditionalAnnotations(WarningAnnotation.Create(
-                        "Adding imports will bring an extension method into scope with the same name as " & node.Name.Identifier.Text))
+                    ' If an extension method is used as a delegate rather than invoked directly,
+                    ' there is no semantically valid transformation that will fully qualify the extension method. 
+                    ' For example `Dim f As Func<int> = x.M;` is not the same as `Dim f As Func<int> = Function(x) Extensions.M(x);`
+                    ' since one captures x by value, and the other by reference.
+                    '
+                    ' We will not visit this node if the parent node was an InvocationExpression, 
+                    ' since we would have expanded the parent node entirely, rather than visiting it.
+                    ' Therefore it's possible that this is an extension method being used as a delegate so we warn.
+                    node = node.WithAdditionalAnnotations(WarningAnnotation.Create(String.Format(
+                        WorkspacesResources.Warning_adding_imports_will_bring_an_extension_method_into_scope_with_the_same_name_as_member_access,
+                        node.Name.Identifier.Text)))
                 End If
 
                 Return node

src/Workspaces/VisualBasicTest/CodeGeneration/AddImportsTests.vb

@@ -452,6 +452,73 @@ Class C
 End Class", safe:=True, useSymbolAnnotations)
         End Function
 
+        <Theory, InlineData(True), InlineData(False)>
+        Public Async Function TestSafeWithMatchingSimpleNameDifferentCase(useSymbolAnnotations As Boolean) As Task
+            Await TestAsync(
+"Imports B
+
+Namespace A
+    Class C1
+    End Class
+
+    Class C2
+    End Class
+End Namespace
+
+Namespace B
+    Class C1
+    End Class
+End Namespace
+
+Class C
+    Private Function M(ByVal c2 As A.C2) As c1
+        Return Nothing
+    End Function
+End Class",
+"Imports A
+Imports B
+
+Namespace A
+    Class C1
+    End Class
+
+    Class C2
+    End Class
+End Namespace
+
+Namespace B
+    Class C1
+    End Class
+End Namespace
+
+Class C
+    Private Function M(ByVal c2 As A.C2) As Global.B.c1
+        Return Nothing
+    End Function
+End Class",
+"Imports A
+Imports B
+
+Namespace A
+    Class C1
+    End Class
+
+    Class C2
+    End Class
+End Namespace
+
+Namespace B
+    Class C1
+    End Class
+End Namespace
+
+Class C
+    Private Function M(ByVal c2 As C2) As B.c1
+        Return Nothing
+    End Function
+End Class", safe:=True, useSymbolAnnotations)
+        End Function
+
         <Theory, InlineData(True), InlineData(False)>
         Public Async Function TestSafeWithMatchingGenericName(useSymbolAnnotations As Boolean) As Task
             Await TestAsync(