diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index c55c78745d1d7..6ff752fbfba82 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -2519,6 +2519,7 @@ private void EnterParameter(ParameterSymbol parameter, TypeWithAnnotations param useLegacyWarnings: false, trackMembers: false, assignmentKind: AssignmentKind.Assignment); + Unsplit(); // If the LHS has annotations, we perform an additional check for nullable value types CheckDisallowedNullAssignment(resultType, parameterAnnotations, equalsValue.Value.Syntax.Location); @@ -3037,6 +3038,7 @@ private void DeclareLocals(ImmutableArray locals) else { valueType = VisitOptionalImplicitConversion(initializer, targetTypeOpt: inferredType ? default : type, useLegacyWarnings: true, trackMembers: true, AssignmentKind.Assignment); + Unsplit(); } if (inferredType) @@ -3600,6 +3602,7 @@ int getOrCreateSlot(int containingSlot, Symbol symbol) conversionCompletion is not null ? (conversionCompletion(type), null) : VisitOptionalImplicitConversion(node.Right, type, useLegacyWarnings: false, trackMembers: true, AssignmentKind.Assignment, delayCompletionForType); + Unsplit(); if (delayCompletionForType) { @@ -3897,6 +3900,7 @@ private int GetOrCreatePlaceholderSlot(object identifier, TypeWithAnnotations ty foreach (var expr in expressions) { _ = VisitOptionalImplicitConversion(expr, elementType, useLegacyWarnings: false, trackMembers: false, AssignmentKind.Assignment); + Unsplit(); } } else @@ -4356,25 +4360,26 @@ private void ReinferBinaryOperatorAndSetResult( // Analyze operator call properly (honoring [Disallow|Allow|Maybe|NotNull] attribute annotations) https://github.com/dotnet/roslyn/issues/32671 var parameters = method.Parameters; - visitOperandConversion(binary.Left, leftOperand, leftConversion, parameters[0], leftUnderlyingType); - visitOperandConversion(binary.Right, rightOperand, rightConversion, parameters[1], rightUnderlyingType); + visitOperandConversionAndPostConditions(binary.Left, leftOperand, leftConversion, parameters[0], leftUnderlyingType); + visitOperandConversionAndPostConditions(binary.Right, rightOperand, rightConversion, parameters[1], rightUnderlyingType); SetUpdatedSymbol(binary, binary.Method!, method); - void visitOperandConversion( + void visitOperandConversionAndPostConditions( BoundExpression expr, BoundExpression operand, Conversion conversion, ParameterSymbol parameter, TypeWithState operandType) { - TypeWithAnnotations targetTypeWithNullability = parameter.TypeWithAnnotations; + var parameterAnnotations = GetParameterAnnotations(parameter); + var targetTypeWithNullability = ApplyLValueAnnotations(parameter.TypeWithAnnotations, parameterAnnotations); if (isLifted && targetTypeWithNullability.Type.IsNonNullableValueType()) { targetTypeWithNullability = TypeWithAnnotations.Create(MakeNullableOf(targetTypeWithNullability)); } - _ = VisitConversion( + var resultType = VisitConversion( expr as BoundConversion, operand, conversion, @@ -4385,6 +4390,9 @@ void visitOperandConversion( useLegacyWarnings: false, AssignmentKind.Argument, parameter); + CheckDisallowedNullAssignment(resultType, parameterAnnotations, expr.Syntax.Location, operand); + + LearnFromPostConditions(operand, parameterAnnotations); } } else @@ -6404,7 +6412,7 @@ private void VisitArgumentOutboundAssignmentsAndPostConditions( case RefKind.In: { // learn from post-conditions [Maybe/NotNull, Maybe/NotNullWhen] without using an assignment - learnFromPostConditions(argument, parameterType, parameterAnnotations); + LearnFromPostConditions(argument, parameterAnnotations); } break; case RefKind.Ref: @@ -6587,50 +6595,53 @@ static TypeWithState applyPostConditionsWhenFalse(TypeWithState typeWithState, F return typeWithState; } + } - void learnFromPostConditions(BoundExpression argument, TypeWithAnnotations parameterType, FlowAnalysisAnnotations parameterAnnotations) - { - // Note: NotNull = NotNullWhen(true) + NotNullWhen(false) - bool notNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenTrue) != 0; - bool notNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenFalse) != 0; + /// + /// Learn from postconditions on a by-value or 'in' argument. + /// + private void LearnFromPostConditions(BoundExpression argument, FlowAnalysisAnnotations parameterAnnotations) + { + // Note: NotNull = NotNullWhen(true) + NotNullWhen(false) + bool notNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenTrue) != 0; + bool notNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.NotNullWhenFalse) != 0; - // Note: MaybeNull = MaybeNullWhen(true) + MaybeNullWhen(false) - bool maybeNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenTrue) != 0; - bool maybeNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenFalse) != 0; + // Note: MaybeNull = MaybeNullWhen(true) + MaybeNullWhen(false) + bool maybeNullWhenTrue = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenTrue) != 0; + bool maybeNullWhenFalse = (parameterAnnotations & FlowAnalysisAnnotations.MaybeNullWhenFalse) != 0; - if (maybeNullWhenTrue && maybeNullWhenFalse && !IsConditionalState && !(notNullWhenTrue && notNullWhenFalse)) + if (maybeNullWhenTrue && maybeNullWhenFalse && !IsConditionalState && !(notNullWhenTrue && notNullWhenFalse)) + { + LearnFromNullTest(argument, ref State); + } + else if (notNullWhenTrue && notNullWhenFalse + && !IsConditionalState + && !(maybeNullWhenTrue || maybeNullWhenFalse)) + { + LearnFromNonNullTest(argument, ref State); + } + else if (notNullWhenTrue || notNullWhenFalse || maybeNullWhenTrue || maybeNullWhenFalse) + { + Split(); + + if (notNullWhenTrue) { - LearnFromNullTest(argument, ref State); + LearnFromNonNullTest(argument, ref StateWhenTrue); } - else if (notNullWhenTrue && notNullWhenFalse - && !IsConditionalState - && !(maybeNullWhenTrue || maybeNullWhenFalse)) + + if (notNullWhenFalse) { - LearnFromNonNullTest(argument, ref State); + LearnFromNonNullTest(argument, ref StateWhenFalse); } - else if (notNullWhenTrue || notNullWhenFalse || maybeNullWhenTrue || maybeNullWhenFalse) - { - Split(); - - if (notNullWhenTrue) - { - LearnFromNonNullTest(argument, ref StateWhenTrue); - } - if (notNullWhenFalse) - { - LearnFromNonNullTest(argument, ref StateWhenFalse); - } - - if (maybeNullWhenTrue) - { - LearnFromNullTest(argument, ref StateWhenTrue); - } + if (maybeNullWhenTrue) + { + LearnFromNullTest(argument, ref StateWhenTrue); + } - if (maybeNullWhenFalse) - { - LearnFromNullTest(argument, ref StateWhenFalse); - } + if (maybeNullWhenFalse) + { + LearnFromNullTest(argument, ref StateWhenFalse); } } } @@ -8260,6 +8271,8 @@ private TypeWithState VisitUserDefinedConversion( fromExplicitCast: conversionOpt?.ExplicitCastInCode ?? false, diagnosticLocation); + LearnFromPostConditions(conversionOperand, parameterAnnotations); + TrackAnalyzedNullabilityThroughConversionGroup(operandType, conversionOpt, conversionOperand); return operandType; @@ -8756,6 +8769,7 @@ private void VisitThisOrBaseReference(BoundExpression node) { var discarded = left is BoundDiscardExpression; rightState = VisitOptionalImplicitConversion(right, targetTypeOpt: discarded ? default : leftLValueType, UseLegacyWarnings(left, leftLValueType), trackMembers: true, AssignmentKind.Assignment); + Unsplit(); } else { @@ -9077,6 +9091,7 @@ private void VisitTupleDeconstructionArguments(ArrayBuilder Iterator() + { + yield return new C(); + yield return true; + } + + public static implicit operator bool([NotNullWhen(true)] C? c) => c != null; +} +", NotNullWhenAttributeDefinition }, options: WithNullableEnable()); + + c.VerifyDiagnostics( + // (9,36): error CS1736: Default parameter value for 'b' must be a compile-time constant + // public static bool M0(bool b = new C()) // 1 + Diagnostic(ErrorCode.ERR_DefaultValueMustBeConstant, "new C()").WithArguments("b").WithLocation(9, 36), + // (16,9): warning CS0162: Unreachable code detected + // return true; // 2 + Diagnostic(ErrorCode.WRN_UnreachableCode, "return").WithLocation(16, 9) + ); + } + + [Fact, WorkItem(32671, "https://github.com/dotnet/roslyn/issues/32671")] + public void DisallowNull_UserDefinedOperator_01() + { + CSharpCompilation c = CreateCompilation(new[] { @" +#pragma warning disable CS0660, CS0661 // no equals, hashcode +using System.Diagnostics.CodeAnalysis; + +public class C +{ + public static void M0(C? c) + { + bool b = c; // 1 + b = new C(); + } + + public static void M0(C? c1, C? c2) + { + bool b = c1 == c2; // 2 + b = c2 != c1; // 3 + b = new C() == new C(); + } + + public static implicit operator bool([DisallowNull] C? c) => c != null; + + public static bool operator ==([DisallowNull] C? c1, C? c2) => (object?)c1 == c2; + public static bool operator !=([DisallowNull] C? c1, C? c2) => (object?)c1 != c2; +} +", DisallowNullAttributeDefinition }, options: WithNullableEnable()); + + c.VerifyDiagnostics( + // (9,18): warning CS8604: Possible null reference argument for parameter 'c' in 'C.implicit operator bool(C? c)'. + // bool b = c; // 1 + Diagnostic(ErrorCode.WRN_NullReferenceArgument, "c").WithArguments("c", "C.implicit operator bool(C? c)").WithLocation(9, 18), + // (15,18): warning CS8604: Possible null reference argument for parameter 'c1' in 'bool C.operator ==(C? c1, C? c2)'. + // bool b = c1 == c2; // 2 + Diagnostic(ErrorCode.WRN_NullReferenceArgument, "c1").WithArguments("c1", "bool C.operator ==(C? c1, C? c2)").WithLocation(15, 18), + // (16,13): warning CS8604: Possible null reference argument for parameter 'c1' in 'bool C.operator !=(C? c1, C? c2)'. + // b = c2 != c1; // 3 + Diagnostic(ErrorCode.WRN_NullReferenceArgument, "c2").WithArguments("c1", "bool C.operator !=(C? c1, C? c2)").WithLocation(16, 13) + ); + } + + [Fact, WorkItem(32671, "https://github.com/dotnet/roslyn/issues/32671")] + public void DisallowNull_UserDefinedOperator_02() + { + CSharpCompilation c = CreateCompilation(new[] { @" +#pragma warning disable CS0660, CS0661 // no equals, hashcode +using System.Diagnostics.CodeAnalysis; + +public struct S +{ + public static void M0(S? s) + { + bool? b = s; // 1 + b = new S(); + } + + public static void M0(S? s1, S? s2) + { + bool b = s1 == s2; // 2 + b = s2 != s1; // 3 + b = new S() == new S(); + } + + public static implicit operator bool([DisallowNull] S? s) => s != null; + + public static bool operator ==([DisallowNull] S? s1, S? s2) => throw null!; + public static bool operator !=([DisallowNull] S? s1, S? s2) => throw null!; +} +", DisallowNullAttributeDefinition }, options: WithNullableEnable()); + + c.VerifyDiagnostics( + // (9,19): warning CS8607: A possible null value may not be used for a type marked with [NotNull] or [DisallowNull] + // bool? b = s; // 1 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "s").WithLocation(9, 19), + // (15,18): warning CS8607: A possible null value may not be used for a type marked with [NotNull] or [DisallowNull] + // bool b = s1 == s2; // 2 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "s1").WithLocation(15, 18), + // (16,13): warning CS8607: A possible null value may not be used for a type marked with [NotNull] or [DisallowNull] + // b = s2 != s1; // 3 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "s2").WithLocation(16, 13) + ); + } + [Fact, WorkItem(58598, "https://github.com/dotnet/roslyn/issues/58598")] public void MemberNotNull_NullConstant() { @@ -39289,6 +39629,7 @@ public class C [WorkItem(48605, "https://github.com/dotnet/roslyn/issues/48605")] [WorkItem(48134, "https://github.com/dotnet/roslyn/issues/48134")] [WorkItem(49136, "https://github.com/dotnet/roslyn/issues/49136")] + [WorkItem(49575, "https://github.com/dotnet/roslyn/issues/49575")] public void AllowNullNotNullInputParam_UpdatesArgumentState() { var source = @" @@ -39311,7 +39652,7 @@ public static void M2(string? s) public static void M3(string? s) { C c = s; - s.ToString(); // 1 + s.ToString(); } public static void MExt([AllowNull, NotNull] this string s) { throw null!; } @@ -39322,13 +39663,8 @@ public class C } } "; - // we should respect postconditions on a conversion parameter - // https://github.com/dotnet/roslyn/issues/49575 var comp = CreateNullableCompilation(new[] { source, AllowNullAttributeDefinition, NotNullAttributeDefinition }); - comp.VerifyDiagnostics( - // (21,9): warning CS8602: Dereference of a possibly null reference. - // s.ToString(); // 1 - Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(21, 9)); + comp.VerifyDiagnostics(); } [Fact] @@ -68209,7 +68545,7 @@ class CL2 ); } - [Fact] + [Fact, WorkItem(32671, "https://github.com/dotnet/roslyn/issues/32671")] public void BinaryOperator_03_WithDisallowAndAllowNull() { CSharpCompilation c = CreateNullableCompilation(new[] { @" @@ -68253,23 +68589,13 @@ void Test4(string x4, CL1 y4, CL2 z4) } ", AllowNullAttributeDefinition, DisallowNullAttributeDefinition }); - // Analyze operator call properly (honoring [Disallow|Allow|Maybe|NotNull] attribute annotations) https://github.com/dotnet/roslyn/issues/32671 c.VerifyDiagnostics( - // (8,24): warning CS8604: Possible null reference argument for parameter 'y' in 'CL0 CL0.operator +(string? x, CL0 y)'. - // CL0? z1 = x1 + y1; - Diagnostic(ErrorCode.WRN_NullReferenceArgument, "y1").WithArguments("y", "CL0 CL0.operator +(string? x, CL0 y)").WithLocation(8, 24), - // (19,18): warning CS8604: Possible null reference argument for parameter 'x' in 'CL1? CL1.operator +(string x, CL1? y)'. - // CL1 z2 = x2 + y2; // 1, 2 - Diagnostic(ErrorCode.WRN_NullReferenceArgument, "x2").WithArguments("x", "CL1? CL1.operator +(string x, CL1? y)").WithLocation(19, 18), // (19,18): warning CS8600: Converting null literal or possible null value to non-nullable type. // CL1 z2 = x2 + y2; // 1, 2 Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "x2 + y2").WithLocation(19, 18), - // (29,23): warning CS8604: Possible null reference argument for parameter 'y' in 'CL0 CL0.operator +(string? x, CL0 y)'. - // CL2 u3 = x3 + y3 + z3; - Diagnostic(ErrorCode.WRN_NullReferenceArgument, "y3").WithArguments("y", "CL0 CL0.operator +(string? x, CL0 y)").WithLocation(29, 23), - // (34,18): warning CS8604: Possible null reference argument for parameter 'x' in 'CL2 CL2.operator +(CL1 x, CL2 y)'. - // CL2 u4 = x4 + y4 + z4; - Diagnostic(ErrorCode.WRN_NullReferenceArgument, "x4 + y4").WithArguments("x", "CL2 CL2.operator +(CL1 x, CL2 y)").WithLocation(34, 18) + // (19,23): warning CS8604: Possible null reference argument for parameter 'y' in 'CL1? CL1.operator +(string x, CL1? y)'. + // CL1 z2 = x2 + y2; // 1, 2 + Diagnostic(ErrorCode.WRN_NullReferenceArgument, "y2").WithArguments("y", "CL1? CL1.operator +(string x, CL1? y)").WithLocation(19, 23) ); } @@ -68850,7 +69176,7 @@ static void F(S x, S? y) comp.VerifyDiagnostics(); } - [Fact] + [Fact, WorkItem(32671, "https://github.com/dotnet/roslyn/issues/32671")] public void BinaryOperator_15_WithDisallowNull() { var source = @" @@ -68868,9 +69194,14 @@ static void F(S? x, S? y) s = y + y; // 2 } }"; - // Analyze operator call properly (honoring [Disallow|Allow|Maybe|NotNull] attribute annotations) https://github.com/dotnet/roslyn/issues/32671 var comp = CreateCompilation(new[] { source, DisallowNullAttributeDefinition }, options: WithNullableEnable()); - comp.VerifyDiagnostics(); + comp.VerifyDiagnostics( + // (11,17): warning CS8607: A possible null value may not be used for a type marked with [NotNull] or [DisallowNull] + // s = x + y; // 1 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "y").WithLocation(11, 17), + // (13,17): warning CS8607: A possible null value may not be used for a type marked with [NotNull] or [DisallowNull] + // s = y + y; // 2 + Diagnostic(ErrorCode.WRN_DisallowNullAttributeForbidsMaybeNullAssignment, "y").WithLocation(13, 17)); } [Fact]