Add a code quality test for switching on a tuple literal

The test exposed some parsing precedence issues for the switch expression which are fixed here. The => following the when clause was being consumed as part of the when expression (as a lambda). Changed the precedence of the when expression to a coalescing expression to forbid a lambda there.

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -7992,7 +7992,7 @@ private SwitchSectionSyntax ParseSwitchSection()
                         }
                         else
                         {
-                            var node = CheckRecursivePatternFeature(ParseExpressionOrPattern(forCase: true, precedence: Precedence.Ternary));
+                            var node = CheckRecursivePatternFeature(ParseExpressionOrPattern(whenIsKeyword: true, precedence: Precedence.Ternary));
                             if (this.CurrentToken.ContextualKind == SyntaxKind.WhenKeyword && node is ExpressionSyntax)
                             {
                                 // if there is a 'where' token, we treat a case expression as a constant pattern.
@@ -8000,7 +8000,7 @@ private SwitchSectionSyntax ParseSwitchSection()
                             }
                             if (node is PatternSyntax)
                             {
-                                var whenClause = ParseWhenClause();
+                                var whenClause = ParseWhenClause(Precedence.Expression);
                                 colon = this.EatToken(SyntaxKind.ColonToken);
                                 label = _syntaxFactory.CasePatternSwitchLabel(specifier, (PatternSyntax)node, whenClause, colon);
                                 label = CheckFeatureAvailability(label, MessageID.IDS_FeaturePatternMatching);
@@ -8311,7 +8311,7 @@ private VariableDesignationSyntax ParseSimpleDesignation()
             }
         }
 
-        private WhenClauseSyntax ParseWhenClause()
+        private WhenClauseSyntax ParseWhenClause(Precedence precedence)
         {
             if (this.CurrentToken.ContextualKind != SyntaxKind.WhenKeyword)
             {
@@ -8319,7 +8319,7 @@ private WhenClauseSyntax ParseWhenClause()
             }
 
             var when = this.EatContextualToken(SyntaxKind.WhenKeyword);
-            var condition = ParseSubExpression(Precedence.Expression);
+            var condition = ParseSubExpression(precedence);
             return _syntaxFactory.WhenClause(when, condition);
         }
 
@@ -9108,63 +9108,12 @@ private ExpressionSyntax ParseSubExpressionCore(Precedence precedence)
 
             else if (tk == SyntaxKind.SwitchKeyword && precedence < Precedence.Coalescing && this.PeekToken(1).Kind == SyntaxKind.OpenBraceToken)
             {
-                // PROTOTYPE(patterns2): for better error recovery when an expression is typed on a line before
-                // a switch statement, we should check if the cases between the parens look like cases with
-                // arrows, and whether the
-                // switch keyword is on the same line as the end of the expression. If those are not satisfied,
-                // we should refuse to consume the switch token here and instead let a "missing semicolon"
-                // occur in a caller. For now we just put up with poor error recovery in that case.
-                var governingExpression = leftOperand;
-                var switchKeyword = this.EatToken();
-                var openBrace = this.EatToken(SyntaxKind.OpenBraceToken);
-                var arms = this.ParseSwitchExpressionArms();
-                // PROTOTYPE(patterns2): Should skip any unexpected tokens up through the close brace token.
-                var closeBrace = this.EatToken(SyntaxKind.CloseBraceToken);
-                leftOperand = _syntaxFactory.SwitchExpression(governingExpression, switchKeyword, openBrace, arms, closeBrace);
-                leftOperand = this.CheckFeatureAvailability(leftOperand, MessageID.IDS_FeatureRecursivePatterns);
+                leftOperand = ParseSwitchExpression(leftOperand);
             }
 
             return leftOperand;
         }
 
-        private SeparatedSyntaxList<SwitchExpressionArmSyntax> ParseSwitchExpressionArms()
-        {
-            // PROTOTYPE(patterns2): Error recovery here leaves much to be desired.
-            var arms = _pool.AllocateSeparated<SwitchExpressionArmSyntax>();
-            do
-            {
-                // Use a precedence that excludes lambdas, assignments, and a ternary which could have a
-                // lambda on the right, because we need the parser to leave the EqualsGreaterThanToken
-                // to be consumed by the switch arm. The strange side-effect of that is that the ternary
-                // expression is not permitted as a constant expression here; it would have to be parenthesized.
-                var pattern = ParsePattern(Precedence.Coalescing);
-                var whenClause = ParseWhenClause();
-                var arrow = this.EatToken(SyntaxKind.EqualsGreaterThanToken);
-                var expression = ParseExpressionCore();
-                var switchExpressionCase = _syntaxFactory.SwitchExpressionArm(pattern, whenClause, arrow, expression);
-                arms.Add(switchExpressionCase);
-                if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
-                {
-                    var commaToken = this.EatToken();
-                    if (this.CurrentToken.Kind == SyntaxKind.CloseBraceToken)
-                    {
-                        commaToken = this.AddError(commaToken, ErrorCode.ERR_UnexpectedToken, this.CurrentToken.Text);
-                    }
-
-                    arms.AddSeparator(commaToken);
-                }
-                else
-                {
-                    break;
-                }
-            }
-            while (this.CurrentToken.Kind != SyntaxKind.CloseBraceToken);
-
-            SeparatedSyntaxList<SwitchExpressionArmSyntax> result = arms;
-            _pool.Free(arms);
-            return result;
-        }
-
         private ExpressionSyntax ParseDeclarationExpression(ParseTypeMode mode, MessageID feature)
         {
             TypeSyntax type = this.ParseType(mode);

src/Compilers/CSharp/Portable/Parser/LanguageParser_Patterns.cs

@@ -312,9 +312,9 @@ private bool ScanDesignation(bool permitTuple)
         /// It is used for parsing patterns in the switch cases. It never returns constant pattern, because for a `case` we
         /// need to use a pre-pattern-matching syntax node for a constant case.
         /// </summary>
-        /// <param name="forCase">prevents the use of "when" for the identifier</param>
+        /// <param name="whenIsKeyword">prevents the use of "when" for the identifier</param>
         /// <returns></returns>
-        private CSharpSyntaxNode ParseExpressionOrPattern(bool forCase, Precedence precedence)
+        private CSharpSyntaxNode ParseExpressionOrPattern(bool whenIsKeyword, Precedence precedence)
         {
             // handle common error recovery situations during typing
             var tk = this.CurrentToken.Kind;
@@ -356,10 +356,10 @@ private CSharpSyntaxNode ParseExpressionOrPattern(bool forCase, Precedence prece
                     }
                 }
 
-                PatternSyntax p = ParsePatternContinued(type, forCase);
+                PatternSyntax p = ParsePatternContinued(type, whenIsKeyword);
                 if (p != null)
                 {
-                    return (forCase && p is ConstantPatternSyntax c) ? c.expression : (CSharpSyntaxNode)p;
+                    return (whenIsKeyword && p is ConstantPatternSyntax c) ? c.expression : (CSharpSyntaxNode)p;
                 }
 
                 this.Reset(ref resetPoint);
@@ -371,7 +371,7 @@ private CSharpSyntaxNode ParseExpressionOrPattern(bool forCase, Precedence prece
             }
         }
 
-        private PatternSyntax ParsePatternContinued(TypeSyntax type, bool forCase)
+        private PatternSyntax ParsePatternContinued(TypeSyntax type, bool whenIsKeyword)
         {
             bool parsePropertySubpattern(out PropertySubpatternSyntax propertySubpatternResult)
             {
@@ -388,7 +388,7 @@ bool parsePropertySubpattern(out PropertySubpatternSyntax propertySubpatternResu
             bool parseDesignation(out VariableDesignationSyntax designationResult)
             {
                 designationResult = null;
-                if (this.IsTrueIdentifier() && (!forCase || this.CurrentToken.ContextualKind != SyntaxKind.WhenKeyword))
+                if (this.IsTrueIdentifier() && (!whenIsKeyword || this.CurrentToken.ContextualKind != SyntaxKind.WhenKeyword))
                 {
                     designationResult = ParseSimpleDesignation();
                     return true;
@@ -483,9 +483,9 @@ bool looksLikeCast()
             return null;
         }
 
-        private PatternSyntax ParsePattern(Precedence precedence)
+        private PatternSyntax ParsePattern(Precedence precedence, bool whenIsKeyword = false)
         {
-            var node = ParseExpressionOrPattern(forCase: false, precedence: precedence);
+            var node = ParseExpressionOrPattern(whenIsKeyword: whenIsKeyword, precedence: precedence);
             switch (node)
             {
                 case PatternSyntax pattern:
@@ -607,5 +607,62 @@ private bool IsPossibleSubpatternElement()
                 p => p.CurrentToken.Kind == closeKind || p.CurrentToken.Kind == SyntaxKind.SemicolonToken || p.IsTerminator(),
                 expected);
         }
+
+        private ExpressionSyntax ParseSwitchExpression(ExpressionSyntax leftOperand)
+        {
+            // PROTOTYPE(patterns2): for better error recovery when an expression is typed on a line before
+            // a switch statement, we should check if the cases between the parens look like cases with
+            // arrows, and whether the
+            // switch keyword is on the same line as the end of the expression. If those are not satisfied,
+            // we should refuse to consume the switch token here and instead let a "missing semicolon"
+            // occur in a caller. For now we just put up with poor error recovery in that case.
+            var governingExpression = leftOperand;
+            var switchKeyword = this.EatToken();
+            var openBrace = this.EatToken(SyntaxKind.OpenBraceToken);
+            var arms = this.ParseSwitchExpressionArms();
+            // PROTOTYPE(patterns2): Should skip any unexpected tokens up through the close brace token.
+            var closeBrace = this.EatToken(SyntaxKind.CloseBraceToken);
+            var result = _syntaxFactory.SwitchExpression(governingExpression, switchKeyword, openBrace, arms, closeBrace);
+            result = this.CheckFeatureAvailability(result, MessageID.IDS_FeatureRecursivePatterns);
+            return result;
+        }
+
+        private SeparatedSyntaxList<SwitchExpressionArmSyntax> ParseSwitchExpressionArms()
+        {
+            // PROTOTYPE(patterns2): Error recovery here leaves much to be desired.
+            var arms = _pool.AllocateSeparated<SwitchExpressionArmSyntax>();
+            do
+            {
+                // Use a precedence that excludes lambdas, assignments, and a ternary which could have a
+                // lambda on the right, because we need the parser to leave the EqualsGreaterThanToken
+                // to be consumed by the switch arm. The strange side-effect of that is that the ternary
+                // expression is not permitted as a constant expression here; it would have to be parenthesized.
+                var pattern = ParsePattern(Precedence.Coalescing, whenIsKeyword: true);
+                var whenClause = ParseWhenClause(Precedence.Coalescing);
+                var arrow = this.EatToken(SyntaxKind.EqualsGreaterThanToken);
+                var expression = ParseExpressionCore();
+                var switchExpressionCase = _syntaxFactory.SwitchExpressionArm(pattern, whenClause, arrow, expression);
+                arms.Add(switchExpressionCase);
+                if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
+                {
+                    var commaToken = this.EatToken();
+                    if (this.CurrentToken.Kind == SyntaxKind.CloseBraceToken)
+                    {
+                        commaToken = this.AddError(commaToken, ErrorCode.ERR_UnexpectedToken, this.CurrentToken.Text);
+                    }
+
+                    arms.AddSeparator(commaToken);
+                }
+                else
+                {
+                    break;
+                }
+            }
+            while (this.CurrentToken.Kind != SyntaxKind.CloseBraceToken);
+
+            SeparatedSyntaxList<SwitchExpressionArmSyntax> result = arms;
+            _pool.Free(arms);
+            return result;
+        }
     }
 }

src/Compilers/CSharp/Test/Emit/CodeGen/PatternTests.cs

@@ -1125,6 +1125,198 @@ .maxstack  2
   IL_0013:  ret
   IL_0014:  ldc.i4.0
   IL_0015:  ret
+}");
+        }
+
+        [Fact, WorkItem(20641, "https://github.com/dotnet/roslyn/issues/20641")]
+        public void TupleSwitch01()
+        {
+            var source = @"using System;
+
+public class Door
+{
+    public DoorState State;
+
+    public enum DoorState { Opened, Closed, Locked }
+
+    public enum Action { Open, Close, Lock, Unlock }
+
+    public void Act(Action action, bool haveKey = false)
+    {
+        Console.Write($""{State} {action}{(haveKey ? "" withKey"" : null)}"");
+        State = ChangeState0(State, action, haveKey);
+        Console.WriteLine($"" -> {State}"");
+    }
+
+    public static DoorState ChangeState0(DoorState state, Action action, bool haveKey = false)
+    {
+        switch ((state, action))
+        {
+            case (DoorState.Opened, Action.Close):
+                return DoorState.Closed;
+            case (DoorState.Closed, Action.Open):
+                return DoorState.Opened;
+            case (DoorState.Closed, Action.Lock) when haveKey:
+                return DoorState.Locked;
+            case (DoorState.Locked, Action.Unlock) when haveKey:
+                return DoorState.Closed;
+            case var (oldState, _):
+                return oldState;
+        }
+    }
+
+    public static DoorState ChangeState1(DoorState state, Action action, bool haveKey = false) =>
+        (state, action) switch {
+            (DoorState.Opened, Action.Close) => DoorState.Closed,
+            (DoorState.Closed, Action.Open) => DoorState.Opened,
+            (DoorState.Closed, Action.Lock) when haveKey => DoorState.Locked,
+            (DoorState.Locked, Action.Unlock) when haveKey => DoorState.Closed,
+            _ => state };
+}
+
+class Program
+{
+    static void Main(string[] args)
+    {
+        var door = new Door();
+        door.Act(Door.Action.Close);
+        door.Act(Door.Action.Lock);
+        door.Act(Door.Action.Lock, true);
+        door.Act(Door.Action.Open);
+        door.Act(Door.Action.Unlock);
+        door.Act(Door.Action.Unlock, true);
+        door.Act(Door.Action.Open);
+    }
+}";
+            var expectedOutput =
+@"Opened Close -> Closed
+Closed Lock -> Closed
+Closed Lock withKey -> Locked
+Locked Open -> Locked
+Locked Unlock -> Locked
+Locked Unlock withKey -> Closed
+Closed Open -> Opened
+";
+            var compilation = CreateCompilation(source, options: TestOptions.ReleaseExe, parseOptions: TestOptions.Regular.WithRecursivePatterns());
+            compilation.VerifyDiagnostics();
+            var compVerifier = CompileAndVerify(compilation, expectedOutput: expectedOutput);
+            compVerifier.VerifyIL("Door.ChangeState0",
+@"{
+  // Code size       94 (0x5e)
+  .maxstack  3
+  .locals init (Door.DoorState V_0,
+                System.ValueTuple<Door.DoorState, Door.Action> V_1,
+                Door.Action V_2)
+  IL_0000:  ldloca.s   V_1
+  IL_0002:  ldarg.0
+  IL_0003:  ldarg.1
+  IL_0004:  call       ""System.ValueTuple<Door.DoorState, Door.Action>..ctor(Door.DoorState, Door.Action)""
+  IL_0009:  ldloc.1
+  IL_000a:  ldfld      ""Door.DoorState System.ValueTuple<Door.DoorState, Door.Action>.Item1""
+  IL_000f:  stloc.0
+  IL_0010:  ldloc.0
+  IL_0011:  switch    (
+        IL_0024,
+        IL_0031,
+        IL_0041)
+  IL_0022:  br.s       IL_005c
+  IL_0024:  ldloc.1
+  IL_0025:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_002a:  stloc.2
+  IL_002b:  ldc.i4.1
+  IL_002c:  ldloc.2
+  IL_002d:  beq.s      IL_004e
+  IL_002f:  br.s       IL_005c
+  IL_0031:  ldloc.1
+  IL_0032:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_0037:  stloc.2
+  IL_0038:  ldloc.2
+  IL_0039:  brfalse.s  IL_0050
+  IL_003b:  ldloc.2
+  IL_003c:  ldc.i4.2
+  IL_003d:  beq.s      IL_0052
+  IL_003f:  br.s       IL_005c
+  IL_0041:  ldloc.1
+  IL_0042:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_0047:  stloc.2
+  IL_0048:  ldc.i4.3
+  IL_0049:  ldloc.2
+  IL_004a:  beq.s      IL_0057
+  IL_004c:  br.s       IL_005c
+  IL_004e:  ldc.i4.1
+  IL_004f:  ret
+  IL_0050:  ldc.i4.0
+  IL_0051:  ret
+  IL_0052:  ldarg.2
+  IL_0053:  brfalse.s  IL_005c
+  IL_0055:  ldc.i4.2
+  IL_0056:  ret
+  IL_0057:  ldarg.2
+  IL_0058:  brfalse.s  IL_005c
+  IL_005a:  ldc.i4.1
+  IL_005b:  ret
+  IL_005c:  ldloc.0
+  IL_005d:  ret
+}");
+            compVerifier.VerifyIL("Door.ChangeState1",
+@"{
+  // Code size       98 (0x62)
+  .maxstack  3
+  .locals init (System.ValueTuple<Door.DoorState, Door.Action> V_0,
+                Door.DoorState V_1,
+                Door.Action V_2,
+                Door.DoorState V_3)
+  IL_0000:  ldloca.s   V_0
+  IL_0002:  ldarg.0
+  IL_0003:  ldarg.1
+  IL_0004:  call       ""System.ValueTuple<Door.DoorState, Door.Action>..ctor(Door.DoorState, Door.Action)""
+  IL_0009:  ldloc.0
+  IL_000a:  ldfld      ""Door.DoorState System.ValueTuple<Door.DoorState, Door.Action>.Item1""
+  IL_000f:  stloc.1
+  IL_0010:  ldloc.1
+  IL_0011:  switch    (
+        IL_0024,
+        IL_0031,
+        IL_0041)
+  IL_0022:  br.s       IL_005e
+  IL_0024:  ldloc.0
+  IL_0025:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_002a:  stloc.2
+  IL_002b:  ldc.i4.1
+  IL_002c:  ldloc.2
+  IL_002d:  beq.s      IL_004e
+  IL_002f:  br.s       IL_005e
+  IL_0031:  ldloc.0
+  IL_0032:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_0037:  stloc.2
+  IL_0038:  ldloc.2
+  IL_0039:  brfalse.s  IL_0052
+  IL_003b:  ldloc.2
+  IL_003c:  ldc.i4.2
+  IL_003d:  beq.s      IL_0056
+  IL_003f:  br.s       IL_005e
+  IL_0041:  ldloc.0
+  IL_0042:  ldfld      ""Door.Action System.ValueTuple<Door.DoorState, Door.Action>.Item2""
+  IL_0047:  stloc.2
+  IL_0048:  ldc.i4.3
+  IL_0049:  ldloc.2
+  IL_004a:  beq.s      IL_005b
+  IL_004c:  br.s       IL_005e
+  IL_004e:  ldc.i4.1
+  IL_004f:  stloc.3
+  IL_0050:  br.s       IL_0060
+  IL_0052:  ldc.i4.0
+  IL_0053:  stloc.3
+  IL_0054:  br.s       IL_0060
+  IL_0056:  ldarg.2
+  IL_0057:  brtrue.s   IL_0056
+  IL_0059:  br.s       IL_005e
+  IL_005b:  ldarg.2
+  IL_005c:  brtrue.s   IL_005b
+  IL_005e:  ldarg.0
+  IL_005f:  stloc.3
+  IL_0060:  ldloc.3
+  IL_0061:  ret
 }");
         }
     }