Merge pull request #17863 from CyrusNajmabadi/locationResolution

Be resilient to being unable to resolve the location of a symbol in a different compilation.

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

@@ -212,7 +212,9 @@ public static IEnumerable<TSymbol> FindSimilarSymbols<TSymbol>(TSymbol symbol, C
             }
 
             var key = symbol.GetSymbolKey();
-            return key.Resolve(compilation, cancellationToken: cancellationToken).GetAllSymbols().OfType<TSymbol>();
+
+            // We may be talking about different compilations.  So do not try to resolve locations.
+            return key.Resolve(compilation, resolveLocations: false, cancellationToken: cancellationToken).GetAllSymbols().OfType<TSymbol>();
         }
     }
 }

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

@@ -52,7 +52,11 @@ public bool Equals(SerializableSymbolAndProjectId other)
             var projectId = ProjectId;
             var project = solution.GetProject(projectId);
             var compilation = await project.GetCompilationAsync(cancellationToken).ConfigureAwait(false);
-            var symbol = SymbolKey.Resolve(SymbolKeyData, compilation, cancellationToken: cancellationToken).GetAnySymbol();
+
+            // The server and client should both be talking about the same compilation.  As such
+            // locations in symbols are save to resolve as we rehydrate the SymbolKey.
+            var symbol = SymbolKey.Resolve(
+                SymbolKeyData, compilation, resolveLocations: true, cancellationToken: cancellationToken).GetAnySymbol();
             Debug.Assert(symbol != null, "We should always be able to resolve a symbol back on the host side.");
             return new SymbolAndProjectId(symbol, projectId);
         }

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.BodyLevelSymbolKey.cs

@@ -76,7 +76,7 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
                     // a SpeculativeSemanticModel, containingSymbol.ContainingAssembly.Compilation
                     // may not have been rebuilt to reflect the trees used by the 
                     // SpeculativeSemanticModel to produce containingSymbol. In that case,
-                    // asking the ContainingAssembly's complation for a SemanticModel based
+                    // asking the ContainingAssembly's compilation for a SemanticModel based
                     // on trees for containingSymbol with throw an ArgumentException.
                     // Unfortunately, the best way to avoid this (currently) is to see if
                     // we're asking for a model for a tree that's part of the compilation.

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.SymbolKeyReader.cs

@@ -216,9 +216,7 @@ public RemoveAssemblySymbolKeysReader()
             }
 
             public void Initialize(string data)
-            {
-                base.Initialize(data, CancellationToken.None);
-            }
+                => base.Initialize(data, CancellationToken.None);
 
             public string RemoveAssemblySymbolKeys()
             {
@@ -247,7 +245,7 @@ public string RemoveAssemblySymbolKeys()
                     }
                     else
                     {
-                        // All ther characters we pass along directly to the string builder.
+                        // All other characters we pass along directly to the string builder.
                         _builder.Append(Eat(ch));
                     }
                 }
@@ -258,7 +256,7 @@ public string RemoveAssemblySymbolKeys()
             protected override object CreateResultForString(int start, int end, bool hasEmbeddedQuote)
             {
                 // 'start' is right after the open quote, and 'end' is right before the close quote.
-                // However, we want to include both quotes in teh result.
+                // However, we want to include both quotes in the result.
                 _builder.Append(DoubleQuoteChar);
                 if (!_skipString)
                 {
@@ -291,6 +289,7 @@ private class SymbolKeyReader : Reader<string>
             public SymbolEquivalenceComparer Comparer { get; private set; }
 
             private List<IMethodSymbol> _methodSymbolStack = new List<IMethodSymbol>();
+            private bool _resolveLocations;
 
             private SymbolKeyReader()
             {
@@ -304,6 +303,7 @@ public override void Dispose()
                 _idToResult.Clear();
                 Compilation = null;
                 IgnoreAssemblyKey = false;
+                _resolveLocations = false;
                 Comparer = null;
                 _methodSymbolStack.Clear();
 
@@ -313,10 +313,11 @@ public override void Dispose()
 
             public static SymbolKeyReader GetReader(
                 string data, Compilation compilation,
-                bool ignoreAssemblyKey, CancellationToken cancellationToken)
+                bool ignoreAssemblyKey, bool resolveLocations,
+                CancellationToken cancellationToken)
             {
                 var reader = s_readerPool.Allocate();
-                reader.Initialize(data, compilation, ignoreAssemblyKey, cancellationToken);
+                reader.Initialize(data, compilation, ignoreAssemblyKey, resolveLocations, cancellationToken);
                 return reader;
             }
 
@@ -324,11 +325,14 @@ public override void Dispose()
                 string data,
                 Compilation compilation,
                 bool ignoreAssemblyKey,
+                bool resolveLocations,
                 CancellationToken cancellationToken)
             {
                 base.Initialize(data, cancellationToken);
                 Compilation = compilation;
                 IgnoreAssemblyKey = ignoreAssemblyKey;
+                _resolveLocations = resolveLocations;
+
                 Comparer = ignoreAssemblyKey
                     ? SymbolEquivalenceComparer.IgnoreAssembliesInstance
                     : SymbolEquivalenceComparer.Instance;
@@ -458,9 +462,7 @@ private SymbolKeyResolution ReadWorker(SymbolKeyType type)
             }
 
             public ImmutableArray<SymbolKeyResolution> ReadSymbolKeyArray()
-            {
-                return ReadArray(_readSymbolKey);
-            }
+                => ReadArray(_readSymbolKey);
 
             #endregion
 
@@ -494,31 +496,45 @@ public Location ReadLocation()
                 }
 
                 var kind = (LocationKind)ReadInteger();
-                if (kind == LocationKind.None)
-                {
-                    return Location.None;
-                }
-                else if (kind == LocationKind.SourceFile)
+                if (kind == LocationKind.SourceFile)
                 {
                     var filePath = ReadString();
                     var start = ReadInteger();
                     var length = ReadInteger();
-                    return CreateSourceLocation(filePath, start, length);
+
+                    if (_resolveLocations)
+                    {
+                        // The syntax tree can be null if we're resolving this location in a compilation
+                        // that does not contain this file.  In this case, just map this location to None.
+                        var syntaxTree = GetSyntaxTree(filePath);
+                        if (syntaxTree != null)
+                        {
+                            return Location.Create(syntaxTree, new TextSpan(start, length));
+                        }
+                    }
                 }
-                else
+                else if (kind == LocationKind.MetadataFile)
                 {
-                    Debug.Assert(kind == LocationKind.MetadataFile);
-                    var assembly = ReadSymbolKey();
+                    var assemblyResolution = ReadSymbolKey();
                     var moduleName = ReadString();
-                    return CreateModuleLocation(assembly, moduleName);
+
+                    if (_resolveLocations)
+                    {
+                        // We may be resolving in a compilation where we don't have a module
+                        // with this name.  In that case, just map this location to none.
+                        if (assemblyResolution.GetAnySymbol() is IAssemblySymbol assembly)
+                        {
+                            var module = assembly.Modules.FirstOrDefault(m => m.MetadataName == moduleName);
+                            var location = module?.Locations.FirstOrDefault();
+                            if (location != null)
+                            {
+                                return location;
+                            }
+                        }
+                    }
                 }
-            }
 
-            private Location CreateSourceLocation(string filePath, int start, int length)
-            {
-                var syntaxTree = GetSyntaxTree(filePath);
-                Debug.Assert(syntaxTree != null);
-                return Location.Create(syntaxTree, new TextSpan(start, length));
+                return Location.None;
             }
 
             private Location CreateModuleLocation(
@@ -531,9 +547,7 @@ private Location CreateSourceLocation(string filePath, int start, int length)
             }
 
             public ImmutableArray<Location> ReadLocationArray()
-            {
-                return ReadArray(_readLocation);
-            }
+                => ReadArray(_readLocation);
 
             #endregion
         }

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.TupleTypeSymbolKey.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;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
+using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis
 {
@@ -14,7 +16,6 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
             {
                 Debug.Assert(symbol.IsTupleType);
 
-
                 var friendlyNames = ArrayBuilder<string>.GetInstance();
                 var locations = ArrayBuilder<Location>.GetInstance();
 
@@ -54,7 +55,7 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
                 {
                     var elementTypes = reader.ReadSymbolKeyArray().SelectAsArray(r => r.GetAnySymbol() as ITypeSymbol);
                     var elementNames = reader.ReadStringArray();
-                    var elementLocations = reader.ReadLocationArray();
+                    var elementLocations = ReadElementLocations(reader);
 
                     if (!elementTypes.Any(t => t == null))
                     {
@@ -73,7 +74,8 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
                 {
                     var underlyingTypeResolution = reader.ReadSymbolKey();
                     var elementNames = reader.ReadStringArray();
-                    var elementLocations = reader.ReadLocationArray();
+                    var elementLocations = ReadElementLocations(reader);
+
                     try
                     {
                         var result = GetAllSymbols<INamedTypeSymbol>(underlyingTypeResolution).Select(
@@ -87,6 +89,19 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
 
                 return new SymbolKeyResolution(reader.Compilation.ObjectType);
             }
+
+            private static ImmutableArray<Location> ReadElementLocations(SymbolKeyReader reader)
+            {
+                // Compiler API requires that all the locations are non-null, or that there is a default
+                // immutable array passed in.
+                var elementLocations = reader.ReadLocationArray();
+                if (elementLocations.All(loc => loc == null))
+                {
+                    elementLocations = default(ImmutableArray<Location>);
+                }
+
+                return elementLocations;
+            }
         }
     }
 }
\ No newline at end of file

src/Workspaces/Core/Portable/SymbolKey/SymbolKey.cs

@@ -100,11 +100,20 @@ public static IEqualityComparer<SymbolKey> GetComparer(bool ignoreCase, bool ign
             return reader.RemoveAssemblySymbolKeys();
         };
 
+        /// <summary>
+        /// Tries to resolve the provided <paramref name="symbolKey"/> in the given 
+        /// <paramref name="compilation"/> to a matching symbol.  <paramref name="resolveLocations"/>
+        /// should only be given <code>true</code> if the symbol was produced from a compilation
+        /// that has the exact same source as the compilation we're resolving against.  Otherwise
+        /// the locations resolved may not actually be correct in the final compilation.
+        /// </summary>
         public static SymbolKeyResolution Resolve(
             string symbolKey, Compilation compilation,
-            bool ignoreAssemblyKey = false, CancellationToken cancellationToken = default(CancellationToken))
+            bool ignoreAssemblyKey = false, bool resolveLocations = false,
+            CancellationToken cancellationToken = default(CancellationToken))
         {
-            using (var reader = SymbolKeyReader.GetReader(symbolKey, compilation, ignoreAssemblyKey, cancellationToken))
+            using (var reader = SymbolKeyReader.GetReader(
+                symbolKey, compilation, ignoreAssemblyKey, resolveLocations, cancellationToken))
             {
                 var result = reader.ReadFirstSymbolKey();
                 Debug.Assert(reader.Position == symbolKey.Length);
@@ -126,15 +135,19 @@ public static string ToString(ISymbol symbol, CancellationToken cancellationToke
             }
         }
 
-        public SymbolKeyResolution Resolve(Compilation compilation, bool ignoreAssemblyKey = false, CancellationToken cancellationToken = default(CancellationToken))
+        public SymbolKeyResolution Resolve(
+            Compilation compilation, 
+            bool ignoreAssemblyKey = false, bool resolveLocations = false,
+            CancellationToken cancellationToken = default(CancellationToken))
         {
-            return Resolve(_symbolKeyData, compilation, ignoreAssemblyKey, cancellationToken);
+            return Resolve(
+                _symbolKeyData, compilation,
+                ignoreAssemblyKey, resolveLocations,
+                cancellationToken);
         }
 
         public override string ToString()
-        {
-            return _symbolKeyData;
-        }
+            => _symbolKeyData;
 
         private static IEnumerable<ISymbol> GetAllSymbols(SymbolKeyResolution info)
         {
@@ -152,9 +165,7 @@ private static IEnumerable<ISymbol> GetAllSymbols(SymbolKeyResolution info)
         }
 
         private static IEnumerable<TType> GetAllSymbols<TType>(SymbolKeyResolution info)
-        {
-            return GetAllSymbols(info).OfType<TType>();
-        }
+            => GetAllSymbols(info).OfType<TType>();
 
         private static SymbolKeyResolution CreateSymbolInfo(IEnumerable<ISymbol> symbols)
         {
@@ -173,14 +184,10 @@ private static SymbolKeyResolution CreateSymbolInfo(ISymbol[] symbols)
         }
 
         private static bool Equals(Compilation compilation, string name1, string name2)
-        {
-            return Equals(compilation.IsCaseSensitive, name1, name2);
-        }
+            => Equals(compilation.IsCaseSensitive, name1, name2);
 
         private static bool Equals(bool isCaseSensitive, string name1, string name2)
-        {
-            return string.Equals(name1, name2, isCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase);
-        }
+            => string.Equals(name1, name2, isCaseSensitive ? StringComparison.Ordinal : StringComparison.OrdinalIgnoreCase);
 
         private static string GetName(string metadataName)
         {

src/Workspaces/CoreTest/SymbolKeyTests.cs

@@ -633,7 +633,7 @@ object LocalFunction<T>()
                 // Ensure we don't crash getting these symbol keys.
                 var id = SymbolKey.ToString(symbol);
                 Assert.NotNull(id);
-                var found = SymbolKey.Resolve(id, compilation).GetAnySymbol();
+                var found = SymbolKey.Resolve(id, compilation: compilation).GetAnySymbol();
                 Assert.NotNull(found);
 
                 // note: we don't check that the symbols are equal.  That's because the compiler
@@ -647,6 +647,80 @@ object LocalFunction<T>()
             Assert.True(tested);
         }
 
+        [Fact, WorkItem(17702, "https://github.com/dotnet/roslyn/issues/17702")]
+        public void TestTupleWithLocalTypeReferences1()
+        {
+            var source = @"
+using System.Linq;
+
+class C
+{
+    void Method((C, int) t)
+    {
+    }
+}";
+            // Tuples store locations along with them.  But we can only recover those locations
+            // if we're re-resolving into a compilation with the same files.
+            var compilation1 = GetCompilation(source, LanguageNames.CSharp, "File1.cs");
+            var compilation2 = GetCompilation(source, LanguageNames.CSharp, "File2.cs");
+
+            var symbol = GetAllSymbols(
+                compilation1.GetSemanticModel(compilation1.SyntaxTrees.Single()),
+                n => n is CSharp.Syntax.MethodDeclarationSyntax).Single();
+
+            // Ensure we don't crash getting these symbol keys.
+            var id = SymbolKey.ToString(symbol);
+            Assert.NotNull(id);
+
+            // Validate that if the client does ask to resolve locations that we
+            // do not crash if those locations cannot be found.
+            var found = SymbolKey.Resolve(id, compilation2, resolveLocations: true).GetAnySymbol();
+            Assert.NotNull(found);
+
+            Assert.Equal(symbol.Name, found.Name);
+            Assert.Equal(symbol.Kind, found.Kind);
+
+            var method = found as IMethodSymbol;
+            Assert.True(method.Parameters[0].Type.IsTupleType);
+        }
+
+        [Fact, WorkItem(17702, "https://github.com/dotnet/roslyn/issues/17702")]
+        public void TestTupleWithLocalTypeReferences2()
+        {
+            var source = @"
+using System.Linq;
+
+class C
+{
+    void Method((C a, int b) t)
+    {
+    }
+}";
+            // Tuples store locations along with them.  But we can only recover those locations
+            // if we're re-resolving into a compilation with the same files.
+            var compilation1 = GetCompilation(source, LanguageNames.CSharp, "File1.cs");
+            var compilation2 = GetCompilation(source, LanguageNames.CSharp, "File2.cs");
+
+            var symbol = GetAllSymbols(
+                compilation1.GetSemanticModel(compilation1.SyntaxTrees.Single()),
+                n => n is CSharp.Syntax.MethodDeclarationSyntax).Single();
+
+            // Ensure we don't crash getting these symbol keys.
+            var id = SymbolKey.ToString(symbol);
+            Assert.NotNull(id);
+
+            // Validate that if the client does ask to resolve locations that we
+            // do not crash if those locations cannot be found.
+            var found = SymbolKey.Resolve(id, compilation2, resolveLocations: true).GetAnySymbol();
+            Assert.NotNull(found);
+
+            Assert.Equal(symbol.Name, found.Name);
+            Assert.Equal(symbol.Kind, found.Kind);
+
+            var method = found as IMethodSymbol;
+            Assert.True(method.Parameters[0].Type.IsTupleType);
+        }
+
         private void TestRoundTrip(IEnumerable<ISymbol> symbols, Compilation compilation, Func<ISymbol, object> fnId = null)
         {
             foreach (var symbol in symbols)
@@ -674,7 +748,7 @@ private void TestRoundTrip(ISymbol symbol, Compilation compilation, Func<ISymbol
             }
         }
 
-        private Compilation GetCompilation(string source, string language)
+        private Compilation GetCompilation(string source, string language, string path = "")
         {
             var references = new[]
             {
@@ -684,12 +758,12 @@ private Compilation GetCompilation(string source, string language)
 
             if (language == LanguageNames.CSharp)
             {
-                var tree = CSharp.SyntaxFactory.ParseSyntaxTree(source);
+                var tree = CSharp.SyntaxFactory.ParseSyntaxTree(source, path: path);
                 return CSharp.CSharpCompilation.Create("Test", syntaxTrees: new[] { tree }, references: references);
             }
             else if (language == LanguageNames.VisualBasic)
             {
-                var tree = VisualBasic.SyntaxFactory.ParseSyntaxTree(source);
+                var tree = VisualBasic.SyntaxFactory.ParseSyntaxTree(source, path: path);
                 return VisualBasic.VisualBasicCompilation.Create("Test", syntaxTrees: new[] { tree }, references: references);
             }