diff --git a/docs/features/nullable-reference-types.md b/docs/features/nullable-reference-types.md index 61e094383145e..7c55d083a78a6 100644 --- a/docs/features/nullable-reference-types.md +++ b/docs/features/nullable-reference-types.md @@ -75,9 +75,17 @@ Flow analysis is used to infer the nullability of variables within executable co ### Warnings _Describe set of warnings. Differentiate W warnings._ +If the analysis determines that a null check always (or never) passes, a hidden diagnostic is produced. For example: `"string" is null`. ### Null tests -_Describe the set of tests that affect flow state._ +A number of null checks affect the flow state when tested for: +- comparisons to `null`: `x == null` and `x != null` +- `is` operator: `x is null`, `x is K` (where `K` is a constant), `x is string`, `x is string s` + +Invocation of methods annotated with the following attributes will also affect flow analysis: +- `[NotNullWhenTrue]` (e.g. `TryGetValue`) and `[NotNullWhenFalse]` (e.g. `string.IsNullOrEmpty`) +- `[EnsuresNotNull]` (e.g. `ThrowIfNull`) +- `[AssertsTrue]` (e.g. `Debug.Assert`) and `[AssertsFalse]` ## `default` `default(T)` is `T?` if `T` is a reference type. diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs index 238bf2cabb646..1962f594735e8 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs @@ -766,69 +766,83 @@ private void EnterParameter(ParameterSymbol parameter, TypeSymbolWithAnnotations public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node) { - // PROTOTYPE(NullableReferenceTypes): Move these asserts to base class. - Debug.Assert(!IsConditionalState); - - // Create slot when the state is unconditional since EnsureCapacity should be - // called on all fields and that is simpler if state is limited to this.State. - int slot = -1; - if (this.State.Reachable) - { - var pattern = node.Pattern; - // PROTOTYPE(NullableReferenceTypes): Handle patterns that ensure x is not null: - // x is T y // where T is not inferred via var - // x is K // where K is a constant other than null - if (pattern.Kind == BoundKind.ConstantPattern && ((BoundConstantPattern)pattern).ConstantValue?.IsNull == true) - { - slot = MakeSlot(node.Expression); - if (slot > 0) - { - Normalize(ref this.State); - } - } - } + VisitRvalue(node.Expression); + var expressionResultType = this._resultType; - var result = base.VisitIsPatternExpression(node); - - Debug.Assert(IsConditionalState); - if (slot > 0) - { - this.StateWhenTrue[slot] = false; - this.StateWhenFalse[slot] = true; - } + VisitPattern(node.Expression, expressionResultType, node.Pattern); SetResult(node); - return result; + return node; } - public override void VisitPattern(BoundExpression expression, BoundPattern pattern) - { - base.VisitPattern(expression, pattern); - var whenFail = StateWhenFalse; - SetState(StateWhenTrue); - AssignPatternVariables(pattern); - SetConditionalState(this.State, whenFail); - SetUnknownResultNullability(); - } - - private void AssignPatternVariables(BoundPattern pattern) + /// + /// Examples: + /// `x is Point p` + /// `switch (x) ... case Point p:` // PROTOTYPE(NullableReferenceTypes): not yet handled + /// + /// If the expression is trackable, we'll return with different null-states for that expression in the two conditional states. + /// If the pattern is a `var` pattern, we'll also have re-inferred the `var` type with nullability and + /// updated the state for that declared local. + /// + private void VisitPattern(BoundExpression expression, TypeSymbolWithAnnotations expressionResultType, BoundPattern pattern) { + bool? isNull = null; // the pattern tells us the expression (1) is null, (2) isn't null, or (3) we don't know. switch (pattern.Kind) { - case BoundKind.DeclarationPattern: - // PROTOTYPE(NullableReferenceTypes): Handle. - break; - case BoundKind.WildcardPattern: - break; case BoundKind.ConstantPattern: + // If the constant is null, the pattern tells us the expression is null. + // If the constant is not null, the pattern tells us the expression is not null. + // If there is no constant, we don't know. + isNull = ((BoundConstantPattern)pattern).ConstantValue?.IsNull; + break; + case BoundKind.DeclarationPattern: + var declarationPattern = (BoundDeclarationPattern)pattern; + if (declarationPattern.IsVar) { - var pat = (BoundConstantPattern)pattern; - this.VisitRvalue(pat.Value); - break; + // The result type and state of the expression carry into the variable declared by var pattern + Symbol variable = declarationPattern.Variable; + // No variable declared for discard (`i is var _`) + if ((object)variable != null) + { + _variableTypes[variable] = expressionResultType; + TrackNullableStateForAssignment(expression, expressionResultType, GetOrCreateSlot(variable), expressionResultType); + } + } + else + { + isNull = false; // the pattern tells us the expression is not null } - default: break; } + + Debug.Assert(!IsConditionalState); + int slot = -1; + if (isNull != null) + { + // Create slot when the state is unconditional since EnsureCapacity should be + // called on all fields and that is simpler if state is limited to this.State. + slot = MakeSlot(expression); + if (slot > 0) + { + Normalize(ref this.State); + } + } + + base.VisitPattern(expression, pattern); + Debug.Assert(IsConditionalState); + + // PROTOTYPE(NullableReferenceTypes): We should only report such + // diagnostics for locals that are set or checked explicitly within this method. + if (expressionResultType?.IsNullable == false && isNull == true) + { + ReportStaticNullCheckingDiagnostics(ErrorCode.HDN_NullCheckIsProbablyAlwaysFalse, pattern.Syntax); + } + + if (slot > 0 && isNull != null) + { + this.StateWhenTrue[slot] = !isNull; + this.StateWhenFalse[slot] = isNull; + } } protected override BoundNode VisitReturnStatementNoAdjust(BoundReturnStatement node) @@ -3663,8 +3677,29 @@ public override BoundNode VisitDefaultExpression(BoundDefaultExpression node) public override BoundNode VisitIsOperator(BoundIsOperator node) { + Debug.Assert(!this.IsConditionalState); + + int slot = -1; + var operand = node.Operand; + if (operand.Type?.IsValueType == false) + { + slot = MakeSlot(operand); + if (slot > 0) + { + Normalize(ref this.State); + } + } + var result = base.VisitIsOperator(node); Debug.Assert(node.Type.SpecialType == SpecialType.System_Boolean); + + if (slot > 0) + { + Split(); + this.StateWhenTrue[slot] = true; + this.StateWhenFalse[slot] = false; + } + SetResult(node); return result; } @@ -4185,6 +4220,11 @@ internal void EnsureCapacity(int capacity) _notNull.EnsureCapacity(capacity); } + /// + /// Use `true` value for not-null. + /// Use `false` value for maybe-null. + /// Use `null` value for unknown. + /// internal bool? this[int slot] { get diff --git a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs index 2711848153486..cb05df44a796a 100644 --- a/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs +++ b/src/Compilers/CSharp/Portable/FlowAnalysis/PreciseAbstractFlowPass.cs @@ -950,9 +950,10 @@ public override BoundNode VisitPassByCopy(BoundPassByCopy node) public override BoundNode VisitIsPatternExpression(BoundIsPatternExpression node) { + Debug.Assert(!IsConditionalState); VisitRvalue(node.Expression); VisitPattern(node.Expression, node.Pattern); - return null; + return node; } public virtual void VisitPattern(BoundExpression expression, BoundPattern pattern) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs index f29b054c6bccf..35c940532964c 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs @@ -944,6 +944,26 @@ public NonNullTypesAttribute(bool flag = true) { } comp.VerifyDiagnostics(); } + [Fact(Skip = "PROTOTYPE(NullableReferenceTypes): stack overflow")] + public void NonNullTypes_DoNotWarnInsideAttributes() + { + string source = @" +using System.Runtime.CompilerServices; +[NonNullTypes(true)] +class C +{ + [System.Obsolete(D.M1(D.M2()))] + class D + { + static void M1(string x) => throw null; + static string? M2() => throw null; + } +} +"; + var comp = CreateCompilation(source + NonNullTypesAttributesDefinition, parseOptions: TestOptions.Regular8); + comp.VerifyDiagnostics(); + } + [Fact] public void NonNullTypes_AttributeDefinedMultipleTimes() { @@ -11715,6 +11735,498 @@ void Test1(object x1, object? y1) ); } + [Fact] + public void ConditionalBranching_Is_ReferenceType() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x is C) + { + x.ToString(); + } + else + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(12, 13) + ); + } + + [Fact] + public void ConditionalBranching_Is_GenericType() + { + CSharpCompilation c = CreateCompilation(@" +class Base { } +class C : Base +{ + void Test(C? x) where T : Base + { + if (x is T) + { + x.ToString(); + } + else + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (13,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(13, 13) + ); + } + + [Fact] + public void ConditionalBranching_Is_UnconstrainedGenericType() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + static void F(object? o) + { + if (o is T) + { + o.ToString(); + } + else + { + o.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // o.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "o").WithLocation(12, 13) + ); + } + + [Fact] + public void ConditionalBranching_Is_StructConstrainedGenericType() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + static void F(object? o) where T : struct + { + if (o is T) + { + o.ToString(); + } + else + { + o.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // o.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "o").WithLocation(12, 13) + ); + } + + [Fact] + public void ConditionalBranching_Is_ClassConstrainedGenericType() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + static void F(object? o) where T : class + { + if (o is T) + { + o.ToString(); + } + else + { + o.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // o.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "o").WithLocation(12, 13) + ); + } + + [Fact] + public void ConditionalBranching_Is_UnconstrainedGenericOperand() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + static void F(T t, object? o) + { + if (t is string) t.ToString(); + if (t is string s) { t.ToString(); s.ToString(); } + if (t != null) t.ToString(); + } +} +", parseOptions: TestOptions.Regular8); + + // PROTOTYPE(NullableReferenceTypes): even unconstrained generic types can be known to be non-null + c.VerifyDiagnostics( + // (6,26): warning CS8602: Possible dereference of a null reference. + // if (t is string) t.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t").WithLocation(6, 26), + // (7,30): warning CS8602: Possible dereference of a null reference. + // if (t is string s) { t.ToString(); s.ToString(); } + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t").WithLocation(7, 30), + // (8,24): warning CS8602: Possible dereference of a null reference. + // if (t != null) t.ToString(); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "t").WithLocation(8, 24) + ); + } + + [Fact] + public void ConditionalBranching_Is_NullOperand() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + static void F(object? o) + { + if (null is string) return; + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (6,13): warning CS0184: The given expression is never of the provided ('string') type + // if (null is string) return; + Diagnostic(ErrorCode.WRN_IsAlwaysFalse, "null is string").WithArguments("string").WithLocation(6, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_Null() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x is null) + { + x.ToString(); // warn + } + else + { + x.ToString(); + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (8,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(8, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_NullInverted() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (!(x is null)) + { + x.ToString(); + } + else + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (12,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(12, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_NonNull() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + const string nonNullConstant = ""hello""; + if (x is nonNullConstant) + { + x.ToString(); + } + else + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (13,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(13, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_NullConstant() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + const string? nullConstant = null; + if (x is nullConstant) + { + x.ToString(); // warn + } + else + { + x.ToString(); + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (9,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(9, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_NonConstant() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + string nonConstant = ""hello""; + if (x is nonConstant) + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (7,18): error CS0150: A constant value is expected + // if (x is nonConstant) + Diagnostic(ErrorCode.ERR_ConstantExpected, "nonConstant").WithLocation(7, 18), + // (9,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(9, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_Null_AlreadyTestedAsNonNull() + { + // PROTOTYPE(NullableReferenceTypes): confirm that we want such hidden warnings + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x != null) + { + if (x is null) // hidden + { + x.ToString(); // warn + } + else + { + x.ToString(); + } + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (8,22): hidden CS8606: Result of the comparison is possibly always false. + // if (x is null) // hidden + Diagnostic(ErrorCode.HDN_NullCheckIsProbablyAlwaysFalse, "null").WithLocation(8, 22), + // (10,17): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(10, 17) + ); + } + + [Fact] + public void ConditionalBranching_IsConstantPattern_Null_AlreadyTestedAsNull() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x == null) + { + if (x is null) + { + x.ToString(); // warn + } + else + { + x.ToString(); + } + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (10,17): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(10, 17) + ); + } + + [Fact] + public void ConditionalBranching_IsDeclarationPattern() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x is C c) + { + x.ToString(); + c.ToString(); + } + else + { + x.ToString(); // warn + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyDiagnostics( + // (13,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(13, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsVarDeclarationPattern() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x is var c) + { + x.ToString(); // warn 1 + c /*T:object?*/ .ToString(); // warn 2 + } + else + { + x.ToString(); // warn 3 + } + + if (x is var _) + { + x.ToString(); // warn 4 + } + else + { + x.ToString(); // warn 5 + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyTypes(); + c.VerifyDiagnostics( + // (8,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn 1 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(8, 13), + // (9,13): warning CS8602: Possible dereference of a null reference. + // c /*T:object?*/ .ToString(); // warn 2 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "c").WithLocation(9, 13), + // (13,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn 3 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(13, 13), + // (18,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn 4 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(18, 13), + // (22,13): warning CS8602: Possible dereference of a null reference. + // x.ToString(); // warn 5 + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "x").WithLocation(22, 13) + ); + } + + [Fact] + public void ConditionalBranching_IsVarDeclarationPattern_AlreadyTestedAsNonNull() + { + CSharpCompilation c = CreateCompilation(@" +class C +{ + void Test(object? x) + { + if (x != null) + { + if (x is var c) + { + c /*T:object!*/ .ToString(); + c = null; // warn + } + } + } +} +", parseOptions: TestOptions.Regular8); + + c.VerifyTypes(); + c.VerifyDiagnostics( + // (11,21): warning CS8600: Converting null literal or possible null value to non-nullable type. + // c = null; // warn + Diagnostic(ErrorCode.WRN_ConvertingNullableToNonNullable, "null").WithLocation(11, 21) + ); + } + [Fact] public void ConditionalOperator_01() {