Merge missing token diagnostics if they occur at the same source location

Author: ahoppen

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -117,11 +117,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
           let previousParent = invalidIdentifier.parent?.as(UnexpectedNodesSyntax.self) {
         addDiagnostic(invalidIdentifier, InvalidIdentifierError(invalidIdentifier: invalidIdentifier), handledNodes: [previousParent.id])
       } else {
-        addDiagnostic(node, MissingTokenError(missingToken: node), fixIts: [
-          FixIt(message: InsertTokenFixIt(missingToken: node), changes: [
-            .makePresent(node: node)
-          ])
-        ])
+        return handleMissingSyntax(node)
       }
     }
     return .skipChildren
@@ -131,7 +127,44 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
-    addDiagnostic(node, position: node.endPosition, MissingNodeError(missingNode: Syntax(node)))
+
+    let position: AbsolutePosition
+    let fixIts: [FixIt]
+    if let token = Syntax(node).as(TokenSyntax.self) {
+      position = token.position
+      fixIts = [
+        FixIt(message: InsertTokenFixIt(missingToken: token), changes: [
+          .makePresent(node: node)
+        ])
+      ]
+    } else {
+      position = node.endPosition
+      fixIts = []
+    }
+
+    // Find the previous sibling, skipping over siblings that don't actually contain any source text.
+    var previousSibling: Syntax? = node.previousSibling(viewMode: .all)
+    while let previousSiblingUnwrapped = previousSibling,
+          !previousSiblingUnwrapped.is(TokenSyntax.self),
+          !previousSiblingUnwrapped.raw.kind.isMissing,
+          previousSiblingUnwrapped.raw.isEmpty {
+      previousSibling = previousSibling?.previousSibling(viewMode: .all)
+    }
+
+    if let previousSibling = previousSibling,
+       let previousMissingTokenDiag = diagnostics.filter({ $0.node == Syntax(previousSibling) }).first,
+       let previousMissingTokenDiagMessage = previousMissingTokenDiag.diagMessage as? MissingNodesError {
+      // Merge into previous diagnostic
+      addDiagnostic(
+        node,
+        position: position,
+        MissingNodesError(missingNodes: previousMissingTokenDiagMessage.missingNodes + [Syntax(node)]),
+        fixIts: previousMissingTokenDiag.fixIts + fixIts,
+        handledNodes: [previousMissingTokenDiag.node.id]
+      )
+    } else {
+      addDiagnostic(node, position: position, MissingNodesError(missingNode: Syntax(node)), fixIts: fixIts)
+    }
     return .visitChildren
   }
 

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -12,6 +12,7 @@
 
 import SwiftDiagnostics
 @_spi(RawSyntax) import SwiftSyntax
+import SwiftBasicFormat
 
 let diagnosticDomain: String = "SwiftParser"
 
@@ -131,79 +132,184 @@ public struct InvalidIdentifierError: ParserError {
   }
 }
 
-public struct MissingNodeError: ParserError {
-  public let missingNode: Syntax
-
-  public var message: String {
-    var message: String
-    var hasNamedParent = false
-    if let parent = missingNode.parent,
-       let childName = parent.childNameForDiagnostics(missingNode.index) {
-      message = "Expected \(childName)"
-      if let parentTypeName = parent.nodeTypeNameForDiagnostics(inherit: false) {
-        message += " of \(parentTypeName)"
-        hasNamedParent = true
-      }
-    } else {
-      message = "Expected \(missingNode.nodeTypeNameForDiagnostics() ?? "syntax")"
-      if let missingDecl = missingNode.as(MissingDeclSyntax.self), let lastModifier = missingDecl.modifiers?.last {
-        message += " after '\(lastModifier.name.text)' modifier"
-      } else if let missingDecl = missingNode.as(MissingDeclSyntax.self), missingDecl.attributes != nil {
-        message += " after attribute"
-      } else if let previousToken = missingNode.previousToken(viewMode: .fixedUp), previousToken.presence == .present {
-        message += " after '\(previousToken.text)'"
-      }
-    }
-    if !hasNamedParent {
-      if let parent = missingNode.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics(allowSourceFile: false) {
-        message += " in \(parentTypeName)"
-      }
+private extension String {
+  /// Remove any leading or trailing whitespace
+  func trimmingWhitespaces() -> String {
+    var result: Substring = Substring(self)
+    result = result.drop(while: { $0 == " " })
+    while result.last == " " {
+      result = result.dropLast(1)
     }
-    return message
+    return String(result)
   }
 }
 
-public struct MissingAttributeArgument: ParserError {
-  /// The name of the attribute that's missing the argument, without `@`.
-  public let attributeName: TokenSyntax
+public struct MissingNodesError: ParserError {
+  public let missingNodes: [Syntax]
+  public let commonParent: Syntax?
 
-  public var message: String {
-    return "Expected argument for '@\(attributeName)' attribute"
+  init(missingNode: Syntax) {
+    self.missingNodes = [missingNode]
+    self.commonParent = missingNode.parent
   }
-}
 
-public struct MissingTokenError: ParserError {
-  public let missingToken: TokenSyntax
+  init(missingNodes: [Syntax]) {
+    assert(!missingNodes.isEmpty)
+    self.missingNodes = missingNodes
+    self.commonParent = missingNodes.first!.parent
+    assert(missingNodes.allSatisfy({ $0.parent == self.commonParent }))
+  }
 
-  public var message: String {
-    var message = "Expected"
-    if missingToken.text.isEmpty {
-      message += " \(missingToken.tokenKind.decomposeToRaw().rawKind.nameForDiagnostics)"
+  /// Returns a list of the tokens that are missing
+  private var missingNodesDescription: String {
+    if let codeBlock = commonParent?.as(CodeBlockSyntax.self),
+        missingNodes.contains(Syntax(codeBlock.leftBrace)),
+        missingNodes.contains(Syntax(codeBlock.rightBrace)) {
+      return "code block"
+    } else if let memberDeclBlock = commonParent?.as(MemberDeclBlockSyntax.self),
+                missingNodes.contains(Syntax(memberDeclBlock.leftBrace)),
+                missingNodes.contains(Syntax(memberDeclBlock.rightBrace)) {
+      return "member block"
+    }
+
+    enum Part {
+      case sourceText(String)
+      case tokenWithoutDefaultText(RawTokenKind)
+      case node(Syntax)
+
+      var description: String {
+        switch self {
+        case .sourceText(let content):
+          return "'\(content.trimmingWhitespaces())'"
+        case .tokenWithoutDefaultText(let tokenKind):
+          return tokenKind.nameForDiagnostics
+        case .node(let node):
+          if let parent = node.parent,
+             let childName = parent.childNameForDiagnostics(node.index) {
+            return "\(childName)"
+          } else {
+            return "\(node.nodeTypeNameForDiagnostics() ?? "syntax")"
+          }
+        }
+      }
+    }
+
+    var parts: [Part] = []
+    for missingNode in missingNodes {
+      if let missingToken = missingNode.as(TokenSyntax.self) {
+        let newPart: Part
+        let (rawKind, text) = missingToken.tokenKind.decomposeToRaw()
+        if let text = text, !text.isEmpty {
+          let presentToken = TokenSyntax(missingToken.tokenKind, presence: .present)
+          newPart = .sourceText("\(Syntax(Format().format(syntax: presentToken)))")
+        } else {
+          let newKind: TokenKind
+          if let defaultText = rawKind.defaultText {
+            newKind = TokenKind.fromRaw(kind: rawKind, text: String(syntaxText: defaultText))
+            let presentToken = TokenSyntax(newKind, presence: .present)
+            newPart = .sourceText("\(Syntax(Format().format(syntax: presentToken)))")
+          } else {
+            newPart = .tokenWithoutDefaultText(rawKind)
+          }
+        }
+
+        switch (parts.last, newPart) {
+        case (.sourceText(let previousContent), .sourceText(let newContent)):
+          parts[parts.count - 1] = .sourceText(previousContent + newContent)
+        default:
+          parts.append(newPart)
+        }
+      } else {
+        parts.append(.node(missingNode))
+      }
+    }
+    let partDescriptions = parts.map({ $0.description })
+    if partDescriptions.count > 1 {
+      return "\(partDescriptions[0..<partDescriptions.count - 1].joined(separator: ", ")) and \(partDescriptions.last!)"
     } else {
-      message += " '\(missingToken.text)'"
+      return "\(partDescriptions.joined(separator: ", "))"
     }
-    if let parent = missingToken.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics() {
+  }
+
+  /// If applicable, returns a string that describes after which node the nodes are expected.
+  private var afterClause: String? {
+    if !missingNodes.first!.is(TokenSyntax.self) {
+      if let missingDecl = missingNodes.first?.as(MissingDeclSyntax.self), let lastModifier = missingDecl.modifiers?.last {
+        return "after '\(lastModifier.name.text)' modifier"
+      } else if let missingDecl = missingNodes.first?.as(MissingDeclSyntax.self), missingDecl.attributes != nil {
+        return "after attribute"
+      } else if let previousToken = missingNodes.first?.previousToken(viewMode: .fixedUp), previousToken.presence == .present {
+        return "after '\(previousToken.text)'"
+      }
+    }
+    return nil
+  }
+
+  /// If applicable, returns a string that describes the node in which the missing nodes are expected.
+  private var parentContextClause: String? {
+    enum ContextType {
+      case toStart
+      case `in`
+      case toEnd
+    }
+    var nodePosition = ContextType.in
+
+    if let missingToken = missingNodes.first?.as(TokenSyntax.self), let commonParent = commonParent {
       switch missingToken.tokenKind {
-      case .leftAngle, .leftBrace, .leftParen, .leftSquareBracket:
-        if parent.children(viewMode: .fixedUp).first?.as(TokenSyntax.self) == missingToken {
-          message += " to start \(parentTypeName)"
+      case .leftBrace, .leftAngle, .leftParen, .leftSquareBracket:
+        if commonParent.children(viewMode: .fixedUp).first?.as(TokenSyntax.self) == missingToken {
+          nodePosition = .toStart
         }
-      case .rightAngle, .rightBrace, .rightParen, .rightSquareBracket:
-        if parent.children(viewMode: .fixedUp).last?.as(TokenSyntax.self) == missingToken {
-          message += " to end \(parentTypeName)"
+      default:
+        break
+      }
+    }
+    if let missingToken = missingNodes.last?.as(TokenSyntax.self), let commonParent = commonParent {
+      switch missingToken.tokenKind {
+      case .rightBrace, .rightAngle, .rightParen, .rightSquareBracket:
+        if commonParent.children(viewMode: .fixedUp).last?.as(TokenSyntax.self) == missingToken {
+          if nodePosition == .toStart {
+            // If the missing tokens encomposs both the start and end, emit an 'in' context clause
+            nodePosition = .in
+          } else {
+            nodePosition = .toEnd
+          }
         }
       default:
-        message += " in \(parentTypeName)"
+        break
       }
     }
-    return message
+    if let commonParent = commonParent, let parentTypeName = commonParent.nodeTypeNameForDiagnostics(allowSourceFile: false) {
+      switch nodePosition {
+      case .toStart:
+        return "to start \(parentTypeName)"
+      case .in:
+        return "in \(parentTypeName)"
+      case .toEnd:
+        return "to end \(parentTypeName)"
+      }
+    }
+    return nil
   }
 
-  public var handledNodes: [Syntax] {
-    if let previous = missingToken.previousToken(viewMode: .all), previous.parent!.is(UnexpectedNodesSyntax.self) {
-      return [Syntax(previous.parent!)]
+  public var message: String {
+    var message = "Expected \(self.missingNodesDescription)"
+    if let afterClause = afterClause {
+      message += " \(afterClause)"
+    }
+    if let parentContextClause = parentContextClause {
+      message += " \(parentContextClause)"
     }
-    return []
+    return message
+  }
+}
+
+public struct MissingAttributeArgument: ParserError {
+  /// The name of the attribute that's missing the argument, without `@`.
+  public let attributeName: TokenSyntax
+
+  public var message: String {
+    return "Expected argument for '@\(attributeName)' attribute"
   }
 }
 

Sources/SwiftSyntax/Syntax.swift

@@ -207,6 +207,31 @@ public extension SyntaxProtocol {
     return parent != nil
   }
 
+  func previousSibling(viewMode: SyntaxTreeViewMode) -> Syntax? {
+    guard let parent = self.parent else {
+      return nil
+    }
+    let siblings = NonNilRawSyntaxChildren(parent, viewMode: viewMode)
+    if let previousAbsoluteRawSibling = siblings[..<self.index].last {
+      return Syntax(SyntaxData(previousAbsoluteRawSibling, parent: parent))
+    } else {
+      return nil
+    }
+  }
+
+  func nextSibling(viewMode: SyntaxTreeViewMode) -> Syntax? {
+    guard let parent = self.parent else {
+      return nil
+    }
+    let siblings = NonNilRawSyntaxChildren(parent, viewMode: viewMode)
+    let nextSiblingIndex = siblings.index(after: self.index)
+    if let previousAbsoluteRawSibling = siblings[nextSiblingIndex...].last {
+      return Syntax(SyntaxData(previousAbsoluteRawSibling, parent: parent))
+    } else {
+      return nil
+    }
+  }
+
   /// Recursively walks through the tree to find the token semantically before
   /// this node.
   var previousToken: TokenSyntax? {

Sources/SwiftSyntax/SyntaxKind.swift.gyb

@@ -27,7 +27,7 @@ public enum SyntaxKind {
   case ${node.swift_syntax_kind}
 % end
 
-  var isSyntaxCollection: Bool {
+  public var isSyntaxCollection: Bool {
     switch self {
 % for node in SYNTAX_NODES:
 %   if node.base_kind == "SyntaxCollection":
@@ -39,7 +39,7 @@ public enum SyntaxKind {
     }
   }
 
-  var isUnknown: Bool {
+  public var isUnknown: Bool {
     switch self {
 % for name in SYNTAX_BASE_KINDS:
 %   if name not in ["Syntax", "SyntaxCollection"]:
@@ -51,7 +51,7 @@ public enum SyntaxKind {
     }
   }
 
-  var isMissing: Bool {
+  public var isMissing: Bool {
     switch self {
 % for name in SYNTAX_BASE_KINDS:
 %   if name not in ["Syntax", "SyntaxCollection"]:

Sources/SwiftSyntax/gyb_generated/SyntaxKind.swift

@@ -288,7 +288,7 @@ public enum SyntaxKind {
   case availabilityVersionRestriction
   case versionTuple
 
-  var isSyntaxCollection: Bool {
+  public var isSyntaxCollection: Bool {
     switch self {
     case .codeBlockItemList: return true
     case .unexpectedNodes: return true
@@ -339,7 +339,7 @@ public enum SyntaxKind {
     }
   }
 
-  var isUnknown: Bool {
+  public var isUnknown: Bool {
     switch self {
     case .unknownDecl: return true
     case .unknownExpr: return true
@@ -351,7 +351,7 @@ public enum SyntaxKind {
     }
   }
 
-  var isMissing: Bool {
+  public var isMissing: Bool {
     switch self {
     case .missingDecl: return true
     case .missingExpr: return true