Skip to content

Commit

Permalink
Check duplicate interfaces only for metadata (#73797)
Browse files Browse the repository at this point in the history 
* Check duplicate interfaces only for metadata

* Improve metadata symbol check

* Add metadata validators

* Remove IL verification

* Extend tests

* Clarify distinction between type symbols
  • Loading branch information
@jjonescz
jjonescz authored Jun 17, 2024
1 parent 7dd9e68 commit b8320e1
Show file tree
Hide file tree
Showing 2 changed files with 309 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/Compilers/CSharp/Portable/Symbols/ConstraintsHelper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@
using System.Collections.Immutable;
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.PooledObjects;
using Roslyn.Utilities;
using System.Runtime.CompilerServices;
using System.Threading;

namespace Microsoft.CodeAnalysis.CSharp.Symbols
{
Expand Down Expand Up @@ -751,6 +751,11 @@ public static bool CheckConstraints(this NamedTypeSymbol type, in CheckConstrain
// metadata and be instantiated in C#. We check to see if that's happened.
private static bool HasDuplicateInterfaces(NamedTypeSymbol type, ConsList<TypeSymbol> basesBeingResolved)
{
if (type.OriginalDefinition is not PENamedTypeSymbol)
{
return false;
}

// PERF: avoid instantiating all interfaces here
// Ex: if class implements just IEnumerable<> and IComparable<> it cannot have conflicting implementations
var array = type.OriginalDefinition.InterfacesNoUseSiteDiagnostics(basesBeingResolved);
Expand Down
302 changes: 302 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Text;
using Microsoft.CodeAnalysis.CSharp.Symbols;
using Microsoft.CodeAnalysis.CSharp.Symbols.Metadata.PE;
using Microsoft.CodeAnalysis.CSharp.Symbols.Retargeting;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.CSharp.Test.Utilities;
using Microsoft.CodeAnalysis.Test.Utilities;
Expand Down Expand Up @@ -136032,6 +136033,307 @@ partial class C2<T> : Base<
);
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_01()
{
var source = """
#nullable enable
var c = new C<int>();

partial class C<T> : I<T> { }

partial class C<T> :
#nullable disable
I<T>
#nullable enable
{
}

public interface I<T> { }
""";

CompileAndVerify(CreateCompilationWithMscorlib40(source), sourceSymbolValidator: validate, symbolValidator: validate).VerifyDiagnostics();

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

if (module is SourceModuleSymbol)
{
var interfaces = c.AllInterfacesNoUseSiteDiagnostics;
Assert.Equal(2, interfaces.Length);
AssertEx.Equal("I<T>", interfaces[0].ToTestDisplayString(includeNonNullable: true));
Assert.Equal(NullableAnnotation.NotAnnotated, interfaces[0].TypeArgumentsWithAnnotationsNoUseSiteDiagnostics.Single().NullableAnnotation);
AssertEx.Equal("I<T>", interfaces[1].ToTestDisplayString(includeNonNullable: true));
Assert.Equal(NullableAnnotation.Oblivious, interfaces[1].TypeArgumentsWithAnnotationsNoUseSiteDiagnostics.Single().NullableAnnotation);
}
else
{
var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T>", i.ToTestDisplayString(includeNonNullable: true));
}
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_02()
{
var source = """
#nullable enable
var c = new C<int>();

partial class C<T> : I<T, T> { }

partial class C<T> : I<T,
#nullable disable
T>
#nullable enable
{
}

public interface I<T1, T2> { }
""";
CompileAndVerify(source, symbolValidator: validate).VerifyDiagnostics();

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T, T>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_03()
{
var source = """
#nullable enable
var c1 = new C1<int>();
var c2 = new C2<int>();

partial class C1<T> : I<T, object?>;

partial class C1<T> : I<
T,
#nullable disable
object>;
#nullable enable

partial class C2<T> : I<object, T>;

partial class C2<T> : I<
object,
#nullable disable
T>;
#nullable enable

interface I<T, U> { }
""";
CompileAndVerify(source, symbolValidator: validate).VerifyDiagnostics();

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C1");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T, System.Object?>", i.ToTestDisplayString(includeNonNullable: true));

c = module.GlobalNamespace.GetTypeMember("C2");
cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<System.Object!, T>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_03b()
{
var source = """
#nullable enable
var c1 = new C1<int>();
var c2 = new C2<int>();

partial class C1<T> : I<
T,
#nullable disable
object>;
#nullable enable

partial class C1<T> : I<T, object?>;

partial class C2<T> : I<
object,
#nullable disable
T>;
#nullable enable

partial class C2<T> : I<object, T>;

interface I<T, U> { }
""";
CompileAndVerify(source, symbolValidator: validate).VerifyDiagnostics();

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C1");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T, System.Object>", i.ToTestDisplayString(includeNonNullable: true));

c = module.GlobalNamespace.GetTypeMember("C2");
cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<System.Object!, T>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_04()
{
var source = """
#nullable enable
var c = new C<int>();

interface I<T> { }

class C<T> : I<T?>, I<T> { }
""";

CompileAndVerify(CreateCompilationWithMscorlib40(source), symbolValidator: validate).VerifyDiagnostics(
// (6,7): warning CS8645: 'I<T>' is already listed in the interface list on type 'C<T>' with different nullability of reference types.
// class C<T> : I<T?>, I<T> { }
Diagnostic(ErrorCode.WRN_DuplicateInterfaceWithNullabilityMismatchInBaseList, "C").WithArguments("I<T>", "C<T>").WithLocation(6, 7));

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T?>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_05()
{
var source = """
#nullable enable
var c = new C<int>();

interface I<T> { }

partial class C<T> : I<T?> { }

partial class C<T> : I<T> { }
""";
CompileAndVerify(source, symbolValidator: validate).VerifyDiagnostics(
// (6,15): warning CS8645: 'I<T>' is already listed in the interface list on type 'C<T>' with different nullability of reference types.
// partial class C<T> : I<T?> { }
Diagnostic(ErrorCode.WRN_DuplicateInterfaceWithNullabilityMismatchInBaseList, "C").WithArguments("I<T>", "C<T>").WithLocation(6, 15));

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T?>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_06()
{
var source = """
#nullable enable
var c = new C<int>();

interface I<T> { }

partial class C<T> : I<T?> { }

partial class C<T> : I<T>, I<T?> { }
""";
CompileAndVerify(source, symbolValidator: validate).VerifyDiagnostics(
// (6,15): warning CS8645: 'I<T>' is already listed in the interface list on type 'C<T>' with different nullability of reference types.
// partial class C<T> : I<T?> { }
Diagnostic(ErrorCode.WRN_DuplicateInterfaceWithNullabilityMismatchInBaseList, "C").WithArguments("I<T>", "C<T>").WithLocation(6, 15));

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T?>", i.ToTestDisplayString(includeNonNullable: true));
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/40538")]
public void AnnotationMismatch_Interface_GenericClass_Retargeting()
{
var source1 = """
#nullable enable
interface I<T> { }

public class C<T> : I<T?>, I<T> { }
""";

var verifier1 = CompileAndVerify(source1, symbolValidator: validate, targetFramework: TargetFramework.Mscorlib40);
verifier1.VerifyDiagnostics(
// (4,14): warning CS8645: 'I<T>' is already listed in the interface list on type 'C<T>' with different nullability of reference types.
// public class C<T> : I<T?>, I<T> { }
Diagnostic(ErrorCode.WRN_DuplicateInterfaceWithNullabilityMismatchInBaseList, "C").WithArguments("I<T>", "C<T>").WithLocation(4, 14));

static void validate(ModuleSymbol module)
{
var c = module.GlobalNamespace.GetTypeMember("C");
var cT = c.TypeParameters.Single();
AssertEx.Equal("T", cT.ToTestDisplayString(includeNonNullable: true));
Assert.Equal(CodeAnalysis.NullableAnnotation.None, cT.GetPublicSymbol().ReferenceTypeConstraintNullableAnnotation);

var i = c.AllInterfacesNoUseSiteDiagnostics.Single();
AssertEx.Equal("I<T?>", i.ToTestDisplayString(includeNonNullable: true));
}

var source2 = """
#nullable enable
var c = new C<int>();
""";
var comp2 = CreateCompilation(source2, [verifier1.Compilation.ToMetadataReference()]);
comp2.VerifyEmitDiagnostics();

var c = comp2.GlobalNamespace.GetMember("C");
Assert.IsType<RetargetingNamedTypeSymbol>(c);
Assert.False(c.HasUnsupportedMetadata);
}

[Fact]
public void PartialMethodsWithConstraints_01()
{
Expand Down

0 comments on commit b8320e1

Please sign in to comment.