Report diagnostics from TryFindDisposePatternMethod when binding `f…

…oreach` loop (#75422)
dotnet · Oct 16, 2024 · f89ffba · f89ffba
1 parent 69b7f75
commit f89ffba
Show file tree

Hide file tree

Showing 6 changed files with 377 additions and 7 deletions.
diff --git a/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md b/docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
@@ -61,3 +61,29 @@ As a workaround, one can define their own `Enumerable.Reverse(this T[])` or use
 int[] x = new[] { 1, 2, 3 };
 var y = Enumerable.Reverse(x); // instead of 'x.Reverse();'
 ```
+
+## 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;
+    }
+}
+```
diff --git a/src/Compilers/CSharp/Portable/Binder/Binder.cs b/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());
             }
 

diff --git a/src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs b/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>);
 

diff --git a/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAwaitForeachTests.cs b/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,9 @@ 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),
@@ -4862,7 +4865,81 @@ 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_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),
+                // (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_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 +8572,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),