diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 100d162268fbc43db53b4c262a803ed150fd635d..1d6672326adcbcf99824e0747d0dd8d1697e1949 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -611,18 +611,50 @@ void enforceMemberNotNullWhenForPendingReturn(PendingBranch pendingReturn, Bound { if (pendingReturn.IsConditionalState) { - enforceMemberNotNullWhen(returnStatement.Syntax, sense: true, pendingReturn.StateWhenTrue); - enforceMemberNotNullWhen(returnStatement.Syntax, sense: false, pendingReturn.StateWhenFalse); + if (returnStatement.ExpressionOpt is { ConstantValue: { IsBoolean: true, BooleanValue: bool value } }) + { + enforceMemberNotNullWhen(returnStatement.Syntax, sense: value, pendingReturn.State); + return; + } + + if (!pendingReturn.StateWhenTrue.Reachable || !pendingReturn.StateWhenFalse.Reachable) + { + return; + } + + if (_symbol is MethodSymbol method) + { + foreach (var memberName in method.NotNullWhenTrueMembers) + { + enforceMemberNotNullWhenIfAffected(returnStatement.Syntax, sense: true, method.ContainingType.GetMembers(memberName), pendingReturn.StateWhenTrue, pendingReturn.StateWhenFalse); + } + + foreach (var memberName in method.NotNullWhenFalseMembers) + { + enforceMemberNotNullWhenIfAffected(returnStatement.Syntax, sense: false, method.ContainingType.GetMembers(memberName), pendingReturn.StateWhenFalse, pendingReturn.StateWhenTrue); + } + } + } + else if (returnStatement.ExpressionOpt is { ConstantValue: { IsBoolean: true, BooleanValue: bool value } }) + { + enforceMemberNotNullWhen(returnStatement.Syntax, sense: value, pendingReturn.State); } } - void enforceMemberNotNullWhen(SyntaxNode? syntaxOpt, bool sense, LocalState state) + void enforceMemberNotNullWhenIfAffected(SyntaxNode? syntaxOpt, bool sense, ImmutableArray members, LocalState state, LocalState otherState) { - if (!state.Reachable) + foreach (var member in members) { - return; + // For non-constant values, only complain if we were able to analyze a difference for this member between two branches + if (memberHasBadState(member, state) != memberHasBadState(member, otherState)) + { + reportMemberIfBadConditionalState(syntaxOpt, sense, member, state); + } } + } + void enforceMemberNotNullWhen(SyntaxNode? syntaxOpt, bool sense, LocalState state) + { if (_symbol is MethodSymbol method) { var notNullMembers = sense ? method.NotNullWhenTrueMembers : method.NotNullWhenFalseMembers; @@ -630,16 +662,21 @@ void enforceMemberNotNullWhen(SyntaxNode? syntaxOpt, bool sense, LocalState stat { foreach (var member in method.ContainingType.GetMembers(memberName)) { - if (memberHasBadState(member, state)) - { - // Member '{name}' must have a non-null value when exiting with '{sense}'. - Diagnostics.Add(ErrorCode.WRN_MemberNotNullWhen, syntaxOpt?.GetLocation() ?? methodMainNode.Syntax.GetLastToken().GetLocation(), member.Name, sense ? "true" : "false"); - } + reportMemberIfBadConditionalState(syntaxOpt, sense, member, state); } } } } + void reportMemberIfBadConditionalState(SyntaxNode? syntaxOpt, bool sense, Symbol member, LocalState state) + { + if (memberHasBadState(member, state)) + { + // Member '{name}' must have a non-null value when exiting with '{sense}'. + Diagnostics.Add(ErrorCode.WRN_MemberNotNullWhen, syntaxOpt?.GetLocation() ?? methodMainNode.Syntax.GetLastToken().GetLocation(), member.Name, sense ? "true" : "false"); + } + } + bool memberHasBadState(Symbol member, LocalState state) { switch (member.Kind) @@ -767,12 +804,45 @@ void enforceNotNullWhenForPendingReturn(PendingBranch pendingReturn, BoundReturn { if (pendingReturn.IsConditionalState) { - enforceParameterNotNullWhen(returnStatement.Syntax, parameters, sense: true, stateWhen: pendingReturn.StateWhenTrue); - enforceParameterNotNullWhen(returnStatement.Syntax, parameters, sense: false, stateWhen: pendingReturn.StateWhenFalse); + if (returnStatement.ExpressionOpt is { ConstantValue: { IsBoolean: true, BooleanValue: bool value } }) + { + enforceParameterNotNullWhen(returnStatement.Syntax, parameters, sense: value, stateWhen: pendingReturn.State); + return; + } + + if (!pendingReturn.StateWhenTrue.Reachable || !pendingReturn.StateWhenFalse.Reachable) + { + return; + } + + foreach (var parameter in parameters) + { + // For non-constant values, only complain if we were able to analyze a difference for this parameter between two branches + if (GetOrCreateSlot(parameter) is > 0 and var slot && pendingReturn.StateWhenTrue[slot] != pendingReturn.StateWhenFalse[slot]) + { + reportParameterIfBadConditionalState(returnStatement.Syntax, parameter, sense: true, stateWhen: pendingReturn.StateWhenTrue); + reportParameterIfBadConditionalState(returnStatement.Syntax, parameter, sense: false, stateWhen: pendingReturn.StateWhenFalse); + } + } + } + else if (returnStatement.ExpressionOpt is { ConstantValue: { IsBoolean: true, BooleanValue: bool value } }) + { + // example: return (bool)true; + enforceParameterNotNullWhen(returnStatement.Syntax, parameters, sense: value, stateWhen: pendingReturn.State); + return; } } } + void reportParameterIfBadConditionalState(SyntaxNode syntax, ParameterSymbol parameter, bool sense, LocalState stateWhen) + { + if (parameterHasBadConditionalState(parameter, sense, stateWhen)) + { + // Parameter '{name}' must have a non-null value when exiting with '{sense}'. + Diagnostics.Add(ErrorCode.WRN_ParameterConditionallyDisallowsNull, syntax.Location, parameter.Name, sense ? "true" : "false"); + } + } + void enforceNotNull(SyntaxNode? syntaxOpt, LocalState state) { if (!state.Reachable) @@ -812,11 +882,7 @@ void enforceParameterNotNullWhen(SyntaxNode syntax, ImmutableArray throw null!; +} +", MemberNotNullWhenAttributeDefinition, MaybeNullWhenAttributeDefinition }); + + c.VerifyDiagnostics( + // (10,5): warning CS8618: Non-nullable field 'field3' is uninitialized. Consider declaring the field as nullable. + // C() // 1 + Diagnostic(ErrorCode.WRN_UninitializedNonNullableField, "C").WithArguments("field", "field3").WithLocation(10, 5) + ); + } + + [Fact] + public void MemberNotNullWhenTrue_EnforcedInMethodBody_NonConstant_ButAnalyzable() + { + var c = CreateNullableCompilation(new[] { @" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string field3; + + C() + { + if (!Init()) throw null!; + } + + [MemberNotNullWhen(true, nameof(field1), nameof(field3))] + bool Init() + { + bool b = true; + if (b) + { + return field1 == null; // 1 + } + if (b) + { + return field1 != null; + } + if (b) + { + return Init(); + } + return !Init(); // 2, 3 + } +} +", MemberNotNullWhenAttributeDefinition, MaybeNullWhenAttributeDefinition }); + + c.VerifyDiagnostics( + // (19,13): warning CS8775: Member 'field1' must have a non-null value when exiting with 'true'. + // return field1 == null; // 1 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return field1 == null;").WithArguments("field1", "true").WithLocation(19, 13), + // (29,9): warning CS8775: Member 'field1' must have a non-null value when exiting with 'true'. + // return !Init(); // 2, 3 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return !Init();").WithArguments("field1", "true").WithLocation(29, 9), + // (29,9): warning CS8775: Member 'field3' must have a non-null value when exiting with 'true'. + // return !Init(); // 2, 3 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return !Init();").WithArguments("field3", "true").WithLocation(29, 9) + ); + } + + [Fact] + public void MemberNotNullWhenFalse_EnforcedInMethodBody_NonConstant_ButAnalyzable() + { + var c = CreateNullableCompilation(new[] { @" +using System.Diagnostics.CodeAnalysis; +public class C +{ + public string field1; + public string field3; + + C() + { + if (Init()) throw null!; + } + + [MemberNotNullWhen(false, nameof(field1), nameof(field3))] + bool Init() + { + bool b = true; + if (b) + { + return field1 == null; + } + if (b) + { + return field1 != null; // 1 + } + if (b) + { + return Init(); + } + return !Init(); // 2, 3 + } + + bool M([MaybeNullWhen(true)]out string s) => throw null!; +} +", MemberNotNullWhenAttributeDefinition, MaybeNullWhenAttributeDefinition }); + + c.VerifyDiagnostics( + // (23,13): warning CS8775: Member 'field1' must have a non-null value when exiting with 'false'. + // return field1 != null; // 1 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return field1 != null;").WithArguments("field1", "false").WithLocation(23, 13), + // (29,9): warning CS8775: Member 'field1' must have a non-null value when exiting with 'false'. + // return !Init(); // 2, 3 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return !Init();").WithArguments("field1", "false").WithLocation(29, 9), + // (29,9): warning CS8775: Member 'field3' must have a non-null value when exiting with 'false'. + // return !Init(); // 2, 3 + Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return !Init();").WithArguments("field3", "false").WithLocation(29, 9) + ); + } + [Fact] public void MemberNotNullWhenTrue_WithMemberNotNull() { @@ -24315,7 +24458,7 @@ bool IsInit2 ", MemberNotNullWhenAttributeDefinition }, parseOptions: TestOptions.Regular9); c.VerifyDiagnostics( - // (21,13): error CS8765: Member 'field2' must have a non-null value when exiting with 'true'. + // (21,13): warning CS8775: Member 'field2' must have a non-null value when exiting with 'true'. // return field2 == null; // 1 Diagnostic(ErrorCode.WRN_MemberNotNullWhen, "return field2 == null;").WithArguments("field2", "true").WithLocation(21, 13) ); @@ -25084,6 +25227,23 @@ public static bool TryGetValue([NotNullWhen(true)] out string? s) comp.VerifyDiagnostics(); } + [Fact] + public void NotNullWhenTrue_EnforceInMethodBody_ConditionalWithThrow() + { + var source = @" +using System.Diagnostics.CodeAnalysis; +public class C +{ + static bool M([NotNullWhen(true)] object? o) + { + return o == null ? true : throw null!; + } +} +"; + var comp = CreateNullableCompilation(new[] { source, NotNullWhenAttributeDefinition }); + comp.VerifyDiagnostics(); + } + [Fact] public void NotNullWhenTrue_EnforceInMethodBody_WithMaybeNull_CallingObliviousAPI() { @@ -25489,35 +25649,39 @@ public void NotNullWhenTrue_EnforceInMethodBody_OnConversionToBool() using System.Diagnostics.CodeAnalysis; public class C { + public static implicit operator bool(C? c) => throw null!; + public static bool TryGetValue(C? c, [NotNullWhen(true)] out string? s) { s = null; - return c; // 1 + return c; } public static bool TryGetValue2(C c, [NotNullWhen(false)] out string? s) { s = null; - return c; // 2 + return c; } - public static implicit operator bool(C? c) => throw null!; - static bool TryGetValue3([MaybeNullWhen(false)]out string s) { s = null; - return (bool)true; // 3 + return (bool)true; // 1 } static bool TryGetValue4([MaybeNullWhen(false)]out string s) { s = null; - return (bool)false; // 4 + return (bool)false; } } "; var comp = CreateNullableCompilation(new[] { source, NotNullWhenAttributeDefinition, MaybeNullWhenAttributeDefinition }); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (22,9): warning CS8762: Parameter 's' must have a non-null value when exiting with 'true'. + // return (bool)true; + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return (bool)true;").WithArguments("s", "true").WithLocation(22, 9) + ); } [Fact] @@ -37294,10 +37458,10 @@ static bool TryGetValue4([AllowNull] TYPE x, [MaybeNullWhen(false)] out TYPE y) "; var comp = CreateNullableCompilation(new[] { AllowNullAttributeDefinition, MaybeNullAttributeDefinition, MaybeNullWhenAttributeDefinition, source.Replace("TYPE", type) }); comp.VerifyDiagnostics( - // (24,9): error CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. + // (24,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(24, 9), - // (30,9): error CS8762: Parameter 'y' must have a non-null value when exiting with 'true'. + // (30,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(30, 9) ); @@ -37387,7 +37551,7 @@ static bool TryGetValue([AllowNull]T x, [MaybeNullWhen(true)]out T y, [MaybeN { y = x; z = x; - return y != null || z != null; // 1, 2 + return y != null || z != null; } static bool TryGetValue2([AllowNull]T x, [MaybeNullWhen(false)]out T y, [MaybeNullWhen(false)]out T z) @@ -37399,14 +37563,7 @@ static bool TryGetValue2([AllowNull]T x, [MaybeNullWhen(false)]out T y, [Mayb } "; var comp = CreateNullableCompilation(new[] { AllowNullAttributeDefinition, MaybeNullWhenAttributeDefinition, source }); - comp.VerifyDiagnostics( - // (12,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. - // return y != null || z != null; // 1, 2 - Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y != null || z != null;").WithArguments("y", "false").WithLocation(12, 9), - // (12,9): warning CS8762: Parameter 'z' must have a non-null value when exiting with 'false'. - // return y != null || z != null; // 1, 2 - Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y != null || z != null;").WithArguments("z", "false").WithLocation(12, 9) - ); + comp.VerifyDiagnostics(); } [Theory] @@ -37440,20 +37597,32 @@ static bool TryGetValue2([NotNullWhen(true)] out TYPE y) return y != null; } + static bool TryGetValue2B([NotNullWhen(true)] out TYPE y) + { + y = null; + return y == null; // 2 + } + static bool TryGetValue3([NotNullWhen(false)] out TYPE y) { y = null; return y == null; } + static bool TryGetValue3B([NotNullWhen(false)] out TYPE y) + { + y = null; + return y != null; // 3 + } + static bool TryGetValue4([NotNull] TYPE x, [NotNullWhen(false)] out TYPE y) { y = null; if (y != null) { - return true; // 2 + return true; // 4 } - return false; // 3, 4 + return false; // 5, 6 } } "; @@ -37462,15 +37631,21 @@ static bool TryGetValue4([NotNull] TYPE x, [NotNullWhen(false)] out TYPE y) // (15,13): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'true'. // return true; // 1 Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return true;").WithArguments("y", "true").WithLocation(15, 13), - // (37,13): warning CS8777: Parameter 'x' must have a non-null value when exiting. - // return true; // 2 - Diagnostic(ErrorCode.WRN_ParameterDisallowsNull, "return true;").WithArguments("x").WithLocation(37, 13), - // (39,9): warning CS8777: Parameter 'x' must have a non-null value when exiting. - // return false; // 3, 4 - Diagnostic(ErrorCode.WRN_ParameterDisallowsNull, "return false;").WithArguments("x").WithLocation(39, 9), - // (39,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. - // return false; // 3, 4 - Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return false;").WithArguments("y", "false").WithLocation(39, 9) + // (29,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(29, 9), + // (41,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. + // return y != null; // 3 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return y != null;").WithArguments("y", "false").WithLocation(41, 9), + // (49,13): warning CS8777: Parameter 'x' must have a non-null value when exiting. + // return true; // 4 + Diagnostic(ErrorCode.WRN_ParameterDisallowsNull, "return true;").WithArguments("x").WithLocation(49, 13), + // (51,9): warning CS8777: Parameter 'x' must have a non-null value when exiting. + // return false; // 5, 6 + Diagnostic(ErrorCode.WRN_ParameterDisallowsNull, "return false;").WithArguments("x").WithLocation(51, 9), + // (51,9): warning CS8762: Parameter 'y' must have a non-null value when exiting with 'false'. + // return false; // 5, 6 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return false;").WithArguments("y", "false").WithLocation(51, 9) ); } @@ -140237,6 +140412,183 @@ void M() Diagnostic(ErrorCode.ERR_DiscardTypeInferenceFailed, "_").WithLocation(10, 13)); } + [Fact, WorkItem(48126, "https://github.com/dotnet/roslyn/issues/48126")] + public void EnforcedInMethodBody_MaybeNullWhen_Issue_48126() + { + var source = @" +using System.Diagnostics.CodeAnalysis; + +#nullable enable + +class C +{ + static bool NullWhenFalseA(bool a, [MaybeNullWhen(false)] out string s1) + { + s1 = null; + return a; + } + + static bool NullWhenFalseNotA(bool a, [MaybeNullWhen(false)] out string s1) + { + s1 = null; + return !a; + } + + static bool NullWhenTrueA(bool a, [MaybeNullWhen(true)] out string s1) + { + s1 = null; + return a; + } + + static bool NullWhenTrueNotA(bool a, [MaybeNullWhen(true)] out string s1) + { + s1 = null; + return !a; + } +}"; + var comp = CreateCompilation(new[] { source, MaybeNullWhenAttributeDefinition }); + comp.VerifyEmitDiagnostics(); + } + + [Fact, WorkItem(48126, "https://github.com/dotnet/roslyn/issues/48126")] + public void EnforcedInMethodBody_MaybeNullWhen_Issue_48126_2() + { + var source = @" +using System.Diagnostics.CodeAnalysis; + +#nullable enable + +class C +{ + static bool M1([MaybeNullWhen(false)] out string s1) + { + const bool b = true; + s1 = null; + return b; // 1 + } + + static bool M2([MaybeNullWhen(false)] out string s1) + { + s1 = null; + return (bool)true; // 2 + } +}"; + var comp = CreateCompilation(new[] { source, MaybeNullWhenAttributeDefinition }); + comp.VerifyEmitDiagnostics( + // (12,9): warning CS8762: Parameter 's1' must have a non-null value when exiting with 'true'. + // return b; // 1 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return b;").WithArguments("s1", "true").WithLocation(12, 9), + // (18,9): warning CS8762: Parameter 's1' must have a non-null value when exiting with 'true'. + // return (bool)true; // 2 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return (bool)true;").WithArguments("s1", "true").WithLocation(18, 9) + ); + } + + [Fact, WorkItem(48126, "https://github.com/dotnet/roslyn/issues/48126")] + public void EnforcedInMethodBody_MaybeNullWhen_Issue_48126_3() + { + var source = @" +using System.Diagnostics.CodeAnalysis; + +#nullable enable + +class C +{ + static bool M1([MaybeNullWhen(false)] out string s1) + { + const bool b = true; + s1 = null; + return b; // 1 + } + + static bool M2([MaybeNullWhen(false)] out string s1) + { + const bool b = true; + s1 = null; + return b && true; // 2 + } + + static bool M3([MaybeNullWhen(false)] out string s1) + { + const bool b = false; + s1 = null; + return b; + } +}"; + var comp = CreateCompilation(new[] { source, MaybeNullWhenAttributeDefinition }); + comp.VerifyEmitDiagnostics( + // (12,9): warning CS8762: Parameter 's1' must have a non-null value when exiting with 'true'. + // return b; // 1 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return b;").WithArguments("s1", "true").WithLocation(12, 9), + // (19,9): warning CS8762: Parameter 's1' must have a non-null value when exiting with 'true'. + // return b && true; // 2 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return b && true;").WithArguments("s1", "true").WithLocation(19, 9) + ); + } + + [Fact, WorkItem(48126, "https://github.com/dotnet/roslyn/issues/48126")] + public void EnforcedInMethodBody_MaybeNullWhen_Issue_48126_4() + { + var source = @" +using System.Diagnostics.CodeAnalysis; + +#nullable enable + +class C +{ + static bool M1([MaybeNullWhen(false)] out string s1) + { + return M1(out s1); + } + + static bool M2([MaybeNullWhen(false)] out string s1) + { + return !M1(out s1); // 1 + } + + static bool M3([MaybeNullWhen(true)] out string s1) + { + return !M1(out s1); + } +}"; + var comp = CreateCompilation(new[] { source, MaybeNullWhenAttributeDefinition }); + comp.VerifyEmitDiagnostics( + // (15,9): warning CS8762: Parameter 's1' must have a non-null value when exiting with 'true'. + // return !M1(out s1); // 1 + Diagnostic(ErrorCode.WRN_ParameterConditionallyDisallowsNull, "return !M1(out s1);").WithArguments("s1", "true").WithLocation(15, 9) + ); + } + + [Fact, WorkItem(48126, "https://github.com/dotnet/roslyn/issues/48126")] + public void EnforcedInMethodBody_MaybeNullWhen_Issue_48126_5() + { + var source = @" +using System.Diagnostics.CodeAnalysis; + +#nullable enable + +internal static class Program +{ + public static bool M1([MaybeNullWhen(true)] out string s) + { + s = null; + return HasAnnotation(out _); + } + + public static bool M2([MaybeNullWhen(true)] out string s) + { + s = null; + return NoAnnotations(); + } + + private static bool HasAnnotation([MaybeNullWhen(true)] out string s) { s = null; return true; } + + private static bool NoAnnotations() => true; +}"; + var comp = CreateCompilation(new[] { source, MaybeNullWhenAttributeDefinition }); + comp.VerifyEmitDiagnostics(); + } + [Fact] [WorkItem(47221, "https://github.com/dotnet/roslyn/issues/47221")] public void PropertyAccessorWithNullableContextAttribute_01()