Fix nested nullability of collection expressions (#74490)

Author: RikkiGibson

src/Compilers/CSharp/Portable/Binder/Semantics/BestTypeInferrer.cs

@@ -69,6 +69,7 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
             HashSet<TypeSymbol> candidateTypes = new HashSet<TypeSymbol>(comparer);
             foreach (BoundExpression expr in exprs)
             {
+                Debug.Assert(!NullableWalker.IsTargetTypedExpression(expr));
                 TypeSymbol? type = expr.GetTypeOrFunctionType();
 
                 if (type is { })
@@ -132,9 +133,9 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
             try
             {
                 var conversionsWithoutNullability = conversions.WithNullability(false);
-                TypeSymbol? type1 = expr1.Type;
 
-                if (type1 is { })
+                Debug.Assert(!NullableWalker.IsTargetTypedExpression(expr1));
+                if (expr1.Type is { } type1)
                 {
                     if (type1.IsErrorType())
                     {
@@ -148,9 +149,8 @@ public static NullableFlowState GetNullableState(ArrayBuilder<TypeWithState> typ
                     }
                 }
 
-                TypeSymbol? type2 = expr2.Type;
-
-                if (type2 is { })
+                Debug.Assert(!NullableWalker.IsTargetTypedExpression(expr2));
+                if (expr2.Type is { } type2)
                 {
                     if (type2.IsErrorType())
                     {

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

@@ -3702,7 +3702,7 @@ TypeWithState convertCollection(BoundCollectionExpression node, TypeWithAnnotati
 
                 // Record the final state
                 NullableFlowState resultState = getResultState(node, collectionKind);
-                var resultTypeWithState = TypeWithState.Create(node.Type, resultState);
+                var resultTypeWithState = TypeWithState.Create(strippedTargetCollectionType, resultState);
                 SetAnalyzedNullability(node, resultTypeWithState);
                 return resultTypeWithState;
             }
@@ -4570,7 +4570,7 @@ static TypeSymbol setSpanElementType(NamedTypeSymbol namedType, TypeWithAnnotati
         /// This is done using <see cref="TargetTypedAnalysisCompletion"/>. All registered completions must be processed
         /// (ie. analyzed via some conversion) before the nullable analysis completes.
         /// </summary>
-        private static bool IsTargetTypedExpression(BoundExpression node)
+        internal static bool IsTargetTypedExpression(BoundExpression node)
         {
             return node is BoundConditionalOperator { WasTargetTyped: true } or
                            BoundConvertedSwitchExpression { WasTargetTyped: true } or
@@ -5817,6 +5817,14 @@ void makeAndAdjustReceiverSlot(BoundExpression receiver)
             {
                 resultType = null;
             }
+            else if (IsTargetTypedExpression(consequence))
+            {
+                resultType = alternativeRValue.Type;
+            }
+            else if (IsTargetTypedExpression(alternative))
+            {
+                resultType = consequenceRValue.Type;
+            }
             else
             {
                 // Determine nested nullability using BestTypeInferrer.
@@ -9305,14 +9313,17 @@ private void TrackAnalyzedNullabilityThroughConversionGroup(TypeWithState result
             while (conversionOpt != null && conversionOpt != convertedNode)
             {
                 Debug.Assert(conversionOpt.ConversionGroupOpt == conversionGroup);
-                visitResult = withType(visitResult, conversionOpt.Type);
+
+                // https://github.com/dotnet/roslyn/issues/35046
+                // SetAnalyzedNullability will drop the type if the visitResult.RValueType.Type differs from conversionOpt.Type.
+                // (It will use the top-level nullability from visitResult, though.)
+                //
+                // Here, the visitResult represents the result of visiting the operand.
+                // Ideally, we would use the visitResult to reinfer the types of the containing conversions, and store those results here.
                 SetAnalyzedNullability(conversionOpt, visitResult);
+
                 conversionOpt = conversionOpt.Operand as BoundConversion;
             }
-
-            static VisitResult withType(VisitResult visitResult, TypeSymbol newType) =>
-                new VisitResult(TypeWithState.Create(newType, visitResult.RValueType.State),
-                                TypeWithAnnotations.Create(newType, visitResult.LValueType.NullableAnnotation));
         }
 
         /// <summary>
@@ -11282,8 +11293,6 @@ private TypeWithState InferResultNullabilityOfBinaryLogicalOperator(BoundExpress
                 }
             }
 
-            // https://github.com/dotnet/roslyn/issues/33344: this fails to produce an updated tuple type for a default expression
-            // (should produce nullable element types for those elements that are of reference types)
             SetResultType(node, TypeWithState.ForType(type));
             return result;
         }

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

@@ -898,8 +898,11 @@ private void VisitSwitchExpressionCore(BoundSwitchExpression node, bool inferTyp
                 resultTypes.Add(armType);
                 Join(ref endState, ref this.State);
 
-                // Build placeholders for inference in order to preserve annotations.
-                placeholderBuilder.Add(CreatePlaceholderIfNecessary(expression, armType.ToTypeWithAnnotations(compilation)));
+                if (!IsTargetTypedExpression(expression))
+                {
+                    // Build placeholders for inference in order to preserve annotations.
+                    placeholderBuilder.Add(CreatePlaceholderIfNecessary(expression, armType.ToTypeWithAnnotations(compilation)));
+                }
             }
 
             SetState(endState);