Flow analysis of with-expressions

src/Compilers/CSharp/Portable/Binder/Binder_WithExpression.cs

@@ -94,7 +94,10 @@ private BoundExpression BindWithExpression(WithExpressionSyntax syntax, Diagnost
                 }
             }
 
-            var cloneReturnType = cloneMethod?.ReturnType;
+            // Even if a clone method is not present, we should come up with
+            // a type to lookup members of the with-expression in for error recovery
+            var cloneReturnType = cloneMethod?.ReturnType ?? receiverType;
+            RoslynDebug.AssertNotNull(cloneReturnType);
 
             var args = ArrayBuilder<(Symbol?, BoundExpression)>.GetInstance();
             // Bind with expression arguments

src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass.cs

@@ -2026,7 +2026,11 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node)
 
         public override BoundNode VisitWithExpression(BoundWithExpression node)
         {
-            // PROTOTYPE: This is wrong
+            VisitRvalue(node.Receiver);
+            foreach (var (_, expr) in node.Arguments)
+            {
+                VisitRvalue(expr);
+            }
             return null;
         }
 

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.DebugVerifier.cs

@@ -187,6 +187,20 @@ private void VisitBinaryOperatorChildren(BoundBinaryOperatorBase node)
                 Visit(node.BoundContainingTypeOpt);
                 return null;
             }
+
+            public override BoundNode? VisitWithExpression(BoundWithExpression node)
+            {
+                Visit(node.Receiver);
+                // PROTOTYPE: We shouldn't need to implement this method manually here.
+                // Either BoundNodeClassWriter should be made aware of tuples,
+                // or the `BoundWithExpression.Arguments` should be changed to
+                // `ImmutableArray<BoundExpression>` as in `BoundObjectInitializerExpressionBase.Initializers`.
+                foreach (var (_, expr) in node.Arguments)
+                {
+                    Visit(expr);
+                }
+                return null;
+            }
         }
 #endif
     }

src/Compilers/CSharp/Portable/FlowAnalysis/NullableWalker.cs

@@ -2222,12 +2222,44 @@ public override BoundNode VisitWhileStatement(BoundWhileStatement node)
             return base.VisitWhileStatement(node);
         }
 
-        public override BoundNode VisitWithExpression(BoundWithExpression expr)
+        public override BoundNode VisitWithExpression(BoundWithExpression withExpr)
         {
-            // PROTOTYPE: This is wrong
-            SetResultType(expr,
-                expr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState()
-                    ?? TypeWithState.Create(expr.Type, NullableFlowState.NotNull));
+            Debug.Assert(!IsConditionalState);
+
+            var receiver = withExpr.Receiver;
+            VisitRvalue(receiver);
+            _ = CheckPossibleNullReceiver(receiver, checkNullableValueType: false);
+
+            var resultSlot = GetOrCreatePlaceholderSlot(withExpr);
+            // carry over the null state of members of 'receiver' to the result of the with-expression.
+            TrackNullableStateForAssignment(receiver, ResultType.ToTypeWithAnnotations(), resultSlot, ResultType, MakeSlot(receiver));
+
+            foreach (var (leftSymbol, right) in withExpr.Arguments)
+            {
+                if (leftSymbol is PropertySymbol prop)
+                {
+                    // PROTOTYPE: attributes on positional record parameters do not propagate
+                    // to the synthesized properties. Perhaps this is by design?
+                    // If we want them to propagate then perhaps we should move
+                    // members such as SourcePropertySymbol.HasAllowNull, etc. to SourceOrRecordPropertySymbol.
+                    var leftAnnotations = GetPropertySetterAnnotations(prop);
+                    var leftLValueType = ApplyLValueAnnotations(prop.TypeWithAnnotations, leftAnnotations);
+
+                    var rightState = VisitOptionalImplicitConversion(right, targetTypeOpt: leftLValueType, useLegacyWarnings: false, trackMembers: true, AssignmentKind.Assignment);
+                    CheckDisallowedNullAssignment(rightState, leftAnnotations, right.Syntax.Location);
+                    var propSlot = GetOrCreateSlot(prop, resultSlot);
+                    TrackNullableStateForAssignment(right, leftLValueType, propSlot, rightState, MakeSlot(right));
+
+                    // PROTOTYPE: Should we AdjustSetValue for properties?
+                    // e.g. a property with [AllowNull, NotNull] would have a NotNull state after assigning a maybe-null.
+                    // This isn't safe at runtime but it is consistent with what we allow users to do with auto properties.
+                    // AdjustSetValue(left, declaredType, leftLValueType, ref rightState);
+                }
+            }
+
+            SetResultType(withExpr,
+                withExpr.CloneMethod?.ReturnTypeWithAnnotations.ToTypeWithState()
+                    ?? TypeWithState.Create(withExpr.Type, NullableFlowState.NotNull));
             return null;
         }
 
@@ -6981,6 +7013,34 @@ FlowAnalysisAnnotations getRValueAnnotations(BoundExpression expr)
             }
         }
 
+        private static FlowAnalysisAnnotations GetPropertySetterAnnotations(PropertySymbol property)
+        {
+            var accessor = property.GetOwnOrInheritedSetMethod();
+            if (accessor is object)
+            {
+                return accessor.Parameters.Last().FlowAnalysisAnnotations;
+            }
+            if (property is SourcePropertySymbol sourceProperty)
+            {
+                return getPropertyAnnotations(sourceProperty);
+            }
+            return FlowAnalysisAnnotations.None;
+
+            static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property)
+            {
+                var annotations = FlowAnalysisAnnotations.None;
+                if (property.HasAllowNull)
+                {
+                    annotations |= FlowAnalysisAnnotations.AllowNull;
+                }
+                if (property.HasDisallowNull)
+                {
+                    annotations |= FlowAnalysisAnnotations.DisallowNull;
+                }
+                return annotations;
+            }
+        }
+
         private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr)
         {
             // Annotations are ignored when binding an attribute to avoid cycles. (Members used
@@ -6992,10 +7052,10 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr)
 
             var annotations = expr switch
             {
-                BoundPropertyAccess property => getSetterAnnotations(property.PropertySymbol),
-                BoundIndexerAccess indexer => getSetterAnnotations(indexer.Indexer),
+                BoundPropertyAccess property => GetPropertySetterAnnotations(property.PropertySymbol),
+                BoundIndexerAccess indexer => GetPropertySetterAnnotations(indexer.Indexer),
                 BoundFieldAccess field => getFieldAnnotations(field.FieldSymbol),
-                BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => getSetterAnnotations(prop),
+                BoundObjectInitializerMember { MemberSymbol: PropertySymbol prop } => GetPropertySetterAnnotations(prop),
                 BoundObjectInitializerMember { MemberSymbol: FieldSymbol field } => getFieldAnnotations(field),
                 BoundParameter { ParameterSymbol: ParameterSymbol parameter }
                     => ToInwardAnnotations(GetParameterAnnotations(parameter) & ~FlowAnalysisAnnotations.NotNull), // NotNull is enforced upon method exit
@@ -7007,37 +7067,9 @@ private FlowAnalysisAnnotations GetLValueAnnotations(BoundExpression expr)
             static FlowAnalysisAnnotations getFieldAnnotations(FieldSymbol field)
             {
                 return field.AssociatedSymbol is PropertySymbol property ?
-                    getSetterAnnotations(property) :
+                    GetPropertySetterAnnotations(property) :
                     field.FlowAnalysisAnnotations;
             }
-
-            static FlowAnalysisAnnotations getSetterAnnotations(PropertySymbol property)
-            {
-                var accessor = property.GetOwnOrInheritedSetMethod();
-                if (accessor is object)
-                {
-                    return accessor.Parameters.Last().FlowAnalysisAnnotations;
-                }
-                if (property is SourcePropertySymbol sourceProperty)
-                {
-                    return getPropertyAnnotations(sourceProperty);
-                }
-                return FlowAnalysisAnnotations.None;
-            }
-
-            static FlowAnalysisAnnotations getPropertyAnnotations(SourcePropertySymbol property)
-            {
-                var annotations = FlowAnalysisAnnotations.None;
-                if (property.HasAllowNull)
-                {
-                    annotations |= FlowAnalysisAnnotations.AllowNull;
-                }
-                if (property.HasDisallowNull)
-                {
-                    annotations |= FlowAnalysisAnnotations.DisallowNull;
-                }
-                return annotations;
-            }
         }
 
         private static FlowAnalysisAnnotations ToInwardAnnotations(FlowAnalysisAnnotations outwardAnnotations)