From 44e0387e6c3df160831804d6f1e74ca25c0509f6 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 8 Feb 2018 00:46:56 -0800 Subject: [PATCH 1/2] Improve error tolerance when parsing invalid implicit arrays. Provide helper for common pattern. Update test. --- .../CSharp/Portable/Parser/LanguageParser.cs | 36 +++- .../CSharp/Portable/Parser/SyntaxParser.cs | 10 + ...rationTests_ArrayCreationAndInitializer.cs | 25 ++- .../Syntax/Parsing/ParserErrorMessageTests.cs | 199 ++++++++++++++++++ 4 files changed, 248 insertions(+), 22 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 7a7803df024bf..222f6496fb000 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -450,19 +450,19 @@ private void AddSkippedNamespaceText( { if (body.Members.Count > 0) { - body.Members[body.Members.Count - 1] = AddTrailingSkippedSyntax(body.Members[body.Members.Count - 1], skippedSyntax); + AddTrailingSkippedSyntax(body.Members, skippedSyntax); } else if (body.Attributes.Count > 0) { - body.Attributes[body.Attributes.Count - 1] = AddTrailingSkippedSyntax(body.Attributes[body.Attributes.Count - 1], skippedSyntax); + AddTrailingSkippedSyntax(body.Attributes, skippedSyntax); } else if (body.Usings.Count > 0) { - body.Usings[body.Usings.Count - 1] = AddTrailingSkippedSyntax(body.Usings[body.Usings.Count - 1], skippedSyntax); + AddTrailingSkippedSyntax(body.Usings, skippedSyntax); } else if (body.Externs.Count > 0) { - body.Externs[body.Externs.Count - 1] = AddTrailingSkippedSyntax(body.Externs[body.Externs.Count - 1], skippedSyntax); + AddTrailingSkippedSyntax(body.Externs, skippedSyntax); } else if (openBrace != null) { @@ -3275,7 +3275,7 @@ private PostSkipAction SkipBadListTokensWithExpectedKindHelper( var action = SkipBadTokensWithExpectedKind(isNotExpectedFunction, abortFunction, expected, out lastItemTrailingTrivia); if (lastItemTrailingTrivia != null) { - list[list.Count - 1] = AddTrailingSkippedSyntax((CSharpSyntaxNode)list[list.Count - 1], lastItemTrailingTrivia); + AddTrailingSkippedSyntax(list, lastItemTrailingTrivia); } trailingTrivia = null; return action; @@ -3299,7 +3299,7 @@ private PostSkipAction SkipBadListTokensWithErrorCodeHelper( var action = SkipBadTokensWithErrorCode(isNotExpectedFunction, abortFunction, error, out lastItemTrailingTrivia); if (lastItemTrailingTrivia != null) { - list[list.Count - 1] = AddTrailingSkippedSyntax(list[list.Count - 1], lastItemTrailingTrivia); + AddTrailingSkippedSyntax(list, lastItemTrailingTrivia); } trailingTrivia = null; return action; @@ -10600,13 +10600,31 @@ private ImplicitArrayCreationExpressionSyntax ParseImplicitlyTypedArrayCreation( var commas = _pool.Allocate(); try { - while (this.CurrentToken.Kind == SyntaxKind.CommaToken) + while (true) { - commas.Add(this.EatToken()); + if (this.IsPossibleExpression()) + { + var size = this.AddError(this.ParseExpressionCore(), ErrorCode.ERR_InvalidArray); + if (commas.Count == 0) + { + openBracket = AddTrailingSkippedSyntax(openBracket, size); + } + else + { + AddTrailingSkippedSyntax(commas, size); + } + } + + if (this.CurrentToken.Kind == SyntaxKind.CommaToken) + { + commas.Add(this.EatToken()); + continue; + } + + break; } var closeBracket = this.EatToken(SyntaxKind.CloseBracketToken); - var initializer = this.ParseArrayInitializer(); return _syntaxFactory.ImplicitArrayCreationExpression(@new, openBracket, commas.ToList(), closeBracket, initializer); diff --git a/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs b/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs index e1b4d5233f7e0..8dbd632cec7f3 100644 --- a/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs @@ -829,6 +829,16 @@ protected TNode AddLeadingSkippedSyntax(TNode node, GreenNode skippedSynt return SyntaxFirstTokenReplacer.Replace(node, oldToken, newToken, skippedSyntax.FullWidth); } + protected void AddTrailingSkippedSyntax(SyntaxListBuilder list, GreenNode skippedSyntax) + { + list[list.Count - 1] = AddTrailingSkippedSyntax((CSharpSyntaxNode)list[list.Count - 1], skippedSyntax); + } + + protected void AddTrailingSkippedSyntax(SyntaxListBuilder list, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode + { + list[list.Count - 1] = AddTrailingSkippedSyntax(list[list.Count - 1], skippedSyntax); + } + protected TNode AddTrailingSkippedSyntax(TNode node, GreenNode skippedSyntax) where TNode : CSharpSyntaxNode { var token = node as SyntaxToken; diff --git a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ArrayCreationAndInitializer.cs b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ArrayCreationAndInitializer.cs index 0dd69fd800ec3..27384c7089cdc 100644 --- a/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ArrayCreationAndInitializer.cs +++ b/src/Compilers/CSharp/Test/Semantic/IOperation/IOperationTests_ArrayCreationAndInitializer.cs @@ -435,27 +435,26 @@ public void F(int dimension) } "; string expectedOperationTree = @" -IArrayCreationOperation (OperationKind.ArrayCreation, Type: System.Int32[], IsInvalid) (Syntax: 'new[2]/**/') +IArrayCreationOperation (OperationKind.ArrayCreation, Type: ?[], IsInvalid) (Syntax: 'new[2]/**/') Dimension Sizes(1): - ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 1, IsInvalid, IsImplicit) (Syntax: 'new[2]/**/') + ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 0, IsInvalid, IsImplicit) (Syntax: 'new[2]/**/') Initializer: - IArrayInitializerOperation (1 elements) (OperationKind.ArrayInitializer, Type: null, IsInvalid) (Syntax: '2]/**/') - Element Values(1): - ILiteralOperation (OperationKind.Literal, Type: System.Int32, Constant: 2, IsInvalid) (Syntax: '2') + IArrayInitializerOperation (0 elements) (OperationKind.ArrayInitializer, Type: null, IsInvalid) (Syntax: '') + Element Values(0) "; var expectedDiagnostics = new DiagnosticDescription[] { - // CS1003: Syntax error, ']' expected + // file.cs(6,31): error CS0178: Invalid rank specifier: expected ',' or ']' // var x = /**/new[2]/**/; - Diagnostic(ErrorCode.ERR_SyntaxError, "2").WithArguments("]", "").WithLocation(6, 31), - // CS1514: { expected + Diagnostic(ErrorCode.ERR_InvalidArray, "2").WithLocation(6, 31), + // file.cs(6,44): error CS1514: { expected // var x = /**/new[2]/**/; - Diagnostic(ErrorCode.ERR_LbraceExpected, "2").WithLocation(6, 31), - // CS1003: Syntax error, ',' expected + Diagnostic(ErrorCode.ERR_LbraceExpected, ";").WithLocation(6, 44), + // file.cs(6,44): error CS1513: } expected // var x = /**/new[2]/**/; - Diagnostic(ErrorCode.ERR_SyntaxError, "]").WithArguments(",", "]").WithLocation(6, 32), - // CS1513: } expected + Diagnostic(ErrorCode.ERR_RbraceExpected, ";").WithLocation(6, 44), + // file.cs(6,27): error CS0826: No best type found for implicitly-typed array // var x = /**/new[2]/**/; - Diagnostic(ErrorCode.ERR_RbraceExpected, ";").WithLocation(6, 44) + Diagnostic(ErrorCode.ERR_ImplicitlyTypedArrayNoBestType, "new[2]/**/").WithLocation(6, 27) }; VerifyOperationTreeAndDiagnosticsForTest(source, expectedOperationTree, expectedDiagnostics); diff --git a/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs b/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs index 0c86ebfeab5e9..98c05b2e3d4f8 100644 --- a/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs +++ b/src/Compilers/CSharp/Test/Syntax/Parsing/ParserErrorMessageTests.cs @@ -271,6 +271,205 @@ public static int Main() Diagnostic(ErrorCode.ERR_SyntaxError, ";").WithArguments("]", ";")); } + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray1() + { + var test = @" +class C { + void Goo() { + var x = new[3] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray2() + { + var test = @" +class C { + void Goo() { + var x = new[3,] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3,] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray3() + { + var test = @" +class C { + void Goo() { + var x = new[,3] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,22): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[,3] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray4() + { + var test = @" +class C { + void Goo() { + var x = new[,3 { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,22): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[,3 { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22), + // (4,24): error CS1003: Syntax error, ']' expected + // var x = new[,3 { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 24)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray5() + { + var test = @" +class C { + void Goo() { + var x = new[3 { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3 { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21), + // (4,23): error CS1003: Syntax error, ']' expected + // var x = new[3 { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 23)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray6() + { + var test = @" +class C { + void Goo() { + var x = new[3, { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3, { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21), + // (4,24): error CS1003: Syntax error, ']' expected + // var x = new[3, { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 24)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray7() + { + var test = @" +class C { + void Goo() { + var x = new[3,,] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3,,] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray8() + { + var test = @" +class C { + void Goo() { + var x = new[,3,] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,22): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[,3,] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 22)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray9() + { + var test = @" +class C { + void Goo() { + var x = new[,,3] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,23): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[,,3] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 23)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray10() + { + var test = @" +class C { + void Goo() { + var x = new[3,,3] { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,21): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3,,3] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 21), + // (4,24): error CS0178: Invalid rank specifier: expected ',' or ']' + // var x = new[3,,3] { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_InvalidArray, "3").WithLocation(4, 24)); + } + + [Fact, WorkItem(24701, "https://github.com/dotnet/roslyn/issues/24701")] + public void CS0178ERR_InvalidArray_ImplicitArray11() + { + var test = @" +class C { + void Goo() { + var x = new[ { 1, 2, 3 }; + } +} +"; + + ParseAndValidate(test, + // (4,22): error CS1003: Syntax error, ']' expected + // var x = new[ { 1, 2, 3 }; + Diagnostic(ErrorCode.ERR_SyntaxError, "{").WithArguments("]", "{").WithLocation(4, 22)); + } + [Fact] public void CS0230ERR_BadForeachDecl() { From da6fa53b2f77fc8db845182dd32ca880ca8909db Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Feb 2018 13:28:25 -0800 Subject: [PATCH 2/2] Be resilient to infinite loops. --- src/Compilers/CSharp/Portable/Parser/LanguageParser.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs index 222f6496fb000..d539c3d3897d3 100644 --- a/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs +++ b/src/Compilers/CSharp/Portable/Parser/LanguageParser.cs @@ -10600,7 +10600,8 @@ private ImplicitArrayCreationExpressionSyntax ParseImplicitlyTypedArrayCreation( var commas = _pool.Allocate(); try { - while (true) + int lastTokenPosition = -1; + while (IsMakingProgress(ref lastTokenPosition)) { if (this.IsPossibleExpression()) { @@ -10834,7 +10835,7 @@ private ParameterListSyntax ParseLambdaParameterList() // additional parameters int tokenProgress = -1; - while(IsMakingProgress(ref tokenProgress)) + while (IsMakingProgress(ref tokenProgress)) { if (this.CurrentToken.Kind == SyntaxKind.CloseParenToken) {