Merge pull request #43207 from CyrusNajmabadi/allowSymbolOOp3

Remove restriction limiting OOPability of some operations.

src/Workspaces/Core/Portable/FindSymbols/IRemoteSymbolFinder.cs

@@ -3,7 +3,7 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.Remote;
@@ -17,19 +17,19 @@ Task FindReferencesAsync(PinnedSolutionInfo solutionInfo, SerializableSymbolAndP
 
         Task FindLiteralReferencesAsync(PinnedSolutionInfo solutionInfo, object value, TypeCode typeCode, CancellationToken cancellationToken);
 
-        Task<IList<SerializableSymbolAndProjectId>> FindAllDeclarationsWithNormalQueryAsync(
+        Task<ImmutableArray<SerializableSymbolAndProjectId>> FindAllDeclarationsWithNormalQueryAsync(
             PinnedSolutionInfo solutionInfo, ProjectId projectId, string name, SearchKind searchKind, SymbolFilter criteria, CancellationToken cancellationToken);
 
-        Task<IList<SerializableSymbolAndProjectId>> FindSolutionSourceDeclarationsWithNormalQueryAsync(
+        Task<ImmutableArray<SerializableSymbolAndProjectId>> FindSolutionSourceDeclarationsWithNormalQueryAsync(
             PinnedSolutionInfo solutionInfo, string name, bool ignoreCase, SymbolFilter criteria, CancellationToken cancellationToken);
 
-        Task<IList<SerializableSymbolAndProjectId>> FindProjectSourceDeclarationsWithNormalQueryAsync(
+        Task<ImmutableArray<SerializableSymbolAndProjectId>> FindProjectSourceDeclarationsWithNormalQueryAsync(
             PinnedSolutionInfo solutionInfo, ProjectId projectId, string name, bool ignoreCase, SymbolFilter criteria, CancellationToken cancellationToken);
 
-        Task<IList<SerializableSymbolAndProjectId>> FindSolutionSourceDeclarationsWithPatternAsync(
+        Task<ImmutableArray<SerializableSymbolAndProjectId>> FindSolutionSourceDeclarationsWithPatternAsync(
             PinnedSolutionInfo solutionInfo, string pattern, SymbolFilter criteria, CancellationToken cancellationToken);
 
-        Task<IList<SerializableSymbolAndProjectId>> FindProjectSourceDeclarationsWithPatternAsync(
+        Task<ImmutableArray<SerializableSymbolAndProjectId>> FindProjectSourceDeclarationsWithPatternAsync(
             PinnedSolutionInfo solutionInfo, ProjectId projectId, string pattern, SymbolFilter criteria, CancellationToken cancellationToken);
     }
 }

src/Workspaces/Core/Portable/FindSymbols/SymbolFinder.FindReferencesServerCallback.cs

@@ -15,15 +15,20 @@ public static partial class SymbolFinder
         /// Callback object we pass to the OOP server to hear about the result 
         /// of the FindReferencesEngine as it executes there.
         /// </summary>
-        internal sealed class FindReferencesServerCallback
+        internal sealed class FindReferencesServerCallback : IEqualityComparer<SerializableSymbolAndProjectId>
         {
             private readonly Solution _solution;
             private readonly IStreamingFindReferencesProgress _progress;
             private readonly CancellationToken _cancellationToken;
 
             private readonly object _gate = new object();
-            private readonly Dictionary<SerializableSymbolAndProjectId, SymbolAndProjectId> _definitionMap =
-                new Dictionary<SerializableSymbolAndProjectId, SymbolAndProjectId>();
+
+            /// <summary>
+            /// Note: for purposes of Equality/Hashing, all that we use is the underlying SymbolKey.  That's because FAR
+            /// only cares if it is looking at the same symbol, it don't care if the symbol came from a different
+            /// project or not.
+            /// </summary>
+            private readonly Dictionary<SerializableSymbolAndProjectId, SymbolAndProjectId> _definitionMap;
 
             public FindReferencesServerCallback(
                 Solution solution,
@@ -33,6 +38,7 @@ public FindReferencesServerCallback(
                 _solution = solution;
                 _progress = progress;
                 _cancellationToken = cancellationToken;
+                _definitionMap = new Dictionary<SerializableSymbolAndProjectId, SymbolAndProjectId>(this);
             }
 
             public Task AddItemsAsync(int count) => _progress.ProgressTracker.AddItemsAsync(count);
@@ -85,6 +91,12 @@ public async Task OnReferenceFoundAsync(
 
                 await _progress.OnReferenceFoundAsync(symbolAndProjectId, referenceLocation).ConfigureAwait(false);
             }
+
+            bool IEqualityComparer<SerializableSymbolAndProjectId>.Equals(SerializableSymbolAndProjectId x, SerializableSymbolAndProjectId y)
+                => y.SymbolKeyData.Equals(x.SymbolKeyData);
+
+            int IEqualityComparer<SerializableSymbolAndProjectId>.GetHashCode(SerializableSymbolAndProjectId obj)
+                => obj.SymbolKeyData.GetHashCode();
         }
     }
 }

src/Workspaces/Core/Portable/FindSymbols/SymbolFinder_FindReferences_Current.cs

@@ -46,7 +46,7 @@ internal static async Task FindReferencesAsync(
                             solution,
                             new object[]
                             {
-                                SerializableSymbolAndProjectId.Dehydrate(symbolAndProjectId),
+                                SerializableSymbolAndProjectId.Dehydrate(solution, symbolAndProjectId.Symbol, cancellationToken),
                                 documents?.Select(d => d.Id).ToArray(),
                                 SerializableFindReferencesSearchOptions.Dehydrate(options),
                             },

src/Workspaces/Core/Portable/Remote/RemoteArguments.cs

@@ -37,35 +37,30 @@ public FindReferencesSearchOptions Rehydrate()
         }
     }
 
-    internal class SerializableSymbolAndProjectId : IEquatable<SerializableSymbolAndProjectId>
+    internal class SerializableSymbolAndProjectId
     {
         public string SymbolKeyData;
         public ProjectId ProjectId;
 
-        public override int GetHashCode()
-            => Hash.Combine(SymbolKeyData, ProjectId.GetHashCode());
-
-        public override bool Equals(object obj)
-            => Equals(obj as SerializableSymbolAndProjectId);
-
-        public bool Equals(SerializableSymbolAndProjectId other)
-            => other != null && SymbolKeyData.Equals(other.SymbolKeyData) && ProjectId.Equals(other.ProjectId);
-
         public static SerializableSymbolAndProjectId Dehydrate(
-            IAliasSymbol alias, Document document)
+            IAliasSymbol alias, Document document, CancellationToken cancellationToken)
         {
             return alias == null
                 ? null
-                : Dehydrate(new SymbolAndProjectId(alias, document.Project.Id));
+                : Dehydrate(document.Project.Solution, alias, cancellationToken);
         }
 
         public static SerializableSymbolAndProjectId Dehydrate(
-            SymbolAndProjectId symbolAndProjectId)
+            Solution solution, ISymbol symbol, CancellationToken cancellationToken)
         {
+            var symbolKey = symbol.GetSymbolKey(cancellationToken);
+            var projectId = solution.GetExactProjectId(symbol);
+            Contract.ThrowIfNull(projectId, WorkspacesResources.Symbols_project_could_not_be_found_in_the_provided_solution);
+
             return new SerializableSymbolAndProjectId
             {
-                SymbolKeyData = symbolAndProjectId.Symbol.GetSymbolKey().ToString(),
-                ProjectId = symbolAndProjectId.ProjectId
+                SymbolKeyData = symbolKey.ToString(),
+                ProjectId = projectId,
             };
         }
 
@@ -163,12 +158,12 @@ internal class SerializableReferenceLocation
         public CandidateReason CandidateReason { get; set; }
 
         public static SerializableReferenceLocation Dehydrate(
-            ReferenceLocation referenceLocation)
+            ReferenceLocation referenceLocation, CancellationToken cancellationToken)
         {
             return new SerializableReferenceLocation
             {
                 Document = referenceLocation.Document.Id,
-                Alias = SerializableSymbolAndProjectId.Dehydrate(referenceLocation.Alias, referenceLocation.Document),
+                Alias = SerializableSymbolAndProjectId.Dehydrate(referenceLocation.Alias, referenceLocation.Document, cancellationToken),
                 Location = referenceLocation.Location.SourceSpan,
                 IsImplicit = referenceLocation.IsImplicit,
                 SymbolUsageInfo = SerializableSymbolUsageInfo.Dehydrate(referenceLocation.SymbolUsageInfo),

src/Workspaces/Core/Portable/Workspace/Solution/Solution.cs

@@ -126,6 +126,29 @@ private static Project CreateProject(ProjectId projectId, Solution solution)
             return projectState == null ? null : GetProject(projectState.Id);
         }
 
+        /// <summary>
+        /// Given a <paramref name="symbol"/> returns the <see cref="ProjectId"/> of the <see cref="Project"/> it came
+        /// from.  Returns <see langword="null"/> if <paramref name="symbol"/> does not come from <paramref name="symbol"/>.
+        /// </summary>
+        /// <remarks>
+        /// This function differs from <see cref="GetProject(IAssemblySymbol, CancellationToken)"/> in terms of how it
+        /// treats <see cref="IAssemblySymbol"/>s.  Specifically, say there is the following:
+        ///
+        /// <c>
+        /// Project-A, containing Symbol-A.<para/>
+        /// Project-B, with a reference to Project-A, and usage of Symbol-A.
+        /// </c>
+        ///
+        /// It is possible (with retargeting, and other complex cases) that Symbol-A from Project-B will be a different
+        /// symbol than Symbol-A from Project-A.  However, <see cref="GetProject(IAssemblySymbol, CancellationToken)"/>
+        /// will always try to return Project-A for either of the Symbol-A's, as it prefers to return the original
+        /// Source-Project of the original definition, not the project that actually produced the symbol.  For many
+        /// features this is an acceptable abstraction.  However, for some cases (Find-References in particular) it is
+        /// necessary to resolve symbols back to the actual project/compilation that produced them for correctness.
+        /// </remarks>
+        internal ProjectId? GetExactProjectId(ISymbol symbol)
+            => _state.GetExactProjectId(symbol);
+
         /// <summary>
         /// True if the solution contains the document in one of its projects
         /// </summary>

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.State.cs

@@ -7,6 +7,7 @@
 using System;
 using System.Collections.Immutable;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using Microsoft.CodeAnalysis;
 using Roslyn.Utilities;
 
@@ -30,7 +31,10 @@ private class State
                 /// <summary>
                 /// The base <see cref="State"/> that starts with everything empty.
                 /// </summary>
-                public static readonly State Empty = new State(compilation: null, declarationOnlyCompilation: null, generatorDriver: new TrackedGeneratorDriver(null));
+                public static readonly State Empty = new State(
+                    compilation: null, declarationOnlyCompilation: null,
+                    generatorDriver: new TrackedGeneratorDriver(null),
+                    assemblyAndModuleSet: null);
 
                 /// <summary>
                 /// A strong reference to the declaration-only compilation. This compilation isn't used to produce symbols,
@@ -46,6 +50,21 @@ private class State
 
                 public TrackedGeneratorDriver GeneratorDriver { get; }
 
+                /// <summary>
+                /// Weak table of the assembly and module symbols that this compilation tracker has created.  This can
+                /// be used to determine which project an assembly symbol came from after the fact.  This is needed as
+                /// the compilation an assembly came from can be GC'ed and further requests to get that compilation (or
+                /// any of it's assemblies) may produce new assembly symbols.
+                /// </summary>
+                /// <remarks>
+                /// Ideally this would just be <c>ConditionalWeakSet&lt;ISymbol&gt;</c>.  Effectively we just want to
+                /// hold onto the symbols as long as someone else is keeping them alive.  And we don't actually need
+                /// them to map to anything.  We just use their existence to know if our project was the project it came
+                /// from.  However, ConditionalWeakTable is the best tool we have, so we simulate a set by just using a
+                /// table and mapping the keys to the <see langword="null"/> value.
+                /// </remarks>
+                public readonly ConditionalWeakTable<ISymbol, object?>? AssemblyAndModuleSet;
+
                 /// <summary>
                 /// Specifies whether <see cref="FinalCompilation"/> and all compilations it depends on contain full information or not. This can return
                 /// <see langword="null"/> if the state isn't at the point where it would know, and it's necessary to transition to <see cref="FinalState"/> to figure that out.
@@ -57,14 +76,19 @@ private class State
                 /// </summary>
                 public virtual ValueSource<Optional<Compilation>>? FinalCompilation => null;
 
-                protected State(ValueSource<Optional<Compilation>>? compilation, Compilation? declarationOnlyCompilation, TrackedGeneratorDriver generatorDriver)
+                protected State(
+                    ValueSource<Optional<Compilation>>? compilation,
+                    Compilation? declarationOnlyCompilation,
+                    TrackedGeneratorDriver generatorDriver,
+                    ConditionalWeakTable<ISymbol, object?>? assemblyAndModuleSet)
                 {
                     // Declaration-only compilations should never have any references
                     Contract.ThrowIfTrue(declarationOnlyCompilation != null && declarationOnlyCompilation.ExternalReferences.Any());
 
                     Compilation = compilation;
                     DeclarationOnlyCompilation = declarationOnlyCompilation;
                     GeneratorDriver = generatorDriver;
+                    AssemblyAndModuleSet = assemblyAndModuleSet;
                 }
 
                 public static State Create(
@@ -90,6 +114,25 @@ public static ValueSource<Optional<Compilation>> CreateValueSource(
                         ? new WeakValueSource<Compilation>(compilation)
                         : (ValueSource<Optional<Compilation>>)new ConstantValueSource<Optional<Compilation>>(compilation);
                 }
+
+                public static ConditionalWeakTable<ISymbol, object?> GetAssemblyAndModuleSet(Compilation compilation)
+                {
+                    var result = new ConditionalWeakTable<ISymbol, object?>();
+
+                    var compAssembly = compilation.Assembly;
+                    result.Add(compAssembly, null);
+
+                    foreach (var reference in compilation.References)
+                    {
+                        var symbol = compilation.GetAssemblyOrModuleSymbol(reference);
+                        if (symbol == null)
+                            continue;
+
+                        result.Add(symbol, null);
+                    }
+
+                    return result;
+                }
             }
 
             /// <summary>
@@ -106,7 +149,8 @@ public InProgressState(
                     ImmutableArray<(ProjectState state, CompilationAndGeneratorDriverTranslationAction action)> intermediateProjects)
                     : base(compilation: new ConstantValueSource<Optional<Compilation>>(inProgressCompilation),
                            declarationOnlyCompilation: null,
-                           generatorDriver: inProgressGeneratorDriver)
+                           generatorDriver: inProgressGeneratorDriver,
+                           GetAssemblyAndModuleSet(inProgressCompilation))
                 {
                     Contract.ThrowIfTrue(intermediateProjects.IsDefault);
                     Contract.ThrowIfFalse(intermediateProjects.Length > 0);
@@ -121,7 +165,10 @@ public InProgressState(
             private sealed class LightDeclarationState : State
             {
                 public LightDeclarationState(Compilation declarationOnlyCompilation)
-                    : base(compilation: null, declarationOnlyCompilation: declarationOnlyCompilation, generatorDriver: new TrackedGeneratorDriver(null))
+                    : base(compilation: null,
+                           declarationOnlyCompilation: declarationOnlyCompilation,
+                           generatorDriver: new TrackedGeneratorDriver(null),
+                           assemblyAndModuleSet: null)
                 {
                 }
             }
@@ -133,7 +180,10 @@ public LightDeclarationState(Compilation declarationOnlyCompilation)
             private sealed class FullDeclarationState : State
             {
                 public FullDeclarationState(Compilation declarationCompilation, TrackedGeneratorDriver generatorDriver)
-                    : base(new WeakValueSource<Compilation>(declarationCompilation), declarationCompilation.Clone().RemoveAllReferences(), generatorDriver)
+                    : base(new WeakValueSource<Compilation>(declarationCompilation),
+                           declarationCompilation.Clone().RemoveAllReferences(),
+                           generatorDriver,
+                           GetAssemblyAndModuleSet(declarationCompilation))
                 {
                 }
             }
@@ -160,8 +210,12 @@ public FinalState(
                     ValueSource<Optional<Compilation>> compilationWithoutGeneratedFilesSource,
                     Compilation compilationWithoutGeneratedFiles,
                     TrackedGeneratorDriver generatorDriver,
-                    bool hasSuccessfullyLoaded)
-                    : base(compilationWithoutGeneratedFilesSource, compilationWithoutGeneratedFiles.Clone().RemoveAllReferences(), generatorDriver)
+                    bool hasSuccessfullyLoaded,
+                    ConditionalWeakTable<ISymbol, object?>? compilationAssemblies)
+                    : base(compilationWithoutGeneratedFilesSource,
+                           compilationWithoutGeneratedFiles.Clone().RemoveAllReferences(),
+                           generatorDriver,
+                           compilationAssemblies)
                 {
                     HasSuccessfullyLoaded = hasSuccessfullyLoaded;
                     FinalCompilation = finalCompilationSource;

src/Workspaces/Core/Portable/Workspace/Solution/SolutionState.CompilationTracker.cs

@@ -100,6 +100,14 @@ public bool HasCompilation
                 }
             }
 
+            public bool ContainsAssemblyOrModule(ISymbol assemblyOrModule)
+            {
+                Debug.Assert(assemblyOrModule.Kind == SymbolKind.Assembly || assemblyOrModule.Kind == SymbolKind.NetModule);
+                var state = this.ReadState();
+                var assemblyAndModuleSet = state.AssemblyAndModuleSet;
+                return assemblyAndModuleSet != null && assemblyAndModuleSet.TryGetValue(assemblyOrModule, out _);
+            }
+
             /// <summary>
             /// Creates a new instance of the compilation info, retaining any already built
             /// compilation state as the now 'old' state
@@ -188,7 +196,8 @@ public CompilationTracker FreezePartialStateWithTree(SolutionState solution, Doc
                         new ConstantValueSource<Optional<Compilation>>(inProgressCompilation),
                         inProgressCompilation,
                         generatorDriver: new TrackedGeneratorDriver(null),
-                        hasSuccessfullyLoaded: false));
+                        hasSuccessfullyLoaded: false,
+                        State.GetAssemblyAndModuleSet(inProgressCompilation)));
             }
 
             /// <summary>
@@ -710,7 +719,8 @@ private async Task<CompilationInfo> FinalizeCompilationAsync(
                             State.CreateValueSource(compilationWithoutGeneratedFiles, solution.Services),
                             compilationWithoutGeneratedFiles,
                             generatorDriver,
-                            hasSuccessfullyLoaded),
+                            hasSuccessfullyLoaded,
+                            State.GetAssemblyAndModuleSet(compilation)),
                         solution.Services);
 
                     return new CompilationInfo(compilation, hasSuccessfullyLoaded);