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.

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

Handle directives in trailing trivia #75595

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
12 changes: 11 additions & 1 deletion src/Compilers/CSharp/Portable/Parser/DirectiveParser.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ public CSharpSyntaxNode ParseDirective(
bool isActive,
bool endIsActive,
bool isAfterFirstTokenInFile,
bool isAfterNonWhitespaceOnLine)
bool isAfterNonWhitespaceOnLine,
bool trailing)
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
{
var hashPosition = lexer.TextWindow.Position;
var hash = this.EatToken(SyntaxKind.HashToken, false);
Expand All @@ -41,6 +42,15 @@ public CSharpSyntaxNode ParseDirective(
hash = this.AddError(hash, ErrorCode.ERR_BadDirectivePlacement);
}

// A directive should never be in trailing trivia, so ignore those.
if (trailing)
{
Debug.Assert(isAfterNonWhitespaceOnLine);
var id = this.EatToken(SyntaxKind.IdentifierToken, false);
var end = this.ParseEndOfDirective(ignoreErrors: true);
return SyntaxFactory.BadDirectiveTrivia(hash, id, end, isActive);
}

// The behavior of these directives when isActive is false is somewhat complicated.
// The key functions in the native compiler are ScanPreprocessorIfSection and
// ScanAndIgnoreDirective in CSourceData::CPreprocessor.
Expand Down
12 changes: 7 additions & 5 deletions src/Compilers/CSharp/Portable/Parser/Lexer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1980,7 +1980,7 @@ private void LexSyntaxTrivia(bool afterFirstToken, bool isTrailing, ref SyntaxLi
case '#':
if (_allowPreprocessorDirectives)
{
this.LexDirectiveAndExcludedTrivia(afterFirstToken, isTrailing || !onlyWhitespaceOnLine, ref triviaList);
this.LexDirectiveAndExcludedTrivia(afterFirstToken, isTrailing || !onlyWhitespaceOnLine, isTrailing, ref triviaList);
break;
}
else
Expand Down Expand Up @@ -2319,11 +2319,12 @@ private static SyntaxTrivia CreateWhitespaceTrivia(SlidingTextWindow textWindow)
private void LexDirectiveAndExcludedTrivia(
bool afterFirstToken,
bool afterNonWhitespaceOnLine,
bool trailing,
ref SyntaxListBuilder triviaList)
{
var directive = this.LexSingleDirective(true, true, afterFirstToken, afterNonWhitespaceOnLine, ref triviaList);
var directive = this.LexSingleDirective(true, true, afterFirstToken, afterNonWhitespaceOnLine, trailing, ref triviaList);

// also lex excluded stuff
// also lex excluded stuff
var branching = directive as BranchingDirectiveTriviaSyntax;
if (branching != null && !branching.BranchTaken)
{
Expand All @@ -2347,7 +2348,7 @@ private void LexExcludedDirectivesAndTrivia(bool endIsActive, ref SyntaxListBuil
break;
}

var directive = this.LexSingleDirective(false, endIsActive, false, false, ref triviaList);
var directive = this.LexSingleDirective(false, endIsActive, false, false, false, ref triviaList);
var branching = directive as BranchingDirectiveTriviaSyntax;
if (directive.Kind == SyntaxKind.EndIfDirectiveTrivia || (branching != null && branching.BranchTaken))
{
Expand All @@ -2365,6 +2366,7 @@ private CSharpSyntaxNode LexSingleDirective(
bool endIsActive,
bool afterFirstToken,
bool afterNonWhitespaceOnLine,
bool trailing,
ref SyntaxListBuilder triviaList)
{
if (SyntaxFacts.IsWhitespace(TextWindow.PeekChar()))
Expand All @@ -2379,7 +2381,7 @@ private CSharpSyntaxNode LexSingleDirective(
_directiveParser ??= new DirectiveParser(this);
_directiveParser.ReInitialize(_directives);

directive = _directiveParser.ParseDirective(isActive, endIsActive, afterFirstToken, afterNonWhitespaceOnLine);
directive = _directiveParser.ParseDirective(isActive, endIsActive, afterFirstToken, afterNonWhitespaceOnLine, trailing);

this.AddTrivia(directive, ref triviaList);
_directives = directive.ApplyDirectives(_directives);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2426,7 +2426,7 @@ class A { } #define XXX
var node = Parse(text);
TestRoundTripping(node, text, false);
VerifyErrorCode(node, (int)ErrorCode.ERR_BadDirectivePlacement); // CS1040
VerifyDirectivesSpecial(node, new DirectiveInfo { Kind = SyntaxKind.DefineDirectiveTrivia, Status = NodeStatus.IsActive, Text = "XXX" });
VerifyDirectivesSpecial(node, new DirectiveInfo { Kind = SyntaxKind.BadDirectiveTrivia, Status = NodeStatus.IsActive });
}

[Fact]
Expand Down Expand Up @@ -2587,7 +2587,7 @@ class A { } #undef XXX
var node = Parse(text);
TestRoundTripping(node, text, false);
VerifyErrorCode(node, (int)ErrorCode.ERR_BadDirectivePlacement);
VerifyDirectivesSpecial(node, new DirectiveInfo { Kind = SyntaxKind.UndefDirectiveTrivia, Status = NodeStatus.IsActive, Text = "XXX" });
VerifyDirectivesSpecial(node, new DirectiveInfo { Kind = SyntaxKind.BadDirectiveTrivia, Status = NodeStatus.IsActive });
}

[Fact]
Expand Down
44 changes: 33 additions & 11 deletions src/Compilers/CSharp/Test/Syntax/Syntax/SyntaxNodeTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -347,24 +347,21 @@ public void TestGetAllDirectivesUsingDescendantNodes()
public void TestContainsDirective()
{
// Empty compilation unit shouldn't have any directives in it.
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.ScopedKeyword; kind++)
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.BadDirectiveTrivia; kind++)
Assert.False(SyntaxFactory.ParseCompilationUnit("").ContainsDirective(kind));

// basic file shouldn't have any directives in it.
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.ScopedKeyword; kind++)
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.BadDirectiveTrivia; kind++)
Assert.False(SyntaxFactory.ParseCompilationUnit("namespace N { }").ContainsDirective(kind));

// directive in trailing trivia is not a thing
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.ScopedKeyword; kind++)
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.BadDirectiveTrivia; kind++)
{
var compilationUnit = SyntaxFactory.ParseCompilationUnit("namespace N { } #if false");
compilationUnit.GetDiagnostics().Verify(
// (1,17): error CS1040: Preprocessor directives must appear as the first non-whitespace character on a line
// namespace N { } #if false
TestBase.Diagnostic(ErrorCode.ERR_BadDirectivePlacement, "#").WithLocation(1, 17),
// (1,26): error CS1027: #endif directive expected
// namespace N { } #if false
TestBase.Diagnostic(ErrorCode.ERR_EndifDirectiveExpected, "").WithLocation(1, 26));
TestBase.Diagnostic(ErrorCode.ERR_BadDirectivePlacement, "#").WithLocation(1, 17));
Assert.False(compilationUnit.ContainsDirective(kind));
}

Expand All @@ -386,14 +383,14 @@ public void TestContainsDirective()
testContainsHelper1("#undef x", SyntaxKind.UndefDirectiveTrivia);
testContainsHelper1("#warning", SyntaxKind.WarningDirectiveTrivia);

// !# is special and is only recognized at start of a script file and nowhere else.
// #! is special and is only recognized at start of a script file and nowhere else.
testContainsHelper2(new[] { SyntaxKind.ShebangDirectiveTrivia }, SyntaxFactory.ParseCompilationUnit("#!command", options: TestOptions.Script));
testContainsHelper2(new[] { SyntaxKind.BadDirectiveTrivia }, SyntaxFactory.ParseCompilationUnit(" #!command", options: TestOptions.Script));
testContainsHelper2(new[] { SyntaxKind.BadDirectiveTrivia }, SyntaxFactory.ParseCompilationUnit("#!command", options: TestOptions.Regular));

return;

void testContainsHelper1(string directive, params SyntaxKind[] directiveKinds)
static void testContainsHelper1(string directive, params SyntaxKind[] directiveKinds)
{
Assert.True(directiveKinds.Length > 0);

Expand Down Expand Up @@ -469,20 +466,45 @@ class D
"""));
}

void testContainsHelper2(SyntaxKind[] directiveKinds, CompilationUnitSyntax compilationUnit)
static void testContainsHelper2(SyntaxKind[] directiveKinds, CompilationUnitSyntax compilationUnit)
{
Assert.True(compilationUnit.ContainsDirectives);
foreach (var directiveKind in directiveKinds)
Assert.True(compilationUnit.ContainsDirective(directiveKind));

for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.ScopedType; kind++)
for (var kind = SyntaxKind.TildeToken; kind < SyntaxKind.BadDirectiveTrivia; kind++)
{
if (!directiveKinds.Contains(kind))
Assert.False(compilationUnit.ContainsDirective(kind));
}
}
}

[Fact, WorkItem("https://github.com/dotnet/roslyn/issues/75583")]
public void TestContainsDirective_IfIf()
{
var compilationUnit = SyntaxFactory.ParseCompilationUnit("""
if (#if)
""");
compilationUnit.GetDiagnostics().Verify(
// (1,5): error CS1040: Preprocessor directives must appear as the first non-whitespace character on a line
// if (#if)
TestBase.Diagnostic(ErrorCode.ERR_BadDirectivePlacement, "#").WithLocation(1, 5),
// (1,9): error CS1733: Expected expression
// if (#if)
TestBase.Diagnostic(ErrorCode.ERR_ExpressionExpected, "").WithLocation(1, 9),
// (1,9): error CS1026: ) expected
// if (#if)
TestBase.Diagnostic(ErrorCode.ERR_CloseParenExpected, "").WithLocation(1, 9),
// (1,9): error CS1733: Expected expression
// if (#if)
TestBase.Diagnostic(ErrorCode.ERR_ExpressionExpected, "").WithLocation(1, 9),
// (1,9): error CS1002: ; expected
// if (#if)
TestBase.Diagnostic(ErrorCode.ERR_SemicolonExpected, "").WithLocation(1, 9));
Assert.False(compilationUnit.ContainsDirective(SyntaxKind.IfDirectiveTrivia));
}

[Fact]
public void TestGetAllAnnotatedNodesUsingDescendantNodes()
{
Expand Down
3 changes: 3 additions & 0 deletions src/Compilers/Core/Portable/Syntax/SyntaxNode.cs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,9 @@ public bool ContainsDirective(int rawKind)
Debug.Assert(!triviaContainsMatch(current.GetLeadingTriviaCore(), rawKind), "Should not have a match if the token doesn't even have leading trivia");
}

// This assert would fail if `rawKind` is not a directive or it corresponds to BadDirectiveTrivia
// (directives after non-whitespace are put into a trailing BadDirectiveTrivia).
// This method should not be called with such `rawKind`s, though.
Debug.Assert(!triviaContainsMatch(current.GetTrailingTriviaCore(), rawKind), "Should never have a match in trailing trivia");
}
else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,11 @@ public async Task ErrorTitleIsShownOnDisablePragma()
{
await TestInMethodAsync(
"""

#pragma warning disable CS0219$$
jjonescz marked this conversation as resolved.
Show resolved Hide resolved
var i = 0;
#pragma warning restore CS0219

""", GetFormattedErrorTitle(ErrorCode.WRN_UnreferencedVarAssg));
}

Expand All @@ -43,9 +45,11 @@ public async Task ErrorTitleIsShownOnRestorePragma()
{
await TestInMethodAsync(
"""

#pragma warning disable CS0219
var i = 0;
#pragma warning restore CS0219$$

""", GetFormattedErrorTitle(ErrorCode.WRN_UnreferencedVarAssg));
}

Expand All @@ -54,7 +58,9 @@ public async Task DisabledWarningNotExistingInCodeIsDisplayedByTitleWithoutCodeD
{
await TestInMethodAsync(
"""

#pragma warning disable CS0219$$

""", GetFormattedErrorTitle(ErrorCode.WRN_UnreferencedVarAssg));
}

Expand Down
3 changes: 1 addition & 2 deletions src/Workspaces/CSharpTest/Formatting/FormattingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5764,8 +5764,7 @@ public async Task PreprocessorOnSameLine()

var expected = @"class C
{
}
#line default
}#line default

#line hidden";

Expand Down