From 56d9df3e6eaeb06ba134c2303c4058ba131f13a5 Mon Sep 17 00:00:00 2001
From: Josef Pihrt <josef.pihrt@outlook.com>
Date: Wed, 15 Mar 2023 18:53:53 +0100
Subject: [PATCH] Check recursion (RCS1235) (#1054)

---
 ChangeLog.md                                  |  1 +
 .../Analysis/OptimizeMethodCallAnalysis.cs    | 15 +++++++++-----
 .../RCS1235OptimizeMethodCallTests.cs         | 20 +++++++++++++++++++
 3 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/ChangeLog.md b/ChangeLog.md
index 28d4b4fe95..13c0c4c94a 100644
--- a/ChangeLog.md
+++ b/ChangeLog.md
@@ -25,6 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - Do not report ([RCS1170](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1170.md)) when `Microsoft.AspNetCore.Components.InjectAttribute` is used ([#1046](https://github.com/JosefPihrt/Roslynator/pull/1046)).
 - Fix ([RCS1235](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1235.md)) ([#1047](https://github.com/JosefPihrt/Roslynator/pull/1047)).
 - Fix ([RCS1206](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1206.md)) ([#1049](https://github.com/JosefPihrt/Roslynator/pull/1049)).
+- Prevent possible recursion in ([RCS1235](https://github.com/JosefPihrt/Roslynator/blob/main/docs/analyzers/RCS1235.md)) ([#1054](https://github.com/JosefPihrt/Roslynator/pull/1054)).
 
 ## [4.2.0] - 2022-11-27
 
diff --git a/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs b/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs
index 0b63b0321d..ec21986237 100644
--- a/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs
+++ b/src/Analyzers/CSharp/Analysis/OptimizeMethodCallAnalysis.cs
@@ -343,7 +343,9 @@ public static void OptimizeAdd(SyntaxNodeAnalysisContext context, in SimpleMembe
             && invocation.ArgumentList.Arguments[0].Expression is IdentifierNameSyntax identifierName
             && identifierName.Identifier.ValueText == forEachStatement.Identifier.ValueText)
         {
-            ITypeSymbol typeSymbol = context.SemanticModel.GetTypeSymbol(invocationInfo.Expression, context.CancellationToken);
+            SemanticModel semanticModel = context.SemanticModel;
+            CancellationToken cancellationToken = context.CancellationToken;
+            ITypeSymbol typeSymbol = semanticModel.GetTypeSymbol(invocationInfo.Expression, cancellationToken);
 
             if (typeSymbol?.IsKind(SymbolKind.ErrorType) == false)
             {
@@ -351,18 +353,21 @@ public static void OptimizeAdd(SyntaxNodeAnalysisContext context, in SimpleMembe
                 {
                     if (member is IMethodSymbol methodSymbol
                         && methodSymbol.Parameters.Length == 1
-                        && context.SemanticModel.IsAccessible(invocation.SpanStart, methodSymbol)
-                        && context.SemanticModel.IsImplicitConversion(forEachStatement.Expression, methodSymbol.Parameters[0].Type)
+                        && semanticModel.IsAccessible(invocation.SpanStart, methodSymbol)
+                        && semanticModel.IsImplicitConversion(forEachStatement.Expression, methodSymbol.Parameters[0].Type)
+                        && !SymbolEqualityComparer.Default.Equals(
+                            methodSymbol,
+                            semanticModel.GetEnclosingSymbol(forEachStatement.SpanStart, cancellationToken))
                         && forEachStatement.CloseParenToken.TrailingTrivia.IsEmptyOrWhitespace()
                         && invocation.GetLeadingTrivia().IsEmptyOrWhitespace()
                         && (block is null
                             || SyntaxTriviaAnalysis.IsExteriorTriviaEmptyOrWhitespace(block.OpenBraceToken)))
                     {
-                        var forEachVariableSymbol = context.SemanticModel.GetDeclaredSymbol(forEachStatement, context.CancellationToken) as ILocalSymbol;
+                        var forEachVariableSymbol = semanticModel.GetDeclaredSymbol(forEachStatement, cancellationToken) as ILocalSymbol;
 
                         if (forEachVariableSymbol is not null)
                         {
-                            ContainsLocalOrParameterReferenceWalker walker = ContainsLocalOrParameterReferenceWalker.GetInstance(forEachVariableSymbol, context.SemanticModel, context.CancellationToken);
+                            ContainsLocalOrParameterReferenceWalker walker = ContainsLocalOrParameterReferenceWalker.GetInstance(forEachVariableSymbol, semanticModel, cancellationToken);
 
                             walker.Visit(invocationInfo.Expression);
 
diff --git a/src/Tests/Analyzers.Tests/RCS1235OptimizeMethodCallTests.cs b/src/Tests/Analyzers.Tests/RCS1235OptimizeMethodCallTests.cs
index de21a8ed5a..4f700c1609 100644
--- a/src/Tests/Analyzers.Tests/RCS1235OptimizeMethodCallTests.cs
+++ b/src/Tests/Analyzers.Tests/RCS1235OptimizeMethodCallTests.cs
@@ -625,6 +625,26 @@ class C
         return (positive, negative);
     }
 }
+");
+    }
+
+    [Fact, Trait(Traits.Analyzer, DiagnosticIdentifiers.OptimizeMethodCall)]
+    public async Task TestNoDiagnostic_Recursion()
+    {
+        await VerifyNoDiagnosticAsync(@"
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+
+class MyCollection : Collection<string>
+{
+    public void AddRange(IEnumerable<string> data)
+    {
+        foreach (string item in data)
+        {
+            this.Add(item);
+        }
+    }
+}
 ");
     }
 }