Fix assignment analysis of ref fields (#75340)

* Fix assignment analysis of ref fields

* Avoid propagating ref assignment

* Set `isRef` to more future-proof values in more places

* Use separate warning

* Document `isRef` parameter

docs/compilers/CSharp/Warnversion Warning Waves.md

@@ -13,6 +13,14 @@ In a typical project, this setting is controlled by the `AnalysisLevel` property
 which determines the `WarningLevel` property (passed to the `Csc` task).
 For more information on `AnalysisLevel`, see https://devblogs.microsoft.com/dotnet/automatically-find-latent-bugs-in-your-code-with-net-5/
 
+## Warning level 10
+
+The compiler shipped with .NET 10 (the C# 14 compiler) contains the following warnings which are reported only under `/warn:10` or higher.
+
+| Warning ID | Description |
+|------------|-------------|
+| CS9265 | [Field is never ref-assigned to, and will always have its default value (null reference)](https://github.com/dotnet/roslyn/issues/75315) |
+
 ## Warning level 8
 
 The compiler shipped with .NET 8 (the C# 12 compiler) contains the following warnings which are reported only under `/warn:8` or higher.

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -1916,6 +1916,12 @@ If such a class is used as a base class and if the deriving class defines a dest
   <data name="WRN_UnassignedInternalField_Title" xml:space="preserve">
     <value>Field is never assigned to, and will always have its default value</value>
   </data>
+  <data name="WRN_UnassignedInternalRefField" xml:space="preserve">
+    <value>Field '{0}' is never ref-assigned to, and will always have its default value (null reference)</value>
+  </data>
+  <data name="WRN_UnassignedInternalRefField_Title" xml:space="preserve">
+    <value>Field is never ref-assigned to, and will always have its default value (null reference)</value>
+  </data>
   <data name="ERR_CStyleArray" xml:space="preserve">
     <value>Bad array declarator: To declare a managed array the rank specifier precedes the variable's identifier. To declare a fixed size buffer field, use the fixed keyword before the field type.</value>
   </data>

src/Compilers/CSharp/Portable/Errors/ErrorCode.cs

@@ -2346,6 +2346,7 @@ internal enum ErrorCode
         ERR_PartialPropertyDuplicateInitializer = 9263,
 
         WRN_UninitializedNonNullableBackingField = 9264,
+        WRN_UnassignedInternalRefField = 9265,
 
         // Note: you will need to do the following after adding errors:
         //  1) Update ErrorFacts.IsBuildOnlyDiagnostic (src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs)

src/Compilers/CSharp/Portable/Errors/ErrorFacts.cs

@@ -210,6 +210,10 @@ internal static int GetWarningLevel(ErrorCode code)
             // docs/compilers/CSharp/Warnversion Warning Waves.md
             switch (code)
             {
+                case ErrorCode.WRN_UnassignedInternalRefField:
+                    // Warning level 10 is exclusively for warnings introduced in the compiler
+                    // shipped with dotnet 10 (C# 14) and that can be reported for pre-existing code.
+                    return 10;
                 case ErrorCode.WRN_AddressOfInAsync:
                 case ErrorCode.WRN_ByValArraySizeConstRequired:
                     // Warning level 8 is exclusively for warnings introduced in the compiler
@@ -2459,6 +2463,7 @@ or ErrorCode.ERR_CannotApplyOverloadResolutionPriorityToOverride
                 or ErrorCode.ERR_CannotApplyOverloadResolutionPriorityToMember
                 or ErrorCode.ERR_PartialPropertyDuplicateInitializer
                 or ErrorCode.WRN_UninitializedNonNullableBackingField
+                or ErrorCode.WRN_UnassignedInternalRefField
                     => false,
             };
 #pragma warning restore CS8524 // The switch expression does not handle some values of its input type (it is not exhaustive) involving an unnamed enum value.

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

@@ -1545,10 +1545,20 @@ private void VisitArgumentsAfterCall(ImmutableArray<BoundExpression> arguments,
             for (int i = 0; i < arguments.Length; i++)
             {
                 RefKind refKind = GetRefKind(refKindsOpt, i);
-                // passing as a byref argument is also a potential write
-                if (refKind != RefKind.None)
+                switch (refKind)
                 {
-                    WriteArgument(arguments[i], refKind, method);
+                    case RefKind.None:
+                    case RefKind.In:
+                    case RefKind.RefReadOnlyParameter:
+                    case RefKindExtensions.StrictIn:
+                        break;
+                    case RefKind.Ref:
+                    case RefKind.Out:
+                        // passing as a byref argument is also a potential write
+                        WriteArgument(arguments[i], refKind, method);
+                        break;
+                    default:
+                        throw ExceptionUtilities.UnexpectedValue(refKind);
                 }
             }
         }

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

@@ -88,7 +88,7 @@ protected override void EnterRegion()
             base.EnterRegion();
         }
 
-        protected override void NoteWrite(Symbol variable, BoundExpression value, bool read)
+        protected override void NoteWrite(Symbol variable, BoundExpression value, bool read, bool isRef)
         {
             // any reachable assignment to a ref or out parameter can be visible to the caller in the face of exceptions.
             if (this.State.Reachable && IsInside)
@@ -107,7 +107,7 @@ protected override void NoteWrite(Symbol variable, BoundExpression value, bool r
 #endif
             }
 
-            base.NoteWrite(variable, value, read);
+            base.NoteWrite(variable, value, read: read, isRef: isRef);
         }
 
 #if DEBUG

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

@@ -346,7 +346,7 @@ protected override ImmutableArray<PendingBranch> Scan(ref bool badRegion)
                         // All primary constructor parameters are definitely assigned outside of the primary constructor
                         foreach (var parameter in primaryConstructor.Parameters)
                         {
-                            NoteWrite(parameter, value: null, read: true);
+                            NoteWrite(parameter, value: null, read: true, isRef: parameter.RefKind != RefKind.None);
                         }
 
                         CurrentSymbol = save;
@@ -846,15 +846,17 @@ private void NoteRead(BoundNode fieldOrEventAccess)
             }
         }
 
-        protected virtual void NoteWrite(Symbol variable, BoundExpression value, bool read)
+        protected virtual void NoteWrite(Symbol variable, BoundExpression value, bool read, bool isRef)
         {
             if ((object)variable != null)
             {
                 _writtenVariables.Add(variable);
                 if ((object)_sourceAssembly != null && variable.Kind == SymbolKind.Field)
                 {
                     var field = (FieldSymbol)variable.OriginalDefinition;
-                    _sourceAssembly.NoteFieldAccess(field, read: read && WriteConsideredUse(field.Type, value), write: true);
+                    _sourceAssembly.NoteFieldAccess(field,
+                        read: read && WriteConsideredUse(field.Type, value),
+                        write: field.RefKind == RefKind.None || isRef);
                 }
 
                 var local = variable as LocalSymbol;
@@ -949,7 +951,10 @@ internal static bool WriteConsideredUse(TypeSymbol type, BoundExpression value)
             }
         }
 
-        private void NoteWrite(BoundExpression n, BoundExpression value, bool read)
+        /// <param name="isRef">
+        /// Whether this write represents a ref-assignment.
+        /// </param>
+        private void NoteWrite(BoundExpression n, BoundExpression value, bool read, bool isRef)
         {
             while (n != null)
             {
@@ -961,12 +966,15 @@ private void NoteWrite(BoundExpression n, BoundExpression value, bool read)
                             if ((object)_sourceAssembly != null)
                             {
                                 var field = fieldAccess.FieldSymbol.OriginalDefinition;
-                                _sourceAssembly.NoteFieldAccess(field, read: value == null || WriteConsideredUse(fieldAccess.FieldSymbol.Type, value), write: true);
+                                _sourceAssembly.NoteFieldAccess(field,
+                                    read: value == null || WriteConsideredUse(fieldAccess.FieldSymbol.Type, value),
+                                    write: field.RefKind == RefKind.None || isRef);
                             }
 
                             if (MayRequireTracking(fieldAccess.ReceiverOpt, fieldAccess.FieldSymbol))
                             {
                                 n = fieldAccess.ReceiverOpt;
+                                isRef = false;
                                 if (n.Kind == BoundKind.Local)
                                 {
                                     _usedVariables.Add(((BoundLocal)n).LocalSymbol);
@@ -1001,19 +1009,19 @@ private void NoteWrite(BoundExpression n, BoundExpression value, bool read)
                         }
 
                     case BoundKind.ThisReference:
-                        NoteWrite(MethodThisParameter, value, read);
+                        NoteWrite(MethodThisParameter, value, read: read, isRef: isRef);
                         return;
 
                     case BoundKind.Local:
-                        NoteWrite(((BoundLocal)n).LocalSymbol, value, read);
+                        NoteWrite(((BoundLocal)n).LocalSymbol, value, read: read, isRef: isRef);
                         return;
 
                     case BoundKind.Parameter:
-                        NoteWrite(((BoundParameter)n).ParameterSymbol, value, read);
+                        NoteWrite(((BoundParameter)n).ParameterSymbol, value, read: read, isRef: isRef);
                         return;
 
                     case BoundKind.RangeVariable:
-                        NoteWrite(((BoundRangeVariable)n).Value, value, read);
+                        NoteWrite(((BoundRangeVariable)n).Value, value, read: read, isRef: isRef);
                         return;
 
                     case BoundKind.InlineArrayAccess:
@@ -1542,7 +1550,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
                             SetSlotState(slot, assigned: written || !this.State.Reachable);
                         }
 
-                        if (written) NoteWrite(pattern.VariableAccess, value, read);
+                        if (written) NoteWrite(pattern.VariableAccess, value, read: read, isRef: isRef);
                         break;
                     }
 
@@ -1553,7 +1561,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
                         LocalSymbol symbol = local.LocalSymbol;
                         int slot = GetOrCreateSlot(symbol);
                         SetSlotState(slot, assigned: written || !this.State.Reachable);
-                        if (written) NoteWrite(symbol, value, read);
+                        if (written) NoteWrite(symbol, value, read: read, isRef: isRef);
                         break;
                     }
 
@@ -1570,7 +1578,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
                         {
                             int slot = MakeSlot(local);
                             SetSlotState(slot, written);
-                            if (written) NoteWrite(local, value, read);
+                            if (written) NoteWrite(local, value, read: read, isRef: isRef);
                         }
                         break;
                     }
@@ -1580,7 +1588,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
                         var elementAccess = (BoundInlineArrayAccess)node;
                         if (written)
                         {
-                            NoteWrite(elementAccess.Expression, value: null, read);
+                            NoteWrite(elementAccess.Expression, value: null, read: read, isRef: isRef);
                         }
 
                         if (elementAccess.Expression.Type.HasInlineArrayAttribute(out int length) &&
@@ -1618,7 +1626,19 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
 
                         int slot = MakeSlot(paramExpr);
                         SetSlotState(slot, written);
-                        if (written) NoteWrite(paramExpr, value, read);
+                        if (written) NoteWrite(paramExpr, value, read: read, isRef: isRef);
+                        break;
+                    }
+
+                case BoundKind.ObjectInitializerMember:
+                    {
+                        var member = (BoundObjectInitializerMember)node;
+                        if (_sourceAssembly is not null && member.MemberSymbol is FieldSymbol field)
+                        {
+                            _sourceAssembly.NoteFieldAccess(field.OriginalDefinition,
+                                read: false,
+                                write: field.RefKind == RefKind.None || isRef);
+                        }
                         break;
                     }
 
@@ -1630,7 +1650,7 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is
                         var expression = (BoundExpression)node;
                         int slot = MakeSlot(expression);
                         SetSlotState(slot, written);
-                        if (written) NoteWrite(expression, value, read);
+                        if (written) NoteWrite(expression, value, read: read, isRef: isRef);
                         break;
                     }
 
@@ -1864,7 +1884,7 @@ protected override void EnterParameter(ParameterSymbol parameter)
             {
                 // this code has no effect except in region analysis APIs such as DataFlowsOut where we unassign things
                 if (slot > 0) SetSlotState(slot, true);
-                NoteWrite(parameter, value: null, read: true);
+                NoteWrite(parameter, value: null, read: true, isRef: parameter.RefKind != RefKind.None);
             }
 
             if (parameter is SourceComplexParameterSymbolBase { ContainingSymbol: LocalFunctionSymbol or LambdaSymbol } sourceComplexParam)
@@ -2748,20 +2768,8 @@ public override void VisitForEachIterationVariables(BoundForEachStatement node)
                 int slot = GetOrCreateSlot(iterationVariable);
                 if (slot > 0) SetSlotAssigned(slot);
                 // NOTE: do not report unused iteration variables. They are always considered used.
-                NoteWrite(iterationVariable, null, read: true);
-            }
-        }
-
-        public override BoundNode VisitObjectInitializerMember(BoundObjectInitializerMember node)
-        {
-            var result = base.VisitObjectInitializerMember(node);
-
-            if ((object)_sourceAssembly != null && node.MemberSymbol != null && node.MemberSymbol.Kind == SymbolKind.Field)
-            {
-                _sourceAssembly.NoteFieldAccess((FieldSymbol)node.MemberSymbol.OriginalDefinition, read: false, write: true);
+                NoteWrite(iterationVariable, null, read: true, isRef: iterationVariable.RefKind != RefKind.None);
             }
-
-            return result;
         }
 
         public override BoundNode VisitDynamicObjectInitializerMember(BoundDynamicObjectInitializerMember node)
@@ -2790,7 +2798,7 @@ protected override void AfterVisitInlineArrayAccess(BoundInlineArrayAccess node)
             if (node.GetItemOrSliceHelper == WellKnownMember.System_Span_T__Slice_Int_Int)
             {
                 // exposing ref is a potential write
-                NoteWrite(node.Expression, value: null, read: false);
+                NoteWrite(node.Expression, value: null, read: false, isRef: false);
             }
         }
 
@@ -2800,7 +2808,7 @@ protected override void AfterVisitConversion(BoundConversion node)
                 node.Type.OriginalDefinition.Equals(compilation.GetWellKnownType(WellKnownType.System_Span_T), TypeCompareKind.AllIgnoreOptions))
             {
                 // exposing ref is a potential write
-                NoteWrite(node.Operand, value: null, read: false);
+                NoteWrite(node.Operand, value: null, read: false, isRef: false);
             }
         }
 

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

@@ -129,11 +129,11 @@ protected override void NoteRead(Symbol variable, ParameterSymbol rangeVariableU
             base.NoteRead(variable, rangeVariableUnderlyingParameter);
         }
 
-        protected override void NoteWrite(Symbol variable, BoundExpression value, bool read)
+        protected override void NoteWrite(Symbol variable, BoundExpression value, bool read, bool isRef)
         {
             if ((object)variable == null) return;
             (IsInside ? _writtenInside : _writtenOutside).Add(variable);
-            base.NoteWrite(variable, value, read);
+            base.NoteWrite(variable, value, read: read, isRef: isRef);
         }
 
         protected override void CheckAssigned(BoundExpression expr, FieldSymbol fieldSymbol, SyntaxNode node)
@@ -233,7 +233,7 @@ protected override void AssignImpl(BoundNode node, BoundExpression value, bool i
             switch (node.Kind)
             {
                 case BoundKind.RangeVariable:
-                    if (written) NoteWrite(((BoundRangeVariable)node).RangeVariableSymbol, value, read);
+                    if (written) NoteWrite(((BoundRangeVariable)node).RangeVariableSymbol, value, read: read, isRef: isRef);
                     break;
 
                 case BoundKind.QueryClause:
@@ -242,7 +242,7 @@ protected override void AssignImpl(BoundNode node, BoundExpression value, bool i
                         var symbol = ((BoundQueryClause)node).DefinedSymbol;
                         if ((object)symbol != null)
                         {
-                            if (written) NoteWrite(symbol, value, read);
+                            if (written) NoteWrite(symbol, value, read: read, isRef: isRef);
                         }
                     }
                     break;

src/Compilers/CSharp/Portable/Symbols/Source/SourceAssemblySymbol.cs

@@ -2740,6 +2740,10 @@ internal ImmutableArray<Diagnostic> GetUnusedFieldWarnings(CancellationToken can
                     {
                         diagnostics.Add(ErrorCode.WRN_UnreferencedField, field.GetFirstLocationOrNone(), field);
                     }
+                    else if (field.RefKind != RefKind.None)
+                    {
+                        diagnostics.Add(ErrorCode.WRN_UnassignedInternalRefField, field.GetFirstLocationOrNone(), field);
+                    }
                     else
                     {
                         diagnostics.Add(ErrorCode.WRN_UnassignedInternalField, field.GetFirstLocationOrNone(), field, DefaultValue(field.Type));