Fix binding order for Dispose in async foreach

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

@@ -106,3 +106,34 @@ https://github.com/dotnet/roslyn/issues/57750
     void M(IEnumerable<char> s = "hello")
     ```
 
+8. <a name="8"></a>Starting with Visual Studio 17.2, an async `foreach` prefers to bind using a pattern-based `DisposeAsync()` method rather than `IAsyncDisposable.DisposeAsync()`.
+    For instance, the `DisposeAsync()` will be picked, rather than the `IAsyncEnumerator<int>.DisposeAsync()` method on `AsyncEnumerator`:
+    ```csharp
+    await foreach (var i in new AsyncEnumerable())
+    {
+    }
+
+    struct AsyncEnumerable
+    {
+        public AsyncEnumerator GetAsyncEnumerator(CancellationToken token) => new AsyncEnumerator();
+    }
+
+    struct AsyncEnumerator : IAsyncDisposable
+    {
+        public int Current => 0;
+        public async ValueTask<bool> MoveNextAsync()
+        {
+            await Task.Yield();
+            return false;
+        }
+        public async ValueTask DisposeAsync()
+        {
+            Console.WriteLine("PICKED");
+            await Task.Yield();
+        }
+        ValueTask IAsyncDisposable.DisposeAsync() => throw null; // no longer picked
+    }
+    ```
+
+9. <a name="9"></a>Starting with Visual Studio 17.2, a `foreach` using a ref struct enumerator type reports an error if the language version is set to 7.3 or earlier.
+

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

@@ -200,6 +200,11 @@ internal override BoundStatement BindForEachDeconstruction(BindingDiagnosticBag
 
         private BoundForEachStatement BindForEachPartsWorker(BindingDiagnosticBag diagnostics, Binder originalBinder)
         {
+            if (IsAsync)
+            {
+                CheckFeatureAvailability(_syntax, MessageID.IDS_FeatureAsyncStreams, diagnostics, _syntax.AwaitKeyword.GetLocation());
+            }
+
             // Use the right binder to avoid seeing iteration variable
             BoundExpression collectionExpr = originalBinder.GetBinder(_syntax.Expression).BindRValueWithoutTargetType(_syntax.Expression, diagnostics);
 
@@ -995,33 +1000,24 @@ private void GetDisposalInfoForEnumerator(ref ForEachEnumeratorInfo.Builder buil
             TypeSymbol enumeratorType = builder.GetEnumeratorInfo.Method.ReturnType;
             CompoundUseSiteInfo<AssemblySymbol> useSiteInfo = GetNewCompoundUseSiteInfo(diagnostics);
 
-            // For async foreach, we don't do the runtime check
-            if ((!enumeratorType.IsSealed && !isAsync) ||
-                this.Conversions.ClassifyImplicitConversionFromType(enumeratorType,
-                    isAsync ? this.Compilation.GetWellKnownType(WellKnownType.System_IAsyncDisposable) : this.Compilation.GetSpecialType(SpecialType.System_IDisposable),
-                    ref useSiteInfo).IsImplicit)
+            MethodSymbol patternDisposeMethod = null;
+            if (enumeratorType.IsRefLikeType || isAsync)
             {
-                builder.NeedsDisposal = true;
-            }
-            else if (Compilation.IsFeatureEnabled(MessageID.IDS_FeatureUsingDeclarations) &&
-                    (enumeratorType.IsRefLikeType || isAsync))
-            {
-                // if it wasn't directly convertable to IDisposable, see if it is pattern-disposable
-                // again, we throw away any binding diagnostics, and assume it's not disposable if we encounter errors
+                // we throw away any binding diagnostics, and assume it's not disposable if we encounter errors
                 var receiver = new BoundDisposableValuePlaceholder(_syntax, enumeratorType);
-                MethodSymbol disposeMethod = TryFindDisposePatternMethod(receiver, _syntax, isAsync, BindingDiagnosticBag.Discarded);
-                if (disposeMethod is object)
+                patternDisposeMethod = TryFindDisposePatternMethod(receiver, _syntax, isAsync, BindingDiagnosticBag.Discarded);
+                if (patternDisposeMethod is object)
                 {
-                    Debug.Assert(!disposeMethod.IsExtensionMethod);
-                    Debug.Assert(disposeMethod.ParameterRefKinds.IsDefaultOrEmpty);
+                    Debug.Assert(!patternDisposeMethod.IsExtensionMethod);
+                    Debug.Assert(patternDisposeMethod.ParameterRefKinds.IsDefaultOrEmpty);
 
-                    var argsBuilder = ArrayBuilder<BoundExpression>.GetInstance(disposeMethod.ParameterCount);
+                    var argsBuilder = ArrayBuilder<BoundExpression>.GetInstance(patternDisposeMethod.ParameterCount);
                     var argsToParams = default(ImmutableArray<int>);
-                    bool expanded = disposeMethod.HasParamsParameter();
+                    bool expanded = patternDisposeMethod.HasParamsParameter();
 
                     BindDefaultArguments(
                         _syntax,
-                        disposeMethod.Parameters,
+                        patternDisposeMethod.Parameters,
                         argsBuilder,
                         argumentRefKindsBuilder: null,
                         ref argsToParams,
@@ -1031,11 +1027,30 @@ private void GetDisposalInfoForEnumerator(ref ForEachEnumeratorInfo.Builder buil
                         diagnostics);
 
                     builder.NeedsDisposal = true;
-                    builder.PatternDisposeInfo = new MethodArgumentInfo(disposeMethod, argsBuilder.ToImmutableAndFree(), argsToParams, defaultArguments, expanded);
+                    builder.PatternDisposeInfo = new MethodArgumentInfo(patternDisposeMethod, argsBuilder.ToImmutableAndFree(), argsToParams, defaultArguments, expanded);
+
+                    if (!isAsync)
+                    {
+                        // We already checked feature availability for async scenarios
+                        CheckFeatureAvailability(expr.Syntax, MessageID.IDS_FeatureDisposalPattern, diagnostics);
+                    }
                 }
             }
 
-            diagnostics.Add(_syntax, useSiteInfo);
+            if (!enumeratorType.IsRefLikeType && patternDisposeMethod is null)
+            {
+                // If it wasn't pattern-disposable, see if it's directly convertable to IDisposable
+                // For async foreach, we don't do the runtime check in unsealed case
+                if ((!enumeratorType.IsSealed && !isAsync) ||
+                    this.Conversions.ClassifyImplicitConversionFromType(enumeratorType,
+                        isAsync ? this.Compilation.GetWellKnownType(WellKnownType.System_IAsyncDisposable) : this.Compilation.GetSpecialType(SpecialType.System_IDisposable),
+                        ref useSiteInfo).IsImplicit)
+                {
+                    builder.NeedsDisposal = true;
+                }
+
+                diagnostics.Add(_syntax, useSiteInfo);
+            }
         }
 
         private ForEachEnumeratorInfo.Builder GetDefaultEnumeratorInfo(ForEachEnumeratorInfo.Builder builder, BindingDiagnosticBag diagnostics, TypeSymbol collectionExprType)

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

@@ -93,6 +93,15 @@ internal static BoundStatement BindUsingStatementOrDeclarationFromParts(SyntaxNo
             bool isExpression = !isUsingDeclaration && syntax.Kind() != SyntaxKind.VariableDeclaration;
             bool hasAwait = awaitKeyword != default;
 
+            if (isUsingDeclaration)
+            {
+                CheckFeatureAvailability(syntax, MessageID.IDS_FeatureUsingDeclarations, diagnostics, usingKeyword.GetLocation());
+            }
+            else if (hasAwait)
+            {
+                CheckFeatureAvailability(syntax, MessageID.IDS_FeatureAsyncUsing, diagnostics, awaitKeyword.GetLocation());
+            }
+
             Debug.Assert(isUsingDeclaration || usingBinderOpt != null);
 
             TypeSymbol disposableInterface = getDisposableInterface(hasAwait);
@@ -200,13 +209,13 @@ bool bindDisposable(bool fromExpression, out MethodArgumentInfo? patternDisposeI
                                                ? expressionOpt
                                                : new BoundLocal(syntax, declarationsOpt[0].LocalSymbol, null, type) { WasCompilerGenerated = true };
 
-                    BindingDiagnosticBag patternDiagnostics = originalBinder.Compilation.IsFeatureEnabled(MessageID.IDS_FeatureUsingDeclarations)
+                    BindingDiagnosticBag patternDiagnostics = originalBinder.Compilation.IsFeatureEnabled(MessageID.IDS_FeatureDisposalPattern)
                                                        ? diagnostics
                                                        : BindingDiagnosticBag.Discarded;
                     MethodSymbol disposeMethod = originalBinder.TryFindDisposePatternMethod(receiver, syntax, hasAwait, patternDiagnostics);
                     if (disposeMethod is object)
                     {
-                        MessageID.IDS_FeatureUsingDeclarations.CheckFeatureAvailability(diagnostics, originalBinder.Compilation, syntax.Location);
+                        MessageID.IDS_FeatureDisposalPattern.CheckFeatureAvailability(diagnostics, originalBinder.Compilation, syntax.Location);
 
                         var argumentsBuilder = ArrayBuilder<BoundExpression>.GetInstance(disposeMethod.ParameterCount);
                         ImmutableArray<int> argsToParams = default;

src/Compilers/CSharp/Portable/CSharpResources.resx

@@ -6070,6 +6070,9 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ
   <data name="IDS_FeatureUsingDeclarations" xml:space="preserve">
     <value>using declarations</value>
   </data>
+  <data name="IDS_FeatureDisposalPattern" xml:space="preserve">
+    <value>pattern-based disposal</value>
+  </data>
   <data name="ERR_FeatureInPreview" xml:space="preserve">
     <value>The feature '{0}' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version.</value>
   </data>

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

@@ -244,6 +244,7 @@ internal enum MessageID
 
         IDS_FeatureCacheStaticMethodGroupConversion = MessageBase + 12816,
         IDS_FeatureRawStringLiterals = MessageBase + 12817,
+        IDS_FeatureDisposalPattern = MessageBase + 12818,
     }
 
     // Message IDs may refer to strings that need to be localized.
@@ -419,9 +420,10 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
                 case MessageID.IDS_FeatureNullableReferenceTypes: // syntax and semantic check
                 case MessageID.IDS_FeatureIndexOperator: // semantic check
                 case MessageID.IDS_FeatureRangeOperator: // semantic check
-                case MessageID.IDS_FeatureAsyncStreams:
+                case MessageID.IDS_FeatureAsyncStreams: // semantic check
                 case MessageID.IDS_FeatureRecursivePatterns:
-                case MessageID.IDS_FeatureUsingDeclarations:
+                case MessageID.IDS_FeatureUsingDeclarations: // semantic check
+                case MessageID.IDS_FeatureDisposalPattern: //semantic check
                 case MessageID.IDS_FeatureStaticLocalFunctions:
                 case MessageID.IDS_FeatureNameShadowingInNestedFunctions:
                 case MessageID.IDS_FeatureUnmanagedConstructedTypes: // semantic check

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -7848,14 +7848,14 @@ private StatementSyntax TryParseStatementStartingWithIdentifier(SyntaxList<Attri
             if (this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword &&
                 this.PeekToken(1).Kind == SyntaxKind.ForEachKeyword)
             {
-                return this.ParseForEachStatement(attributes, ParseAwaitKeyword(MessageID.IDS_FeatureAsyncStreams));
+                return this.ParseForEachStatement(attributes, this.EatContextualToken(SyntaxKind.AwaitKeyword));
             }
             else if (IsPossibleAwaitUsing())
             {
                 if (PeekToken(2).Kind == SyntaxKind.OpenParenToken)
                 {
                     // `await using Type ...` is handled below in ParseLocalDeclarationStatement
-                    return this.ParseUsingStatement(attributes, ParseAwaitKeyword(MessageID.IDS_FeatureAsyncUsing));
+                    return this.ParseUsingStatement(attributes, this.EatContextualToken(SyntaxKind.AwaitKeyword));
                 }
             }
             else if (this.IsPossibleLabeledStatement())
@@ -7885,13 +7885,6 @@ private StatementSyntax ParseStatementStartingWithUsing(SyntaxList<AttributeList
         private StatementSyntax TryParseStatementStartingWithUnsafe(SyntaxList<AttributeListSyntax> attributes)
             => IsPossibleUnsafeStatement() ? ParseUnsafeStatement(attributes) : null;
 
-        private SyntaxToken ParseAwaitKeyword(MessageID feature)
-        {
-            Debug.Assert(this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword);
-            SyntaxToken awaitToken = this.EatContextualToken(SyntaxKind.AwaitKeyword);
-            return feature != MessageID.None ? CheckFeatureAvailability(awaitToken, feature) : awaitToken;
-        }
-
         private bool IsPossibleAwaitUsing()
             => CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword && PeekToken(1).Kind == SyntaxKind.UsingKeyword;
 
@@ -9676,7 +9669,7 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxList<AttributeListS
             bool canParseAsLocalFunction = false;
             if (IsPossibleAwaitUsing())
             {
-                awaitKeyword = ParseAwaitKeyword(MessageID.None);
+                awaitKeyword = this.EatContextualToken(SyntaxKind.AwaitKeyword);
                 usingKeyword = EatToken();
             }
             else if (this.CurrentToken.Kind == SyntaxKind.UsingKeyword)
@@ -9691,11 +9684,6 @@ private StatementSyntax ParseLocalDeclarationStatement(SyntaxList<AttributeListS
                 canParseAsLocalFunction = true;
             }
 
-            if (usingKeyword != null)
-            {
-                usingKeyword = CheckFeatureAvailability(usingKeyword, MessageID.IDS_FeatureUsingDeclarations);
-            }
-
             var mods = _pool.Allocate();
             this.ParseDeclarationModifiers(mods);