Skip to content

Commit

Permalink
Requiring semicolon after embedded statements (#17668)
Browse files Browse the repository at this point in the history 
Requiring semicolon after embedded statements
  • Loading branch information
@ivanbasov
ivanbasov authored Mar 14, 2017
1 parent c59e62a commit a192dc6
Show file tree
Hide file tree
Showing 4 changed files with 201 additions and 3 deletions.
24 changes: 22 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7354,15 +7354,35 @@ private StatementSyntax ParseEmbeddedStatement(bool complexCheck)
statement = SyntaxFactory.EmptyStatement(EatToken(SyntaxKind.SemicolonToken));
}

// An "embedded" statement is simply a statement that is not a labelled
// statement or a declaration statement.
switch (statement.Kind)
{
// An "embedded" statement is simply a statement that is not a labelled
// statement or a declaration statement.
case SyntaxKind.LabeledStatement:
case SyntaxKind.LocalDeclarationStatement:
case SyntaxKind.LocalFunctionStatement:
statement = this.AddError(statement, ErrorCode.ERR_BadEmbeddedStmt);
break;
// In scripts, stand-alone expression statements may not be followed by semicolons.
// ParseExpressionStatement hides the error.
// However, embedded expression statements are required to be followed by semicolon.
case SyntaxKind.ExpressionStatement:
if (IsScript)
{
var expressionStatementSyntax = (ExpressionStatementSyntax)statement;
var semicolonToken = expressionStatementSyntax.SemicolonToken;

// Do not add a new error if the same error was already added.
if (semicolonToken.IsMissing
&& (!semicolonToken.ContainsDiagnostics
|| !semicolonToken.GetDiagnostics()
.Contains(diagnosticInfo => (ErrorCode)diagnosticInfo.Code == ErrorCode.ERR_SemicolonExpected)))
{
semicolonToken = this.AddError(semicolonToken, ErrorCode.ERR_SemicolonExpected);
statement = expressionStatementSyntax.Update(expressionStatementSyntax.Expression, semicolonToken);
}
}
break;
}

return statement;
Expand Down
89 changes: 88 additions & 1 deletion src/Compilers/CSharp/Test/Syntax/IncrementalParsing/IncrementalParsingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

namespace Microsoft.CodeAnalysis.CSharp.UnitTests
{
public class IncrementalParsingTests
public class IncrementalParsingTests : TestBase
{
private CSharpParseOptions GetOptions(string[] defines)
{
Expand Down Expand Up @@ -2341,6 +2341,93 @@ public void InsertOpenBraceBeforeCodes()
CompareIncToFullParseErrors(reparsedTree, parsedTree);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void InsertExpressionStatementWithoutSemicolonBefore()
{
SourceText oldText = SourceText.From(@"System.Console.WriteLine(true)
");
var startTree = SyntaxFactory.ParseSyntaxTree(oldText, options: TestOptions.Script);

startTree.GetDiagnostics().Verify();

var newText = oldText.WithChanges(new TextChange(new TextSpan(0, 0), @"System.Console.WriteLine(false)
"));

AssertEx.AreEqual(@"System.Console.WriteLine(false)
System.Console.WriteLine(true)
",
newText.ToString());

var reparsedTree = startTree.WithChangedText(newText);
var parsedTree = SyntaxFactory.ParseSyntaxTree(newText, options: TestOptions.Script);

parsedTree.GetDiagnostics().Verify(
// (1,32): error CS1002: ; expected
// System.Console.WriteLine(false)
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(1, 32));

CompareIncToFullParseErrors(reparsedTree, parsedTree);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void InsertExpressionStatementWithoutSemicolonAfter()
{
SourceText oldText = SourceText.From(@"System.Console.WriteLine(true)
");
var startTree = SyntaxFactory.ParseSyntaxTree(oldText, options: TestOptions.Script);

startTree.GetDiagnostics().Verify();

var newText = oldText.WithInsertAt(
oldText.Length,
@"System.Console.WriteLine(false)
");

AssertEx.Equal(@"System.Console.WriteLine(true)
System.Console.WriteLine(false)
", newText.ToString());

var reparsedTree = startTree.WithChangedText(newText);

var parsedTree = SyntaxFactory.ParseSyntaxTree(newText, options: TestOptions.Script);
parsedTree.GetDiagnostics().Verify(
// (1,31): error CS1002: ; expected
// System.Console.WriteLine(true)
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(1, 31));

CompareIncToFullParseErrors(reparsedTree, parsedTree);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void MakeEmbeddedExpressionStatementWithoutSemicolon()
{
SourceText oldText = SourceText.From(@"System.Console.WriteLine(true)
");
var startTree = SyntaxFactory.ParseSyntaxTree(oldText, options: TestOptions.Script);

startTree.GetDiagnostics().Verify();

var newText = oldText.WithChanges(new TextChange(new TextSpan(0, 0), @"if (false)
"));

AssertEx.Equal(@"if (false)
System.Console.WriteLine(true)
", newText.ToString());

var reparsedTree = startTree.WithChangedText(newText);
var parsedTree = SyntaxFactory.ParseSyntaxTree(newText, options: TestOptions.Script);

parsedTree.GetDiagnostics().Verify(
// (2,31): error CS1002: ; expected
// System.Console.WriteLine(true)
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(2, 31));

CompareIncToFullParseErrors(reparsedTree, parsedTree);
}

[WorkItem(531404, "http://vstfdevdiv:8080/DevDiv2/DevDiv/_workitems/edit/531404")]
[Fact]
public void AppendDisabledText()
Expand Down
15 changes: 15 additions & 0 deletions src/Compilers/CSharp/Test/Syntax/Parsing/StatementParsingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2661,6 +2661,21 @@ protected override void M()
CSharpTestBase.Diagnostic(ErrorCode.ERR_RbraceExpected, "").WithLocation(9, 10));
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunEmbeddedStatementNotFollowedBySemicolon()
{
var text = @"if (true)
System.Console.WriteLine(true)";
var statement = this.ParseStatement(text);

Assert.NotNull(statement);
Assert.Equal(SyntaxKind.IfStatement, statement.Kind());
Assert.Equal(text, statement.ToString());
Assert.Equal(1, statement.Errors().Length);
Assert.Equal((int)ErrorCode.ERR_SemicolonExpected, statement.Errors()[0].Code);
}

private sealed class TokenAndTriviaWalker : CSharpSyntaxWalker
{
public int Tokens;
Expand Down
76 changes: 76 additions & 0 deletions src/Scripting/CSharpTest/ScriptTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,82 @@ public void Do()
, ScriptOptions.Default.WithReferences(MscorlibRef, SystemRef, SystemCoreRef, CSharpRef));
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunEmbeddedStatementNotFollowedBySemicolon()
{
var exceptionThrown = false;

try
{
var state = CSharpScript.RunAsync(@"if (true)
System.Console.WriteLine(true)", globals: new ScriptTests());
}
catch (CompilationErrorException ex)
{
exceptionThrown = true;
ex.Diagnostics.Verify(
// (2,32): error CS1002: ; expected
// System.Console.WriteLine(true)
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(2, 32));
}

Assert.True(exceptionThrown);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunEmbeddedStatementFollowedBySemicolon()
{
var state = CSharpScript.RunAsync(@"if (true)
System.Console.WriteLine(true);", globals: new ScriptTests());
Assert.Null(state.Exception);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunStatementFollowedBySpace()
{
var state = CSharpScript.RunAsync(@"System.Console.WriteLine(true) ", globals: new ScriptTests());
Assert.Null(state.Exception);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunStatementFollowedByNewLineNoSemicolon()
{
var state = CSharpScript.RunAsync(@"
System.Console.WriteLine(true)
", globals: new ScriptTests());
Assert.Null(state.Exception);
}

[WorkItem(6676, "https://github.com/dotnet/roslyn/issues/6676")]
[Fact]
public void TestRunEmbeddedNoSemicolonFollowedByAnotherStatement()
{
var exceptionThrown = false;

try
{
var state = CSharpScript.RunAsync(@"if (e) a = b
throw e;", globals: new ScriptTests());
}
catch (CompilationErrorException ex)
{
exceptionThrown = true;
// Verify that it produces a single ExpectedSemicolon error.
// No duplicates for the same error.
ex.Diagnostics.Verify(
// (1,13): error CS1002: ; expected
// if (e) a = b
Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(1, 13));
}

Assert.True(exceptionThrown);
}

[Fact]
public async Task TestRunScriptWithGlobals()
{
Expand Down

0 comments on commit a192dc6

Please sign in to comment.