From ff4091f378feed1ac87632b85f24688c498438ac Mon Sep 17 00:00:00 2001
From: Sam Harwell <Sam.Harwell@microsoft.com>
Date: Tue, 27 Oct 2020 17:11:24 -0700
Subject: [PATCH] Clean up 'foreach' handling to match invocation handling

---
 .../Core/DoNotCopyValue.cs                    | 57 +++++++++++--------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs b/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs
index d1ef9618f4..29a9933cab 100644
--- a/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs
+++ b/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs
@@ -547,38 +547,45 @@ public override void VisitForEachLoop(IForEachLoopOperation operation)
                     CheckLocalSymbolInUnsupportedContext(operation, local);
                 }
 
-                var instance = operation.Collection;
+                // 'foreach' operations have an identity conversion for the collection property, and then invoke the
+                // GetEnumerator method.
+                var instance = operation.Collection as IConversionOperation;
                 var instance2 = (operation.Collection as IConversionOperation)?.Operand;
 
-                switch (Acquire(operation.Collection))
+                if (instance2 is null)
                 {
-                    case RefKind.Ref:
-                        // No special requirements
-                        break;
-
-                    case RefKind.RefReadOnly when operation.Syntax is CommonForEachStatementSyntax syntax && operation.SemanticModel.GetForEachStatementInfo(syntax).GetEnumeratorMethod is { IsReadOnly: true }:
-                        // Requirement of readonly GetEnumerator is met
-                        break;
-
-                    default:
-                        instance = null;
-                        instance2 = null;
-                        break;
+                    // Didn't match the known pattern
+                    instance = null;
                 }
-
-                switch (Acquire(instance2))
+                else if (instance?.Conversion is not { IsIdentity: true, MethodSymbol: null })
                 {
-                    case RefKind.Ref:
-                        // No special requirements
-                        break;
-
-                    case RefKind.RefReadOnly when operation.Syntax is CommonForEachStatementSyntax syntax && operation.SemanticModel.GetForEachStatementInfo(syntax).GetEnumeratorMethod is { IsReadOnly: true }:
-                        // Requirement of readonly GetEnumerator is met
-                        break;
+                    // Not a supported conversion
+                    instance = null;
+                    instance2 = null;
+                }
+                else
+                {
+                    // Treat this as an invocation of the GetEnumerator method.
+                    if (operation.Syntax is CommonForEachStatementSyntax syntax
+                        && operation.SemanticModel.GetForEachStatementInfo(syntax).GetEnumeratorMethod is { } getEnumeratorMethod)
+                    {
+                        CheckMethodSymbolInUnsupportedContext(operation, getEnumeratorMethod);
 
-                    default:
+                        if (instance2 is not null
+                            && _cache.IsNonCopyableType(getEnumeratorMethod.ReceiverType)
+                            && !getEnumeratorMethod.IsReadOnly
+                            && Acquire(instance) == RefKind.In)
+                        {
+                            // mark the instance as not checked by this method
+                            instance2 = null;
+                        }
+                    }
+                    else
+                    {
+                        // Not supported
+                        instance = null;
                         instance2 = null;
-                        break;
+                    }
                 }
 
                 using var releaser = TryAddForVisit(_handledOperations, instance, out _);