From fb7473c4e263690b2444e2edbf10ce94d3a4e86e Mon Sep 17 00:00:00 2001 From: Julien Couvreur Date: Thu, 11 Jul 2019 18:37:40 -0700 Subject: [PATCH] Apply Maybe/NotNull when getting default value of fields and properties (#37135) --- .../Portable/FlowAnalysis/NullableWalker.cs | 16 +- .../Semantics/NullableReferenceTypesTests.cs | 195 ++++++++++++++++++ 2 files changed, 202 insertions(+), 9 deletions(-) diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 4bcca221e17..e08ceb8d3ee 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -701,7 +701,7 @@ private NullableFlowState GetDefaultState(ref LocalState state, int slot) case SymbolKind.Field: case SymbolKind.Property: case SymbolKind.Event: - return symbol.GetTypeOrReturnType().ToTypeWithState().State; + return GetDefaultState(symbol); case SymbolKind.ErrorType: return NullableFlowState.NotNull; default: @@ -1230,11 +1230,15 @@ private void InheritDefaultState(int targetSlot) { continue; } - SetStateAndTrackForFinally(ref this.State, slot, variable.Symbol.GetTypeOrReturnType().ToTypeWithState().State); + + SetStateAndTrackForFinally(ref this.State, slot, GetDefaultState(variable.Symbol)); InheritDefaultState(slot); } } + private NullableFlowState GetDefaultState(Symbol symbol) + => ApplyUnconditionalAnnotations(symbol.GetTypeOrReturnType().ToTypeWithState(), GetRValueAnnotations(symbol)).State; + private void InheritNullableStateOfTrackableType(int targetSlot, int valueSlot, int skipSlot) { Debug.Assert(targetSlot > 0); @@ -1254,11 +1258,6 @@ private void InheritNullableStateOfTrackableType(int targetSlot, int valueSlot, } } - private TypeSymbol GetSlotType(int slot) - { - return variableBySlot[slot].Symbol.GetTypeOrReturnType().Type; - } - protected override LocalState TopState() { var state = LocalState.ReachableState(capacity: nextVariableSlot); @@ -6081,7 +6080,6 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt } } - resultType = ApplyUnconditionalAnnotations(resultType, GetRValueAnnotations(member)); SetResult(node, resultType, type); } @@ -6108,7 +6106,7 @@ private void VisitMemberAccess(BoundExpression node, BoundExpression receiverOpt private int GetNullableOfTValueSlot(TypeSymbol containingType, int containingSlot, out Symbol valueProperty, bool forceSlotEvenIfEmpty = false) { Debug.Assert(containingType.IsNullableType()); - Debug.Assert(TypeSymbol.Equals(GetSlotType(containingSlot), containingType, TypeCompareKind.ConsiderEverything2)); + Debug.Assert(TypeSymbol.Equals(variableBySlot[containingSlot].Symbol.GetTypeOrReturnType().Type, containingType, TypeCompareKind.ConsiderEverything2)); var getValue = (MethodSymbol)compilation.GetSpecialTypeMember(SpecialMember.System_Nullable_T_get_Value); valueProperty = getValue?.AsMember((NamedTypeSymbol)containingType)?.AssociatedSymbol; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index 20c3cc24c3d..c0ad6a9df33 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -20963,6 +20963,131 @@ void symbolValidator(ModuleSymbol m) } } + [Fact] + public void MaybeNull_Field_WithAssignment() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [MaybeNull] string f1 = """"; + void M() + { + f1.ToString(); // 1 + f1 = """"; + f1.ToString(); + } +}"; + + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (7,9): warning CS8602: Dereference of a possibly null reference. + // f1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "f1").WithLocation(7, 9) + ); + } + + [Fact] + public void MaybeNull_Field_WithAssignment_NullableInt() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [MaybeNull] int? f1 = 1; + void M() + { + f1.Value.ToString(); // 1 + f1 = 2; + f1.Value.ToString(); + } +}"; + + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (7,9): warning CS8629: Nullable value type may be null. + // f1.Value.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "f1").WithLocation(7, 9) + ); + } + + [Fact] + public void MaybeNull_Field_WithContainerAssignment() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [MaybeNull] public string f1 = """"; + void M(C c) + { + c.f1.ToString(); // 1 + c.f1 = """"; + + c = new C(); + c.f1.ToString(); // 2 + } +}"; + + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (7,9): warning CS8602: Dereference of a possibly null reference. + // c.f1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.f1").WithLocation(7, 9), + // (11,9): warning CS8602: Dereference of a possibly null reference. + // c.f1.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c.f1").WithLocation(11, 9) + ); + } + + [Fact, WorkItem(36830, "https://github.com/dotnet/roslyn/issues/36830")] + public void MaybeNull_Field_InDebugAssert() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +class C +{ + [MaybeNull] string f1 = """"; + void M() + { + System.Diagnostics.Debug.Assert(f1 != null); + f1.ToString(); + } +}"; + + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics(); + } + + [Fact] + public void MaybeNull_Field_WithContainerAssignment_NullableInt() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [MaybeNull] public int? f1 = 1; + void M(C c) + { + c.f1.Value.ToString(); // 1 + c.f1 = 2; + + c = new C(); + c.f1.Value.ToString(); // 2 + } +}"; + + var comp = CreateNullableCompilation(new[] { source, MaybeNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (7,9): warning CS8629: Nullable value type may be null. + // c.f1.Value.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "c.f1").WithLocation(7, 9), + // (11,9): warning CS8629: Nullable value type may be null. + // c.f1.Value.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullableValueTypeMayBeNull, "c.f1").WithLocation(11, 9) + ); + } + [Fact] public void MaybeNullWhenTrue_OnImplicitReferenceConversion() { @@ -30646,6 +30771,30 @@ static void M1(T t1) comp2.VerifyDiagnostics(); } + [Fact] + public void NotNull_Property_WithAssignment() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [NotNull] string? P { get; set; } + void M() + { + P.ToString(); + P = null; + P.ToString(); // 1 + } +}"; + + var comp = CreateNullableCompilation(new[] { source, NotNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (9,9): warning CS8602: Dereference of a possibly null reference. + // P.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "P").WithLocation(9, 9) + ); + } + [Fact] public void NotNull_Property_OnVirtualProperty() { @@ -30901,6 +31050,52 @@ void symbolValidator(ModuleSymbol m) } } + [Fact] + public void NotNull_Field_WithAssignment() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [NotNull] string? f1 = default!; + void M() + { + f1.ToString(); + f1 = null; + f1.ToString(); // 1 + } +}"; + + var comp = CreateNullableCompilation(new[] { source, NotNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (9,9): warning CS8602: Dereference of a possibly null reference. + // f1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "f1").WithLocation(9, 9) + ); + } + + [Fact] + public void NotNull_Field_WithContainerAssignment() + { + var source = +@"using System.Diagnostics.CodeAnalysis; +public class C +{ + [NotNull] public string? f1 = default!; + void M(C c) + { + c.f1.ToString(); + c.f1 = null; + + c = new C(); + c.f1.ToString(); + } +}"; + + var comp = CreateNullableCompilation(new[] { source, NotNullAttributeDefinition }); + comp.VerifyDiagnostics(); + } + [Fact, WorkItem(35949, "https://github.com/dotnet/roslyn/issues/35949")] public void NotNull_Complexity() { -- GitLab