From f1a8213e888faffd10afc0c3f9f5aa07ab5cc98e Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 30 Jul 2020 11:39:14 -0700 Subject: [PATCH] Cannot assign maybe-null value to TNotNull variable (#41445) --- .../Portable/FlowAnalysis/NullableWalker.cs | 2 +- .../Portable/Symbols/TypeSymbolExtensions.cs | 1 + .../Semantics/NullableReferenceTypesTests.cs | 121 +++++++++++++----- 3 files changed, 91 insertions(+), 33 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 63e64542219..3e25ea1cd03 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -1415,7 +1415,7 @@ private static bool ShouldReportNullableAssignment(TypeWithAnnotations type, Nul return false; case NullableFlowState.MaybeNull: // https://github.com/dotnet/roslyn/issues/46044: Skip the following check if /langversion > 8? - if (type.Type.IsTypeParameterDisallowingAnnotationInCSharp8()) + if (type.Type.IsTypeParameterDisallowingAnnotationInCSharp8() && !(type.Type is TypeParameterSymbol { IsNotNullable: true })) { return false; } diff --git a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs index 5bb2b9b1d33..97edb71ed25 100644 --- a/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs +++ b/src/Compilers/CSharp/Portable/Symbols/TypeSymbolExtensions.cs @@ -52,6 +52,7 @@ public static bool CanBeConst(this TypeSymbol typeSymbol) /// T where T : class? => true /// T where T : IComparable => true /// T where T : IComparable? => true + /// T where T : notnull => true /// /// /// In C#9, annotations are allowed regardless of constraints. diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 2da015b3cdc..98804e2abe3 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -35137,96 +35137,97 @@ class C comp.VerifyDiagnostics(); } - [Theory] - [InlineData("T")] - [InlineData("TNotNull")] + [Fact] [WorkItem(39922, "https://github.com/dotnet/roslyn/issues/39922")] - public void EnforcedInMethodBody_MaybeNullWhen(string type) + public void EnforcedInMethodBody_MaybeNullWhen() { var source = @" #nullable enable using System.Diagnostics.CodeAnalysis; -class C - where TNotNull : notnull +class C { - static void GetValue(TYPE x, [MaybeNull] out TYPE y) + static void GetValue(T x, [MaybeNull] out T y) { y = x; } - static bool TryGetValue(TYPE x, [MaybeNullWhen(true)] out TYPE y) + static bool TryGetValue(T x, [MaybeNullWhen(true)] out T y) { y = x; return y == null; } - static bool TryGetValue2(TYPE x, [MaybeNullWhen(true)] out TYPE y) + static bool TryGetValue2(T x, [MaybeNullWhen(true)] out T y) { y = x; return y != null; } - static bool TryGetValue3(TYPE x, [MaybeNullWhen(false)] out TYPE y) + static bool TryGetValue3(T x, [MaybeNullWhen(false)] out T y) { y = x; return y == null; } - static bool TryGetValue4(TYPE x, [MaybeNullWhen(false)] out TYPE y) + static bool TryGetValue4(T x, [MaybeNullWhen(false)] out T y) { y = x; return y != null; } } "; - var comp = CreateNullableCompilation(new[] { MaybeNullAttributeDefinition, MaybeNullWhenAttributeDefinition, source.Replace("TYPE", type) }); + var comp = CreateNullableCompilation(new[] { MaybeNullAttributeDefinition, MaybeNullWhenAttributeDefinition, source }); comp.VerifyDiagnostics(); } - [Fact, WorkItem(39922, "https://github.com/dotnet/roslyn/issues/39922")] - public void EnforcedInMethodBody_MaybeNullWhen_ReferenceType() + [Theory] + [InlineData("TClass")] + [InlineData("TNotNull")] + [WorkItem(39922, "https://github.com/dotnet/roslyn/issues/39922")] + public void EnforcedInMethodBody_MaybeNullWhen_NotNullableTypes(string type) { var source = @" #nullable enable using System.Diagnostics.CodeAnalysis; -class C +class C where TClass : class + where TNotNull : notnull { - static bool TryGetValue(TClass x, [MaybeNullWhen(true)] out TClass y) + static bool TryGetValue(TYPE x, [MaybeNullWhen(true)] out TYPE y) { y = x; return y == null; } - static bool TryGetValue2(TClass x, [MaybeNullWhen(true)] out TClass y) + static bool TryGetValue2(TYPE x, [MaybeNullWhen(true)] out TYPE y) { y = x; return y != null; // 1 } - static bool TryGetValue3(TClass x, [MaybeNullWhen(false)] out TClass y) + static bool TryGetValue3(TYPE x, [MaybeNullWhen(false)] out TYPE y) { y = x; return y == null; // 2 } - static bool TryGetValue4(TClass x, [MaybeNullWhen(false)] out TClass y) + static bool TryGetValue4(TYPE x, [MaybeNullWhen(false)] out TYPE y) { y = x; return y != null; } } "; - var comp = CreateNullableCompilation(new[] { MaybeNullWhenAttributeDefinition, source }); + var comp = CreateNullableCompilation(new[] { MaybeNullWhenAttributeDefinition, source.Replace("TYPE", type) }); comp.VerifyDiagnostics( - // (17,9): error CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. + // (18,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. // return y != null; // 1 - Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y != null;").WithArguments("y", "false").WithLocation(17, 9), - // (23,9): error CS8762: Parameter 'y' must have a non-null value when exiting with 'true'. + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y != null;").WithArguments("y", "false").WithLocation(18, 9), + // (24,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'true'. // return y == null; // 2 - Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y == null;").WithArguments("y", "true").WithLocation(23, 9) + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y == null;").WithArguments("y", "true").WithLocation(24, 9) ); } @@ -36133,11 +36134,8 @@ public class Derived : Base } "; var comp = CreateNullableCompilation(new[] { MaybeNullWhenAttributeDefinition, source }); - // Note: we're missing warnings on F1 because the overridden method with substituted T from Derived has an oblivious T + // Note: we're missing warnings on F1 and F6 because the overridden method with substituted T from Derived has an oblivious T // https://github.com/dotnet/roslyn/issues/41368 - - // Note: assignment of `[MaybeNull] TNotNull` to `TNotNull` is currently not detected - // This is a known issue: https://github.com/dotnet/roslyn/issues/41437 comp.VerifyDiagnostics( // (14,26): warning CS8765: Type of parameter 't2' doesn't match overridden member because of nullability attributes. // public override bool F2([MaybeNullWhen(false)]T t1, [MaybeNullWhen(false)] out T t2, [MaybeNullWhen(false)] ref T t3, [MaybeNullWhen(false)] in T t4) where T : class => throw null!; // t2, t3 @@ -36148,6 +36146,29 @@ public class Derived : Base ); } + [Fact, WorkItem(41437, "https://github.com/dotnet/roslyn/issues/41437")] + public void AssigningMaybeNullTNotNullToTNotNullInOverride() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class Base +{ + public virtual bool F6([AllowNull] in T t4) where T : notnull => throw null!; +} +public class Derived : Base +{ + public override bool F6(in T t4) => throw null!; +} +"; + var comp = CreateNullableCompilation(new[] { AllowNullAttributeDefinition, source }); + + comp.VerifyDiagnostics( + // (8,26): warning CS8765: Nullability of type of parameter 't4' doesn't match overridden member (possibly because of nullability attributes). + // public override bool F6(in T t4) => throw null!; + Diagnostic(ErrorCode.WRN_TopLevelNullabilityMismatchInParameterTypeOnOverride, "F6").WithArguments("t4").WithLocation(8, 26) + ); + } + [Fact, WorkItem(41437, "https://github.com/dotnet/roslyn/issues/41437")] public void MaybeNullWhenTrue_Parameter_Generic_OnOverrides_WithMaybeNull() { @@ -36172,11 +36193,8 @@ public class Derived : Base public override bool F6([MaybeNull]T t1, [MaybeNull] out T t2, [MaybeNull] ref T t3, [MaybeNull] in T t4) => throw null!; } "; - // Note: we're missing warnings on F1 because the overridden method with substituted T from Derived has an oblivious T + // Note: we're missing warnings on F1 and F6 because the overridden method with substituted T from Derived has an oblivious T // https://github.com/dotnet/roslyn/issues/41368 - - // Note: assignment of `[MaybeNull] TNotNull` to `TNotNull` is currently not detected - // This is a known issue: https://github.com/dotnet/roslyn/issues/41437 var comp = CreateNullableCompilation(new[] { MaybeNullWhenAttributeDefinition, MaybeNullAttributeDefinition, source }); comp.VerifyDiagnostics( // (14,26): warning CS8765: Type of parameter 't2' doesn't match overridden member because of nullability attributes. @@ -133261,6 +133279,45 @@ public static void Main() CompileAndVerify(executeComp, expectedOutput: "ran"); } + [Fact, WorkItem(41437, "https://github.com/dotnet/roslyn/issues/41437")] + public void CannotAssignMaybeNullToTNotNull() + { + var source = @" +class C +{ + TNotNull M(TNotNull t) where TNotNull : notnull + { + if (t is null) System.Console.WriteLine(); + + return t; // 1 + } +}"; + var comp = CreateNullableCompilation(source); + comp.VerifyDiagnostics( + // (8,16): warning CS8603: Possible null reference return. + // return t; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReturn, "t").WithLocation(8, 16) + ); + } + + [Fact, WorkItem(41437, "https://github.com/dotnet/roslyn/issues/41437")] + public void CanAssignMaybeNullToMaybeNullTNotNull() + { + var source = @" +using System.Diagnostics.CodeAnalysis; +class C +{ + [return: MaybeNull] TNotNull M(TNotNull t) where TNotNull : notnull + { + if (t is null) System.Console.WriteLine(); + + return t; + } +}"; + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics(); + } + [Fact] [WorkItem(35602, "https://github.com/dotnet/roslyn/issues/35602")] public void PureNullTestOnUnconstrainedType() -- GitLab