Report diagnostics from TryFindDisposePatternMethod when binding foreach loop

docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md

@@ -41,3 +41,30 @@ static class C
     public static void R<T>(ReadOnlySpan<T> s) => Console.Write(3);
 }
 ```
+
+## Diagnostics now reported for pattern-based disposal method in `foreach`
+
+***Introduced in Visual Studio 2022 version 17.13***
+
+For instance, an obsolete `DisposeAsync` method is now reported in `await foreach`.
+```csharp
+await foreach (var i in new C()) { } // 'C.AsyncEnumerator.DisposeAsync()' is obsolete
+
+class C
+{
+    public AsyncEnumerator GetAsyncEnumerator(System.Threading.CancellationToken token = default)
+    {
+        throw null;
+    }
+
+    public sealed class AsyncEnumerator : System.IAsyncDisposable
+    {
+        public int Current { get => throw null; }
+        public Task<bool> MoveNextAsync() => throw null;
+
+        [System.Obsolete]
+        public ValueTask DisposeAsync() => throw null;
+    }
+}
+```
+

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

@@ -8,6 +8,7 @@
 using System.Diagnostics;
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Symbols;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Roslyn.Utilities;
 
@@ -727,6 +728,11 @@ internal static ObsoleteDiagnosticKind ReportDiagnosticsIfObsoleteInternal(Diagn
 
             if (info != null)
             {
+                if (node.AsNode() is ForEachStatementSyntax foreachSyntax)
+                {
+                    node = foreachSyntax.ForEachKeyword;
+                }
+
                 diagnostics.Add(info, node.GetLocation());
             }
 

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

@@ -1193,15 +1193,16 @@ private void GetDisposalInfoForEnumerator(SyntaxNode syntax, ref ForEachEnumerat
 
             if (enumeratorType.IsRefLikeType || isAsync)
             {
-                // we throw away any binding diagnostics, and assume it's not disposable if we encounter errors
                 var receiver = new BoundDisposableValuePlaceholder(syntax, enumeratorType);
-                MethodSymbol patternDisposeMethod = TryFindDisposePatternMethod(receiver, syntax, isAsync, BindingDiagnosticBag.Discarded, out bool expanded);
+                BindingDiagnosticBag patternDiagnostics = BindingDiagnosticBag.GetInstance(diagnostics);
+                MethodSymbol patternDisposeMethod = TryFindDisposePatternMethod(receiver, syntax, isAsync, patternDiagnostics, out bool expanded);
                 if (patternDisposeMethod is object)
                 {
                     Debug.Assert(!patternDisposeMethod.IsExtensionMethod);
                     Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty ||
                         patternDisposeMethod.ParameterRefKinds.All(static refKind => refKind is RefKind.None or RefKind.In or RefKind.RefReadOnlyParameter));
 
+                    diagnostics.AddRangeAndFree(patternDiagnostics);
                     var argsBuilder = ArrayBuilder<BoundExpression>.GetInstance(patternDisposeMethod.ParameterCount);
                     var argsToParams = default(ImmutableArray<int>);
 

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAwaitForeachTests.cs

@@ -4821,8 +4821,8 @@ public static class Extensions
             CompileAndVerify(comp, expectedOutput: "NextAsync(0) Current(1) Got(1,-1) NextAsync(1) Current(2) Got(2,-2) NextAsync(2) Dispose(3) Done");
         }
 
-        [Fact]
-        public void TestWithPatternAndObsolete()
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/30257")]
+        public void TestWithPatternAndObsolete_WithDisposableInterface()
         {
             string source = @"
 using System.Threading.Tasks;
@@ -4852,6 +4852,54 @@ public sealed class AsyncEnumerator : System.IAsyncDisposable
 }";
             var comp = CreateCompilationWithTasksExtensions(source + s_IAsyncEnumerable, options: TestOptions.DebugExe);
             comp.VerifyDiagnostics(
+                // (7,15): warning CS0612: 'C.AsyncEnumerator.DisposeAsync()' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.DisposeAsync()").WithLocation(7, 15),
+                // (7,15): warning CS0612: 'C.GetAsyncEnumerator(CancellationToken)' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetAsyncEnumerator(System.Threading.CancellationToken)").WithLocation(7, 15),
+                // (7,15): warning CS0612: 'C.AsyncEnumerator.MoveNextAsync()' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.MoveNextAsync()").WithLocation(7, 15),
+                // (7,15): warning CS0612: 'C.AsyncEnumerator.Current' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.Current").WithLocation(7, 15));
+        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/30257")]
+        public void TestWithPatternAndObsolete_WithoutDisposableInterface()
+        {
+            string source = @"
+using System.Threading.Tasks;
+class C
+{
+    static async System.Threading.Tasks.Task Main()
+    {
+        await foreach (var i in new C())
+        {
+        }
+    }
+    [System.Obsolete]
+    public AsyncEnumerator GetAsyncEnumerator(System.Threading.CancellationToken token = default)
+    {
+        throw null;
+    }
+    [System.Obsolete]
+    public sealed class AsyncEnumerator
+    {
+        [System.Obsolete]
+        public int Current { get => throw null; }
+        [System.Obsolete]
+        public Task<bool> MoveNextAsync() => throw null;
+        [System.Obsolete]
+        public ValueTask DisposeAsync() => throw null;
+    }
+}";
+            var comp = CreateCompilationWithTasksExtensions(source + s_IAsyncEnumerable, options: TestOptions.DebugExe);
+            comp.VerifyDiagnostics(
+                // (7,15): warning CS0612: 'C.AsyncEnumerator.DisposeAsync()' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.DisposeAsync()").WithLocation(7, 15),
                 // (7,15): warning CS0612: 'C.GetAsyncEnumerator(CancellationToken)' is obsolete
                 //         await foreach (var i in new C())
                 Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetAsyncEnumerator(System.Threading.CancellationToken)").WithLocation(7, 15),
@@ -4862,7 +4910,35 @@ public sealed class AsyncEnumerator : System.IAsyncDisposable
                 //         await foreach (var i in new C())
                 Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.Current").WithLocation(7, 15)
                 );
-            // Note: Obsolete on DisposeAsync is not reported since always called through IAsyncDisposable interface
+        }
+
+        [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/30257")]
+        public void TestWithPatternAndObsolete_WithExplicitInterfaceImplementation()
+        {
+            string source = @"
+using System.Threading.Tasks;
+class C
+{
+    static async System.Threading.Tasks.Task Main()
+    {
+        await foreach (var i in new C())
+        {
+        }
+    }
+    public AsyncEnumerator GetAsyncEnumerator(System.Threading.CancellationToken token = default)
+    {
+        throw null;
+    }
+    public sealed class AsyncEnumerator : System.IAsyncDisposable
+    {
+        public int Current { get => throw null; }
+        public Task<bool> MoveNextAsync() => throw null;
+        [System.Obsolete]
+        ValueTask System.IAsyncDisposable.DisposeAsync() => throw null;
+    }
+}";
+            var comp = CreateCompilationWithTasksExtensions(source + s_IAsyncEnumerable);
+            comp.VerifyEmitDiagnostics();
         }
 
         [Fact]
@@ -8495,6 +8571,9 @@ public static class Extensions
 }";
             var comp = CreateCompilationWithMscorlib46(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular9);
             comp.VerifyDiagnostics(
+                // (8,15): warning CS0612: 'C.Enumerator.DisposeAsync()' is obsolete
+                //         await foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.DisposeAsync()").WithLocation(8, 15),
                 // (8,15): warning CS0612: 'Extensions.GetAsyncEnumerator(C)' is obsolete
                 //         await foreach (var i in new C())
                 Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("Extensions.GetAsyncEnumerator(C)").WithLocation(8, 15),

src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenForEachTests.cs

@@ -5593,5 +5593,195 @@ public static void Dispose(this int i, params int[] other) { }
             CompileAndVerify(comp, expectedOutput: ExecutionConditionUtil.IsMonoOrCoreClr ? "42" : null, verify: Verification.Skipped)
                 .VerifyDiagnostics();
         }
+
+        [Fact]
+        public void TestWithPatternAndObsolete_WithDisposableInterface()
+        {
+            string source = """
+foreach (var i in new C())
+{
+}
+class C
+{
+    [System.Obsolete]
+    public MyEnumerator GetEnumerator()
+    {
+        throw null;
+    }
+    [System.Obsolete]
+    public sealed class MyEnumerator : System.IDisposable
+    {
+        [System.Obsolete]
+        public int Current { get => throw null; }
+        [System.Obsolete]
+        public bool MoveNext() => throw null;
+        [System.Obsolete("error", true)]
+        public void Dispose() => throw null;
+    }
+}
+""";
+            var comp = CreateCompilation(source);
+            comp.VerifyEmitDiagnostics(
+                // (1,1): warning CS0612: 'C.GetEnumerator()' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetEnumerator()").WithLocation(1, 1),
+                // (1,1): warning CS0612: 'C.MyEnumerator.MoveNext()' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.MoveNext()").WithLocation(1, 1),
+                // (1,1): warning CS0612: 'C.MyEnumerator.Current' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.Current").WithLocation(1, 1));
+            var verifier = CompileAndVerify(comp);
+            verifier.VerifyIL("<top-level-statements-entry-point>", """
+{
+  // Code size       41 (0x29)
+  .maxstack  1
+  .locals init (C.MyEnumerator V_0)
+  IL_0000:  newobj     "C..ctor()"
+  IL_0005:  call       "C.MyEnumerator C.GetEnumerator()"
+  IL_000a:  stloc.0
+  .try
+  {
+    IL_000b:  br.s       IL_0014
+    IL_000d:  ldloc.0
+    IL_000e:  callvirt   "int C.MyEnumerator.Current.get"
+    IL_0013:  pop
+    IL_0014:  ldloc.0
+    IL_0015:  callvirt   "bool C.MyEnumerator.MoveNext()"
+    IL_001a:  brtrue.s   IL_000d
+    IL_001c:  leave.s    IL_0028
+  }
+  finally
+  {
+    IL_001e:  ldloc.0
+    IL_001f:  brfalse.s  IL_0027
+    IL_0021:  ldloc.0
+    IL_0022:  callvirt   "void System.IDisposable.Dispose()"
+    IL_0027:  endfinally
+  }
+  IL_0028:  ret
+}
+""");
+        }
+
+        [Fact]
+        public void TestWithPatternAndObsolete_WithoutDisposableInterface()
+        {
+            string source = """
+foreach (var i in new C())
+{
+}
+class C
+{
+    [System.Obsolete]
+    public MyEnumerator GetEnumerator()
+    {
+        throw null;
+    }
+    [System.Obsolete]
+    public sealed class MyEnumerator
+    {
+        [System.Obsolete]
+        public int Current { get => throw null; }
+        [System.Obsolete]
+        public bool MoveNext() => throw null;
+        [System.Obsolete("error", true)]
+        public void Dispose() => throw null;
+    }
+}
+""";
+            var comp = CreateCompilation(source);
+            comp.VerifyEmitDiagnostics(
+                // (1,1): warning CS0612: 'C.GetEnumerator()' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetEnumerator()").WithLocation(1, 1),
+                // (1,1): warning CS0612: 'C.MyEnumerator.MoveNext()' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.MoveNext()").WithLocation(1, 1),
+                // (1,1): warning CS0612: 'C.MyEnumerator.Current' is obsolete
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.Current").WithLocation(1, 1));
+            var verifier = CompileAndVerify(comp);
+            verifier.VerifyIL("<top-level-statements-entry-point>", """
+{
+  // Code size       29 (0x1d)
+  .maxstack  1
+  .locals init (C.MyEnumerator V_0)
+  IL_0000:  newobj     "C..ctor()"
+  IL_0005:  call       "C.MyEnumerator C.GetEnumerator()"
+  IL_000a:  stloc.0
+  IL_000b:  br.s       IL_0014
+  IL_000d:  ldloc.0
+  IL_000e:  callvirt   "int C.MyEnumerator.Current.get"
+  IL_0013:  pop
+  IL_0014:  ldloc.0
+  IL_0015:  callvirt   "bool C.MyEnumerator.MoveNext()"
+  IL_001a:  brtrue.s   IL_000d
+  IL_001c:  ret
+}
+""");
+        }
+
+        [Fact]
+        public void TestWithPatternAndObsolete_WithoutDisposableInterface_RefStructEnumerator()
+        {
+            string source = """
+foreach (var i in new C())
+{
+}
+
+class C
+{
+    public MyEnumerator GetEnumerator()
+    {
+        throw null;
+    }
+
+    public ref struct MyEnumerator
+    {
+        public int Current { get => throw null; }
+        public bool MoveNext() => throw null;
+
+        [System.Obsolete("error", true)]
+        public void Dispose() => throw null;
+    }
+}
+""";
+            var comp = CreateCompilation(source);
+            comp.VerifyEmitDiagnostics(
+                // (1,1): error CS0619: 'C.MyEnumerator.Dispose()' is obsolete: 'error'
+                // foreach (var i in new C())
+                Diagnostic(ErrorCode.ERR_DeprecatedSymbolStr, "foreach").WithArguments("C.MyEnumerator.Dispose()", "error").WithLocation(1, 1));
+        }
+
+        [Fact]
+        public void TestWithPatternAndObsolete_WithoutDisposableInterface_RefStructEnumerator_CollectionExpression()
+        {
+            string source = """
+int[] a = [42, ..new C()];
+
+class C
+{
+    public MyEnumerator GetEnumerator()
+    {
+        throw null;
+    }
+
+    public ref struct MyEnumerator
+    {
+        public int Current { get => throw null; }
+        public bool MoveNext() => throw null;
+
+        [System.Obsolete("error", true)]
+        public void Dispose() => throw null;
+    }
+}
+""";
+            var comp = CreateCompilation(source);
+            comp.VerifyEmitDiagnostics(
+                // (1,16): error CS0619: 'C.MyEnumerator.Dispose()' is obsolete: 'error'
+                // int[] a = [42, ..new C()];
+                Diagnostic(ErrorCode.ERR_DeprecatedSymbolStr, "..new C()").WithArguments("C.MyEnumerator.Dispose()", "error").WithLocation(1, 16));
+        }
     }
 }