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

Move feature check from lexer to binder. #57113

Merged
merged 3 commits into from
Oct 13, 2021
Merged

Move feature check from lexer to binder. #57113

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0168b61
Move feature check from lexer to binder.
CyrusNajmabadi Oct 12, 2021
71b5b2d
Add braces
CyrusNajmabadi Oct 12, 2021
267fc94
Inline check
CyrusNajmabadi Oct 12, 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
9 changes: 9 additions & 0 deletions src/Compilers/CSharp/Portable/Binder/Binder_InterpolatedString.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,15 @@ internal partial class Binder
{
private BoundExpression BindInterpolatedString(InterpolatedStringExpressionSyntax node, BindingDiagnosticBag diagnostics)
{
var startText = node.StringStartToken.Text;
if (startText.StartsWith("@$\"") && !Compilation.IsFeatureEnabled(MessageID.IDS_FeatureAltInterpolatedVerbatimStrings))
{
Error(diagnostics,
ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable,
node.StringStartToken.GetLocation(),
new CSharpRequiredLanguageVersion(MessageID.IDS_FeatureAltInterpolatedVerbatimStrings.RequiredVersion()));
}

var builder = ArrayBuilder<BoundExpression>.GetInstance();
var stringType = GetSpecialType(SpecialType.System_String, diagnostics, node);
ConstantValue? resultConstant = null;
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/CSharp/Portable/Errors/MessageID.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ internal static LanguageVersion RequiredVersion(this MessageID feature)
return LanguageVersion.CSharp9;

// C# 8.0 features.
case MessageID.IDS_FeatureAltInterpolatedVerbatimStrings:
case MessageID.IDS_FeatureAltInterpolatedVerbatimStrings: // semantic check
case MessageID.IDS_FeatureCoalesceAssignmentExpression:
case MessageID.IDS_FeatureUnconstrainedTypeParameterInNullCoalescingOperator:
case MessageID.IDS_FeatureNullableReferenceTypes: // syntax and semantic check
Expand Down
5 changes: 0 additions & 5 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser_InterpolatedString.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,6 @@ private ExpressionSyntax ParseInterpolatedStringToken()
: "$\"";
var openQuote = SyntaxFactory.Token(originalToken.GetLeadingTrivia(), openQuoteKind, openQuoteText, openQuoteText, trailing: null);

if (isAltInterpolatedVerbatim)
{
openQuote = CheckFeatureAvailability(openQuote, MessageID.IDS_FeatureAltInterpolatedVerbatimStrings);
}
Copy link
Member Author

Choose a reason for hiding this comment

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

parser check.


// Make a token for the close quote " (even if it was missing)
var closeQuoteIndex = closeQuoteMissing ? originalText.Length : originalText.Length - 1;
Debug.Assert(closeQuoteMissing || originalText[closeQuoteIndex] == '"');
Expand Down
1 change: 0 additions & 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 @@ -771,7 +771,6 @@ private void ScanSyntaxToken(ref TokenInfo info)
else if (TextWindow.PeekChar(1) == '$' && TextWindow.PeekChar(2) == '"')
{
this.ScanInterpolatedStringLiteral(isVerbatim: true, ref info);
CheckFeatureAvailability(MessageID.IDS_FeatureAltInterpolatedVerbatimStrings);
Copy link
Member Author

Choose a reason for hiding this comment

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

lexer check #1. intended to be overridden by parser.

break;
}
else if (!this.ScanIdentifierOrKeyword(ref info))
Expand Down
1 change: 0 additions & 1 deletion 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 @@ -628,7 +628,6 @@ private void ScanInterpolatedStringLiteralHoleBalancedText(char endingChar, bool
}
else if (_lexer.TextWindow.PeekChar(1) == '$' && _lexer.TextWindow.PeekChar(2) == '"')
{
_lexer.CheckFeatureAvailability(MessageID.IDS_FeatureAltInterpolatedVerbatimStrings);
Copy link
Member Author

Choose a reason for hiding this comment

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

lexer check #2, during recursive interpolated string lexing. Intended to be overridden by parser.

var interpolations = (ArrayBuilder<Interpolation>?)null;
var info = default(TokenInfo);
bool wasVerbatim = _isVerbatim;
Expand Down
6 changes: 0 additions & 6 deletions src/Compilers/CSharp/Portable/Parser/SyntaxParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1091,12 +1091,6 @@ protected TNode CheckFeatureAvailability<TNode>(TNode node, MessageID feature, b
return availableVersion >= LanguageVersion.CSharp2
? node
: this.AddError(node, ErrorCode.WRN_NonECMAFeature, feature.Localize());

case MessageID.IDS_FeatureAltInterpolatedVerbatimStrings:
return availableVersion >= requiredVersion
? node
: this.AddError(node, ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable,
new CSharpRequiredLanguageVersion(requiredVersion));
Copy link
Member Author

Choose a reason for hiding this comment

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

special case error message. moved to https://github.com/dotnet/roslyn/pull/57113/files#diff-a1cb0e0d2615ca34e6c6cfc96a0300cb85c046ca6f90bf0d8931021ec8ccc480R334

}

var info = feature.GetFeatureAvailabilityDiagnosticInfo(this.Options);
Expand Down
62 changes: 51 additions & 11 deletions src/Compilers/CSharp/Test/Syntax/Parsing/ExpressionParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,11 +60,20 @@ public void TestInterpolatedVerbatimString()
[Fact]
public void TestAltInterpolatedVerbatimString_CSharp73()
{
UsingExpression(@"@$""hello""", TestOptions.Regular7_3,
// (1,1): error CS8401: To use '@$' instead of '$@' for an interpolated verbatim string, please use language version '8.0' or greater.
// @$"hello"
Diagnostic(ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable, @"@$""").WithArguments("8.0").WithLocation(1, 1)
);
var text = @"@$""hello""";
CreateCompilation($@"
class C
{{
void M()
{{
var v = {text};
}}
}}", parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_3)).VerifyDiagnostics(
// (6,17): error CS8370: Feature 'alternative interpolated verbatim strings' is not available in C# 7.3. Please use language version 8.0 or greater.
// var v = @$"hello";
Diagnostic(ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable, @"@$""").WithArguments("8.0").WithLocation(6, 17));

UsingExpression(text, TestOptions.Regular7_3);

N(SyntaxKind.InterpolatedStringExpression);
{
Expand All @@ -81,7 +90,17 @@ public void TestAltInterpolatedVerbatimString_CSharp73()
[Fact]
public void TestAltInterpolatedVerbatimString_CSharp8()
{
UsingExpression(@"@$""hello""", TestOptions.Regular8);
var text = @"@$""hello""";
CreateCompilation($@"
class C
{{
void M()
{{
var v = {text};
}}
}}", parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8)).VerifyDiagnostics();

UsingExpression(text, TestOptions.Regular8);
N(SyntaxKind.InterpolatedStringExpression);
{
N(SyntaxKind.InterpolatedVerbatimStringStartToken);
Expand All @@ -97,11 +116,20 @@ public void TestAltInterpolatedVerbatimString_CSharp8()
[Fact]
public void TestNestedAltInterpolatedVerbatimString_CSharp73()
{
UsingExpression("$@\"aaa{@$\"bbb\nccc\"}ddd\"", TestOptions.Regular7_3,
// (1,8): error CS8401: To use '@$' instead of '$@' for an interpolated verbatim string, please use language version '8.0' or greater.
var text = "$@\"aaa{@$\"bbb\nccc\"}ddd\"";
CreateCompilation($@"
class C
{{
void M()
{{
var v = {text};
}}
}}", parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp7_3)).VerifyDiagnostics(
// (6, 24): error CS8401: To use '@$' instead of '$@' for an interpolated verbatim string, please use language version '8.0' or greater.
// $@"aaa{@$"bbb
Diagnostic(ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable, @"@$""").WithArguments("8.0").WithLocation(1, 8)
);
Diagnostic(ErrorCode.ERR_AltInterpolatedVerbatimStringsNotAvailable, @"@$""").WithArguments("8.0").WithLocation(6, 24));

UsingExpression(text, TestOptions.Regular7_3);

N(SyntaxKind.InterpolatedStringExpression);
{
Expand Down Expand Up @@ -136,7 +164,19 @@ public void TestNestedAltInterpolatedVerbatimString_CSharp73()
[Fact]
public void TestNestedAltInterpolatedVerbatimString_CSharp8()
{
UsingExpression("$@\"aaa{@$\"bbb\nccc\"}ddd\"", TestOptions.Regular8);
var text = "$@\"aaa{@$\"bbb\nccc\"}ddd\"";

CreateCompilation($@"
class C
{{
void M()
{{
var v = {text};
}}
}}", parseOptions: CSharpParseOptions.Default.WithLanguageVersion(LanguageVersion.CSharp8)).VerifyDiagnostics();

UsingExpression(text, TestOptions.Regular8);

N(SyntaxKind.InterpolatedStringExpression);
{
N(SyntaxKind.InterpolatedVerbatimStringStartToken);
Expand Down