Skip to content

Commit

Permalink
Permit a trailing commas in two places
Browse files Browse the repository at this point in the history 
- Permit a trailing comma after the last arm of a switch expression
- Permit a trailing comma after the last subpattern of a property pattern clause
Fixes dotnet#32292
  • Loading branch information
@gafter
gafter committed Jan 14, 2019
1 parent acaa82c commit 788b90b
Show file tree
Hide file tree
Showing 6 changed files with 295 additions and 71 deletions.
27 changes: 11 additions & 16 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser_Patterns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,11 @@ private bool LooksLikeTypeOfPattern(SyntaxKind tk)
//
// Parse an expression where a declaration expression would be permitted. This is suitable for use after
// the `out` keyword in an argument list, or in the elements of a tuple literal (because they may
// be on the left-hand-side of a deconstruction). The first element of a tuple is handled slightly
// be on the left-hand-side of a positional). The first element of a tuple is handled slightly
// differently, as we check for the comma before concluding that the identifier should cause a
// disambiguation. For example, for the input `(A < B , C > D)`, we treat this as a tuple with
// two elements, because if we considered the `A<B,C>` to be a type, it wouldn't be a tuple at
// all. Since we don't have such a thing as a one-element tuple (even for deconstruction), the
// all. Since we don't have such a thing as a one-element tuple (even for positional), the
// absence of the comma after the `D` means we don't treat the `D` as contributing to the
// disambiguation of the expression/type. More formally, ...
//
Expand Down Expand Up @@ -283,7 +283,7 @@ private bool ScanDesignation(bool permitTuple)
/// pattern
/// : declaration_pattern
/// | constant_pattern
/// | deconstruction_pattern
/// | positional_pattern
/// | property_pattern
/// | discard_pattern
/// ;
Expand All @@ -293,7 +293,7 @@ private bool ScanDesignation(bool permitTuple)
/// constant_pattern
/// : expression
/// ;
/// deconstruction_pattern
/// positional_pattern
/// : type? '(' subpatterns? ')' property_subpattern? identifier?
/// ;
/// subpatterns
Expand Down Expand Up @@ -446,7 +446,7 @@ private PatternSyntax ParsePatternContinued(TypeSyntax type, bool whenIsKeyword)
{
if (subPatterns[0].Pattern is ConstantPatternSyntax cp)
{
// There is an ambiguity between a deconstruction pattern `(` pattern `)`
// There is an ambiguity between a positional pattern `(` pattern `)`
// and a constant expression pattern that happens to be parenthesized.
// Per 2017-11-20 LDM we treat such syntax as a parenthesized expression always.
return _syntaxFactory.ConstantPattern(_syntaxFactory.ParenthesizedExpression(openParenToken, cp.Expression, closeParenToken));
Expand Down Expand Up @@ -577,6 +577,10 @@ private void ParseSubpatternList(
else if (this.CurrentToken.Kind == SyntaxKind.CommaToken || this.IsPossibleSubpatternElement())
{
list.AddSeparator(this.EatToken(SyntaxKind.CommaToken));
if (this.CurrentToken.Kind == SyntaxKind.CloseBraceToken)
{
break;
}
list.Add(this.ParseSubpatternElement());
continue;
}
Expand Down Expand Up @@ -668,20 +672,11 @@ private SeparatedSyntaxList<SwitchExpressionArmSyntax> ParseSwitchExpressionArms
var expression = ParseExpressionCore();
var switchExpressionCase = _syntaxFactory.SwitchExpressionArm(pattern, whenClause, arrow, expression);
arms.Add(switchExpressionCase);
if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
if (this.CurrentToken.Kind != SyntaxKind.CloseBraceToken)
{
var commaToken = this.EatToken();
if (this.CurrentToken.Kind == SyntaxKind.CloseBraceToken)
{
commaToken = this.AddError(commaToken, ErrorCode.ERR_UnexpectedToken, this.CurrentToken.Text);
}

var commaToken = this.EatToken(SyntaxKind.CommaToken);
arms.AddSeparator(commaToken);
}
else
{
break;
}
}

SeparatedSyntaxList<SwitchExpressionArmSyntax> result = arms;
Expand Down
59 changes: 13 additions & 46 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests2.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,13 +375,13 @@ public void SwitchExpression_01()
{
public static void Main()
{
var r = 1 switch { _ => 0 };
var r = 1 switch { _ => 0, };
}
}";
CreateCompilation(source, options: TestOptions.DebugExe, parseOptions: TestOptions.RegularWithoutRecursivePatterns).VerifyDiagnostics(
// (5,17): error CS8370: Feature 'recursive patterns' is not available in C# 7.3. Please use language version 8.0 or greater.
// var r = 1 switch { _ => 0 };
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7_3, "1 switch { _ => 0 }").WithArguments("recursive patterns", "8.0").WithLocation(5, 17)
// var r = 1 switch { _ => 0, };
Diagnostic(ErrorCode.ERR_FeatureNotAvailableInVersion7_3, "1 switch { _ => 0, }").WithArguments("recursive patterns", "8.0").WithLocation(5, 17)
);
}

Expand Down Expand Up @@ -433,48 +433,15 @@ public static void Main()
// (6,34): error CS1525: Invalid expression term '?'
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_InvalidExprTerm, "?").WithArguments("?").WithLocation(6, 34),
// (6,48): error CS1513: } expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_RbraceExpected, "=>").WithLocation(6, 48),
// (6,48): error CS1003: Syntax error, ',' expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_SyntaxError, "=>").WithArguments(",", "=>").WithLocation(6, 48),
// (6,51): error CS1002: ; expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "true").WithLocation(6, 51),
// (6,55): error CS1002: ; expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_SemicolonExpected, ",").WithLocation(6, 55),
// (6,55): error CS1513: } expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_RbraceExpected, ",").WithLocation(6, 55),
// (6,63): error CS1002: ; expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "=>").WithLocation(6, 63),
// (6,63): error CS1513: } expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_RbraceExpected, "=>").WithLocation(6, 63),
// (6,72): error CS1002: ; expected
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_SemicolonExpected, "}").WithLocation(6, 72),
// (6,73): error CS1597: Semicolon after method or accessor block is not valid
// (6,48): error CS8504: Pattern missing
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.ERR_UnexpectedSemicolon, ";").WithLocation(6, 73),
// (9,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(9, 1),
// (7,9): error CS0825: The contextual keyword 'var' may only appear within a local variable declaration or in script code
// var r2 = b switch { (true ? true : true) => true, false => false };
Diagnostic(ErrorCode.ERR_TypeVarNotFound, "var").WithLocation(7, 9),
// (7,18): error CS0103: The name 'b' does not exist in the current context
// var r2 = b switch { (true ? true : true) => true, false => false };
Diagnostic(ErrorCode.ERR_NameNotInContext, "b").WithArguments("b").WithLocation(7, 18),
// (7,20): warning CS8409: The switch expression does not handle all possible inputs (it is not exhaustive).
// var r2 = b switch { (true ? true : true) => true, false => false };
Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithLocation(7, 20),
// (6,20): warning CS8409: The switch expression does not handle all possible inputs (it is not exhaustive).
Diagnostic(ErrorCode.ERR_MissingPattern, "=>").WithLocation(6, 48),
// (6,57): error CS8510: The pattern has already been handled by a previous arm of the switch expression.
// var r1 = b switch { true ? true : true => true, false => false };
Diagnostic(ErrorCode.WRN_SwitchExpressionNotExhaustive, "switch").WithLocation(6, 20)
Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "false").WithLocation(6, 57)
);
}

Expand All @@ -488,7 +455,7 @@ public void SwitchExpression_04()
public static void Main()
{
var b = (true, false);
var r1 = b switch { (true ? true : true, _) => true, _ => false };
var r1 = b switch { (true ? true : true, _) => true, _ => false, };
var r2 = b is (true ? true : true, _);
switch (b.Item1) { case true ? true : true: break; }
}
Expand Down Expand Up @@ -609,7 +576,7 @@ public static void Main()
{
int q = 1;
int u;
var x = q switch { 0 => u=0, 1 => u=M(u), _ => u=2 };
var x = q switch { 0 => u=0, 1 => u=M(u), _ => u=2, };
System.Console.WriteLine(u);
}
static int M(int i) => i;
Expand All @@ -633,9 +600,9 @@ public void SwitchExpression_10()
public static void Main()
{
int a = 1;
var b = a switch { var x1 => x1 };
var b = a switch { var x1 => x1, };
var c = a switch { var x2 when x2 is var x3 => x3 };
var d = a switch { var x4 => x4 is var x5 ? x5 : 1 };
var d = a switch { var x4 => x4 is var x5 ? x5 : 1, };
}
static int M(int i) => i;
}";
Expand Down Expand Up @@ -2066,13 +2033,13 @@ class Point
};
var compilation = CreateCompilation(source, options: TestOptions.ReleaseDll);
compilation.VerifyDiagnostics(
// (10,24): error CS8400: Pattern missing
// (10,24): error CS8504: Pattern missing
// if (p is Point(, { })) { }
Diagnostic(ErrorCode.ERR_MissingPattern, ",").WithLocation(10, 24),
// (9,23): error CS1501: No overload for method 'Deconstruct' takes 3 arguments
// if (p is Point({ }, { }, { })) { }
Diagnostic(ErrorCode.ERR_BadArgCount, "({ }, { }, { })").WithArguments("Deconstruct", "3").WithLocation(9, 23),
// (9,23): error CS8129: No suitable Deconstruct instance or extension method was found for type 'Point', with 3 out parameters and a void return type.
// (9,23): error CS8129: No suitable 'Deconstruct' instance or extension method was found for type 'Point', with 3 out parameters and a void return type.
// if (p is Point({ }, { }, { })) { }
Diagnostic(ErrorCode.ERR_MissingDeconstruct, "({ }, { }, { })").WithArguments("Point", "3").WithLocation(9, 23)
);
Expand Down
6 changes: 3 additions & 3 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests3.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ class Program
public static void Main()
{
Point p = null;
Console.WriteLine(p is { X: 3, Y: 4 });
Console.WriteLine(p is { X: 3, Y: 4, });
}
}
interface I1
Expand Down Expand Up @@ -144,7 +144,7 @@ class Program
public static void Main()
{
Point p = null;
Console.WriteLine(p is { X: 3, Y: 4 });
Console.WriteLine(p is { X: 3, Y: 4, });
}
}
class Point
Expand Down Expand Up @@ -411,7 +411,7 @@ public static void Main()
{
switch (anon)
{
case { B: true, V: var val }:
case { B: true, V: var val, }:
sum += val;
break;
}
Expand Down
8 changes: 4 additions & 4 deletions src/Compilers/CSharp/Test/Semantic/Semantics/PatternMatchingTests4.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1641,7 +1641,7 @@ static int M1(bool? b1, bool? b2)
(false, false) => 1,
(false, true) => 2,
// (true, false) => 3,
(true, true) => 4
(true, true) => 4,
};
}
}
Expand All @@ -1667,7 +1667,7 @@ static int M1(bool? b1, bool? b2)
(false, false) => 1,
(false, true) => 2,
(true, false) => 3,
(true, true) => 4
(true, true) => 4,
};
}
}
Expand Down Expand Up @@ -1718,15 +1718,15 @@ static int M1(bool? b1, bool? b2)
(true, false) => 3,
(true, true) => 4,
_ => 5,
(null, true) => 6
(null, true) => 6,
};
}
}
";
var compilation = CreatePatternCompilation(source);
compilation.VerifyDiagnostics(
// (13,13): error CS8510: The pattern has already been handled by a previous arm of the switch expression.
// (null, true) => 6
// (null, true) => 6,
Diagnostic(ErrorCode.ERR_SwitchArmSubsumed, "(null, true)").WithLocation(13, 13)
);
}
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Test/Syntax/Parsing/ParsingTests.cs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

//#define PARSING_TESTS_DUMP
#define PARSING_TESTS_DUMP

using System.Collections.Generic;
using System.Diagnostics;
Expand Down
Loading

0 comments on commit 788b90b

Please sign in to comment.