Skip to content

Commit

Permalink
Report obsolete DisposeAsync method in await foreach
Browse files Browse the repository at this point in the history
  • Loading branch information
@jcouv
jcouv committed Oct 8, 2024
1 parent 244b7e5 commit e2dac8f
Show file tree
Hide file tree
Showing 6 changed files with 243 additions and 12 deletions.
46 changes: 46 additions & 0 deletions docs/compilers/CSharp/Compiler Breaking Changes - DotNet 10.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# This document lists known breaking changes in Roslyn after .NET 9 all the way to .NET 10.

## Diagnostic now reported for improper use of 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;
}
}
```

Similarly, an `[UnmanagedCallersOnly]` `Dispose` method is now reported in `foreach` with a `ref struct` enumerator.
```csharp
public struct S
{
public static void M2(S s)
{
foreach (var i in s) { } // 'SEnumerator.Dispose()' is attributed with 'UnmanagedCallersOnly' and cannot be called directly.
}
public static SEnumerator GetEnumerator() => throw null;
}
public ref struct SEnumerator
{
public bool MoveNext() => throw null;
public int Current => throw null;
[UnmanagedCallersOnly]
public void Dispose() => throw null;
}
```
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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());
}

Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Binder/ForEachLoopBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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>);

Expand Down
62 changes: 55 additions & 7 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenAwaitForeachTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Expand All @@ -4860,9 +4863,52 @@ public sealed class AsyncEnumerator : System.IAsyncDisposable
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)
);
// Note: Obsolete on DisposeAsync is not reported since always called through IAsyncDisposable interface
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),
// (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]
Expand Down Expand Up @@ -8495,6 +8541,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),
Expand All @@ -8503,8 +8552,7 @@ public static class Extensions
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.MoveNextAsync()").WithLocation(8, 15),
// (8,15): warning CS0612: 'C.Enumerator.Current' is obsolete
// await foreach (var i in new C())
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.Current").WithLocation(8, 15)
);
Diagnostic(ErrorCode.WRN_DeprecatedSymbol, "foreach").WithArguments("C.Enumerator.Current").WithLocation(8, 15));
CompileAndVerify(comp, expectedOutput: "123Disposed");
}

Expand Down
128 changes: 128 additions & 0 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenForEachTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5576,5 +5576,133 @@ struct S
// r.S.F++;
Diagnostic(ErrorCode.ERR_AssgReadonlyLocal2Cause, "r.S.F").WithArguments("r", "foreach iteration variable").WithLocation(7, 5));
}

[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
}
""");
}
}
}
8 changes: 5 additions & 3 deletions src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenFunctionPointersTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9424,10 +9424,12 @@ public static class CExt
", UnmanagedCallersOnlyAttribute });

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

[Fact]
Expand Down

0 comments on commit e2dac8f

Please sign in to comment.