Fix broken 'convert if to switch' case with recursive analysis

src/Features/CSharp/Portable/ConvertIfToSwitch/CSharpConvertIfToSwitchCodeRefactoringProvider.cs

@@ -14,15 +14,11 @@
 namespace Microsoft.CodeAnalysis.CSharp.ConvertIfToSwitch;
 
 [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.ConvertIfToSwitch), Shared]
-internal sealed partial class CSharpConvertIfToSwitchCodeRefactoringProvider
+[method: ImportingConstructor]
+[method: SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
+internal sealed partial class CSharpConvertIfToSwitchCodeRefactoringProvider()
     : AbstractConvertIfToSwitchCodeRefactoringProvider<IfStatementSyntax, ExpressionSyntax, BinaryExpressionSyntax, PatternSyntax>
 {
-    [ImportingConstructor]
-    [SuppressMessage("RoslynDiagnosticsReliability", "RS0033:Importing constructor should be [Obsolete]", Justification = "Used in test code: https://github.com/dotnet/roslyn/issues/42814")]
-    public CSharpConvertIfToSwitchCodeRefactoringProvider()
-    {
-    }
-
     public override string GetTitle(bool forSwitchExpression)
         => forSwitchExpression
             ? CSharpFeaturesResources.Convert_to_switch_expression

src/Features/CSharpTest/ConvertIfToSwitch/ConvertIfToSwitchTests.cs

@@ -18,7 +18,7 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.CodeActions.ConvertIfTo
 
 [UseExportProvider]
 [Trait(Traits.Feature, Traits.Features.CodeActionsConvertIfToSwitch)]
-public class ConvertIfToSwitchTests
+public sealed class ConvertIfToSwitchTests
 {
     [Fact]
     public async Task TestUnreachableEndPoint()
@@ -3121,4 +3121,59 @@ void M()
             CodeActionValidationMode = CodeActionValidationMode.None,
         }.RunAsync();
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/71295")]
+    public async Task TestCodeAfterElseIf()
+    {
+        var source =
+            """
+            class C
+            {
+                void TestThing(int a, int b)
+                {
+                    $$if (a == 1 && b == 0)
+                    {
+                        TestThing(0, 1);
+                    }
+                    else
+                    {
+                        if (a == 2 && b == 1)
+                        {
+                            a = b; b = 0;
+                        }
+                        TestThing(a, b);
+                    }
+                }
+            }
+            """;
+        var fixedSource =
+            """
+            class C
+            {
+                void TestThing(int a, int b)
+                {
+                    switch (a)
+                    {
+                        case 1 when b == 0:
+                            TestThing(0, 1);
+                            break;
+                        default:
+                            if (a == 2 && b == 1)
+                            {
+                                a = b; b = 0;
+                            }
+                            TestThing(a, b);
+                            break;
+                    }
+                }
+            }
+            """;
+
+        await new VerifyCS.Test
+        {
+            TestCode = source,
+            FixedCode = fixedSource,
+            CodeActionValidationMode = CodeActionValidationMode.None,
+        }.RunAsync();
+    }
 }

src/Features/Core/Portable/ConvertIfToSwitch/AbstractConvertIfToSwitchCodeRefactoringProvider.Analyzer.cs

@@ -44,7 +44,7 @@ internal abstract partial class AbstractConvertIfToSwitchCodeRefactoringProvider
     //        | ( <expr0> <= <const> | <const> >= <expr0> )
     //           && ( <expr0> >= <const> | <const> <= <expr0> )  // C#, VB
     //
-    internal abstract class Analyzer
+    internal abstract class Analyzer(ISyntaxFacts syntaxFacts, AbstractConvertIfToSwitchCodeRefactoringProvider<TIfStatementSyntax, TExpressionSyntax, TIsExpressionSyntax, TPatternSyntax>.Feature features)
     {
         public abstract bool CanConvert(IConditionalOperation operation);
         public abstract bool HasUnreachableEndPoint(IOperation operation);
@@ -61,23 +61,17 @@ internal abstract class Analyzer
         /// Holds the type of the <see cref="_switchTargetExpression"/>
         /// </summary>
         private ITypeSymbol? _switchTargetType = null!;
-        private readonly ISyntaxFacts _syntaxFacts;
+        private readonly ISyntaxFacts _syntaxFacts = syntaxFacts;
 
-        protected Analyzer(ISyntaxFacts syntaxFacts, Feature features)
-        {
-            _syntaxFacts = syntaxFacts;
-            Features = features;
-        }
-
-        public Feature Features { get; }
+        public Feature Features { get; } = features;
 
         public bool Supports(Feature feature)
             => (Features & feature) != 0;
 
         public (ImmutableArray<AnalyzedSwitchSection>, SyntaxNode TargetExpression) AnalyzeIfStatementSequence(ReadOnlySpan<IOperation> operations)
         {
             using var _ = ArrayBuilder<AnalyzedSwitchSection>.GetInstance(out var sections);
-            if (!ParseIfStatementSequence(operations, sections, out var defaultBodyOpt))
+            if (!ParseIfStatementSequence(operations, sections, topLevel: true, out var defaultBodyOpt))
             {
                 return default;
             }
@@ -98,7 +92,11 @@ public bool Supports(Feature feature)
         //        : if (<section-expr>) { <unreachable-end-point> }, ( return | throw )
         //        | <if-statement>
         //
-        private bool ParseIfStatementSequence(ReadOnlySpan<IOperation> operations, ArrayBuilder<AnalyzedSwitchSection> sections, out IOperation? defaultBodyOpt)
+        private bool ParseIfStatementSequence(
+            ReadOnlySpan<IOperation> operations,
+            ArrayBuilder<AnalyzedSwitchSection> sections,
+            bool topLevel,
+            out IOperation? defaultBodyOpt)
         {
             var current = 0;
             while (current < operations.Length &&
@@ -113,7 +111,17 @@ private bool ParseIfStatementSequence(ReadOnlySpan<IOperation> operations, Array
             if (current == 0)
             {
                 // didn't consume a sequence of if-statements with unreachable ends.  Check for the last case.
-                return operations.Length > 0 && ParseIfStatement(operations[0], sections, out defaultBodyOpt);
+                if (operations.Length == 0)
+                    return false;
+
+                // If we're in the initial state, it's fine for there to be many operations that follow.  We're just
+                // trying to check if the first one completes our analysis (and we'll not touch the ones that
+                // follow). However, if we're actually in one of the recursive calls, it's *not* ok to ignore the 
+                // following ops as those may impact if the higher call into us is ok.  So in that case, we do not
+                // allow the parsing to succeed if we have more than one operation left.
+                return topLevel
+                    ? operations is [var op1, ..] && ParseIfStatement(op1, sections, out defaultBodyOpt)
+                    : operations is [var op2] && ParseIfStatement(op2, sections, out defaultBodyOpt);
             }
             else
             {
@@ -173,7 +181,7 @@ private bool ParseIfStatement(IOperation operation, ArrayBuilder<AnalyzedSwitchS
         private bool ParseIfStatementOrBlock(IOperation op, ArrayBuilder<AnalyzedSwitchSection> sections, out IOperation? defaultBodyOpt)
         {
             return op is IBlockOperation block
-                ? ParseIfStatementSequence(block.Operations.AsSpan(), sections, out defaultBodyOpt)
+                ? ParseIfStatementSequence(block.Operations.AsSpan(), sections, topLevel: false, out defaultBodyOpt)
                 : ParseIfStatement(op, sections, out defaultBodyOpt);
         }