Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix parsing certain postfix expressions with top level collection literals #68787

Merged
merged 2 commits into from
Jun 26, 2023
Merged

Fix parsing certain postfix expressions with top level collection literals #68787

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5945eb3
Fix parsing certain postfix expressions with top level collection lit…
CyrusNajmabadi Jun 26, 2023
b50cb75
Update test
CyrusNajmabadi Jun 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7440,8 +7440,15 @@ private SyntaxList<AttributeListSyntax> ParseStatementAttributeDeclarations()
while (this.CurrentToken.Kind == SyntaxKind.OpenBracketToken)
ParseBracketedArgumentList();

// Check the next token to see if it indicates the `[...]` sequence we have is a term or not.
var isCollectionExpression = this.CurrentToken.Kind is SyntaxKind.DotToken or SyntaxKind.QuestionToken or SyntaxKind.ExclamationToken;
// Check the next token to see if it indicates the `[...]` sequence we have is a term or not. This is the
// same set of tokens that ParsePostFixExpression looks for.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are we going to remember to make a corresponding change here if another postfix operator is ever added to the language? :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add comment

var isCollectionExpression = this.CurrentToken.Kind
is SyntaxKind.DotToken
or SyntaxKind.QuestionToken
or SyntaxKind.ExclamationToken
or SyntaxKind.PlusPlusToken
or SyntaxKind.MinusMinusToken
or SyntaxKind.MinusGreaterThanToken;

// If this was a collection expression, not an attribute declaration, return no attributes so that the
// caller will parse this out as a collection expression. Otherwise re-parse the code as the actual
Expand Down
283 changes: 283 additions & 0 deletions src/Compilers/CSharp/Test/Syntax/Parsing/CollectionLiteralParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10609,6 +10609,289 @@ public void MemberAccess22A()
EOF();
}

[Fact]
public void MemberAccess23()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider naming these tests something like PostFixOperator

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix in next pr

{
UsingTree("""
class Program
{
static void Main()
{
[]++;
}
}
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.ClassDeclaration);
{
N(SyntaxKind.ClassKeyword);
N(SyntaxKind.IdentifierToken, "Program");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.MethodDeclaration);
{
N(SyntaxKind.StaticKeyword);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.VoidKeyword);
}
N(SyntaxKind.IdentifierToken, "Main");
N(SyntaxKind.ParameterList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
N(SyntaxKind.Block);
{
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.PostIncrementExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.PlusPlusToken);
}
N(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.CloseBraceToken);
}
}
N(SyntaxKind.CloseBraceToken);
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void MemberAccess23A()
{
UsingTree("""
[]++;
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.GlobalStatement);
{
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.PostIncrementExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.PlusPlusToken);
}
N(SyntaxKind.SemicolonToken);
}
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void MemberAccess24()
{
UsingTree("""
class Program
{
static void Main()
{
[]--;
}
}
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.ClassDeclaration);
{
N(SyntaxKind.ClassKeyword);
N(SyntaxKind.IdentifierToken, "Program");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.MethodDeclaration);
{
N(SyntaxKind.StaticKeyword);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.VoidKeyword);
}
N(SyntaxKind.IdentifierToken, "Main");
N(SyntaxKind.ParameterList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
N(SyntaxKind.Block);
{
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.PostDecrementExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.MinusMinusToken);
}
N(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.CloseBraceToken);
}
}
N(SyntaxKind.CloseBraceToken);
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void MemberAccess24A()
{
UsingTree("""
[]--;
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.GlobalStatement);
{
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.PostDecrementExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.MinusMinusToken);
}
N(SyntaxKind.SemicolonToken);
}
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void MemberAccess25()
{
UsingTree("""
class Program
{
static void Main()
{
[]->Goo();
}
}
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.ClassDeclaration);
{
N(SyntaxKind.ClassKeyword);
N(SyntaxKind.IdentifierToken, "Program");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.MethodDeclaration);
{
N(SyntaxKind.StaticKeyword);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.VoidKeyword);
}
N(SyntaxKind.IdentifierToken, "Main");
N(SyntaxKind.ParameterList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
N(SyntaxKind.Block);
{
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.InvocationExpression);
{
N(SyntaxKind.PointerMemberAccessExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.MinusGreaterThanToken);
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.IdentifierToken, "Goo");
}
}
N(SyntaxKind.ArgumentList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
}
N(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.CloseBraceToken);
}
}
N(SyntaxKind.CloseBraceToken);
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void MemberAccess25A()
{
UsingTree("""
[]->Goo;
""");

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.GlobalStatement);
{
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.PointerMemberAccessExpression);
{
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.MinusGreaterThanToken);
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.IdentifierToken, "Goo");
}
}
N(SyntaxKind.SemicolonToken);
}
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
public void AttributeOnTopLevelFunction1()
{
Expand Down
61 changes: 39 additions & 22 deletions src/Compilers/CSharp/Test/Syntax/Parsing/StatementAttributeParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5512,14 +5512,18 @@ void Goo(int i)
[Fact]
public void AttributeOnExpressionStatement_PrefixUnary()
{
var test = UsingTree(@"
class C
{
void Goo(int i)
{
[A]++i;
}
}");
var test = UsingTree("""
class C
{
void Goo(int i)
{
[A]++i;
}
}
""",
// (5,14): error CS1002: ; expected
// [A]++i;
Diagnostic(ErrorCode.ERR_SemicolonExpected, "i").WithLocation(5, 14));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was an existing error recovery case that i added a few years back (recovering when attributes were put in incorrect places). It's still an error after this pr, just one that views the above as an invalid operation on a collection-expr, not an invalid attribute on an expression statement. Trying to detect this case and keep the old parsing isn't worth it IMO.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, I was just about to comment and say that this needed to be updated.


N(SyntaxKind.CompilationUnit);
{
Expand Down Expand Up @@ -5553,25 +5557,29 @@ void Goo(int i)
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.AttributeList);
N(SyntaxKind.PostIncrementExpression);
{
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.Attribute);
N(SyntaxKind.CollectionExpression);
{
N(SyntaxKind.IdentifierName);
N(SyntaxKind.OpenBracketToken);
N(SyntaxKind.ExpressionElement);
{
N(SyntaxKind.IdentifierToken, "A");
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.IdentifierToken, "A");
}
}
N(SyntaxKind.CloseBracketToken);
}
N(SyntaxKind.CloseBracketToken);
N(SyntaxKind.PlusPlusToken);
}
N(SyntaxKind.PreIncrementExpression);
M(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.PlusPlusToken);
N(SyntaxKind.IdentifierName);
{
N(SyntaxKind.IdentifierToken, "i");
}
N(SyntaxKind.IdentifierToken, "i");
}
N(SyntaxKind.SemicolonToken);
}
Expand All @@ -5585,9 +5593,18 @@ void Goo(int i)
EOF();

CreateCompilation(test).GetDiagnostics().Verify(
// (6,9): error CS7014: Attributes are not valid in this context.
// (5,9): error CS8652: The feature 'collection literals' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version.
// [A]++i;
Diagnostic(ErrorCode.ERR_AttributesNotAllowed, "[A]").WithLocation(6, 9));
Diagnostic(ErrorCode.ERR_FeatureInPreview, "[").WithArguments("collection literals").WithLocation(5, 9),
// (5,10): error CS0103: The name 'A' does not exist in the current context
// [A]++i;
Diagnostic(ErrorCode.ERR_NameNotInContext, "A").WithArguments("A").WithLocation(5, 10),
// (5,14): error CS1002: ; expected
// [A]++i;
Diagnostic(ErrorCode.ERR_SemicolonExpected, "i").WithLocation(5, 14),
// (5,14): error CS0201: Only assignment, call, increment, decrement, await, and new object expressions can be used as a statement
// [A]++i;
Diagnostic(ErrorCode.ERR_IllegalStatement, "i").WithLocation(5, 14));
}

[Fact]
Expand Down