Address review comments

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

@@ -129,11 +129,12 @@ internal override async Task<CompletionChange> GetChangeAsync(Document document,
 
                 var rootWithImport = addImportService.AddImport(compilation, root, addImportContextNode, importNode, placeSystemNamespaceFirst, cancellationToken);
                 var documentWithImport = document.WithSyntaxRoot(rootWithImport);
+                // This only formats the annotated import we just added, not the entire document.
                 var formattedDocumentWithImport = await Formatter.FormatAsync(documentWithImport, Formatter.Annotation, cancellationToken: cancellationToken).ConfigureAwait(false);
 
                 var builder = ArrayBuilder<TextChange>.GetInstance();
 
-                // Get text change for add improt
+                // Get text change for add import
                 var importChanges = await formattedDocumentWithImport.GetTextChangesAsync(document, cancellationToken).ConfigureAwait(false);
                 builder.AddRange(importChanges);
 

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

@@ -263,7 +263,7 @@ private enum ItemPropertyKind : byte
             {
                 IsPublic = 0x1,
                 IsGeneric = 0x2,
-                IsAttribute = 0x4
+                IsAttribute = 0x4,
             }
         }
     }

src/Features/Core/Portable/Completion/Providers/ImportCompletionProvider/ExtensionMethodImportCompletionService.CacheEntry.cs

@@ -65,19 +65,13 @@ public void AddItem(SyntaxTreeIndex syntaxIndex)
                     {
                         foreach (var index in symbolInfoIndices)
                         {
-                            if (syntaxIndex.TryGetDeclaredSymbolInfo(index, out var methodInfo))
-                            {
-                                _simpleItemBuilder.Add(targetType, methodInfo);
-                            }
+                            _simpleItemBuilder.Add(targetType, syntaxIndex.DeclaredSymbolInfos[index]);
                         }
                     }
 
                     foreach (var index in syntaxIndex.ComplexExtensionMethodInfo)
                     {
-                        if (syntaxIndex.TryGetDeclaredSymbolInfo(index, out var methodInfo))
-                        {
-                            _complexItemBuilder.Add(methodInfo);
-                        }
+                        _complexItemBuilder.Add(syntaxIndex.DeclaredSymbolInfos[index]);
                     }
                 }
 
@@ -107,6 +101,8 @@ public CacheServiceFactory()
             IImportCompletionCacheService<CacheEntry, object> cacheService,
             CancellationToken cancellationToken)
         {
+            // While we are caching data from SyntaxTreeInfo, all the things we cared about here are actually based on sources symbols.
+            // So using source symbol checksum would suffice.
             var checksum = await SymbolTreeInfo.GetSourceSymbolsChecksumAsync(project, cancellationToken).ConfigureAwait(false);
 
             // Cache miss, create all requested items.

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

@@ -37,6 +37,10 @@ public SerializableImportCompletionItem(string symbolKeyData, string name, int a
         }
     }
 
+    /// <summary>
+    /// Provides completion items for extension methods from unimported namespace.
+    /// </summary>
+    /// <remarks>It runs out-of-proc if it's enabled</remarks>
     internal static partial class ExtensionMethodImportCompletionService
     {
         private static readonly char[] s_dotSeparator = new char[] { '.' };
@@ -174,6 +178,9 @@ internal static partial class ExtensionMethodImportCompletionService
             using var symbolDisposer = ArrayBuilder<SymbolTreeInfo>.GetInstance(out var symbolBuilder);
             var peReferences = PooledHashSet<PortableExecutableReference>.GetInstance();
 
+            // We will only create missing indices in the following cases, neither would block completion.
+            // 1. User explicitly asked for them using expander. 
+            // 2. We are trying to populate the data in background.
             var shouldCreateIndex = isExpandedCompletion || isPrecalculation;
 
             try

src/Workspaces/CSharp/Portable/FindSymbols/CSharpDeclaredSymbolInfoFactoryService.cs

@@ -492,6 +492,7 @@ private bool IsExtensionMethod(MethodDeclarationSyntax method)
             => method.ParameterList.Parameters.Count > 0 &&
                method.ParameterList.Parameters[0].Modifiers.Any(SyntaxKind.ThisKeyword);
 
+        // Root namespace is a VB only concept, which basically means root namespace is always global in C#.
         public override string GetRootNamespace(CompilationOptions compilationOptions)
             => string.Empty;
 

src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex.cs

@@ -80,8 +80,10 @@ public static Task<SyntaxTreeIndex> GetIndexAsync(Document document, Cancellatio
                     return null;
                 }
 
-                // Populate our cache with this data.
+                // Populate our caches with this data.
                 s_documentToIndex.GetValue(document, _ => index);
+                s_documentIdToIndex.Remove(document.Id);
+                s_documentIdToIndex.GetValue(document.Id, _ => index);
             }
 
             return index;
@@ -106,18 +108,16 @@ public static Task<SyntaxTreeIndex> GetIndexAsync(Document document, Cancellatio
 
             // What we have in memory isn't valid.  Try to load from the persistence service.
             index = await LoadAsync(document, checksum, cancellationToken).ConfigureAwait(false);
-            if (index == null && !loadOnly)
+            if (index != null || loadOnly)
             {
-                // alright, we don't have cached information, re-calculate them here.
-                index = await CreateIndexAsync(document, checksum, cancellationToken).ConfigureAwait(false);
+                return index;
+            }
 
-                // okay, persist this info
-                await index.SaveAsync(document, cancellationToken).ConfigureAwait(false);
+            // alright, we don't have cached information, re-calculate them here.
+            index = await CreateIndexAsync(document, checksum, cancellationToken).ConfigureAwait(false);
 
-                // Populate our cache with this data.
-                s_documentIdToIndex.Remove(document.Id);
-                s_documentIdToIndex.GetValue(document.Id, _ => index);
-            }
+            // okay, persist this info
+            await index.SaveAsync(document, cancellationToken).ConfigureAwait(false);
 
             return index;
         }

src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex_Create.cs

@@ -248,7 +248,7 @@ internal sealed partial class SyntaxTreeIndex
                     new DeclarationInfo(
                             declaredSymbolInfos.ToImmutableAndFree()),
                     new ExtensionMethodInfo(
-                        simpleExtensionMethodInfoBuilder.ToImmutableDictionary(GetKey, GetImmutableValue),
+                        simpleExtensionMethodInfoBuilder.ToImmutableDictionary(s_getKey, s_getValuesAsImmutableArray),
                         complexExtensionMethodInfoBuilder.ToImmutable()));
             }
             finally
@@ -260,53 +260,50 @@ internal sealed partial class SyntaxTreeIndex
                 complexExtensionMethodInfoBuilder.Free();
                 usingAliases.Free();
             }
+        }
 
-            static void AddExtensionMethodInfo(
-                IDeclaredSymbolInfoFactoryService infoFactory,
-                SyntaxNode node,
-                PooledDictionary<string, string> aliases,
-                int declaredSymbolInfoIndex,
-                DeclaredSymbolInfo declaredSymbolInfo,
-                PooledDictionary<string, ArrayBuilder<int>> simpleInfoBuilder,
-                ArrayBuilder<int> complexInfoBuilder)
+        private static readonly Func<KeyValuePair<string, ArrayBuilder<int>>, string> s_getKey = kvp => kvp.Key;
+        private static readonly Func<KeyValuePair<string, ArrayBuilder<int>>, ImmutableArray<int>> s_getValuesAsImmutableArray = kvp => kvp.Value.ToImmutableAndFree();
+
+        private static void AddExtensionMethodInfo(
+            IDeclaredSymbolInfoFactoryService infoFactory,
+            SyntaxNode node,
+            PooledDictionary<string, string> aliases,
+            int declaredSymbolInfoIndex,
+            DeclaredSymbolInfo declaredSymbolInfo,
+            PooledDictionary<string, ArrayBuilder<int>> simpleInfoBuilder,
+            ArrayBuilder<int> complexInfoBuilder)
+        {
+            if (declaredSymbolInfo.Kind != DeclaredSymbolInfoKind.ExtensionMethod)
             {
-                if (declaredSymbolInfo.Kind != DeclaredSymbolInfoKind.ExtensionMethod)
-                {
-                    return;
-                }
-
-                if (!infoFactory.TryGetTargetTypeName(node, out var targetTypeName))
-                {
-                    return;
-                }
-
-                // complex type
-                if (targetTypeName == null)
-                {
-                    complexInfoBuilder.Add(declaredSymbolInfoIndex);
-                    return;
-                }
+                return;
+            }
 
-                if (aliases.TryGetValue(targetTypeName, out var originalName))
-                {
-                    targetTypeName = originalName;
-                }
+            if (!infoFactory.TryGetTargetTypeName(node, out var targetTypeName))
+            {
+                return;
+            }
 
-                // simple type
-                if (!simpleInfoBuilder.TryGetValue(targetTypeName, out var arrayBuilder))
-                {
-                    arrayBuilder = ArrayBuilder<int>.GetInstance();
-                    simpleInfoBuilder[targetTypeName] = arrayBuilder;
-                }
+            // complex type
+            if (targetTypeName == null)
+            {
+                complexInfoBuilder.Add(declaredSymbolInfoIndex);
+                return;
+            }
 
-                arrayBuilder.Add(declaredSymbolInfoIndex);
+            if (aliases.TryGetValue(targetTypeName, out var originalName))
+            {
+                targetTypeName = originalName;
             }
 
-            static string GetKey(KeyValuePair<string, ArrayBuilder<int>> kvp)
-                => kvp.Key;
+            // simple type
+            if (!simpleInfoBuilder.TryGetValue(targetTypeName, out var arrayBuilder))
+            {
+                arrayBuilder = ArrayBuilder<int>.GetInstance();
+                simpleInfoBuilder[targetTypeName] = arrayBuilder;
+            }
 
-            static ImmutableArray<int> GetImmutableValue(KeyValuePair<string, ArrayBuilder<int>> kvp)
-                => kvp.Value.ToImmutableAndFree();
+            arrayBuilder.Add(declaredSymbolInfoIndex);
         }
 
         private static StringTable GetStringTable(Project project)

src/Workspaces/Core/Portable/FindSymbols/SyntaxTree/SyntaxTreeIndex_Forwarders.cs

@@ -17,18 +17,6 @@ public ImmutableArray<int> ComplexExtensionMethodInfo
 
         public bool ContainsExtensionMethod => SimpleExtensionMethodInfo.Count > 0 || ComplexExtensionMethodInfo.Length > 0;
 
-        public bool TryGetDeclaredSymbolInfo(int index, out DeclaredSymbolInfo declaredSymbolInfo)
-        {
-            if (index >= DeclaredSymbolInfos.Length)
-            {
-                declaredSymbolInfo = default;
-                return false;
-            }
-
-            declaredSymbolInfo = DeclaredSymbolInfos[index];
-            return true;
-        }
-
         public bool ProbablyContainsIdentifier(string identifier) => _identifierInfo.ProbablyContainsIdentifier(identifier);
         public bool ProbablyContainsEscapedIdentifier(string identifier) => _identifierInfo.ProbablyContainsEscapedIdentifier(identifier);