is operator informs nullability analysis (#28686)

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.

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)
+        /// <summary>
+        /// 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.
+        /// </summary>
+        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);
             }
 
+            /// <summary>
+            /// Use `true` value for not-null.
+            /// Use `false` value for maybe-null.
+            /// Use `null` value for unknown.
+            /// </summary>
             internal bool? this[int slot]
             {
                 get

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)