[Performance] Simplify UseSpanBasedStringConcat for better performance (#6870)

* [Performance] Simplify UseSpanBasedStringConcat for better performance

* Remove unused using

Author: Youssef1313

src/NetAnalyzers/CSharp/Microsoft.NetCore.Analyzers/Runtime/CSharpUseSpanBasedStringConcat.cs

@@ -1,6 +1,5 @@
 // Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
 
-using System.Diagnostics.CodeAnalysis;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
@@ -11,20 +10,10 @@ namespace Microsoft.NetCore.CSharp.Analyzers.Runtime
     [DiagnosticAnalyzer(LanguageNames.CSharp)]
     public sealed class CSharpUseSpanBasedStringConcat : UseSpanBasedStringConcat
     {
-        private protected override bool TryGetTopMostConcatOperation(IBinaryOperation binaryOperation, [NotNullWhen(true)] out IBinaryOperation? rootBinaryOperation)
+        private protected override bool IsTopMostConcatOperation(IBinaryOperation binaryOperation)
         {
-            if (!IsConcatOperation(binaryOperation))
-            {
-                rootBinaryOperation = default;
-                return false;
-            }
-
-            var current = binaryOperation;
-            while (current.Parent is IBinaryOperation parentBinaryOperation && IsConcatOperation(parentBinaryOperation))
-                current = parentBinaryOperation;
-
-            rootBinaryOperation = current;
-            return true;
+            return IsConcatOperation(binaryOperation) &&
+                (binaryOperation.Parent is not IBinaryOperation parentBinary || !IsConcatOperation(parentBinary));
 
             static bool IsConcatOperation(IBinaryOperation operation)
             {

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/UseSpanBasedStringConcat.cs

@@ -6,7 +6,6 @@
 using System.Linq;
 using Analyzer.Utilities;
 using Analyzer.Utilities.Extensions;
-using Analyzer.Utilities.PooledObjects;
 using Microsoft.CodeAnalysis;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
@@ -32,11 +31,9 @@ public abstract class UseSpanBasedStringConcat : DiagnosticAnalyzer
             isDataflowRule: false);
 
         /// <summary>
-        /// If the specified binary operation is a string concatenation operation, we try to walk up to the top-most
-        /// string-concatenation operation that it is part of. If it is not a string-concatenation operation, we simply
-        /// return false.
+        /// Returns true if the specified binary operation is a top-most string concatenation operation
         /// </summary>
-        private protected abstract bool TryGetTopMostConcatOperation(IBinaryOperation binaryOperation, [NotNullWhen(true)] out IBinaryOperation? rootBinaryOperation);
+        private protected abstract bool IsTopMostConcatOperation(IBinaryOperation binaryOperation);
 
         /// <summary>
         /// Remove the built in implicit conversion on operands to concat.
@@ -59,49 +56,19 @@ private void OnCompilationStart(CompilationStartAnalysisContext context)
             if (!RequiredSymbols.TryGetSymbols(context.Compilation, out RequiredSymbols symbols))
                 return;
 
-            context.RegisterOperationBlockStartAction(OnOperationBlockStart);
-            return;
-
-            // Local functions
-            void OnOperationBlockStart(OperationBlockStartAnalysisContext context)
+            context.RegisterOperationAction(context =>
             {
-                //  Maintain set of all top-most concat operations so we don't report sub-expressions of an
-                //  already-reported violation. 
+                //  Report diagnostic only if the operation is the top-most concat operation so we don't report sub-expressions of an
+                //  already-reported violation.
                 //  We also don't report any diagnostic if the concat operation has too many operands for the span-based
                 //  Concat overloads to handle.
-                var topMostConcatOperations = TemporarySet<IBinaryOperation>.Empty;
-
-                context.RegisterOperationAction(PopulateTopMostConcatOperations, OperationKind.Binary);
-                context.RegisterOperationBlockEndAction(ReportDiagnosticsOnRootConcatOperationsWithSubstringCalls);
-
-                void PopulateTopMostConcatOperations(OperationAnalysisContext context)
-                {
-                    //  If the current operation is a string-concatenation operation, walk up to the top-most concat
-                    //  operation and add it to the set.
-                    var binary = (IBinaryOperation)context.Operation;
-                    if (!TryGetTopMostConcatOperation(binary, out var topMostConcatOperation))
-                        return;
-
-                    topMostConcatOperations.Add(topMostConcatOperation, context.CancellationToken);
-                }
-
-                void ReportDiagnosticsOnRootConcatOperationsWithSubstringCalls(OperationBlockAnalysisContext context)
-                {
-                    //  We report diagnostics for all top-most concat operations that contain 
-                    //  direct or conditional substring invocations when there is an applicable span-based overload of
-                    //  the string.Concat method.
-                    //  We don't report when the concatenation contains anything other than strings or character literals.
-                    foreach (var operation in topMostConcatOperations.NonConcurrentEnumerable)
-                    {
-                        if (ShouldBeReported(operation))
-                        {
-                            context.ReportDiagnostic(operation.CreateDiagnostic(Rule));
-                        }
-                    }
+                var binary = (IBinaryOperation)context.Operation;
+                if (IsTopMostConcatOperation(binary) && ShouldBeReported(binary))
+                    context.ReportDiagnostic(binary.CreateDiagnostic(Rule));
+            }, OperationKind.Binary);
+            return;
 
-                    topMostConcatOperations.Free(context.CancellationToken);
-                }
-            }
+            // Local functions
 
             bool ShouldBeReported(IBinaryOperation topMostConcatOperation)
             {
@@ -133,8 +100,8 @@ bool ShouldBeReported(IBinaryOperation topMostConcatOperation)
 
             bool IsAnyDirectOrConditionalSubstringInvocation(IOperation operation)
             {
-                if (operation is IConditionalAccessOperation conditionallAccessOperation)
-                    operation = conditionallAccessOperation.WhenNotNull;
+                if (operation is IConditionalAccessOperation conditionalAccessOperation)
+                    operation = conditionalAccessOperation.WhenNotNull;
 
                 return operation is IInvocationOperation invocation && symbols.IsAnySubstringMethod(invocation.TargetMethod);
             }

src/NetAnalyzers/VisualBasic/Microsoft.NetCore.Analyzers/Runtime/BasicUseSpanBasedStringConcat.vb

@@ -10,22 +10,13 @@ Namespace Microsoft.NetCore.VisualBasic.Analyzers.Runtime
     <DiagnosticAnalyzer(LanguageNames.VisualBasic)>
     Public NotInheritable Class BasicUseSpanBasedStringConcat : Inherits UseSpanBasedStringConcat
 
-        Private Protected Overrides Function TryGetTopMostConcatOperation(binaryOperation As IBinaryOperation, ByRef rootBinaryOperation As IBinaryOperation) As Boolean
-
+        Private Protected Overrides Function IsTopMostConcatOperation(binaryOperation As IBinaryOperation) As Boolean
             If Not IsStringConcatOperation(binaryOperation) Then
-                rootBinaryOperation = Nothing
                 Return False
             End If
 
-            Dim parentBinaryOperation = binaryOperation
-            Dim current As IBinaryOperation
-            Do
-                current = parentBinaryOperation
-                parentBinaryOperation = TryCast(WalkUpImplicitConversionToObject(current.Parent), IBinaryOperation)
-            Loop While parentBinaryOperation IsNot Nothing AndAlso IsStringConcatOperation(parentBinaryOperation)
-
-            rootBinaryOperation = current
-            Return True
+            Dim parentBinaryOperation = TryCast(WalkUpImplicitConversionToObject(binaryOperation.Parent), IBinaryOperation)
+            Return parentBinaryOperation Is Nothing OrElse Not IsStringConcatOperation(parentBinaryOperation)
         End Function
 
         Private Protected Overrides Function WalkDownBuiltInImplicitConversionOnConcatOperand(operand As IOperation) As IOperation