Skip to content

Commit 884fdff

Browse files
CyrusNajmabadiCyrusNajmabadi
authored
Merge pull request #44713 from CyrusNajmabadi/missingNamespace
Fix OOP crashes due to inability to roundtrip error symbols.
2 parents da26bc3 + 8e4264f commit 884fdff

File tree

4 files changed

+310
-16
lines changed

4 files changed

+310
-16
lines changed

‎src/EditorFeatures/Test2/FindReferences/FindReferencesTests.OrdinaryMethodSymbols.vb‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3253,6 +3253,51 @@ End Class
32533253
}
32543254
</Document>
32553255
</Project>
3256+
</Workspace>
3257+
Await TestAPIAndFeature(input, kind, host)
3258+
End Function
3259+
3260+
<WpfTheory, CombinatorialData, Trait(Traits.Feature, Traits.Features.FindReferences)>
3261+
Public Async Function TestOrdinaryMethodWithMissingReferences_CSharp(kind As TestKind, host As TestHost) As Task
3262+
Dim input =
3263+
<Workspace>
3264+
<Project Language="C#" CommonReferences="false">
3265+
<Document>
3266+
class C
3267+
{
3268+
// string will be an error type because we have no actual references.
3269+
private void {|Definition:Goo|}(string s) { }
3270+
3271+
void Bar()
3272+
{
3273+
[|Go$$o|]("");
3274+
[|Goo|](s);
3275+
}
3276+
}
3277+
</Document>
3278+
</Project>
3279+
</Workspace>
3280+
Await TestAPIAndFeature(input, kind, host)
3281+
End Function
3282+
3283+
<WpfTheory, CombinatorialData, Trait(Traits.Feature, Traits.Features.FindReferences)>
3284+
Public Async Function TestOrdinaryMethodWithMissingReferences_VB(kind As TestKind, host As TestHost) As Task
3285+
Dim input =
3286+
<Workspace>
3287+
<Project Language="Visual Basic" CommonReferences="false">
3288+
<Document>
3289+
class C
3290+
' string will be an error type because we have no actual references.
3291+
private sub {|Definition:Goo|}(s as string)
3292+
end sub
3293+
3294+
sub Bar()
3295+
[|Go$$o|]("")
3296+
[|Goo|](s)
3297+
end sub
3298+
end class
3299+
</Document>
3300+
</Project>
32563301
</Workspace>
32573302
Await TestAPIAndFeature(input, kind, host)
32583303
End Function

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

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
// See the LICENSE file in the project root for more information.
44

55
using System.Collections.Immutable;
6+
using Microsoft.CodeAnalysis.PooledObjects;
7+
using Roslyn.Utilities;
68

79
namespace Microsoft.CodeAnalysis
810
{
@@ -13,9 +15,22 @@ private static class ErrorTypeSymbolKey
1315
public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
1416
{
1517
visitor.WriteString(symbol.Name);
16-
visitor.WriteSymbolKey(symbol.ContainingSymbol as INamespaceOrTypeSymbol);
17-
visitor.WriteInteger(symbol.Arity);
18+
switch (symbol.ContainingSymbol)
19+
{
20+
case INamedTypeSymbol parentType:
21+
visitor.WriteInteger(0);
22+
visitor.WriteSymbolKey(parentType);
23+
break;
24+
case INamespaceSymbol parentNamespace:
25+
visitor.WriteInteger(1);
26+
visitor.WriteStringArray(GetContainingNamespaceNamesInReverse(parentNamespace));
27+
break;
28+
default:
29+
visitor.WriteInteger(2);
30+
break;
31+
}
1832

33+
visitor.WriteInteger(symbol.Arity);
1934
if (!symbol.Equals(symbol.ConstructedFrom))
2035
{
2136
visitor.WriteSymbolKeyArray(symbol.TypeArguments);
@@ -26,10 +41,26 @@ public static void Create(INamedTypeSymbol symbol, SymbolKeyWriter visitor)
2641
}
2742
}
2843

44+
/// <summary>
45+
/// For a symbol like <c>System.Collections.Generic.IEnumerable</c>, this would produce <c>"Generic",
46+
/// "Collections", "System"</c>
47+
/// </summary>
48+
private static ImmutableArray<string> GetContainingNamespaceNamesInReverse(INamespaceSymbol namespaceSymbol)
49+
{
50+
using var _ = ArrayBuilder<string>.GetInstance(out var builder);
51+
while (namespaceSymbol != null && namespaceSymbol.Name != "")
52+
{
53+
builder.Add(namespaceSymbol.Name);
54+
namespaceSymbol = namespaceSymbol.ContainingNamespace;
55+
}
56+
57+
return builder.ToImmutable();
58+
}
59+
2960
public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
3061
{
3162
var name = reader.ReadString();
32-
var containingSymbolResolution = reader.ReadSymbolKey();
63+
var containingSymbolResolution = ResolveContainer(reader);
3364
var arity = reader.ReadInteger();
3465

3566
using var typeArguments = reader.ReadSymbolKeyArray<ITypeSymbol>();
@@ -57,6 +88,31 @@ public static SymbolKeyResolution Resolve(SymbolKeyReader reader)
5788
return CreateResolution(result);
5889
}
5990

91+
private static SymbolKeyResolution ResolveContainer(SymbolKeyReader reader)
92+
{
93+
var type = reader.ReadInteger();
94+
95+
if (type == 0)
96+
return reader.ReadSymbolKey();
97+
98+
if (type == 1)
99+
{
100+
using var namespaceNames = reader.ReadStringArray();
101+
var currentNamespace = reader.Compilation.GlobalNamespace;
102+
103+
// have to walk the namespaces in reverse because that's how we encoded them.
104+
for (var i = namespaceNames.Count - 1; i >= 0; i--)
105+
currentNamespace = reader.Compilation.CreateErrorNamespaceSymbol(currentNamespace, namespaceNames[i]);
106+
107+
return new SymbolKeyResolution(currentNamespace);
108+
}
109+
110+
if (type == 2)
111+
return default;
112+
113+
throw ExceptionUtilities.UnexpectedValue(type);
114+
}
115+
60116
private static INamedTypeSymbol Construct(SymbolKeyReader reader, INamespaceOrTypeSymbol container, string name, int arity, ITypeSymbol[] typeArguments)
61117
{
62118
var result = reader.Compilation.CreateErrorTypeSymbol(container, name, arity);

‎src/Workspaces/CoreTest/SymbolKeyTests.cs‎

Lines changed: 171 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using System.Threading.Tasks;
1010
using Microsoft.CodeAnalysis;
1111
using Microsoft.CodeAnalysis.Shared.Extensions;
12+
using Microsoft.CodeAnalysis.Shared.Utilities;
1213
using Microsoft.CodeAnalysis.Test.Utilities;
1314
using Microsoft.CodeAnalysis.Text;
1415
using Roslyn.Test.Utilities;
@@ -784,6 +785,174 @@ void Method((C a, int b) t)
784785
Assert.True(method.Parameters[0].Type.IsTupleType);
785786
}
786787

788+
[Fact, WorkItem(14365, "https://github.com/dotnet/roslyn/issues/14365")]
789+
public void TestErrorType_CSharp()
790+
{
791+
var source = @"
792+
class C
793+
{
794+
int i { get; }
795+
}";
796+
797+
// We don't add metadata references, so even `int` will be an error type.
798+
var compilation1 = GetCompilation(source, LanguageNames.CSharp, "File1.cs", Array.Empty<MetadataReference>());
799+
var compilation2 = GetCompilation(source, LanguageNames.CSharp, "File2.cs", Array.Empty<MetadataReference>());
800+
801+
var symbol = (IPropertySymbol)GetAllSymbols(
802+
compilation1.GetSemanticModel(compilation1.SyntaxTrees.Single()),
803+
n => n is CSharp.Syntax.PropertyDeclarationSyntax).Single();
804+
805+
var propType = symbol.Type;
806+
Assert.Equal(SymbolKind.ErrorType, propType.Kind);
807+
808+
// Ensure we don't crash getting these symbol keys.
809+
var id = SymbolKey.CreateString(propType);
810+
Assert.NotNull(id);
811+
812+
// Validate that if the client does ask to resolve locations that we
813+
// do not crash if those locations cannot be found.
814+
var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol();
815+
Assert.NotNull(found);
816+
817+
Assert.Equal(propType.Name, found.Name);
818+
Assert.Equal(propType.Kind, found.Kind);
819+
820+
var method = (IErrorTypeSymbol)found;
821+
Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
822+
}
823+
824+
[Fact, WorkItem(14365, "https://github.com/dotnet/roslyn/issues/14365")]
825+
public void TestErrorType_VB()
826+
{
827+
var source = @"
828+
class C
829+
public readonly property i as integer
830+
end class";
831+
832+
// We don't add metadata references, so even `int` will be an error type.
833+
var compilation1 = GetCompilation(source, LanguageNames.VisualBasic, "File1.vb", Array.Empty<MetadataReference>());
834+
var compilation2 = GetCompilation(source, LanguageNames.VisualBasic, "File2.vb", Array.Empty<MetadataReference>());
835+
836+
var symbol = (IPropertySymbol)GetAllSymbols(
837+
compilation1.GetSemanticModel(compilation1.SyntaxTrees.Single()),
838+
n => n is VisualBasic.Syntax.PropertyStatementSyntax).Single();
839+
840+
var propType = symbol.Type;
841+
Assert.Equal(SymbolKind.ErrorType, propType.Kind);
842+
843+
// Ensure we don't crash getting these symbol keys.
844+
var id = SymbolKey.CreateString(propType);
845+
Assert.NotNull(id);
846+
847+
// Validate that if the client does ask to resolve locations that we
848+
// do not crash if those locations cannot be found.
849+
var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol();
850+
Assert.NotNull(found);
851+
852+
Assert.Equal(propType.Name, found.Name);
853+
Assert.Equal(propType.Kind, found.Kind);
854+
855+
var method = (IErrorTypeSymbol)found;
856+
Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
857+
}
858+
859+
[Fact, WorkItem(14365, "https://github.com/dotnet/roslyn/issues/14365")]
860+
public void TestErrorTypeInNestedNamespace()
861+
{
862+
var source1 = @"
863+
public class C
864+
{
865+
public System.Collections.IEnumerable I { get; }
866+
}";
867+
868+
var source2 = @"
869+
class X
870+
{
871+
void M()
872+
{
873+
new C().I;
874+
}
875+
}";
876+
877+
// We don't add metadata to the second compilation, so even `System.Collections.IEnumerable` will be an
878+
// error type.
879+
var compilation1 = GetCompilation(source1, LanguageNames.CSharp, "File1.cs");
880+
var compilation2 = GetCompilation(source2, LanguageNames.CSharp, "File2.cs",
881+
new[] { compilation1.ToMetadataReference() });
882+
883+
var symbol = (IPropertySymbol)GetAllSymbols(
884+
compilation2.GetSemanticModel(compilation2.SyntaxTrees.Single()),
885+
n => n is CSharp.Syntax.MemberAccessExpressionSyntax).Single();
886+
887+
var propType = symbol.Type;
888+
Assert.Equal(SymbolKind.ErrorType, propType.Kind);
889+
Assert.Equal("Collections", propType.ContainingNamespace.Name);
890+
Assert.Equal("System", propType.ContainingNamespace.ContainingNamespace.Name);
891+
892+
// Ensure we don't crash getting these symbol keys.
893+
var id = SymbolKey.CreateString(propType);
894+
Assert.NotNull(id);
895+
896+
// Validate that if the client does ask to resolve locations that we
897+
// do not crash if those locations cannot be found.
898+
var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol();
899+
Assert.NotNull(found);
900+
901+
Assert.Equal(propType.Name, found.Name);
902+
Assert.Equal(propType.Kind, found.Kind);
903+
Assert.Equal(propType.ContainingNamespace.Name, found.ContainingNamespace.Name);
904+
905+
var method = (IErrorTypeSymbol)found;
906+
Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
907+
}
908+
909+
[Fact, WorkItem(14365, "https://github.com/dotnet/roslyn/issues/14365")]
910+
public void TestErrorTypeInNestedNamespace_VB()
911+
{
912+
var source1 = @"
913+
public class C
914+
public readonly property I as System.Collections.IEnumerable
915+
end class";
916+
917+
var source2 = @"
918+
class X
919+
sub M()
920+
dim y = new C().I;
921+
end sub
922+
end class";
923+
924+
// We don't add metadata to the second compilation, so even `System.Collections.IEnumerable` will be an
925+
// error type.
926+
var compilation1 = GetCompilation(source1, LanguageNames.VisualBasic, "File1.vb");
927+
var compilation2 = GetCompilation(source2, LanguageNames.VisualBasic, "File2.vb",
928+
new[] { compilation1.ToMetadataReference() });
929+
930+
var symbol = (IPropertySymbol)GetAllSymbols(
931+
compilation2.GetSemanticModel(compilation2.SyntaxTrees.Single()),
932+
n => n is VisualBasic.Syntax.MemberAccessExpressionSyntax).Single();
933+
934+
var propType = symbol.Type;
935+
Assert.Equal(SymbolKind.ErrorType, propType.Kind);
936+
Assert.Equal("Collections", propType.ContainingNamespace.Name);
937+
Assert.Equal("System", propType.ContainingNamespace.ContainingNamespace.Name);
938+
939+
// Ensure we don't crash getting these symbol keys.
940+
var id = SymbolKey.CreateString(propType);
941+
Assert.NotNull(id);
942+
943+
// Validate that if the client does ask to resolve locations that we
944+
// do not crash if those locations cannot be found.
945+
var found = SymbolKey.ResolveString(id, compilation2).GetAnySymbol();
946+
Assert.NotNull(found);
947+
948+
Assert.Equal(propType.Name, found.Name);
949+
Assert.Equal(propType.Kind, found.Kind);
950+
Assert.Equal(propType.ContainingNamespace.Name, found.ContainingNamespace.Name);
951+
952+
var method = (IErrorTypeSymbol)found;
953+
Assert.True(SymbolEquivalenceComparer.Instance.Equals(propType, found));
954+
}
955+
787956
[Fact]
788957
public void TestFunctionPointerTypeSymbols()
789958
{
@@ -826,9 +995,9 @@ private static void TestRoundTrip(ISymbol symbol, Compilation compilation, Func<
826995
}
827996
}
828997

829-
private static Compilation GetCompilation(string source, string language, string path = "")
998+
private static Compilation GetCompilation(string source, string language, string path = "", MetadataReference[] references = null)
830999
{
831-
var references = new[]
1000+
references ??= new[]
8321001
{
8331002
MetadataReference.CreateFromFile(typeof(object).Assembly.Location),
8341003
MetadataReference.CreateFromFile(typeof(Enumerable).Assembly.Location)

‎src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Utilities/SymbolEquivalenceComparer.EquivalenceVisitor.cs‎

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -358,19 +358,43 @@ private bool HandleNamedTypesWorker(INamedTypeSymbol x, INamedTypeSymbol y, Dict
358358
if (x.IsTupleType)
359359
return HandleTupleTypes(x, y, equivalentTypesWithDifferingAssemblies);
360360

361-
if (!AreEquivalent(x.ContainingSymbol, y.ContainingSymbol, equivalentTypesWithDifferingAssemblies))
362-
return false;
361+
if (x.Kind == SymbolKind.ErrorType &&
362+
x.ContainingSymbol is INamespaceSymbol xNamespace &&
363+
y.ContainingSymbol is INamespaceSymbol yNamespace)
364+
{
365+
Debug.Assert(y.Kind == SymbolKind.ErrorType);
363366

364-
// Above check makes sure that the containing assemblies are considered the same by the assembly comparer being used.
365-
// If they are in fact not the same (have different name) and the caller requested to know about such types add {x, y}
366-
// to equivalentTypesWithDifferingAssemblies map.
367-
if (equivalentTypesWithDifferingAssemblies != null &&
368-
x.ContainingType == null &&
369-
x.ContainingAssembly != null &&
370-
!AssemblyIdentityComparer.SimpleNameComparer.Equals(x.ContainingAssembly.Name, y.ContainingAssembly.Name) &&
371-
!equivalentTypesWithDifferingAssemblies.ContainsKey(x))
367+
// For error types, we just ensure that the containing namespaces are equivalent up to the root.
368+
while (true)
369+
{
370+
if (xNamespace.Name != yNamespace.Name)
371+
return false;
372+
373+
// Error namespaces don't set the IsGlobalNamespace bit unfortunately. So we just do the
374+
// nominal check to see if we've actually hit the root.
375+
if (xNamespace.Name == "")
376+
break;
377+
378+
xNamespace = xNamespace.ContainingNamespace;
379+
yNamespace = yNamespace.ContainingNamespace;
380+
}
381+
}
382+
else
372383
{
373-
equivalentTypesWithDifferingAssemblies.Add(x, y);
384+
if (!AreEquivalent(x.ContainingSymbol, y.ContainingSymbol, equivalentTypesWithDifferingAssemblies))
385+
return false;
386+
387+
// Above check makes sure that the containing assemblies are considered the same by the assembly comparer being used.
388+
// If they are in fact not the same (have different name) and the caller requested to know about such types add {x, y}
389+
// to equivalentTypesWithDifferingAssemblies map.
390+
if (equivalentTypesWithDifferingAssemblies != null &&
391+
x.ContainingType == null &&
392+
x.ContainingAssembly != null &&
393+
!AssemblyIdentityComparer.SimpleNameComparer.Equals(x.ContainingAssembly.Name, y.ContainingAssembly.Name) &&
394+
!equivalentTypesWithDifferingAssemblies.ContainsKey(x))
395+
{
396+
equivalentTypesWithDifferingAssemblies.Add(x, y);
397+
}
374398
}
375399

376400
if (x.IsAnonymousType)

0 commit comments

Comments
 (0)