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

Allow newlines in the holes inside interpolated strings #56853

Merged
merged 21 commits into from
Oct 25, 2021
Merged

Allow newlines in the holes inside interpolated strings #56853

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
8ed75ec
Allow newlines in the holes inside interpolated strings
CyrusNajmabadi Sep 29, 2021
fe7c2d9
Update tests
CyrusNajmabadi Sep 29, 2021
10a0fb5
Update tests
CyrusNajmabadi Sep 29, 2021
06afc0b
Remove unnecessary extra error
CyrusNajmabadi Sep 29, 2021
3a8f99b
Use simple using
CyrusNajmabadi Sep 29, 2021
7dffb4b
Inline variables
CyrusNajmabadi Sep 29, 2021
dde9fdc
Use named parameters
CyrusNajmabadi Sep 29, 2021
61bcfbb
Simplify
CyrusNajmabadi Sep 29, 2021
bde11ad
Move error to binding phase
CyrusNajmabadi Oct 14, 2021
437fe79
Add tests
CyrusNajmabadi Oct 14, 2021
edd0497
Update optinos
CyrusNajmabadi Oct 14, 2021
da75881
remove
CyrusNajmabadi Oct 14, 2021
9dd3189
Update comment
CyrusNajmabadi Oct 14, 2021
d5eb8f6
Remove prototype
CyrusNajmabadi Oct 15, 2021
1b05285
PR feedback and comment
CyrusNajmabadi Oct 15, 2021
f0b1cd8
fix loc strings
CyrusNajmabadi Oct 15, 2021
3ec40db
Update src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
CyrusNajmabadi Oct 25, 2021
2459df5
Update src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
CyrusNajmabadi Oct 25, 2021
1e445cb
Update src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
CyrusNajmabadi Oct 25, 2021
65e91b2
Update language string
CyrusNajmabadi Oct 25, 2021
59e4649
Update src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
CyrusNajmabadi Oct 25, 2021
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
6 changes: 4 additions & 2 deletions src/Compilers/CSharp/Portable/Errors/ErrorCode.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1308,7 +1308,8 @@ internal enum ErrorCode
ERR_DictionaryInitializerInExpressionTree = 8074,
ERR_ExtensionCollectionElementInitializerInExpressionTree = 8075,
ERR_UnclosedExpressionHole = 8076,
ERR_SingleLineCommentInExpressionHole = 8077,
// It is now legal to have single line comments in expression holes.
// ERR_SingleLineCommentInExpressionHole = 8077,
ERR_InsufficientStack = 8078,
ERR_UseDefViolationProperty = 8079,
ERR_AutoPropertyMustOverrideSet = 8080,
Expand Down Expand Up @@ -1990,7 +1991,8 @@ internal enum ErrorCode
ERR_BadCallerArgumentExpressionParamWithoutDefaultValue = 8964,
WRN_CallerArgumentExpressionAttributeSelfReferential = 8965,
WRN_CallerArgumentExpressionParamForUnconsumedLocation = 8966,
ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString = 8967,
// It is now legal to have newlines in expression holes.
// ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString = 8967,
ERR_AttrTypeArgCannotBeTypeVar = 8968,
// WRN_AttrDependentTypeNotAllowed = 8969, // Backed out of of warning wave 6, may be reintroduced later
ERR_AttrDependentTypeNotAllowed = 8970,
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Parser/Lexer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ private void ScanSyntaxToken(ref TokenInfo info)
case '@':
if (TextWindow.PeekChar(1) == '"')
{
var errorCode = this.ScanVerbatimStringLiteral(ref info, allowNewlines: true);
var errorCode = this.ScanVerbatimStringLiteral(ref info);
if (errorCode is ErrorCode code)
this.AddError(code);
}
Expand Down
63 changes: 12 additions & 51 deletions src/Compilers/CSharp/Portable/Parser/Lexer_StringLiteral.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ private char ScanEscapeSequence(out char surrogateCharacter)
/// <summary>
/// Returns an appropriate error code if scanning this verbatim literal ran into an error.
/// </summary>
private ErrorCode? ScanVerbatimStringLiteral(ref TokenInfo info, bool allowNewlines)
private ErrorCode? ScanVerbatimStringLiteral(ref TokenInfo info)
{
_builder.Length = 0;

Expand Down Expand Up @@ -180,14 +180,6 @@ private char ScanEscapeSequence(out char surrogateCharacter)
break;
}

// If we hit a new line when it's not allowed. Give an error at that new line, but keep on consuming
// the verbatim literal to the end to avoid the contents of the string being lexed as C# (which will
// cause a ton of cascaded errors). Only need to do this on the first newline we hit.
if (!allowNewlines && SyntaxFacts.IsNewLine(ch))
{
error ??= ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString;
}

TextWindow.AdvanceChar();
_builder.Append(ch);
}
Expand Down Expand Up @@ -272,7 +264,6 @@ private class InterpolatedStringScanner
{
private readonly Lexer _lexer;
private bool _isVerbatim;
private bool _allowNewlines;

/// <summary>
/// There are two types of errors we can encounter when trying to scan out an interpolated string (and its
Expand All @@ -285,18 +276,15 @@ private class InterpolatedStringScanner
public SyntaxDiagnosticInfo? Error;
private bool EncounteredUnrecoverableError;

public InterpolatedStringScanner(
Lexer lexer,
bool isVerbatim)
public InterpolatedStringScanner(Lexer lexer, bool isVerbatim)
{
_lexer = lexer;
_isVerbatim = isVerbatim;
_allowNewlines = isVerbatim;
}

private bool IsAtEnd()
{
return IsAtEnd(_isVerbatim && _allowNewlines);
return IsAtEnd(_isVerbatim);
}

private bool IsAtEnd(bool allowNewline)
Expand Down Expand Up @@ -529,15 +517,8 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
{
char ch = _lexer.TextWindow.PeekChar();

// See if we ran into a disallowed new line. If so, this is recoverable, so just skip past it but
// give a good message about the issue. This will prevent a lot of cascading issues with the
// remainder of the interpolated string that comes on the following lines.
var allowNewLines = _isVerbatim && _allowNewlines;
if (!allowNewLines && SyntaxFacts.IsNewLine(ch))
{
TrySetRecoverableError(_lexer.MakeError(_lexer.TextWindow.Position, width: 0, ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString));
}

// Note: within a hole new-lines are always allowed. The retriction on if new-lines are allowed or not
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
// is only within a text-portion of the interpolated stirng.
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
if (IsAtEnd(allowNewline: true))
{
// the caller will complain
Expand All @@ -558,17 +539,14 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
var interpolations = (ArrayBuilder<Interpolation>?)null;
var info = default(TokenInfo);
bool wasVerbatim = _isVerbatim;
bool wasAllowNewlines = _allowNewlines;
try
{
_isVerbatim = isVerbatimSubstring;
_allowNewlines &= _isVerbatim;
ScanInterpolatedStringLiteralTop(interpolations, ref info, closeQuoteMissing: out _);
}
finally
{
_isVerbatim = wasVerbatim;
_allowNewlines = wasAllowNewlines;
}
continue;
}
Expand Down Expand Up @@ -618,7 +596,7 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
// to be a normal verbatim string, so we can continue on an attempt to understand the
// outer interpolated string properly.
var discarded = default(TokenInfo);
var errorCode = _lexer.ScanVerbatimStringLiteral(ref discarded, _allowNewlines);
var errorCode = _lexer.ScanVerbatimStringLiteral(ref discarded);
if (errorCode is ErrorCode code)
{
TrySetRecoverableError(_lexer.MakeError(nestedStringPosition, width: 2, code));
Expand All @@ -632,17 +610,14 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
var interpolations = (ArrayBuilder<Interpolation>?)null;
var info = default(TokenInfo);
bool wasVerbatim = _isVerbatim;
bool wasAllowNewlines = _allowNewlines;
try
{
_isVerbatim = true;
_allowNewlines = true;
ScanInterpolatedStringLiteralTop(interpolations, ref info, closeQuoteMissing: out _);
}
finally
{
_isVerbatim = wasVerbatim;
_allowNewlines = wasAllowNewlines;
}
continue;
}
Expand All @@ -652,15 +627,10 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
switch (_lexer.TextWindow.PeekChar(1))
{
case '/':
if (!_isVerbatim || !_allowNewlines)
{
// error: single-line comment not allowed in an interpolated string.
// report the error but keep going for good error recovery.
TrySetRecoverableError(_lexer.MakeError(_lexer.TextWindow.Position, 2, ErrorCode.ERR_SingleLineCommentInExpressionHole));
}

_lexer.TextWindow.AdvanceChar(); // skip /
_lexer.TextWindow.AdvanceChar(); // skip /

// read up to the end of the line.
while (!IsAtEnd(allowNewline: false))
{
_lexer.TextWindow.AdvanceChar(); // skip // comment character
Expand Down Expand Up @@ -710,26 +680,17 @@ private void ScanInterpolatedStringLiteralNestedComment()
_lexer.TextWindow.AdvanceChar();
while (true)
{
var ch = _lexer.TextWindow.PeekChar();

// See if we ran into a disallowed new line. If so, this is recoverable, so just skip past it but
// give a good message about the issue. This will prevent a lot of cascading issues with the remainder
// of the interpolated string that comes on the following lines.
var allowNewLines = _isVerbatim && _allowNewlines;
if (!allowNewLines && SyntaxFacts.IsNewLine(ch))
{
TrySetRecoverableError(_lexer.MakeError(_lexer.TextWindow.Position, width: 0, ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString));
}

if (IsAtEnd(allowNewline: true))
{
return; // let the caller complain about the unterminated quote
this.TrySetUnrecoverableError(_lexer.MakeError(_lexer.TextWindow.Position, 1, ErrorCode.ERR_OpenEndedComment));
return;
}

var ch = _lexer.TextWindow.PeekChar();
_lexer.TextWindow.AdvanceChar();
if (ch == '*' && _lexer.TextWindow.PeekChar() == '/')
{
_lexer.TextWindow.AdvanceChar(); // skip */
_lexer.TextWindow.AdvanceChar();
return;
}
}
Expand Down
29 changes: 15 additions & 14 deletions src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,9 @@ public static void Main(string[] args)
// (5,63): error CS1010: Newline in constant
// Console.WriteLine($"Jenny don\'t change your number { ");
Diagnostic(ErrorCode.ERR_NewlineInConst, "").WithLocation(5, 63),
// (5,66): error CS8967: Newlines are not allowed inside a non-verbatim interpolated string
// Console.WriteLine($"Jenny don\'t change your number { ");
Diagnostic(ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString, "").WithLocation(5, 66),
// (6,5): error CS1010: Newline in constant
// }
Diagnostic(ErrorCode.ERR_NewlineInConst, "}").WithLocation(6, 5),
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
// (6,6): error CS1026: ) expected
// }
Diagnostic(ErrorCode.ERR_CloseParenExpected, "").WithLocation(6, 6),
Expand All @@ -159,9 +159,9 @@ public static void Main(string[] args)
}";
// too many diagnostics perhaps, but it starts the right way.
CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (5,71): error CS8077: A single-line comment may not be used in an interpolated string.
// Console.WriteLine($"Jenny don\'t change your number { 8675309 // ");
Diagnostic(ErrorCode.ERR_SingleLineCommentInExpressionHole, "//").WithLocation(5, 71),
// (6,5): error CS1010: Newline in constant
// }
Diagnostic(ErrorCode.ERR_NewlineInConst, "}").WithLocation(6, 5),
// (6,6): error CS1026: ) expected
// }
Diagnostic(ErrorCode.ERR_CloseParenExpected, "").WithLocation(6, 6),
Expand Down Expand Up @@ -189,9 +189,9 @@ public static void Main(string[] args)
// (5,71): error CS1035: End-of-file found, '*/' expected
// Console.WriteLine($"Jenny don\'t change your number { 8675309 /* ");
Diagnostic(ErrorCode.ERR_OpenEndedComment, "").WithLocation(5, 71),
// (5,77): error CS8967: Newlines are not allowed inside a non-verbatim interpolated string
// Console.WriteLine($"Jenny don\'t change your number { 8675309 /* ");
Diagnostic(ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString, "").WithLocation(5, 77),
// (7,2): error CS1035: End-of-file found, '*/' expected
// }
Diagnostic(ErrorCode.ERR_OpenEndedComment, "").WithLocation(7, 2),
Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi Sep 29, 2021 •
edited by jcouv
Loading

Choose a reason for hiding this comment

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

getting a duplicated error. investigating. #Closed

// (7,2): error CS1026: ) expected
// }
Diagnostic(ErrorCode.ERR_CloseParenExpected, "").WithLocation(7, 2),
Expand Down Expand Up @@ -358,12 +358,13 @@ static void Main(string[] args)
Console.WriteLine( $""{"" );
}
}";
CreateCompilationWithMscorlib45(source).VerifyDiagnostics( // (6,31): error CS1010: Newline in constant
// Console.WriteLine( $"{" );
Diagnostic(ErrorCode.ERR_NewlineInConst, "").WithLocation(6, 31),
// (6,35): error CS8958: Newlines are not allowed inside a non-verbatim interpolated string
CreateCompilationWithMscorlib45(source).VerifyDiagnostics(
// (6,31): error CS1010: Newline in constant
// Console.WriteLine( $"{" );
Diagnostic(ErrorCode.ERR_NewlinesAreNotAllowedInsideANonVerbatimInterpolatedString, "").WithLocation(6, 35),
Diagnostic(ErrorCode.ERR_NewlineInConst, "").WithLocation(6, 31),
// (7,5): error CS1010: Newline in constant
// }
Diagnostic(ErrorCode.ERR_NewlineInConst, "}").WithLocation(7, 5),
// (7,6): error CS1026: ) expected
// }
Diagnostic(ErrorCode.ERR_CloseParenExpected, "").WithLocation(7, 6),
Expand Down
Loading