Support foreach over parameters in DoNotCopyValue

sharwell · sharwell · commit 653580dfa5f9 · 2020-10-27T16:49:39.000-07:00
diff --git a/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs b/src/Roslyn.Diagnostics.Analyzers/Core/DoNotCopyValue.cs
@@ -9,6 +9,8 @@
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
 using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CSharp;
+using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.FlowAnalysis;
 using Microsoft.CodeAnalysis.Operations;
@@ -367,7 +369,7 @@ bool IsSupportedConversion()
                     switch (Acquire(operation.Operand))
                     {
                         case RefKind.None:
-                        case RefKind.Ref when operation.Conversion.IsIdentity:
+                        case RefKind.Ref or RefKind.RefReadOnly when operation.Conversion.IsIdentity:
                             return true;
 
                         default:
@@ -547,14 +549,36 @@ public override void VisitForEachLoop(IForEachLoopOperation operation)
 
                 var instance = operation.Collection;
                 var instance2 = (operation.Collection as IConversionOperation)?.Operand;
-                if (Acquire(operation.Collection) != RefKind.Ref)
+
+                switch (Acquire(operation.Collection))
                 {
-                    instance = null;
-                    instance2 = 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;
                 }
-                else if (Acquire(instance2) != RefKind.Ref)
+
+                switch (Acquire(instance2))
                 {
-                    instance2 = 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:
+                        instance2 = null;
+                        break;
                 }
 
                 using var releaser = TryAddForVisit(_handledOperations, instance, out _);
diff --git a/src/Roslyn.Diagnostics.Analyzers/UnitTests/DoNotCopyValueTests.cs b/src/Roslyn.Diagnostics.Analyzers/UnitTests/DoNotCopyValueTests.cs
@@ -3,6 +3,7 @@
 #nullable enable
 
 using System.Threading.Tasks;
+using Microsoft.CodeAnalysis.Testing;
 using Xunit;
 using VerifyCS = Test.Utilities.CSharpSecurityCodeFixVerifier<
     Roslyn.Diagnostics.Analyzers.DoNotCopyValue,
@@ -685,6 +686,58 @@ internal sealed class NonCopyableAttribute : System.Attribute { }
             }.RunAsync();
         }
 
+        [Theory]
+        [CombinatorialData]
+        public async Task AllowCustomForeachEnumeratorParameterReference(
+            [CombinatorialValues("", "ref", "in")] string parameterModifiers,
+            [CombinatorialValues("", "readonly")] string getEnumeratorModifiers)
+        {
+            var source = $@"
+using System.Runtime.InteropServices;
+
+class C
+{{
+    void Method({parameterModifiers} CannotCopy cannotCopy)
+    {{
+        foreach (var obj in {{|#0:cannotCopy|}})
+        {{
+        }}
+    }}
+}}
+
+[NonCopyable]
+struct CannotCopy
+{{
+    public {getEnumeratorModifiers} Enumerator GetEnumerator() => throw null;
+
+    public struct Enumerator
+    {{
+        public object Current => throw null;
+        public bool MoveNext() => throw null;
+    }}
+}}
+
+internal sealed class NonCopyableAttribute : System.Attribute {{ }}
+";
+
+            var expected = (parameterModifiers, getEnumeratorModifiers) switch
+            {
+                // /0/Test0.cs(8,29): warning RS0042: Unsupported use of non-copyable type 'CannotCopy' in 'ParameterReference' operation
+                ("in", "") => new[] { VerifyCS.Diagnostic(DoNotCopyValue.UnsupportedUseRule).WithLocation(0).WithArguments("CannotCopy", "ParameterReference") },
+
+                _ => DiagnosticResult.EmptyDiagnosticResults,
+            };
+
+            var test = new VerifyCS.Test
+            {
+                TestCode = source,
+                LanguageVersion = Microsoft.CodeAnalysis.CSharp.LanguageVersion.CSharp8,
+            };
+
+            test.ExpectedDiagnostics.AddRange(expected);
+            await test.RunAsync();
+        }
+
         [Fact]
         public async Task AllowCustomForeachEnumeratorDisposableObject()
         {
