From e7427b4ae3ca035ad8239eb0b0b1241274a5f42f Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 6 Jul 2022 16:13:52 -0700 Subject: [PATCH 1/5] Fix crash with type parameter on type constrained to file-local type --- .../Portable/Binder/Binder_Constraints.cs | 2 +- .../Symbols/Source/FileModifierTests.cs | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index a827850296103..c1444888526e3 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -411,7 +411,7 @@ private TypeParameterConstraintClause GetDefaultTypeParameterConstraintClause(Ty diagnostics.Add(ErrorCode.ERR_BadVisBound, location, containingSymbol, constraintType.Type); } - if (constraintType.Type.IsFileTypeOrUsesFileTypes() && !containingSymbol.ContainingType.IsFileTypeOrUsesFileTypes()) + if (constraintType.Type.IsFileTypeOrUsesFileTypes() && !(containingSymbol as TypeSymbol ?? containingSymbol.ContainingType).IsFileTypeOrUsesFileTypes()) { diagnostics.Add(ErrorCode.ERR_FileTypeDisallowedInSignature, location, constraintType.Type, containingSymbol); } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs index 47029fb59b42d..f47bc784dcdb8 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Test.Utilities; using Microsoft.CodeAnalysis.Test.Utilities; +using Roslyn.Test.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.CSharp.UnitTests; @@ -2074,6 +2075,28 @@ class E Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "E.M(T)").WithLocation(10, 30)); } + [Fact, WorkItem(62435, "https://github.com/dotnet/roslyn/issues/62435")] + public void Constraints_02() + { + var source = """ + file class C { } + + file class D where T : C // ok + { + } + + class E where T : C // 1 + { + } + """; + + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (7,22): error CS9051: File type 'C' cannot be used in a member signature in non-file type 'E'. + // class E where T : C // 1 + Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "E").WithLocation(7, 22)); + } + [Fact] public void PrimaryConstructor_01() { From 48608d16af418ab09b5fae75dae0ecd17ef2cd28 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 6 Jul 2022 17:45:33 -0700 Subject: [PATCH 2/5] Address feedback --- .../Portable/Binder/Binder_Constraints.cs | 4 +- .../Symbols/Source/FileModifierTests.cs | 67 +++++++++++++++++-- 2 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index c1444888526e3..4dd2afaf3021f 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -411,7 +411,9 @@ private TypeParameterConstraintClause GetDefaultTypeParameterConstraintClause(Ty diagnostics.Add(ErrorCode.ERR_BadVisBound, location, containingSymbol, constraintType.Type); } - if (constraintType.Type.IsFileTypeOrUsesFileTypes() && !(containingSymbol as TypeSymbol ?? containingSymbol.ContainingType).IsFileTypeOrUsesFileTypes()) + // Whether a member is effectively file-local is determined on the type level. When non-type members have constraints we need to check the containing symbol. + // If the 'containingSymbol.ContainingSymbol' is not a type, then 'containingSymbol' is not a member, and we don't need to enforce file types here. + if (constraintType.Type.IsFileTypeOrUsesFileTypes() && (containingSymbol as TypeSymbol ?? containingSymbol.ContainingSymbol as TypeSymbol)?.IsFileTypeOrUsesFileTypes() == false) { diagnostics.Add(ErrorCode.ERR_FileTypeDisallowedInSignature, location, constraintType.Type, containingSymbol); } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs index f47bc784dcdb8..8ed740888ffdc 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs @@ -2075,26 +2075,79 @@ class E Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "E.M(T)").WithLocation(10, 30)); } - [Fact, WorkItem(62435, "https://github.com/dotnet/roslyn/issues/62435")] - public void Constraints_02() + [Theory, WorkItem(62435, "https://github.com/dotnet/roslyn/issues/62435")] + [InlineData("class")] + [InlineData("struct")] + [InlineData("interface")] + [InlineData("record")] + [InlineData("record struct")] + public void Constraints_02(string typeKind) + { + var source = $$""" + file {{typeKind}} C { } + + file {{typeKind}} D where T : C // ok + { + } + + {{typeKind}} E where T : C // 1 + { + } + """; + + var comp = CreateCompilation(source); + comp.VerifyDiagnostics( + // (7,{{17 + typeKind.Length}}): error CS9051: File type 'C' cannot be used in a member signature in non-file type 'E'. + // {{typeKind}} E where T : C // 1 + Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "E").WithLocation(7, 17 + typeKind.Length)); + } + + [Fact] + public void Constraints_03() { var source = """ file class C { } - file class D where T : C // ok + file class D { + void M() + { + local(new C()); + void local(T t) where T : C { } // ok + } } - class E where T : C // 1 + class E { + void M() + { + local(new C()); + void local(T t) where T : C { } // ok + } } """; + // Local functions aren't members, so we don't give any diagnostics when their signatures contain file types. + var comp = CreateCompilation(source); + comp.VerifyDiagnostics(); + } + + [Fact] + public void Constraints_04() + { + var source = """ + file class C { } + + file delegate void D1(T t) where T : C; // ok + + delegate void D2(T t) where T : C; // 1 + """; + var comp = CreateCompilation(source); comp.VerifyDiagnostics( - // (7,22): error CS9051: File type 'C' cannot be used in a member signature in non-file type 'E'. - // class E where T : C // 1 - Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "E").WithLocation(7, 22)); + // (5,36): error CS9051: File type 'C' cannot be used in a member signature in non-file type 'D2'. + // delegate void D2(T t) where T : C; // 1 + Diagnostic(ErrorCode.ERR_FileTypeDisallowedInSignature, "C").WithArguments("C", "D2").WithLocation(5, 36)); } [Fact] From 6f41f115d0e897212ac79181861b62f19ad91716 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Wed, 6 Jul 2022 18:25:13 -0700 Subject: [PATCH 3/5] fix test --- .../CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs index 8ed740888ffdc..b2322b27767d1 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/Source/FileModifierTests.cs @@ -2084,7 +2084,7 @@ class E public void Constraints_02(string typeKind) { var source = $$""" - file {{typeKind}} C { } + file class C { } file {{typeKind}} D where T : C // ok { From f49b5ca42970f0da1cb4ce2facf7ecdbc0d26736 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 7 Jul 2022 15:28:21 -0700 Subject: [PATCH 4/5] Handle a more limited set of expected symbols --- .../CSharp/Portable/Binder/Binder_Constraints.cs | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index 4dd2afaf3021f..6f7799b7932c2 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -411,11 +411,21 @@ private TypeParameterConstraintClause GetDefaultTypeParameterConstraintClause(Ty diagnostics.Add(ErrorCode.ERR_BadVisBound, location, containingSymbol, constraintType.Type); } - // Whether a member is effectively file-local is determined on the type level. When non-type members have constraints we need to check the containing symbol. - // If the 'containingSymbol.ContainingSymbol' is not a type, then 'containingSymbol' is not a member, and we don't need to enforce file types here. if (constraintType.Type.IsFileTypeOrUsesFileTypes() && (containingSymbol as TypeSymbol ?? containingSymbol.ContainingSymbol as TypeSymbol)?.IsFileTypeOrUsesFileTypes() == false) { - diagnostics.Add(ErrorCode.ERR_FileTypeDisallowedInSignature, location, constraintType.Type, containingSymbol); + // if the containing symbol of the constraint is a member, we need to ensure the nearest containing type is a file type. + var possibleFileType = containingSymbol switch + { + TypeSymbol typeSymbol => typeSymbol, + LocalFunctionSymbol => null, + MethodSymbol method => (TypeSymbol)method.ContainingSymbol, + _ => throw ExceptionUtilities.UnexpectedValue(containingSymbol) + }; + Debug.Assert(possibleFileType?.IsDefinition != false); + if (possibleFileType?.IsFileTypeOrUsesFileTypes() == false) + { + diagnostics.Add(ErrorCode.ERR_FileTypeDisallowedInSignature, location, constraintType.Type, containingSymbol); + } } diagnostics.Add(location, useSiteInfo); From 65ecbccc8586b8730e224fd7dcc0b450a21aded3 Mon Sep 17 00:00:00 2001 From: Rikki Gibson Date: Thu, 7 Jul 2022 15:56:06 -0700 Subject: [PATCH 5/5] remove unnecessary check --- src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs index 6f7799b7932c2..01e38f749c160 100644 --- a/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs +++ b/src/Compilers/CSharp/Portable/Binder/Binder_Constraints.cs @@ -411,7 +411,7 @@ private TypeParameterConstraintClause GetDefaultTypeParameterConstraintClause(Ty diagnostics.Add(ErrorCode.ERR_BadVisBound, location, containingSymbol, constraintType.Type); } - if (constraintType.Type.IsFileTypeOrUsesFileTypes() && (containingSymbol as TypeSymbol ?? containingSymbol.ContainingSymbol as TypeSymbol)?.IsFileTypeOrUsesFileTypes() == false) + if (constraintType.Type.IsFileTypeOrUsesFileTypes()) { // if the containing symbol of the constraint is a member, we need to ensure the nearest containing type is a file type. var possibleFileType = containingSymbol switch