Warn for yield return in lock

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 9
+
+The compiler shipped with .NET 9 (the C# 13 compiler) contains the following warnings which are reported only under `/warn:9` or higher.
+
+| Warning ID | Description |
+|------------|-------------|
+| CS9230 | ['yield return' should not be used in the body of a lock statement](https://github.com/dotnet/roslyn/issues/72443) |
+
 ## 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/Binder/Binder_Operators.cs

@@ -648,25 +648,8 @@ resultOperatorKind is BinaryOperatorKind.ObjectEqual or BinaryOperatorKind.Objec
 
                 if (needsFilterDiagnostics)
                 {
-                    Debug.Assert(conversionDiagnostics != diagnostics);
-                    diagnostics.AddDependencies(conversionDiagnostics);
-
-                    var sourceBag = conversionDiagnostics.DiagnosticBag;
-                    Debug.Assert(sourceBag is not null);
-
-                    if (!sourceBag.IsEmptyWithoutResolution)
-                    {
-                        foreach (var diagnostic in sourceBag.AsEnumerableWithoutResolution())
-                        {
-                            var code = diagnostic is DiagnosticWithInfo { HasLazyInfo: true, LazyInfo.Code: var lazyCode } ? lazyCode : diagnostic.Code;
-                            if ((ErrorCode)code is not ErrorCode.WRN_ConvertingLock)
-                            {
-                                diagnostics.Add(diagnostic);
-                            }
-                        }
-                    }
-
-                    conversionDiagnostics.Free();
+                    conversionDiagnostics.CopyFilteredToAndFree(diagnostics,
+                        static code => code is not ErrorCode.WRN_ConvertingLock);
                 }
 
                 resultConstant = FoldBinaryOperator(node, resultOperatorKind, resultLeft, resultRight, resultType, diagnostics);

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

@@ -268,6 +268,10 @@ private BoundStatement BindYieldReturnStatement(YieldStatementSyntax node, Bindi
             {
                 Error(diagnostics, ErrorCode.ERR_YieldNotAllowedInScript, node.YieldKeyword);
             }
+            else if (this.Flags.Includes(BinderFlags.InLockBody))
+            {
+                Error(diagnostics, ErrorCode.WRN_BadYieldInLock, node.YieldKeyword);
+            }
 
             CheckRequiredLangVersionForIteratorMethods(node, diagnostics);
             return new BoundYieldReturnStatement(node, argument);

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

@@ -5,6 +5,7 @@
 using System;
 using System.Collections.Generic;
 using System.Collections.Immutable;
+using System.Diagnostics;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
 using Microsoft.CodeAnalysis.PooledObjects;
 
@@ -185,5 +186,30 @@ internal void Add(DiagnosticInfo? info, Location location)
                 DiagnosticBag?.Add(info, location);
             }
         }
+
+        internal void CopyFilteredToAndFree(BindingDiagnosticBag target, Func<ErrorCode, bool> predicate)
+        {
+            Debug.Assert(target != this);
+            target.AddDependencies(this);
+
+            var bag = DiagnosticBag;
+            Debug.Assert(bag is not null);
+
+            if (bag?.IsEmptyWithoutResolution == false)
+            {
+                foreach (var diagnostic in bag.AsEnumerableWithoutResolution())
+                {
+                    var code = diagnostic is DiagnosticWithInfo { HasLazyInfo: true, LazyInfo.Code: var lazyCode }
+                        ? lazyCode
+                        : diagnostic.Code;
+                    if (predicate((ErrorCode)code))
+                    {
+                        target.Add(diagnostic);
+                    }
+                }
+            }
+
+            Free();
+        }
     }
 }

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

@@ -55,8 +55,9 @@ internal override BoundStatement BindLockStatementParts(BindingDiagnosticBag dia
                 hasErrors = true;
             }
 
-            if (exprType?.IsWellKnownTypeLock() == true &&
-                TryFindLockTypeInfo(exprType, diagnostics, exprSyntax) is { } lockTypeInfo)
+            bool isLockObjectBased = exprType?.IsWellKnownTypeLock() == true;
+            if (isLockObjectBased &&
+                TryFindLockTypeInfo(exprType!, diagnostics, exprSyntax) is { } lockTypeInfo)
             {
                 CheckFeatureAvailability(exprSyntax, MessageID.IDS_FeatureLockObject, diagnostics);
 
@@ -73,8 +74,18 @@ internal override BoundStatement BindLockStatementParts(BindingDiagnosticBag dia
                     errorCode: ErrorCode.ERR_BadSpecialByRefLock);
             }
 
-            BoundStatement stmt = originalBinder.BindPossibleEmbeddedStatement(_syntax.Statement, diagnostics);
+            var needsFilterDiagnostics = isLockObjectBased && diagnostics.AccumulatesDiagnostics;
+            var bodyDiagnostics = needsFilterDiagnostics ? BindingDiagnosticBag.GetInstance(template: diagnostics) : diagnostics;
+
+            BoundStatement stmt = originalBinder.BindPossibleEmbeddedStatement(_syntax.Statement, bodyDiagnostics);
             Debug.Assert(this.Locals.IsDefaultOrEmpty);
+
+            if (needsFilterDiagnostics)
+            {
+                bodyDiagnostics.CopyFilteredToAndFree(diagnostics,
+                    static code => code is not ErrorCode.WRN_BadYieldInLock);
+            }
+
             return new BoundLockStatement(_syntax, expr, stmt, hasErrors);
         }
 

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -7905,4 +7905,10 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="ERR_NoModifiersOnUsing" xml:space="preserve">
     <value>Modifiers cannot be placed on using declarations</value>
   </data>
+  <data name="WRN_BadYieldInLock" xml:space="preserve">
+    <value>'yield return' should not be used in the body of a lock statement</value>
+  </data>
+  <data name="WRN_BadYieldInLock_Title" xml:space="preserve">
+    <value>'yield return' should not be used in the body of a lock statement</value>
+  </data>
 </root>

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

@@ -2304,6 +2304,8 @@ internal enum ErrorCode
 
         #endregion
 
+        WRN_BadYieldInLock = 9230,
+
         // Note: you will need to do the following after adding warnings:
         //  1) Re-generate compiler code (eng\generate-compiler-code.cmd).
     }

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

@@ -207,6 +207,10 @@ internal static int GetWarningLevel(ErrorCode code)
             // docs/compilers/CSharp/Warnversion Warning Waves.md
             switch (code)
             {
+                case ErrorCode.WRN_BadYieldInLock:
+                    // Warning level 9 is exclusively for warnings introduced in the compiler
+                    // shipped with dotnet 9 (C# 13) and that can be reported for pre-existing code.
+                    return 9;
                 case ErrorCode.WRN_AddressOfInAsync:
                 case ErrorCode.WRN_ByValArraySizeConstRequired:
                     // Warning level 8 is exclusively for warnings introduced in the compiler
@@ -2431,6 +2435,7 @@ internal static bool IsBuildOnlyDiagnostic(ErrorCode code)
                 case ErrorCode.ERR_ParamsCollectionExtensionAddMethod:
                 case ErrorCode.ERR_ParamsCollectionMissingConstructor:
                 case ErrorCode.ERR_NoModifiersOnUsing:
+                case ErrorCode.WRN_BadYieldInLock:
                     return false;
                 default:
                     // NOTE: All error codes must be explicitly handled in this switch statement