Repository: apple/swift-syntax (swift)

Issue:
Additional declaration allowed on same line after `#endif`
### Issue Kind

Other

### Source Code

```swift
struct Foo {
  #if false
  var x: Int
#endif; var x = 1
}
```


### Description

The above example produces an error *Extra tokens following conditional compilation directive* in the old parser but is incorrectly accepted by the new parser.

Pull request:
Resolves #1823

Patch:
diff --git a/Sources/SwiftBasicFormat/BasicFormat.swift b/Sources/SwiftBasicFormat/BasicFormat.swift
index 090218d75ff..6dec7d7962f 100644
--- a/Sources/SwiftBasicFormat/BasicFormat.swift
+++ b/Sources/SwiftBasicFormat/BasicFormat.swift
@@ -186,12 +186,12 @@ open class BasicFormat: SyntaxRewriter {
     }
 
     switch (first?.tokenKind, second?.tokenKind) {
-
     case (.multilineStringQuote, .backslash),  // string interpolation segment inside a multi-line string literal
       (.multilineStringQuote, .multilineStringQuote),  // empty multi-line string literal
       (.multilineStringQuote, .stringSegment),  // segment starting a multi-line string literal
       (.stringSegment, .multilineStringQuote),  // ending a multi-line string literal that has a string interpolation segment at its end
       (.rightParen, .multilineStringQuote),  // ending a multi-line string literal that has a string interpolation segment at its end
+      (.poundEndifKeyword, _),
       (_, .poundElseKeyword),
       (_, .poundElseifKeyword),
       (_, .poundEndifKeyword),
diff --git a/Sources/SwiftParser/Directives.swift b/Sources/SwiftParser/Directives.swift
index ac5d3aeda22..6ac4d73f516 100644
--- a/Sources/SwiftParser/Directives.swift
+++ b/Sources/SwiftParser/Directives.swift
@@ -106,12 +106,14 @@ extension Parser {
     // Parse #if
     let (unexpectedBeforePoundIfKeyword, poundIfKeyword) = self.expect(.poundIfKeyword)
     let condition = RawExprSyntax(self.parseSequenceExpression(.basic, forDirective: true))
+    let unexpectedBetweenConditionAndElements = self.consumeRemainingTokenOnLine()
 
     clauses.append(
       RawIfConfigClauseSyntax(
         unexpectedBeforePoundIfKeyword,
         poundKeyword: poundIfKeyword,
         condition: condition,
+        unexpectedBetweenConditionAndElements,
         elements: syntax(&self, parseIfConfigClauseElements(parseElement, addSemicolonIfNeeded: addSemicolonIfNeeded)),
         arena: self.arena
       )
@@ -123,11 +125,13 @@ extension Parser {
       var unexpectedBeforePoundKeyword: RawUnexpectedNodesSyntax?
       var poundKeyword: RawTokenSyntax
       let condition: RawExprSyntax?
+      let unexpectedBetweenConditionAndElements: RawUnexpectedNodesSyntax?
 
       switch match {
       case .poundElseifKeyword:
         (unexpectedBeforePoundKeyword, poundKeyword) = self.eat(handle)
         condition = RawExprSyntax(self.parseSequenceExpression(.basic, forDirective: true))
+        unexpectedBetweenConditionAndElements = self.consumeRemainingTokenOnLine()
       case .poundElseKeyword:
         (unexpectedBeforePoundKeyword, poundKeyword) = self.eat(handle)
         if let ifToken = self.consume(if: .init(.if, allowAtStartOfLine: false)) {
@@ -137,6 +141,7 @@ extension Parser {
         } else {
           condition = nil
         }
+        unexpectedBetweenConditionAndElements = self.consumeRemainingTokenOnLine()
       case .pound:
         if self.atElifTypo() {
           (unexpectedBeforePoundKeyword, poundKeyword) = self.eat(handle)
@@ -146,6 +151,7 @@ extension Parser {
           unexpectedBeforePoundKeyword = RawUnexpectedNodesSyntax(combining: unexpectedBeforePoundKeyword, poundKeyword, elif, arena: self.arena)
           poundKeyword = self.missingToken(.poundElseifKeyword)
           condition = RawExprSyntax(self.parseSequenceExpression(.basic, forDirective: true))
+          unexpectedBetweenConditionAndElements = self.consumeRemainingTokenOnLine()
         } else {
           break LOOP
         }
@@ -156,6 +162,7 @@ extension Parser {
           unexpectedBeforePoundKeyword,
           poundKeyword: poundKeyword,
           condition: condition,
+          unexpectedBetweenConditionAndElements,
           elements: syntax(&self, parseIfConfigClauseElements(parseElement, addSemicolonIfNeeded: addSemicolonIfNeeded)),
           arena: self.arena
         )
@@ -163,10 +170,13 @@ extension Parser {
     }
 
     let (unexpectedBeforePoundEndIf, poundEndIf) = self.expect(.poundEndifKeyword)
+    let unexpectedAfterPoundEndif = self.consumeRemainingTokenOnLine()
+
     return RawIfConfigDeclSyntax(
       clauses: RawIfConfigClauseListSyntax(elements: clauses, arena: self.arena),
       unexpectedBeforePoundEndIf,
       poundEndif: poundEndIf,
+      unexpectedAfterPoundEndif,
       arena: self.arena
     )
   }
@@ -256,6 +266,8 @@ extension Parser {
       args = nil
     }
     let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen)
+    let unexpectedAfterRightParen = self.consumeRemainingTokenOnLine()
+
     return RawPoundSourceLocationSyntax(
       poundSourceLocation: line,
       unexpectedBeforeLParen,
@@ -263,6 +275,7 @@ extension Parser {
       args: args,
       unexpectedBeforeRParen,
       rightParen: rparen,
+      unexpectedAfterRightParen,
       arena: self.arena
     )
   }
diff --git a/Sources/SwiftParser/Parser.swift b/Sources/SwiftParser/Parser.swift
index c8e0e8896dd..3e9a5404845 100644
--- a/Sources/SwiftParser/Parser.swift
+++ b/Sources/SwiftParser/Parser.swift
@@ -257,6 +257,22 @@ extension Parser {
     return self.consumeAnyToken()
   }
 
+  /// Consumes remaining token on the line and returns a ``RawUnexpectedNodesSyntax``
+  /// if there is any tokens consumed.
+  mutating func consumeRemainingTokenOnLine() -> RawUnexpectedNodesSyntax? {
+    guard !self.currentToken.isAtStartOfLine else {
+      return nil
+    }
+
+    var unexpectedTokens = [RawTokenSyntax]()
+    var loopProgress = LoopProgressCondition()
+    while !self.at(.eof), !currentToken.isAtStartOfLine, loopProgress.evaluate(self.currentToken) {
+      unexpectedTokens += [self.consumeAnyToken()]
+    }
+
+    return RawUnexpectedNodesSyntax(unexpectedTokens, arena: self.arena)
+  }
+
 }
 
 // MARK: Check if we can recover to a token
diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
index e41df780f42..efbab454832 100644
--- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
+++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift
@@ -1100,6 +1100,22 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: IfConfigClauseSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if let unexpectedBetweenConditionAndElements = node.unexpectedBetweenConditionAndElements {
+      addDiagnostic(
+        unexpectedBetweenConditionAndElements,
+        .extraTokensFollowingConditionalCompilationDirective,
+        handledNodes: [unexpectedBetweenConditionAndElements.id]
+      )
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: IfConfigDeclSyntax) -> SyntaxVisitorContinueKind {
     for clause in node.clauses where clause.hasError {
       if let unexpectedBeforePoundKeyword = clause.unexpectedBeforePoundKeyword,
@@ -1136,6 +1152,15 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         }
       }
     }
+
+    if let unexpectedAfterPoundEndif = node.unexpectedAfterPoundEndif {
+      addDiagnostic(
+        unexpectedAfterPoundEndif,
+        .extraTokensFollowingConditionalCompilationDirective,
+        handledNodes: [unexpectedAfterPoundEndif.id]
+      )
+    }
+
     return .visitChildren
   }
 
@@ -1415,6 +1440,22 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: PoundSourceLocationSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if let unexpectedAfterRightParen = node.unexpectedAfterRightParen {
+      addDiagnostic(
+        unexpectedAfterRightParen,
+        .extraTokensAtTheEndOfSourceLocationDirective,
+        handledNodes: [unexpectedAfterRightParen.id]
+      )
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
diff --git a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift
index 8b000d2757e..349aa5641fb 100644
--- a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift
+++ b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift
@@ -156,6 +156,12 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var expectedSequenceExpressionInForEachLoop: Self {
     .init("expected Sequence expression for for-each loop")
   }
+  public static var extraTokensAtTheEndOfSourceLocationDirective: Self {
+    .init("extra tokens following the #sourceLocation directive")
+  }
+  public static var extraTokensFollowingConditionalCompilationDirective: Self {
+    .init("extra tokens following conditional compilation directive")
+  }
   public static var extraRightBracket: Self {
     .init("unexpected ']' in type; did you mean to write an array type?")
   }


Test patch:
diff --git a/Tests/SwiftParserTest/DirectiveTests.swift b/Tests/SwiftParserTest/DirectiveTests.swift
index 6f76a1d02de..780832aec02 100644
--- a/Tests/SwiftParserTest/DirectiveTests.swift
+++ b/Tests/SwiftParserTest/DirectiveTests.swift
@@ -197,4 +197,128 @@ final class DirectiveTests: XCTestCase {
     )
   }
 
+  func testEndIfFollowedByDeclarations() {
+    assertParse(
+      """
+      struct Foo {
+        #if false
+        var x: Int
+        #endif1️⃣; var x = 1
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+  }
+
+  func testIfFollowByDeclarations() {
+    assertParse(
+      """
+      struct Foo {
+        #if DEBUG1️⃣; var x = 1
+        var x: Int
+        #endif
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+
+    assertParse(
+      """
+      struct Foo {
+        #if DEBUG || UAT1️⃣; var x = 1
+        var x: Int
+        #endif
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+  }
+
+  func testElseIfFollowByDeclarations() {
+    assertParse(
+      """
+      struct Foo {
+        #if DEBUG
+        var x: Int = 1
+        #elseif UAT1️⃣; var x = 1
+        var x: Int = 2
+        #endif
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+
+    assertParse(
+      """
+      struct Foo {
+        #if DEBUG
+        var x: Int = 1
+        #elseif UAT || UAT1️⃣; var x = 1
+        var x: Int = 2
+        #endif
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+  }
+
+  func testElseFollowByDeclarations() {
+    assertParse(
+      """
+      struct Foo {
+        #if DEBUG
+        var x: Int = 1
+        #else1️⃣; var x = 1
+        var x: Int = 2
+        #endif
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following conditional compilation directive"
+        )
+      ]
+    )
+  }
+
+  func testSourcelocationDirectiveFollowedByDeclarations() {
+    assertParse(
+      """
+      var sometName: Int
+      #sourceLocation(file: "other.swift", line: 1)
+      var someName: Int
+      """
+    )
+
+    assertParse(
+      """
+      #sourceLocation(file: "other.swift", line: 1)1️⃣; let x = 1
+      """,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "extra tokens following the #sourceLocation directive"
+        )
+      ]
+    )
+  }
 }