From 29ffc0735846469073d0d81010793fe777f3407f Mon Sep 17 00:00:00 2001 From: Charles Stoner Date: Wed, 8 Apr 2020 20:11:51 -0700 Subject: [PATCH] Avoid using SymbolDisplay while binding type (#43181) --- .../CSharp/Portable/Binder/Binder_Lookup.cs | 26 ++++++- .../CSharp/Portable/Binder/Binder_Symbols.cs | 6 +- .../Test/Emit/Attributes/AttributeTests.cs | 76 +++++++++++++++++++ .../Compilation/GetSemanticInfoTests.cs | 33 ++++++++ .../Symbol/Symbols/Source/PropertyTests.cs | 13 +++- 5 files changed, 145 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs index c05d8608d1ff0..609228d5a6d7c 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Lookup.cs @@ -553,8 +553,7 @@ private Symbol ResolveMultipleSymbolsInAttributeTypeLookup(ArrayBuilder var mdSymbol = symbols[secondBest.Index]; //if names match, arities match, and containing symbols match (recursively), ... - if (srcSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat) == - mdSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)) + if (NameAndArityMatchRecursively(srcSymbol, mdSymbol)) { return originalSymbols[best.Index]; } @@ -563,6 +562,29 @@ private Symbol ResolveMultipleSymbolsInAttributeTypeLookup(ArrayBuilder return null; } + private static bool NameAndArityMatchRecursively(Symbol x, Symbol y) + { + while (true) + { + if (isRoot(x)) + { + return isRoot(y); + } + if (isRoot(y)) + { + return false; + } + if (x.Name != y.Name || x.GetArity() != y.GetArity()) + { + return false; + } + x = x.ContainingSymbol; + y = y.ContainingSymbol; + } + + static bool isRoot(Symbol symbol) => symbol is null || symbol is NamespaceSymbol { IsGlobalNamespace: true }; + } + private bool IsSingleViableAttributeType(LookupResult result, out Symbol symbol) { if (IsAmbiguousResult(result, out symbol)) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs index dc4f6da4d5032..b18be172ca9b0 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Symbols.cs @@ -1589,8 +1589,7 @@ Symbol resultSymbol( } //if names match, arities match, and containing symbols match (recursively), ... - if (srcSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat) == - mdSymbol.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)) + if (NameAndArityMatchRecursively(srcSymbol, mdSymbol)) { if (srcSymbol.Kind == SymbolKind.Namespace && mdSymbol.Kind == SymbolKind.NamedType) { @@ -1647,8 +1646,7 @@ Symbol resultSymbol( //if names match, arities match, and containing symbols match (recursively), ... if (first != second && - first.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat) == - second.ToDisplayString(SymbolDisplayFormat.QualifiedNameArityFormat)) + NameAndArityMatchRecursively(first, second)) { // suppress reporting the error if we found multiple symbols from source module // since an error has already been reported from the declaration diff --git a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs index 6897fb9614511..80124d3a674da 100644 --- a/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs +++ b/src/Compilers/CSharp/Test/Emit/Attributes/AttributeTests.cs @@ -7653,6 +7653,82 @@ class Goo: Attribute Diagnostic(ErrorCode.ERR_NotAnAttributeClass, "X").WithArguments("X")); } + [Fact] + public void AmbiguousClassNamespaceLookup_Container() + { + var source1 = +@"using System; +public class A : Attribute { } +namespace N1 +{ + public class B : Attribute { } + public class C : Attribute { } +}"; + var comp = CreateCompilation(source1, assemblyName: "A"); + var ref1 = comp.EmitToImageReference(); + + var source2 = +@"using System; +using N1; +using N2; +namespace N1 +{ + class A : Attribute { } + class B : Attribute { } +} +namespace N2 +{ + class C : Attribute { } +} +[A] +[B] +[C] +class D +{ +}"; + comp = CreateCompilation(source2, references: new[] { ref1 }, assemblyName: "B"); + comp.VerifyDiagnostics( + // (14,2): warning CS0436: The type 'B' in '' conflicts with the imported type 'B' in 'A, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null'. Using the type defined in ''. + // [B] + Diagnostic(ErrorCode.WRN_SameFullNameThisAggAgg, "B").WithArguments("", "N1.B", "A, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "N1.B").WithLocation(14, 2), + // (15,2): error CS0104: 'C' is an ambiguous reference between 'N2.C' and 'N1.C' + // [C] + Diagnostic(ErrorCode.ERR_AmbigContext, "C").WithArguments("C", "N2.C", "N1.C").WithLocation(15, 2)); + } + + [Fact] + public void AmbiguousClassNamespaceLookup_Generic() + { + var source1 = +@"public class A { } +public class B { } +public class C { }"; + var comp = CreateCompilation(source1); + var ref1 = comp.EmitToImageReference(); + + var source2 = +@"class A { } +class B { } +class C { } +[A] +[B] +[C] +class D +{ +}"; + comp = CreateCompilation(source2, references: new[] { ref1 }); + comp.VerifyDiagnostics( + // (4,2): error CS0616: 'A' is not an attribute class + // [A] + Diagnostic(ErrorCode.ERR_NotAnAttributeClass, "A").WithArguments("A").WithLocation(4, 2), + // (5,2): error CS0404: Cannot apply attribute class 'B' because it is generic + // [B] + Diagnostic(ErrorCode.ERR_AttributeCantBeGeneric, "B").WithArguments("B").WithLocation(5, 2), + // (6,2): error CS0305: Using the generic type 'C' requires 1 type arguments + // [C] + Diagnostic(ErrorCode.ERR_BadArity, "C").WithArguments("C", "type", "1").WithLocation(6, 2)); + } + [WorkItem(546283, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/546283")] [Fact] public void ApplyIndexerNameAttributeTwice() diff --git a/src/Compilers/CSharp/Test/Symbol/Compilation/GetSemanticInfoTests.cs b/src/Compilers/CSharp/Test/Symbol/Compilation/GetSemanticInfoTests.cs index 43e93148c0f06..d1b00dfd7a51b 100644 --- a/src/Compilers/CSharp/Test/Symbol/Compilation/GetSemanticInfoTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Compilation/GetSemanticInfoTests.cs @@ -5971,5 +5971,38 @@ static void M() { } Assert.Null(info.Symbol); Assert.Equal(CandidateReason.NotReferencable, info.CandidateReason); } + + [Fact] + [WorkItem(42840, "https://github.com/dotnet/roslyn/issues/42840")] + public void DuplicateTypeArgument() + { + var source = +@"class A +{ +} +class B + where T : A + where U : class +{ +}"; + + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (4,15): error CS0692: Duplicate type parameter 'U' + // class B + Diagnostic(ErrorCode.ERR_DuplicateTypeParameter, "U").WithArguments("U").WithLocation(4, 15), + // (5,17): error CS0229: Ambiguity between 'U' and 'U' + // where T : A + Diagnostic(ErrorCode.ERR_AmbigMember, "U").WithArguments("U", "U").WithLocation(5, 17)); + + comp = CreateCompilation(source); + var tree = comp.SyntaxTrees[0]; + var model = comp.GetSemanticModel(tree); + var typeParameters = tree.GetRoot().DescendantNodes().OfType().ToArray(); + var symbol = model.GetDeclaredSymbol(typeParameters[typeParameters.Length - 1]); + Assert.False(symbol.IsReferenceType); + symbol = model.GetDeclaredSymbol(typeParameters[typeParameters.Length - 2]); + Assert.True(symbol.IsReferenceType); + } } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/PropertyTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/PropertyTests.cs index 12fc799c1cf6c..c80d2818891cf 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/PropertyTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/PropertyTests.cs @@ -1561,6 +1561,11 @@ static void Main() { public void CanNotReadPropertyFromAmbiguousGenericClass() { const string ilSource = @" +.assembly extern mscorlib { .ver 4:0:0:0 .publickeytoken = (B7 7A 5C 56 19 34 E0 89) } +.assembly '09f9df97-a228-4ca4-9b71-151909f205e6' +{ +} + .class public A`1 { .method public static int32 get_Goo() { ldnull throw } .property int32 Goo() { .get int32 A`1::get_Goo() } @@ -1571,6 +1576,8 @@ .class public A { .property int32 Goo() { .get int32 A::get_Goo() } } "; + var ref0 = CompileIL(ilSource, prependDefaultHeader: false); + const string cSharpSource = @" class B { static void Main() { @@ -1578,10 +1585,10 @@ static void Main() { } } "; - CreateCompilationWithILAndMscorlib40(cSharpSource, ilSource).VerifyDiagnostics( - // (4,16): error CS0104: 'A<>' is an ambiguous reference between 'A' and 'A' + CreateCompilation(cSharpSource, references: new[] { ref0 }).VerifyDiagnostics( + // (4,16): error CS0433: The type 'A' exists in both '09f9df97-a228-4ca4-9b71-151909f205e6, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' and '09f9df97-a228-4ca4-9b71-151909f205e6, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null' // object x = A.Goo; - Diagnostic(ErrorCode.ERR_AmbigContext, "A").WithArguments("A<>", "A", "A").WithLocation(4, 16)); + Diagnostic(ErrorCode.ERR_SameFullNameAggAgg, "A").WithArguments("09f9df97-a228-4ca4-9b71-151909f205e6, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null", "A", "09f9df97-a228-4ca4-9b71-151909f205e6, Version=0.0.0.0, Culture=neutral, PublicKeyToken=null").WithLocation(4, 16)); } [WorkItem(538789, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/538789")]