Skip to content

Commit 9bab481

Browse files
jcouvjcouv
committed
Report obsolete DisposeAsync method in await foreach
1 parent a656d1a commit 9bab481

File tree

6 files changed

+244
-13
lines changed

6 files changed

+244
-13
lines changed

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

+46
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
# This document lists known breaking changes in Roslyn after .NET 9 all the way to .NET 10.
2+
3+
## Diagnostics now reported for improper use of pattern-based disposal method in `foreach`
4+
5+
***Introduced in Visual Studio 2022 version 17.13***
6+
7+
For instance, an obsolete `DisposeAsync` method is now reported in `await foreach`.
8+
```csharp
9+
await foreach (var i in new C()) { } // 'C.AsyncEnumerator.DisposeAsync()' is obsolete
10+
11+
class C
12+
{
13+
public AsyncEnumerator GetAsyncEnumerator(System.Threading.CancellationToken token = default)
14+
{
15+
throw null;
16+
}
17+
18+
public sealed class AsyncEnumerator : System.IAsyncDisposable
19+
{
20+
public int Current { get => throw null; }
21+
public Task<bool> MoveNextAsync() => throw null;
22+
23+
[System.Obsolete]
24+
public ValueTask DisposeAsync() => throw null;
25+
}
26+
}
27+
```
28+
29+
Similarly, an `[UnmanagedCallersOnly]` `Dispose` method is now reported in `foreach` with a `ref struct` enumerator.
30+
```csharp
31+
public struct S
32+
{
33+
public static void M2(S s)
34+
{
35+
foreach (var i in s) { } // 'SEnumerator.Dispose()' is attributed with 'UnmanagedCallersOnly' and cannot be called directly.
36+
}
37+
public static SEnumerator GetEnumerator() => throw null;
38+
}
39+
public ref struct SEnumerator
40+
{
41+
public bool MoveNext() => throw null;
42+
public int Current => throw null;
43+
[UnmanagedCallersOnly]
44+
public void Dispose() => throw null;
45+
}
46+
```

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

+6
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
using System.Diagnostics;
99
using System.Threading;
1010
using Microsoft.CodeAnalysis.CSharp.Symbols;
11+
using Microsoft.CodeAnalysis.CSharp.Syntax;
1112
using Microsoft.CodeAnalysis.PooledObjects;
1213
using Roslyn.Utilities;
1314

@@ -727,6 +728,11 @@ internal static ObsoleteDiagnosticKind ReportDiagnosticsIfObsoleteInternal(Diagn
727728

728729
if (info != null)
729730
{
731+
if (node.AsNode() is ForEachStatementSyntax foreachSyntax)
732+
{
733+
node = foreachSyntax.ForEachKeyword;
734+
}
735+
730736
diagnostics.Add(info, node.GetLocation());
731737
}
732738

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

+3-2
Original file line numberDiff line numberDiff line change
@@ -1193,15 +1193,16 @@ private void GetDisposalInfoForEnumerator(SyntaxNode syntax, ref ForEachEnumerat
11931193

11941194
if (enumeratorType.IsRefLikeType || isAsync)
11951195
{
1196-
// we throw away any binding diagnostics, and assume it's not disposable if we encounter errors
11971196
var receiver = new BoundDisposableValuePlaceholder(syntax, enumeratorType);
1198-
MethodSymbol patternDisposeMethod = TryFindDisposePatternMethod(receiver, syntax, isAsync, BindingDiagnosticBag.Discarded, out bool expanded);
1197+
BindingDiagnosticBag patternDiagnostics = BindingDiagnosticBag.GetInstance(diagnostics);
1198+
MethodSymbol patternDisposeMethod = TryFindDisposePatternMethod(receiver, syntax, isAsync, patternDiagnostics, out bool expanded);
11991199
if (patternDisposeMethod is object)
12001200
{
12011201
Debug.Assert(!patternDisposeMethod.IsExtensionMethod);
12021202
Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty ||
12031203
patternDisposeMethod.ParameterRefKinds.All(static refKind => refKind is RefKind.None or RefKind.In or RefKind.RefReadOnlyParameter));
12041204

1205+
diagnostics.AddRangeAndFree(patternDiagnostics);
12051206
var argsBuilder = ArrayBuilder<BoundExpression>.GetInstance(patternDisposeMethod.ParameterCount);
12061207
var argsToParams = default(ImmutableArray<int>);
12071208

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

+55-7
Original file line numberDiff line numberDiff line change
@@ -4821,8 +4821,8 @@ public static class Extensions
48214821
CompileAndVerify(comp, expectedOutput: "NextAsync(0) Current(1) Got(1,-1) NextAsync(1) Current(2) Got(2,-2) NextAsync(2) Dispose(3) Done");
48224822
}
48234823

4824-
[Fact]
4825-
public void TestWithPatternAndObsolete()
4824+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/30257")]
4825+
public void TestWithPatternAndObsolete_WithDisposableInterface()
48264826
{
48274827
string source = @"
48284828
using System.Threading.Tasks;
@@ -4852,6 +4852,9 @@ public sealed class AsyncEnumerator : System.IAsyncDisposable
48524852
}";
48534853
var comp = CreateCompilationWithTasksExtensions(source + s_IAsyncEnumerable, options: TestOptions.DebugExe);
48544854
comp.VerifyDiagnostics(
4855+
// (7,15): warning CS0612: 'C.AsyncEnumerator.DisposeAsync()' is obsolete
4856+
// await foreach (var i in new C())
4857+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.DisposeAsync()").WithLocation(7, 15),
48554858
// (7,15): warning CS0612: 'C.GetAsyncEnumerator(CancellationToken)' is obsolete
48564859
// await foreach (var i in new C())
48574860
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetAsyncEnumerator(System.Threading.CancellationToken)").WithLocation(7, 15),
@@ -4860,9 +4863,52 @@ public sealed class AsyncEnumerator : System.IAsyncDisposable
48604863
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.MoveNextAsync()").WithLocation(7, 15),
48614864
// (7,15): warning CS0612: 'C.AsyncEnumerator.Current' is obsolete
48624865
// await foreach (var i in new C())
4863-
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.Current").WithLocation(7, 15)
4864-
);
4865-
// Note: Obsolete on DisposeAsync is not reported since always called through IAsyncDisposable interface
4866+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.Current").WithLocation(7, 15));
4867+
}
4868+
4869+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/30257")]
4870+
public void TestWithPatternAndObsolete_WithoutDisposableInterface()
4871+
{
4872+
string source = @"
4873+
using System.Threading.Tasks;
4874+
class C
4875+
{
4876+
static async System.Threading.Tasks.Task Main()
4877+
{
4878+
await foreach (var i in new C())
4879+
{
4880+
}
4881+
}
4882+
[System.Obsolete]
4883+
public AsyncEnumerator GetAsyncEnumerator(System.Threading.CancellationToken token = default)
4884+
{
4885+
throw null;
4886+
}
4887+
[System.Obsolete]
4888+
public sealed class AsyncEnumerator
4889+
{
4890+
[System.Obsolete]
4891+
public int Current { get => throw null; }
4892+
[System.Obsolete]
4893+
public Task<bool> MoveNextAsync() => throw null;
4894+
[System.Obsolete]
4895+
public ValueTask DisposeAsync() => throw null;
4896+
}
4897+
}";
4898+
var comp = CreateCompilationWithTasksExtensions(source + s_IAsyncEnumerable, options: TestOptions.DebugExe);
4899+
comp.VerifyDiagnostics(
4900+
// (7,15): warning CS0612: 'C.AsyncEnumerator.DisposeAsync()' is obsolete
4901+
// await foreach (var i in new C())
4902+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.DisposeAsync()").WithLocation(7, 15),
4903+
// (7,15): warning CS0612: 'C.GetAsyncEnumerator(CancellationToken)' is obsolete
4904+
// await foreach (var i in new C())
4905+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetAsyncEnumerator(System.Threading.CancellationToken)").WithLocation(7, 15),
4906+
// (7,15): warning CS0612: 'C.AsyncEnumerator.MoveNextAsync()' is obsolete
4907+
// await foreach (var i in new C())
4908+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.MoveNextAsync()").WithLocation(7, 15),
4909+
// (7,15): warning CS0612: 'C.AsyncEnumerator.Current' is obsolete
4910+
// await foreach (var i in new C())
4911+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.AsyncEnumerator.Current").WithLocation(7, 15));
48664912
}
48674913

48684914
[Fact]
@@ -8495,6 +8541,9 @@ public static class Extensions
84958541
}";
84968542
var comp = CreateCompilationWithMscorlib46(source, options: TestOptions.DebugExe, parseOptions: TestOptions.Regular9);
84978543
comp.VerifyDiagnostics(
8544+
// (8,15): warning CS0612: 'C.Enumerator.DisposeAsync()' is obsolete
8545+
// await foreach (var i in new C())
8546+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.DisposeAsync()").WithLocation(8, 15),
84988547
// (8,15): warning CS0612: 'Extensions.GetAsyncEnumerator(C)' is obsolete
84998548
// await foreach (var i in new C())
85008549
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("Extensions.GetAsyncEnumerator(C)").WithLocation(8, 15),
@@ -8503,8 +8552,7 @@ public static class Extensions
85038552
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.MoveNextAsync()").WithLocation(8, 15),
85048553
// (8,15): warning CS0612: 'C.Enumerator.Current' is obsolete
85058554
// await foreach (var i in new C())
8506-
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.Current").WithLocation(8, 15)
8507-
);
8555+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.Current").WithLocation(8, 15));
85088556
CompileAndVerify(comp, expectedOutput: "123Disposed");
85098557
}
85108558

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

+128
Original file line numberDiff line numberDiff line change
@@ -5593,5 +5593,133 @@ public static void Dispose(this int i, params int[] other) { }
55935593
CompileAndVerify(comp, expectedOutput: ExecutionConditionUtil.IsMonoOrCoreClr ? "42" : null, verify: Verification.Skipped)
55945594
.VerifyDiagnostics();
55955595
}
5596+
5597+
[Fact]
5598+
public void TestWithPatternAndObsolete_WithDisposableInterface()
5599+
{
5600+
string source = """
5601+
foreach (var i in new C())
5602+
{
5603+
}
5604+
class C
5605+
{
5606+
[System.Obsolete]
5607+
public MyEnumerator GetEnumerator()
5608+
{
5609+
throw null;
5610+
}
5611+
[System.Obsolete]
5612+
public sealed class MyEnumerator : System.IDisposable
5613+
{
5614+
[System.Obsolete]
5615+
public int Current { get => throw null; }
5616+
[System.Obsolete]
5617+
public bool MoveNext() => throw null;
5618+
[System.Obsolete("error", true)]
5619+
public void Dispose() => throw null;
5620+
}
5621+
}
5622+
""";
5623+
var comp = CreateCompilation(source);
5624+
comp.VerifyEmitDiagnostics(
5625+
// (1,1): warning CS0612: 'C.GetEnumerator()' is obsolete
5626+
// foreach (var i in new C())
5627+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetEnumerator()").WithLocation(1, 1),
5628+
// (1,1): warning CS0612: 'C.MyEnumerator.MoveNext()' is obsolete
5629+
// foreach (var i in new C())
5630+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.MoveNext()").WithLocation(1, 1),
5631+
// (1,1): warning CS0612: 'C.MyEnumerator.Current' is obsolete
5632+
// foreach (var i in new C())
5633+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.Current").WithLocation(1, 1));
5634+
var verifier = CompileAndVerify(comp);
5635+
verifier.VerifyIL("<top-level-statements-entry-point>", """
5636+
{
5637+
// Code size 41 (0x29)
5638+
.maxstack 1
5639+
.locals init (C.MyEnumerator V_0)
5640+
IL_0000: newobj "C..ctor()"
5641+
IL_0005: call "C.MyEnumerator C.GetEnumerator()"
5642+
IL_000a: stloc.0
5643+
.try
5644+
{
5645+
IL_000b: br.s IL_0014
5646+
IL_000d: ldloc.0
5647+
IL_000e: callvirt "int C.MyEnumerator.Current.get"
5648+
IL_0013: pop
5649+
IL_0014: ldloc.0
5650+
IL_0015: callvirt "bool C.MyEnumerator.MoveNext()"
5651+
IL_001a: brtrue.s IL_000d
5652+
IL_001c: leave.s IL_0028
5653+
}
5654+
finally
5655+
{
5656+
IL_001e: ldloc.0
5657+
IL_001f: brfalse.s IL_0027
5658+
IL_0021: ldloc.0
5659+
IL_0022: callvirt "void System.IDisposable.Dispose()"
5660+
IL_0027: endfinally
5661+
}
5662+
IL_0028: ret
5663+
}
5664+
""");
5665+
}
5666+
5667+
[Fact]
5668+
public void TestWithPatternAndObsolete_WithoutDisposableInterface()
5669+
{
5670+
string source = """
5671+
foreach (var i in new C())
5672+
{
5673+
}
5674+
class C
5675+
{
5676+
[System.Obsolete]
5677+
public MyEnumerator GetEnumerator()
5678+
{
5679+
throw null;
5680+
}
5681+
[System.Obsolete]
5682+
public sealed class MyEnumerator
5683+
{
5684+
[System.Obsolete]
5685+
public int Current { get => throw null; }
5686+
[System.Obsolete]
5687+
public bool MoveNext() => throw null;
5688+
[System.Obsolete("error", true)]
5689+
public void Dispose() => throw null;
5690+
}
5691+
}
5692+
""";
5693+
var comp = CreateCompilation(source);
5694+
comp.VerifyEmitDiagnostics(
5695+
// (1,1): warning CS0612: 'C.GetEnumerator()' is obsolete
5696+
// foreach (var i in new C())
5697+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.GetEnumerator()").WithLocation(1, 1),
5698+
// (1,1): warning CS0612: 'C.MyEnumerator.MoveNext()' is obsolete
5699+
// foreach (var i in new C())
5700+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.MoveNext()").WithLocation(1, 1),
5701+
// (1,1): warning CS0612: 'C.MyEnumerator.Current' is obsolete
5702+
// foreach (var i in new C())
5703+
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.MyEnumerator.Current").WithLocation(1, 1));
5704+
var verifier = CompileAndVerify(comp);
5705+
verifier.VerifyIL("<top-level-statements-entry-point>", """
5706+
{
5707+
// Code size 29 (0x1d)
5708+
.maxstack 1
5709+
.locals init (C.MyEnumerator V_0)
5710+
IL_0000: newobj "C..ctor()"
5711+
IL_0005: call "C.MyEnumerator C.GetEnumerator()"
5712+
IL_000a: stloc.0
5713+
IL_000b: br.s IL_0014
5714+
IL_000d: ldloc.0
5715+
IL_000e: callvirt "int C.MyEnumerator.Current.get"
5716+
IL_0013: pop
5717+
IL_0014: ldloc.0
5718+
IL_0015: callvirt "bool C.MyEnumerator.MoveNext()"
5719+
IL_001a: brtrue.s IL_000d
5720+
IL_001c: ret
5721+
}
5722+
""");
5723+
}
55965724
}
55975725
}

‎src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs

+6-4
Original file line numberDiff line numberDiff line change
@@ -9398,7 +9398,7 @@ public static class CExt
93989398
);
93999399
}
94009400

9401-
[Fact]
9401+
[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/73934")]
94029402
public void UnmanagedCallersOnlyDeclaredOnPatternDispose()
94039403
{
94049404
var comp = CreateCompilation(new[] { @"
@@ -9424,10 +9424,12 @@ public static class CExt
94249424
", UnmanagedCallersOnlyAttribute });
94259425

94269426
comp.VerifyDiagnostics(
9427-
// (14,6): error CS8896: 'UnmanagedCallersOnly' can only be applied to ordinary static non-abstract, non-virtual methods or static local functions.
9427+
// 0.cs(7,9): error CS8901: 'SEnumerator.Dispose()' is attributed with 'UnmanagedCallersOnly' and cannot be called directly. Obtain a function pointer to this method.
9428+
// foreach (var i in s) {}
9429+
Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyMethodsCannotBeCalledDirectly, "foreach (var i in s) {}").WithArguments("SEnumerator.Dispose()").WithLocation(7, 9),
9430+
// 0.cs(14,6): error CS8896: 'UnmanagedCallersOnly' can only be applied to ordinary static non-abstract, non-virtual methods or static local functions.
94289431
// [UnmanagedCallersOnly]
9429-
Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyRequiresStatic, "UnmanagedCallersOnly").WithLocation(14, 6)
9430-
);
9432+
Diagnostic(ErrorCode.ERR_UnmanagedCallersOnlyRequiresStatic, "UnmanagedCallersOnly").WithLocation(14, 6));
94319433
}
94329434

94339435
[Fact]

0 commit comments

Comments
 (0)