diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 8056e83b73970..123ff8ce5b272 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -669,6 +669,12 @@ void checkMemberStateOnConstructorExit(MethodSymbol constructor, Symbol member, { return; } + + if (symbol.IsRequired()) + { + return; + } + var annotations = symbol.GetFlowAnalysisAnnotations(); if ((annotations & FlowAnalysisAnnotations.AllowNull) != 0) { @@ -811,46 +817,43 @@ void makeNotNullMembersMaybeNull() { if (method.IsConstructor()) { - if (needsDefaultInitialStateForMembers()) + foreach (var member in getMembersNeedingDefaultInitialState()) { - foreach (var member in method.ContainingType.GetMembersUnordered()) + if (member.IsStatic != method.IsStatic) { - if (member.IsStatic != method.IsStatic) - { - continue; - } + continue; + } - var memberToInitialize = member; - switch (member) - { - case PropertySymbol: - // skip any manually implemented properties. - continue; - case FieldSymbol { IsConst: true }: + var memberToInitialize = member; + switch (member) + { + case PropertySymbol: + // skip any manually implemented properties. + continue; + case FieldSymbol { IsConst: true }: + continue; + case FieldSymbol { AssociatedSymbol: PropertySymbol prop }: + // this is a property where assigning 'default' causes us to simply update + // the state to the output state of the property + // thus we skip setting an initial state for it here + if (IsPropertyOutputMoreStrictThanInput(prop)) + { continue; - case FieldSymbol { AssociatedSymbol: PropertySymbol prop }: - // this is a property where assigning 'default' causes us to simply update - // the state to the output state of the property - // thus we skip setting an initial state for it here - if (IsPropertyOutputMoreStrictThanInput(prop)) - { - continue; - } + } - // We want to initialize auto-property state to the default state, but not computed properties. - memberToInitialize = prop; - break; - default: - break; - } - var memberSlot = getSlotForFieldOrPropertyOrEvent(memberToInitialize); - if (memberSlot > 0) + // We want to initialize auto-property state to the default state, but not computed properties. + memberToInitialize = prop; + break; + default: + break; + } + var memberSlot = getSlotForFieldOrPropertyOrEvent(memberToInitialize); + if (memberSlot > 0) + { + var type = memberToInitialize.GetTypeOrReturnType(); + if (!type.NullableAnnotation.IsOblivious()) { - var type = memberToInitialize.GetTypeOrReturnType(); - if (!type.NullableAnnotation.IsOblivious()) - { - this.State[memberSlot] = type.Type.IsPossiblyNullableReferenceTypeTypeParameter() ? NullableFlowState.MaybeDefault : NullableFlowState.MaybeNull; - } + this.State[memberSlot] = type.Type.IsPossiblyNullableReferenceTypeTypeParameter() ? NullableFlowState.MaybeDefault : NullableFlowState.MaybeNull; } } } @@ -868,21 +871,47 @@ void makeNotNullMembersMaybeNull() } } - bool needsDefaultInitialStateForMembers() + ImmutableArray getMembersNeedingDefaultInitialState() { if (_hasInitialState) { - return false; + return ImmutableArray.Empty; } // We don't use a default initial state for value type instance constructors without `: this()` because // any usages of uninitialized fields will get definite assignment errors anyway. - if (!method.HasThisConstructorInitializer(out _) && (!method.ContainingType.IsValueType || method.IsStatic)) + bool hasConstructorInitializer = method.HasThisConstructorInitializer(out _); + if (!hasConstructorInitializer && (!method.ContainingType.IsValueType || method.IsStatic)) { - return true; + return membersToBeInitialized(method.ContainingType, includeAllMembers: true); } - return method.IncludeFieldInitializersInBody(); + return membersToBeInitialized(method.ContainingType, includeAllMembers: method.IncludeFieldInitializersInBody()); + + static ImmutableArray membersToBeInitialized(NamedTypeSymbol containingType, bool includeAllMembers) + { + var requiredMembers = containingType.AllRequiredMembers; + var members = includeAllMembers ? containingType.GetMembersUnordered() : ImmutableArray.Empty; + if (requiredMembers.IsEmpty) + { + return members; + } + + var builder = ArrayBuilder.GetInstance(members.Length + requiredMembers.Count); + builder.AddRange(members); + foreach (var (_, requiredMember) in requiredMembers) + { + if (!members.IsEmpty && requiredMember.ContainingType.Equals(containingType, TypeCompareKind.ConsiderEverything)) + { + continue; + } + + // We want to assume that all required members were _not_ set by the chained constructor + builder.Add(requiredMember is SourcePropertySymbol { IsAutoPropertyWithGetAccessor: true } prop ? prop.BackingField : requiredMember); + } + + return builder.ToImmutableAndFree(); + } } } diff --git a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs index a5788ad6a0aec..bc1701d1b9733 100644 --- a/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs +++ b/src/Compilers/CSharp/Test/Symbol/Symbols/RequiredMembersTests.cs @@ -2850,4 +2850,409 @@ public class RequiredMemberAttribute : Attribute Diagnostic(ErrorCode.ERR_ExplicitRequiredMember, "RequiredMember").WithLocation(6, 6) ); } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_01() + { + var code = @" +#pragma warning disable CS0649 // Field is never assigned to +#nullable enable +class C +{ + public required string P1 { get; set; } + public required string F1; + public string P2 { get; set; } + public string F2; +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (8,19): warning CS8618: Non-nullable property 'P2' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. + // public string P2 { get; set; } + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "P2").WithArguments("property", "P2").WithLocation(8, 19), + // (9,19): warning CS8618: Non-nullable field 'F2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public string F2; + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "F2").WithArguments("field", "F2").WithLocation(9, 19) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_02() + { + var code = @" +#pragma warning disable CS0649 // Field is never assigned to +#nullable enable +class C +{ + public required string P1 { get; set; } + public required string F1; + public string P2 { get; set; } + public string F2; + + public C() + { + } + + public C(int _) {} +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (11,12): warning CS8618: Non-nullable property 'P2' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. + // public C() + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("property", "P2").WithLocation(11, 12), + // (11,12): warning CS8618: Non-nullable field 'F2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public C() + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("field", "F2").WithLocation(11, 12), + // (15,12): warning CS8618: Non-nullable property 'P2' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. + // public C(int _) {} + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("property", "P2").WithLocation(15, 12), + // (15,12): warning CS8618: Non-nullable field 'F2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public C(int _) {} + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("field", "F2").WithLocation(15, 12) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_03() + { + var code = @" +#pragma warning disable CS0649 // Field is never assigned to +#nullable enable +struct S +{ + public required string P1 { get; set; } + public required string F1; + public string P2 { get; set; } + public string F2; +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_04() + { + var code = @" +#pragma warning disable CS0649 // Field is never assigned to +#nullable enable +struct S +{ + public required string P1 { get; set; } + public required string F1; + public string P2 { get; set; } + public string F2; + + public S() {} +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + // PROTOTYPE(req): These errors should change with Rikki's DA changes. + comp.VerifyDiagnostics( + // (11,12): warning CS8618: Non-nullable property 'P2' must contain a non-null value when exiting constructor. Consider declaring the property as nullable. + // public S() {} + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "S").WithArguments("property", "P2").WithLocation(11, 12), + // (11,12): warning CS8618: Non-nullable field 'F2' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public S() {} + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "S").WithArguments("field", "F2").WithLocation(11, 12), + // (11,12): error CS0171: Field 'S.F1' must be fully assigned before control is returned to the caller + // public S() {} + Diagnostic(ErrorCode.ERR_UnassignedThis, "S").WithArguments("S.F1").WithLocation(11, 12), + // (11,12): error CS0843: Auto-implemented property 'S.P2' must be fully assigned before control is returned to the caller. + // public S() {} + Diagnostic(ErrorCode.ERR_UnassignedThisAutoProperty, "S").WithArguments("S.P2").WithLocation(11, 12), + // (11,12): error CS0171: Field 'S.F2' must be fully assigned before control is returned to the caller + // public S() {} + Diagnostic(ErrorCode.ERR_UnassignedThis, "S").WithArguments("S.F2").WithLocation(11, 12), + // (11,12): error CS0843: Auto-implemented property 'S.P1' must be fully assigned before control is returned to the caller. + // public S() {} + Diagnostic(ErrorCode.ERR_UnassignedThisAutoProperty, "S").WithArguments("S.P1").WithLocation(11, 12) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_MemberNotNull_NoChainedConstructor() + { + var code = @" +using System.Diagnostics.CodeAnalysis; +#nullable enable +class C +{ + private string _field; + public required string Field { get => _field; [MemberNotNull(nameof(_field))] set => _field = value; } + + public C() { } +}"; + + var comp = CreateCompilationWithRequiredMembers(new[] { code, MemberNotNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (9,12): warning CS8618: Non-nullable field '_field' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public C() { } + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("field", "_field").WithLocation(9, 12) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_MemberNotNull_ChainedConstructor() + { + var code = @" +using System.Diagnostics.CodeAnalysis; +#nullable enable +class C +{ + private string _field; + public required string Field { get => _field; [MemberNotNull(nameof(_field))] set => _field = value; } + + public C() { } + public C(bool unused) : this() + { + _field.ToString(); + Field.ToString(); + } +}"; + + var comp = CreateCompilationWithRequiredMembers(new[] { code, MemberNotNullAttributeDefinition }); + comp.VerifyDiagnostics( + // (9,12): warning CS8618: Non-nullable field '_field' must contain a non-null value when exiting constructor. Consider declaring the field as nullable. + // public C() { } + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("field", "_field").WithLocation(9, 12) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_01() + { + var code = @" +#nullable enable +public class C +{ + public required string Prop { get; set; } + + public C(bool unused) { } + + public C() : this(true) + { + Prop.ToString(); + } +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (11,9): warning CS8602: Dereference of a possibly null reference. + // Prop.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(11, 9) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_02() + { + var code = @" +#nullable enable +public struct C +{ + public required string Prop { get; set; } + + public C(bool unused) { } + + public C() : this(true) + { + Prop.ToString(); + } +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + // PROTOTYPE(req): These errors should change with Rikki's DA changes. + comp.VerifyDiagnostics( + // (7,12): error CS0843: Auto-implemented property 'C.Prop' must be fully assigned before control is returned to the caller. + // public C(bool unused) { } + Diagnostic(ErrorCode.ERR_UnassignedThisAutoProperty, "C").WithArguments("C.Prop").WithLocation(7, 12), + // (11,9): warning CS8602: Dereference of a possibly null reference. + // Prop.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(11, 9) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_03() + { + var code = @" +#nullable enable +public struct C +{ + public required string Prop { get; set; } + + public C(bool unused) : this() + { + Prop.ToString(); + } +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (9,9): warning CS8602: Dereference of a possibly null reference. + // Prop.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop").WithLocation(9, 9) + ); + } + + [Theory, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + [InlineData(": base()")] + [InlineData("")] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_04(string baseSyntax) + { + var code = $@" +#nullable enable +public class Base +{{ + public required string Prop1 {{ get; set; }} + public string Prop2 {{ get; set; }} = null!; +}} + +public class Derived : Base +{{ + public Derived() {baseSyntax} + {{ + Prop1.ToString(); + Prop2.ToString(); + }} +}} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (13,9): warning CS8602: Dereference of a possibly null reference. + // Prop1.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(13, 9) + ); + } + + [Theory, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + [InlineData(": base()")] + [InlineData("")] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_05(string baseSyntax) + { + var code = @$" +#nullable enable +public class Base +{{ + public required string Prop1 {{ get; set; }} + public string Prop2 {{ get; set; }} = null!; +}} + +public class Derived : Base +{{ + public required string Prop3 {{ get; set; }} + public string Prop4 {{ get; set; }} = null!; + + public Derived() {baseSyntax} + {{ + Prop1.ToString(); // 1 + Prop2.ToString(); + Prop3.ToString(); // 2 + Prop4.ToString(); + }} +}} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (16,9): warning CS8602: Dereference of a possibly null reference. + // Prop1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(16, 9), + // (18,9): warning CS8602: Dereference of a possibly null reference. + // Prop3.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(18, 9) + ); + } + + [Theory, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + [InlineData(": base()")] + [InlineData("")] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_06(string baseSyntax) + { + var code = @$" +#nullable enable +public class Base +{{ + public required string Prop1 {{ get; set; }} + public string Prop2 {{ get; set; }} = null!; +}} + +public class Derived : Base +{{ + public required string Prop3 {{ get; set; }} + public string Prop4 {{ get; set; }} + + public Derived() {baseSyntax} + {{ + Prop1.ToString(); // 1 + Prop2.ToString(); + Prop3.ToString(); // 2 + Prop4.ToString(); // 3 + }} +}} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (16,9): warning CS8602: Dereference of a possibly null reference. + // Prop1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(16, 9), + // (18,9): warning CS8602: Dereference of a possibly null reference. + // Prop3.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(18, 9), + // (19,9): warning CS8602: Dereference of a possibly null reference. + // Prop4.ToString(); // 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop4").WithLocation(19, 9) + ); + } + + [Fact, CompilerTrait(CompilerFeature.NullableReferenceTypes)] + public void RequiredMemberSuppressesNullabilityWarnings_ChainedConstructor_07() + { + var code = @" +#nullable enable +public class Base +{ + public required string Prop1 { get; set; } + public string Prop2 { get; set; } = null!; +} + +public class Derived : Base +{ + public required string Prop3 { get; set; } + public string Prop4 { get; set; } + + public Derived(bool unused) { Prop4 = null!; } + + public Derived() : this(true) + { + Prop1.ToString(); // 1 + Prop2.ToString(); + Prop3.ToString(); // 2 + Prop4.ToString(); + } +} +"; + + var comp = CreateCompilationWithRequiredMembers(code); + comp.VerifyDiagnostics( + // (18,9): warning CS8602: Dereference of a possibly null reference. + // Prop1.ToString(); // 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop1").WithLocation(18, 9), + // (20,9): warning CS8602: Dereference of a possibly null reference. + // Prop3.ToString(); // 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "Prop3").WithLocation(20, 9) + ); + } }