Fix issue where we were recommending removing the GetEnumerator method used in a foreach (#75903)

CyrusNajmabadi · web-flow · commit afc0e8a1ba4d · 2024-11-14T08:29:18.000-05:00
Fixes #57470
diff --git a/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/CSharp/Analyzers/RemoveUnusedMembers/CSharpRemoveUnusedMembersDiagnosticAnalyzer.cs
@@ -7,6 +7,7 @@
 using System.Threading;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.RemoveUnusedMembers;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 
@@ -20,6 +21,8 @@ internal sealed class CSharpRemoveUnusedMembersDiagnosticAnalyzer
         TypeDeclarationSyntax,
         MemberDeclarationSyntax>
 {
+    protected override ISemanticFacts SemanticFacts => CSharpSemanticFacts.Instance;
+
     protected override IEnumerable<TypeDeclarationSyntax> GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken)
     {
         return namedType.DeclaringSyntaxReferences
diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedMembers/RemoveUnusedMembersTests.cs
@@ -15,7 +15,6 @@
 
 namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnusedMembers;
 
-using static Microsoft.CodeAnalysis.CSharp.UsePatternCombinators.AnalyzedPattern;
 using VerifyCS = CSharpCodeFixVerifier<
     CSharpRemoveUnusedMembersDiagnosticAnalyzer,
     CSharpRemoveUnusedMembersCodeFixProvider>;
@@ -3243,4 +3242,33 @@ class C
 
         await VerifyCS.VerifyCodeFixAsync(code, fixedCode);
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/57470")]
+    public async Task TestForeach()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                using System.Collections;
+
+                static class Program
+                {
+                    static void Main(string[] args)
+                    {
+                        foreach (var i in 1..10)
+                            Console.WriteLine(i);
+                    }
+
+                    static IEnumerator GetEnumerator(this Range range)
+                    {
+                        for (int i = range.Start.Value; i < range.End.Value; i++)
+                            yield return i;
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp13,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net90,
+        }.RunAsync();
+    }
 }
diff --git a/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnusedMembers/AbstractRemoveUnusedMembersDiagnosticAnalyzer.cs
@@ -13,6 +13,7 @@
 using Microsoft.CodeAnalysis.CodeQuality;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.LanguageService;
 using Microsoft.CodeAnalysis.Operations;
 using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
@@ -25,8 +26,11 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
     TDocumentationCommentTriviaSyntax,
     TIdentifierNameSyntax,
     TTypeDeclarationSyntax,
-    TMemberDeclarationSyntax>
-    : AbstractCodeQualityDiagnosticAnalyzer
+    TMemberDeclarationSyntax>()
+    : AbstractCodeQualityDiagnosticAnalyzer(
+        [s_removeUnusedMembersRule, s_removeUnreadMembersRule],
+        // We want to analyze references in generated code, but not report unused members in generated code.
+        GeneratedCodeAnalysisFlags.Analyze)
     where TDocumentationCommentTriviaSyntax : SyntaxNode
     where TIdentifierNameSyntax : SyntaxNode
     where TTypeDeclarationSyntax : TMemberDeclarationSyntax
@@ -56,11 +60,7 @@ internal abstract class AbstractRemoveUnusedMembersDiagnosticAnalyzer<
         new LocalizableResourceString(nameof(AnalyzersResources.Private_member_0_can_be_removed_as_the_value_assigned_to_it_is_never_read), AnalyzersResources.ResourceManager, typeof(AnalyzersResources)),
         hasAnyCodeStyleOption: false, isUnnecessary: true);
 
-    protected AbstractRemoveUnusedMembersDiagnosticAnalyzer()
-        : base([s_removeUnusedMembersRule, s_removeUnreadMembersRule],
-               GeneratedCodeAnalysisFlags.Analyze) // We want to analyze references in generated code, but not report unused members in generated code.
-    {
-    }
+    protected abstract ISemanticFacts SemanticFacts { get; }
 
     protected abstract IEnumerable<TTypeDeclarationSyntax> GetTypeDeclarations(INamedTypeSymbol namedType, CancellationToken cancellationToken);
     protected abstract SyntaxList<TMemberDeclarationSyntax> GetMembers(TTypeDeclarationSyntax typeDeclaration);
@@ -86,7 +86,8 @@ protected virtual void HandleNamedTypeSymbolStart(SymbolStartAnalysisContext con
 
     private sealed class CompilationAnalyzer
     {
-        private readonly object _gate;
+        private readonly object _gate = new();
+
         /// <summary>
         /// State map for candidate member symbols, with the value indicating how each symbol is used in executable code.
         /// </summary>
@@ -114,7 +115,6 @@ private CompilationAnalyzer(
             Compilation compilation,
             AbstractRemoveUnusedMembersDiagnosticAnalyzer<TDocumentationCommentTriviaSyntax, TIdentifierNameSyntax, TTypeDeclarationSyntax, TMemberDeclarationSyntax> analyzer)
         {
-            _gate = new object();
             _analyzer = analyzer;
 
             _taskType = compilation.TaskType();
@@ -204,15 +204,17 @@ private void RegisterActions(CompilationStartAnalysisContext compilationStartCon
                 symbolStartContext.RegisterOperationAction(AnalyzeInvocationOperation, OperationKind.Invocation);
                 symbolStartContext.RegisterOperationAction(AnalyzeNameOfOperation, OperationKind.NameOf);
                 symbolStartContext.RegisterOperationAction(AnalyzeObjectCreationOperation, OperationKind.ObjectCreation);
+                symbolStartContext.RegisterOperationAction(AnalyzeLoopOperation, OperationKind.Loop);
 
                 // We bail out reporting diagnostics for named types if it contains following kind of operations:
                 //  1. Invalid operations, i.e. erroneous code:
                 //     We do so to ensure that we don't report false positives during editing scenarios in the IDE, where the user
                 //     is still editing code and fixing unresolved references to symbols, such as overload resolution errors.
                 //  2. Dynamic operations, where we do not know the exact member being referenced at compile time.
                 //  3. Operations with OperationKind.None.
-                symbolStartContext.RegisterOperationAction(_ => hasUnsupportedOperation = true, OperationKind.Invalid, OperationKind.None,
-                    OperationKind.DynamicIndexerAccess, OperationKind.DynamicInvocation, OperationKind.DynamicMemberReference, OperationKind.DynamicObjectCreation);
+                symbolStartContext.RegisterOperationAction(
+                    _ => hasUnsupportedOperation = true,
+                    OperationKind.Invalid, OperationKind.None, OperationKind.DynamicIndexerAccess, OperationKind.DynamicInvocation, OperationKind.DynamicMemberReference, OperationKind.DynamicObjectCreation);
 
                 symbolStartContext.RegisterSymbolEndAction(symbolEndContext => OnSymbolEnd(symbolEndContext, hasUnsupportedOperation));
 
@@ -369,6 +371,18 @@ memberReference.Parent is IIncrementOrDecrementOperation ||
             }
         }
 
+        private void AnalyzeLoopOperation(OperationAnalysisContext operationContext)
+        {
+            var operation = operationContext.Operation;
+            if (operation is not IForEachLoopOperation loopOperation)
+                return;
+
+            var symbols = _analyzer.SemanticFacts.GetForEachSymbols(operation.SemanticModel!, loopOperation.Syntax);
+            OnSymbolUsage(symbols.CurrentProperty, ValueUsageInfo.Read);
+            OnSymbolUsage(symbols.GetEnumeratorMethod, ValueUsageInfo.Read);
+            OnSymbolUsage(symbols.MoveNextMethod, ValueUsageInfo.Read);
+        }
+
         private void AnalyzeInvocationOperation(OperationAnalysisContext operationContext)
         {
             var targetMethod = ((IInvocationOperation)operationContext.Operation).TargetMethod.OriginalDefinition;
@@ -380,9 +394,7 @@ private void AnalyzeInvocationOperation(OperationAnalysisContext operationContex
             // If the invoked method is a reduced extension method, also mark the original
             // method from which it was reduced as "used".
             if (targetMethod.ReducedFrom != null)
-            {
                 OnSymbolUsage(targetMethod.ReducedFrom, ValueUsageInfo.Read);
-            }
         }
 
         private void AnalyzeNameOfOperation(OperationAnalysisContext operationContext)
diff --git a/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb b/src/Analyzers/VisualBasic/Analyzers/RemoveUnusedMembers/VisualBasicRemoveUnusedMembersDiagnosticAnalyzer.vb
@@ -4,6 +4,7 @@
 
 Imports System.Threading
 Imports Microsoft.CodeAnalysis.Diagnostics
+Imports Microsoft.CodeAnalysis.LanguageService
 Imports Microsoft.CodeAnalysis.RemoveUnusedMembers
 Imports Microsoft.CodeAnalysis.VisualBasic.Syntax
 
@@ -16,6 +17,8 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedMembers
             TypeBlockSyntax,
             StatementSyntax)
 
+        Protected Overrides ReadOnly Property SemanticFacts As ISemanticFacts = VisualBasicSemanticFacts.Instance
+
         Protected Overrides Sub HandleNamedTypeSymbolStart(context As SymbolStartAnalysisContext, onSymbolUsageFound As Action(Of ISymbol, ValueUsageInfo))
             ' Mark all methods with handles clause as having a read reference
             ' to ensure that we consider the method as "used".
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SemanticFacts/CSharpSemanticFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SemanticFacts/CSharpSemanticFacts.cs
@@ -166,22 +166,18 @@ private static void AppendAliasNames(IEnumerable<BaseNamespaceDeclarationSyntax>
         }
     }
 
-    public ForEachSymbols GetForEachSymbols(SemanticModel semanticModel, SyntaxNode forEachStatement)
+    public ForEachSymbols GetForEachSymbols(SemanticModel semanticModel, SyntaxNode node)
     {
-        if (forEachStatement is CommonForEachStatementSyntax csforEachStatement)
-        {
-            var info = semanticModel.GetForEachStatementInfo(csforEachStatement);
-            return new ForEachSymbols(
-                info.GetEnumeratorMethod,
-                info.MoveNextMethod,
-                info.CurrentProperty,
-                info.DisposeMethod,
-                info.ElementType);
-        }
-        else
-        {
+        if (node is not CommonForEachStatementSyntax forEachStatement)
             return default;
-        }
+
+        var info = semanticModel.GetForEachStatementInfo(forEachStatement);
+        return new ForEachSymbols(
+            info.GetEnumeratorMethod,
+            info.MoveNextMethod,
+            info.CurrentProperty,
+            info.DisposeMethod,
+            info.ElementType);
     }
 
     public SymbolInfo GetCollectionInitializerSymbolInfo(SemanticModel semanticModel, SyntaxNode node, CancellationToken cancellationToken)
