Parse directives in trailing trivia as skipped tokens trivia (#75724)

* Handle directives in trailing trivia

* Revert the change and improve tests

* Attach trailing directives as skipped tokens trivia

* Update tests

* Parse as bad directive trivia

* Simplify tests

* Remove trailing parameter

* Parse as skipped tokens trivia

* Carry trivia over

* Wrap trivia in a token

* Put text into one token

* Simplify width logic

* Refactor to simplify the code

* Add more tests

* Pass `false` and explain

Author: jjonescz

src/Compilers/CSharp/Portable/Parser/DirectiveParser.cs

@@ -31,15 +31,10 @@ public void ReInitialize(DirectiveStack context)
         public CSharpSyntaxNode ParseDirective(
             bool isActive,
             bool endIsActive,
-            bool isAfterFirstTokenInFile,
-            bool isAfterNonWhitespaceOnLine)
+            bool isFollowingToken)
         {
             var hashPosition = lexer.TextWindow.Position;
             var hash = this.EatToken(SyntaxKind.HashToken, false);
-            if (isAfterNonWhitespaceOnLine)
-            {
-                hash = this.AddError(hash, ErrorCode.ERR_BadDirectivePlacement);
-            }
 
             // The behavior of these directives when isActive is false is somewhat complicated.
             // The key functions in the native compiler are ScanPreprocessorIfSection and
@@ -81,7 +76,7 @@ public CSharpSyntaxNode ParseDirective(
 
                 case SyntaxKind.DefineKeyword:
                 case SyntaxKind.UndefKeyword:
-                    result = this.ParseDefineOrUndefDirective(hash, this.EatContextualToken(contextualKind), isActive, isAfterFirstTokenInFile && !isAfterNonWhitespaceOnLine);
+                    result = this.ParseDefineOrUndefDirective(hash, this.EatContextualToken(contextualKind), isActive, isFollowingToken);
                     break;
 
                 case SyntaxKind.ErrorKeyword:
@@ -101,11 +96,11 @@ public CSharpSyntaxNode ParseDirective(
                     break;
 
                 case SyntaxKind.ReferenceKeyword:
-                    result = this.ParseReferenceDirective(hash, this.EatContextualToken(contextualKind), isActive, isAfterFirstTokenInFile && !isAfterNonWhitespaceOnLine);
+                    result = this.ParseReferenceDirective(hash, this.EatContextualToken(contextualKind), isActive, isFollowingToken);
                     break;
 
                 case SyntaxKind.LoadKeyword:
-                    result = this.ParseLoadDirective(hash, this.EatContextualToken(contextualKind), isActive, isAfterFirstTokenInFile && !isAfterNonWhitespaceOnLine);
+                    result = this.ParseLoadDirective(hash, this.EatContextualToken(contextualKind), isActive, isFollowingToken);
                     break;
 
                 case SyntaxKind.NullableKeyword:
@@ -121,16 +116,13 @@ public CSharpSyntaxNode ParseDirective(
                     {
                         var id = this.EatToken(SyntaxKind.IdentifierToken, false);
                         var end = this.ParseEndOfDirective(ignoreErrors: true);
-                        if (!isAfterNonWhitespaceOnLine)
+                        if (!id.IsMissing)
                         {
-                            if (!id.IsMissing)
-                            {
-                                id = this.AddError(id, ErrorCode.ERR_PPDirectiveExpected);
-                            }
-                            else
-                            {
-                                hash = this.AddError(hash, ErrorCode.ERR_PPDirectiveExpected);
-                            }
+                            id = this.AddError(id, ErrorCode.ERR_PPDirectiveExpected);
+                        }
+                        else
+                        {
+                            hash = this.AddError(hash, ErrorCode.ERR_PPDirectiveExpected);
                         }
 
                         result = SyntaxFactory.BadDirectiveTrivia(hash, id, end, isActive);

src/Compilers/CSharp/Portable/Parser/Lexer.cs

@@ -303,7 +303,7 @@ private static int GetFullWidth(SyntaxListBuilder? builder)
         private SyntaxToken LexSyntaxToken()
         {
             _leadingTriviaCache.Clear();
-            this.LexSyntaxTrivia(afterFirstToken: TextWindow.Position > 0, isTrailing: false, triviaList: ref _leadingTriviaCache);
+            this.LexSyntaxTrivia(isFollowingToken: TextWindow.Position > 0, isTrailing: false, triviaList: ref _leadingTriviaCache);
             var leading = _leadingTriviaCache;
 
             var tokenInfo = default(TokenInfo);
@@ -313,7 +313,7 @@ private SyntaxToken LexSyntaxToken()
             var errors = this.GetErrors(GetFullWidth(leading));
 
             _trailingTriviaCache.Clear();
-            this.LexSyntaxTrivia(afterFirstToken: true, isTrailing: true, triviaList: ref _trailingTriviaCache);
+            this.LexSyntaxTrivia(isFollowingToken: true, isTrailing: true, triviaList: ref _trailingTriviaCache);
             var trailing = _trailingTriviaCache;
 
             return Create(in tokenInfo, leading, trailing, errors);
@@ -322,15 +322,15 @@ private SyntaxToken LexSyntaxToken()
         internal SyntaxTriviaList LexSyntaxLeadingTrivia()
         {
             _leadingTriviaCache.Clear();
-            this.LexSyntaxTrivia(afterFirstToken: TextWindow.Position > 0, isTrailing: false, triviaList: ref _leadingTriviaCache);
+            this.LexSyntaxTrivia(isFollowingToken: TextWindow.Position > 0, isTrailing: false, triviaList: ref _leadingTriviaCache);
             return new SyntaxTriviaList(default(Microsoft.CodeAnalysis.SyntaxToken),
                 _leadingTriviaCache.ToListNode(), position: 0, index: 0);
         }
 
         internal SyntaxTriviaList LexSyntaxTrailingTrivia()
         {
             _trailingTriviaCache.Clear();
-            this.LexSyntaxTrivia(afterFirstToken: true, isTrailing: true, triviaList: ref _trailingTriviaCache);
+            this.LexSyntaxTrivia(isFollowingToken: true, isTrailing: true, triviaList: ref _trailingTriviaCache);
             return new SyntaxTriviaList(default(Microsoft.CodeAnalysis.SyntaxToken),
                 _trailingTriviaCache.ToListNode(), position: 0, index: 0);
         }
@@ -1889,7 +1889,7 @@ private bool ScanIdentifierOrKeyword(ref TokenInfo info)
             }
         }
 
-        private void LexSyntaxTrivia(bool afterFirstToken, bool isTrailing, ref SyntaxListBuilder triviaList)
+        private void LexSyntaxTrivia(bool isFollowingToken, bool isTrailing, ref SyntaxListBuilder triviaList)
         {
             bool onlyWhitespaceOnLine = !isTrailing;
 
@@ -1995,7 +1995,33 @@ private void LexSyntaxTrivia(bool afterFirstToken, bool isTrailing, ref SyntaxLi
                     case '#':
                         if (_allowPreprocessorDirectives)
                         {
-                            this.LexDirectiveAndExcludedTrivia(afterFirstToken, isTrailing || !onlyWhitespaceOnLine, ref triviaList);
+                            if (isTrailing || !onlyWhitespaceOnLine)
+                            {
+                                // Directives cannot be in trailing trivia.
+                                // We parse the directive (ignoring its effects like disabled text or defined symbols).
+                                // We extract the text corresponding to the parsed directive
+                                // and add it as a BadToken in SkippedTokensTrivia.
+
+                                var savePosition = TextWindow.Position;
+
+                                // All the `false` arguments affect only error reporting and the resulting node, both of which we discard.
+                                // However, passing `false` can skip some unnecessary checks.
+                                this.ParseDirective(isActive: false, endIsActive: false, isFollowingToken: false);
+
+                                var text = TextWindow.Text.GetSubText(TextSpan.FromBounds(savePosition, TextWindow.Position));
+
+                                var error = new SyntaxDiagnosticInfo(offset: 0, width: 1, code: ErrorCode.ERR_BadDirectivePlacement);
+
+                                var token = SyntaxFactory.BadToken(null, text.ToString(), null).WithDiagnosticsGreen([error]);
+
+                                this.AddTrivia(SyntaxFactory.SkippedTokensTrivia(token), ref triviaList);
+                            }
+                            else
+                            {
+                                this.LexDirectiveAndExcludedTrivia(isFollowingToken, ref triviaList);
+                            }
+
+                            onlyWhitespaceOnLine = true;
                             break;
                         }
                         else
@@ -2332,13 +2358,12 @@ private static SyntaxTrivia CreateWhitespaceTrivia(SlidingTextWindow textWindow)
         }
 
         private void LexDirectiveAndExcludedTrivia(
-            bool afterFirstToken,
-            bool afterNonWhitespaceOnLine,
+            bool isFollowingToken,
             ref SyntaxListBuilder triviaList)
         {
-            var directive = this.LexSingleDirective(true, true, afterFirstToken, afterNonWhitespaceOnLine, ref triviaList);
+            var directive = this.LexSingleDirective(true, true, isFollowingToken, ref triviaList);
 
-            // also lex excluded stuff            
+            // also lex excluded stuff
             var branching = directive as BranchingDirectiveTriviaSyntax;
             if (branching != null && !branching.BranchTaken)
             {
@@ -2362,7 +2387,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, ref triviaList);
                 var branching = directive as BranchingDirectiveTriviaSyntax;
                 if (directive.Kind == SyntaxKind.EndIfDirectiveTrivia || (branching != null && branching.BranchTaken))
                 {
@@ -2378,8 +2403,7 @@ private void LexExcludedDirectivesAndTrivia(bool endIsActive, ref SyntaxListBuil
         private CSharpSyntaxNode LexSingleDirective(
             bool isActive,
             bool endIsActive,
-            bool afterFirstToken,
-            bool afterNonWhitespaceOnLine,
+            bool isFollowingToken,
             ref SyntaxListBuilder triviaList)
         {
             if (SyntaxFacts.IsWhitespace(TextWindow.PeekChar()))
@@ -2388,16 +2412,25 @@ private CSharpSyntaxNode LexSingleDirective(
                 this.AddTrivia(this.ScanWhitespace(), ref triviaList);
             }
 
-            CSharpSyntaxNode directive;
+            CSharpSyntaxNode directive = ParseDirective(isActive, endIsActive, isFollowingToken);
+
+            this.AddTrivia(directive, ref triviaList);
+            _directives = directive.ApplyDirectives(_directives);
+            return directive;
+        }
+
+        private CSharpSyntaxNode ParseDirective(
+            bool isActive,
+            bool endIsActive,
+            bool isFollowingToken)
+        {
             var saveMode = _mode;
 
             _directiveParser ??= new DirectiveParser(this);
             _directiveParser.ReInitialize(_directives);
 
-            directive = _directiveParser.ParseDirective(isActive, endIsActive, afterFirstToken, afterNonWhitespaceOnLine);
+            CSharpSyntaxNode directive = _directiveParser.ParseDirective(isActive, endIsActive, isFollowingToken);
 
-            this.AddTrivia(directive, ref triviaList);
-            _directives = directive.ApplyDirectives(_directives);
             _mode = saveMode;
             return directive;
         }

src/Compilers/CSharp/Test/Syntax/LexicalAndXml/PreprocessorTests.cs

@@ -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);
         }
 
         [Fact]
@@ -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);
         }
 
         [Fact]