Address review comments to swiftlang#1650

This addresses the review comments I made on swiftlang#1650.

Co-Authored-By: Rintaro Ishizaki <rintaro@apple.com>

Author: ahoppen

Sources/SwiftParser/Declarations.swift

@@ -30,24 +30,30 @@ extension DeclarationModifier {
 
 extension TokenConsumer {
   mutating func atStartOfFreestandingMacroExpansion() -> Bool {
+    // Check if "'#' <identifier>" where the identifier is on the sameline.
     if !self.at(.pound) {
       return false
     }
-    if self.peek().rawTokenKind != .identifier && !self.peek().isLexerClassifiedKeyword {
+    if self.peek().isAtStartOfLine {
       return false
     }
-    if self.currentToken.trailingTriviaByteLength != 0 || self.peek().leadingTriviaByteLength != 0 {
+    switch self.peek().rawTokenKind {
+    case .identifier:
+      return true
+    case .keyword:
+      // allow keywords right after '#' so we can diagnose it when parsing.
+      return (self.currentToken.trailingTriviaByteLength == 0 && self.peek().leadingTriviaByteLength == 0)
+    default:
       return false
     }
-    return true
   }
 
   mutating func atStartOfDeclaration(
     isAtTopLevel: Bool = false,
     allowInitDecl: Bool = true,
     allowRecovery: Bool = false
   ) -> Bool {
-    if self.at(anyIn: PoundDeclarationStart.self) != nil {
+    if self.at(.poundIfKeyword) {
       return true
     }
 
@@ -188,13 +194,9 @@ extension Parser {
   /// If `inMemberDeclList` is `true`, we know that the next item must be a
   /// declaration and thus start with a keyword. This allows futher recovery.
   mutating func parseDeclaration(inMemberDeclList: Bool = false) -> RawDeclSyntax {
-    switch self.at(anyIn: PoundDeclarationStart.self) {
-    case (.poundIfKeyword, _)?:
-      if self.withLookahead({ $0.consumeIfConfigOfAttributes() }) {
-        // If we are at a `#if` of attributes, the `#if` directive should be
-        // parsed when we're parsing the attributes.
-        break
-      }
+    // If we are at a `#if` of attributes, the `#if` directive should be
+    // parsed when we're parsing the attributes.
+    if self.at(.poundIfKeyword) && !self.withLookahead({ $0.consumeIfConfigOfAttributes() }) {
       let directive = self.parsePoundIfDirective { (parser, _) in
         let parsedDecl = parser.parseDeclaration()
         let semicolon = parser.consume(if: .semicolon)
@@ -220,8 +222,6 @@ extension Parser {
         return .decls(RawMemberDeclListSyntax(elements: elements, arena: parser.arena))
       }
       return RawDeclSyntax(directive)
-    case nil:
-      break
     }
 
     let attrs = DeclAttributes(
@@ -2022,25 +2022,24 @@ extension Parser {
     _ handle: RecoveryConsumptionHandle
   ) -> RawMacroExpansionDeclSyntax {
 
-    let (unexpectedBeforePound, poundKeyword) = self.eat(handle)
-    // Don't allow space between '#' and the macro name.
-    if poundKeyword.trailingTriviaByteLength != 0 || self.currentToken.leadingTriviaByteLength != 0 {
-      return RawMacroExpansionDeclSyntax(
-        attributes: attrs.attributes,
-        modifiers: attrs.modifiers,
-        unexpectedBeforePound,
-        poundToken: poundKeyword,
-        macro: self.missingToken(.identifier),
-        genericArguments: nil,
-        leftParen: nil,
-        argumentList: .init(elements: [], arena: self.arena),
-        rightParen: nil,
-        trailingClosure: nil,
-        additionalTrailingClosures: nil,
-        arena: self.arena
-      )
+    var (unexpectedBeforePound, pound) = self.eat(handle)
+    if pound.trailingTriviaByteLength != 0 {
+      // `#` and the macro name must not be separated by a newline.
+      unexpectedBeforePound = RawUnexpectedNodesSyntax(combining: unexpectedBeforePound, pound, arena: self.arena)
+      pound = RawTokenSyntax(missing: .pound, text: "#", leadingTriviaPieces: pound.leadingTriviaPieces, arena: self.arena)
+    }
+    var unexpectedBeforeMacro: RawUnexpectedNodesSyntax?
+    var macro: RawTokenSyntax
+    if !self.currentToken.isAtStartOfLine {
+      (unexpectedBeforeMacro, macro) = self.expectIdentifier(keywordRecovery: true)
+      if macro.leadingTriviaByteLength != 0 {
+        unexpectedBeforeMacro = RawUnexpectedNodesSyntax(combining: unexpectedBeforeMacro, macro, arena: self.arena)
+        pound = self.missingToken(.identifier, text: macro.tokenText)
+      }
+    } else {
+      unexpectedBeforeMacro = nil
+      macro = self.missingToken(.identifier)
     }
-    let (unexpectedBeforeMacro, macro) = self.expectIdentifier(keywordRecovery: true)
 
     // Parse the optional generic argument list.
     let generics: RawGenericArgumentClauseSyntax?
@@ -2081,7 +2080,7 @@ extension Parser {
       attributes: attrs.attributes,
       modifiers: attrs.modifiers,
       unexpectedBeforePound,
-      poundToken: poundKeyword,
+      poundToken: pound,
       unexpectedBeforeMacro,
       macro: macro,
       genericArguments: generics,

Sources/SwiftParser/Expressions.swift

@@ -1409,21 +1409,25 @@ extension Parser {
     pattern: PatternContext,
     flavor: ExprFlavor
   ) -> RawMacroExpansionExprSyntax {
-    if !atStartOfFreestandingMacroExpansion() {
-      return RawMacroExpansionExprSyntax(
-        poundToken: self.consumeAnyToken(),
-        macro: self.missingToken(.identifier),
-        genericArguments: nil,
-        leftParen: nil,
-        argumentList: .init(elements: [], arena: self.arena),
-        rightParen: nil,
-        trailingClosure: nil,
-        additionalTrailingClosures: nil,
-        arena: self.arena
-      )
+    var (unexpectedBeforePound, pound) = self.expect(.pound)
+    if pound.trailingTriviaByteLength != 0 {
+      // If there are whitespaces after '#' diagnose.
+      unexpectedBeforePound = RawUnexpectedNodesSyntax(combining: unexpectedBeforePound, pound, arena: self.arena)
+      pound = self.missingToken(.pound)
+    }
+    var unexpectedBeforeMacro: RawUnexpectedNodesSyntax?
+    var macro: RawTokenSyntax
+    if !self.currentToken.isAtStartOfLine {
+      (unexpectedBeforeMacro, macro) = self.expectIdentifier(keywordRecovery: true)
+      if macro.leadingTriviaByteLength != 0 {
+        // If there're whitespaces after '#' diagnose.
+        unexpectedBeforeMacro = RawUnexpectedNodesSyntax(combining: unexpectedBeforeMacro, macro, arena: self.arena)
+        pound = self.missingToken(.identifier, text: macro.tokenText)
+      }
+    } else {
+      unexpectedBeforeMacro = nil
+      macro = self.missingToken(.identifier)
     }
-    let poundKeyword = self.consumeAnyToken()
-    let (unexpectedBeforeMacro, macro) = self.expectIdentifier(keywordRecovery: true)
 
     // Parse the optional generic argument list.
     let generics: RawGenericArgumentClauseSyntax?
@@ -1458,7 +1462,8 @@ extension Parser {
     }
 
     return RawMacroExpansionExprSyntax(
-      poundToken: poundKeyword,
+      unexpectedBeforePound,
+      poundToken: pound,
       unexpectedBeforeMacro,
       macro: macro,
       genericArguments: generics,

Sources/SwiftParser/TokenSpecSet.swift

@@ -397,23 +397,6 @@ enum OperatorLike: TokenSpecSet {
   }
 }
 
-enum PoundDeclarationStart: TokenSpecSet {
-  case poundIfKeyword
-
-  init?(lexeme: Lexer.Lexeme) {
-    switch lexeme.rawTokenKind {
-    case .poundIfKeyword: self = .poundIfKeyword
-    default: return nil
-    }
-  }
-
-  var spec: TokenSpec {
-    switch self {
-    case .poundIfKeyword: return .poundIfKeyword
-    }
-  }
-}
-
 enum SwitchCaseStart: TokenSpecSet {
   case caseKeyword
   case defaultKeyword

Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

@@ -59,7 +59,7 @@ extension FixIt.MultiNodeChange {
   /// removed node will be transferred to the trailing trivia of the previous token.
   static func makeMissing(_ tokens: [TokenSyntax], transferTrivia: Bool = true) -> Self {
     precondition(!tokens.isEmpty)
-    precondition(tokens.allSatisfy({ $0.presence == .present }))
+    precondition(tokens.allSatisfy({ $0.isPresent }))
     var changes = tokens.map {
       FixIt.Change.replace(
         oldNode: Syntax($0),
@@ -111,7 +111,7 @@ extension FixIt.MultiNodeChange {
 class MissingNodesBasicFormatter: BasicFormat {
   override func isMutable(_ token: TokenSyntax) -> Bool {
     // Assume that all missing nodes will be made present by the Fix-It.
-    return token.presence == .missing
+    return token.isMissing
   }
 }
 
@@ -153,7 +153,7 @@ extension FixIt.MultiNodeChange {
     }
 
     if let previousToken = node.previousToken(viewMode: .fixedUp),
-      previousToken.presence == .present,
+      previousToken.isPresent,
       let firstToken = node.firstToken(viewMode: .all),
       previousToken.trailingTrivia.allSatisfy({ $0.isWhitespace }),
       !BasicFormat().requiresWhitespace(between: previousToken, and: firstToken),

Sources/SwiftParserDiagnostics/MissingNodesError.swift

@@ -71,7 +71,7 @@ fileprivate enum NodesDescriptionPart {
     for missingNode in nodes {
       if let token = missingNode.as(TokenSyntax.self) {
         let newPart: NodesDescriptionPart
-        if token.presence == .present {
+        if token.isPresent {
           newPart = .tokensWithDefaultText([token])
         } else {
           let (rawKind, text) = token.tokenKind.decomposeToRaw()
@@ -356,7 +356,7 @@ extension ParseDiagnosticsGenerator {
       let siblings = parentWithTokens.children(viewMode: .all)
       let siblingsAfter = siblings[siblings.index(after: index)...]
       for sibling in siblingsAfter {
-        if sibling.as(TokenSyntax.self)?.presence == .missing {
+        if sibling.as(TokenSyntax.self)?.isMissing ?? false {
           // Handle missing sibling tokens
           missingNodes += [sibling]
         } else if sibling.raw.kind.isMissing {

Sources/SwiftParserDiagnostics/MissingTokenError.swift

@@ -105,7 +105,7 @@ extension ParseDiagnosticsGenerator {
 
     if let identifier = missingToken.nextToken(viewMode: .all),
       identifier.tokenView.rawKind == .identifier,
-      identifier.presence == .missing
+      identifier.isMissing
     {
       // The extraneous whitespace caused a missing identifier, output a
       // diagnostic inserting it instead of a diagnostic to fix the trivia
@@ -123,7 +123,7 @@ extension ParseDiagnosticsGenerator {
       )
     } else {
       let fixIt = FixIt(message: .removeExtraneousWhitespace, changes: changes)
-      addDiagnostic(invalidToken, .invalidWhitespaceAfterPeriod, fixIts: [fixIt], handledNodes: [invalidTokenContainer.id])
+      addDiagnostic(invalidToken, ExtraneousWhitespace(tokenWithWhitespace: invalidToken), fixIts: [fixIt], handledNodes: [invalidTokenContainer.id])
     }
     return true
   }