Merge pull request #739 from ahoppen/ahoppen/diagnose-missing-nodes

Emit diagnostics for missing nodes

Author: ahoppen

Sources/SwiftDiagnostics/Diagnostic.swift

@@ -19,16 +19,21 @@ public struct Diagnostic: CustomDebugStringConvertible {
   /// The node at whose start location the message should be displayed.
   public let node: Syntax
 
+  /// The position at which the location should be anchored.
+  /// By default, this is the start location of `node`.
+  public let position: AbsolutePosition
+
   /// Nodes that should be highlighted in the source code.
   public let highlights: [Syntax]
 
   /// Fix-Its that can be applied to resolve this diagnostic.
   /// Each Fix-It offers a different way to resolve the diagnostic. Usually, there's only one.
   public let fixIts: [FixIt]
 
-  public init(node: Syntax, message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = []) {
-    self.diagMessage = message
+  public init(node: Syntax, position: AbsolutePosition? = nil, message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = []) {
     self.node = node
+    self.position = position ?? node.positionAfterSkippingLeadingTrivia
+    self.diagMessage = message
     self.highlights = highlights
     self.fixIts = fixIts
   }
@@ -46,7 +51,7 @@ public struct Diagnostic: CustomDebugStringConvertible {
 
   /// The location at which the diagnostic should be displayed.
   public func location(converter: SourceLocationConverter) -> SourceLocation {
-    return node.startLocation(converter: converter)
+    return converter.location(for: position)
   }
 
   public var debugDescription: String {

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -67,15 +67,15 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   // MARK: - Private helper functions
 
   /// Produce a diagnostic.
-  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
+  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, position: AbsolutePosition? = nil, _ message: DiagnosticMessage, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
     diagnostics.removeAll(where: { handledNodes.contains($0.node.id) })
-    diagnostics.append(Diagnostic(node: Syntax(node), message: message, highlights: highlights, fixIts: fixIts))
+    diagnostics.append(Diagnostic(node: Syntax(node), position: position, message: message, highlights: highlights, fixIts: fixIts))
     self.handledNodes.append(contentsOf: handledNodes)
   }
 
   /// Produce a diagnostic.
-  private func addDiagnostic<T: SyntaxProtocol>(_ node: T, _ message: StaticParserError, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
-    addDiagnostic(node, message as DiagnosticMessage, highlights: highlights, fixIts: fixIts, handledNodes: handledNodes)
+  private func addDiagnostic<T: SyntaxProtocol>(_ node: T,position: AbsolutePosition? = nil,  _ message: StaticParserError, highlights: [Syntax] = [], fixIts: [FixIt] = [], handledNodes: [SyntaxIdentifier] = []) {
+    addDiagnostic(node, position: position, message as DiagnosticMessage, highlights: highlights, fixIts: fixIts, handledNodes: handledNodes)
   }
 
   /// Whether the node should be skipped for diagnostic emission.
@@ -118,8 +118,40 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .skipChildren
   }
 
+  private func handleMissingSyntax<T: SyntaxProtocol>(_ node: T) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    addDiagnostic(node, position: node.endPosition, MissingNodeError(missingNode: Syntax(node)))
+    return .visitChildren
+  }
+
   // MARK: - Specialized diagnostic generation
 
+  public override func visit(_ node: MissingDeclSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
+  public override func visit(_ node: MissingExprSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
+  public override func visit(_ node: MissingPatternSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
+  public override func visit(_ node: MissingStmtSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
+  public override func visit(_ node: MissingSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
+  public override func visit(_ node: MissingTypeSyntax) -> SyntaxVisitorContinueKind {
+    return handleMissingSyntax(node)
+  }
+
   public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
@@ -145,7 +177,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
           Syntax(node.unexpectedBetweenWhereClauseAndBody),
           Syntax(unexpectedCondition)
         ] as [Syntax?]).compactMap({ $0 }),
-        handledNodes: [node.inKeyword.id, unexpectedCondition.id]
+        handledNodes: [node.inKeyword.id, node.sequenceExpr.id, unexpectedCondition.id]
       )
     }
     return .visitChildren

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -22,13 +22,19 @@ extension SyntaxProtocol {
   /// default.
   /// Pass inherit: false to prevent this behavior, in which case `nil` will be
   /// returned instead.
-  func nodeTypeNameForDiagnostics(inherit: Bool = true) -> String? {
+  /// If `allowSourceFile` is `false`, `nil` will be returned if the inherited
+  /// node type name is "source file".
+  func nodeTypeNameForDiagnostics(inherit: Bool = true, allowSourceFile: Bool = true) -> String? {
     if let name = Syntax(self).as(SyntaxEnum.self).nameForDiagnostics {
-      return name
+      if Syntax(self).is(SourceFileSyntax.self) && !allowSourceFile {
+        return nil
+      } else {
+        return name
+      }
     }
     if inherit {
       if let parent = self.parent {
-        return parent.nodeTypeNameForDiagnostics(inherit: inherit)
+        return parent.nodeTypeNameForDiagnostics(inherit: inherit, allowSourceFile: allowSourceFile)
       }
     }
     return nil
@@ -135,6 +141,38 @@ 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)"
+      }
+    }
+    return message
+  }
+}
+
 public struct MissingTokenError: ParserError {
   public let missingToken: TokenSyntax
 

Sources/SwiftParser/Directives.swift

@@ -89,7 +89,7 @@ extension Parser {
         do {
           var elementsProgress = LoopProgressCondition()
           while !self.at(any: [.eof, .poundElseKeyword, .poundElseifKeyword, .poundEndifKeyword]) && elementsProgress.evaluate(currentToken) {
-            guard let element = parseElement(&self), element.raw.byteLength > 0 else {
+            guard let element = parseElement(&self), !element.isEmpty else {
               break
             }
             elements.append(element)

Sources/SwiftParser/Expressions.swift

@@ -1617,21 +1617,27 @@ extension Parser {
 
         switch elementKind! {
         case .array(let el):
-          elements.append(RawSyntax(RawArrayElementSyntax(
-            expression: el, trailingComma: comma, arena: self.arena)))
-          if el.is(RawMissingExprSyntax.self) {
+          let element = RawArrayElementSyntax(
+            expression: el, trailingComma: comma, arena: self.arena
+          )
+          if element.isEmpty {
             break COLLECTION_LOOP
+          } else {
+            elements.append(RawSyntax(element))
           }
         case .dictionary(let key, let unexpectedBeforeColon, let colon, let value):
-          elements.append(RawSyntax(RawDictionaryElementSyntax(
+          let element = RawDictionaryElementSyntax(
             keyExpression: key,
             unexpectedBeforeColon,
             colon: colon,
             valueExpression: value,
             trailingComma: comma,
-            arena: self.arena)))
-          if key.is(RawMissingExprSyntax.self), colon.isMissing, value.is(RawMissingExprSyntax.self) {
+            arena: self.arena
+          )
+          if element.isEmpty {
             break COLLECTION_LOOP
+          } else {
+            elements.append(RawSyntax(element))
           }
         }
 

Sources/SwiftParser/Statements.swift

@@ -865,7 +865,7 @@ extension Parser {
     let expr: RawExprSyntax?
     if
       !self.at(any: [
-        RawTokenKind.rightBrace, .semicolon, .eof,
+        .rightBrace, .caseKeyword, .semicolon, .eof,
         .poundIfKeyword, .poundErrorKeyword, .poundWarningKeyword,
         .poundEndifKeyword, .poundElseKeyword, .poundElseifKeyword
       ])

Sources/SwiftParser/TopLevel.swift

@@ -119,7 +119,7 @@ extension Parser {
     let item = self.parseItem(isAtTopLevel: isAtTopLevel)
     let semi = self.consume(if: .semicolon)
 
-    if item.raw.byteLength == 0 && semi == nil {
+    if item.isEmpty && semi == nil {
       return nil
     }
     return .init(item: item, semicolon: semi, errorTokens: nil, arena: self.arena)

Sources/SwiftSyntax/Misc.swift.gyb

@@ -39,6 +39,19 @@ extension Syntax {
 % for node in NON_BASE_SYNTAX_NODES:
     case .${node.swift_syntax_kind}(let node):
       return node
+% end
+    }
+  }
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    switch self.as(SyntaxEnum.self) {
+    case .token(let node):
+      return node.childNameForDiagnostics(index)
+    case .unknown(let node):
+      return node.childNameForDiagnostics(index)
+% for node in NON_BASE_SYNTAX_NODES:
+    case .${node.swift_syntax_kind}(let node):
+      return node.childNameForDiagnostics(index)
 % end
     }
   }

Sources/SwiftSyntax/Raw/RawSyntaxNodeProtocol.swift

@@ -42,6 +42,10 @@ public extension RawSyntaxNodeProtocol {
   func write<Target>(to target: inout Target) where Target : TextOutputStream {
     raw.write(to: &target)
   }
+
+  var isEmpty: Bool {
+    return raw.byteLength == 0
+  }
 }
 
 /// `RawSyntax` itself conforms to `RawSyntaxNodeProtocol`.

Sources/SwiftSyntax/Syntax.swift

@@ -131,6 +131,10 @@ public protocol SyntaxProtocol: CustomStringConvertible,
   /// Converts the given `Syntax` node to this type. Returns `nil` if the
   /// conversion is not possible.
   init?(_ syntaxNode: Syntax)
+
+  /// Return a name with which the child at the given `index` can be referred to
+  /// in diagnostics.
+  func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String?
 }
 
 extension SyntaxProtocol {

Sources/SwiftSyntax/SyntaxBaseNodes.swift.gyb

@@ -120,6 +120,10 @@ public struct ${node.name}: ${node.name}Protocol, SyntaxHashable {
   public func asProtocol(_: ${node.name}Protocol.Protocol) -> ${node.name}Protocol {
     return Syntax(self).asProtocol(${node.name}Protocol.self)!
   }
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    return Syntax(self).childNameForDiagnostics(index)
+  }
 }
 
 extension ${node.name}: CustomReflectable {

Sources/SwiftSyntax/SyntaxChildren.swift

@@ -20,7 +20,7 @@ public struct SyntaxChildrenIndexData: Comparable {
   fileprivate let offset: UInt32
   /// The index of the node in the parent.
   /// See `AbsoluteSyntaxPosition.indexInParent`
-  fileprivate let indexInParent: UInt32
+  let indexInParent: UInt32
   /// Unique value for a node within its own tree.
   /// See `SyntaxIdentifier.indexIntree`
   fileprivate let indexInTree: SyntaxIndexInTree
@@ -61,7 +61,7 @@ public struct SyntaxChildrenIndex: Comparable, ExpressibleByNilLiteral {
   /// SyntaxChildrenIndex` an enumbecause the optional value can be
   /// force-unwrapped when we know that an index is not the end index, saving a
   /// switch case comparison.
-  fileprivate let data: SyntaxChildrenIndexData?
+  let data: SyntaxChildrenIndexData?
 
   fileprivate init(offset: UInt32, indexInParent: UInt32,
                    indexInTree: SyntaxIndexInTree) {

Sources/SwiftSyntax/SyntaxCollections.swift.gyb

@@ -217,6 +217,10 @@ public struct ${node.name}: SyntaxCollection, SyntaxHashable {
       self = withTrailingTrivia(newValue ?? [])
     }
   }
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    return nil
+  }
 }
 
 /// Conformance for `${node.name}` to the `BidirectionalCollection` protocol.

Sources/SwiftSyntax/SyntaxNodes.swift.gyb.template

@@ -171,6 +171,21 @@ public struct ${node.name}: ${base_type}Protocol, SyntaxHashable {
     return ${node.name}(newData)
   }
 %     end
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    switch index.data?.indexInParent {
+%     for (index, child) in enumerate(node.children):
+    case ${index}:
+%       if child.name_for_diagnostics:
+      return "${child.name_for_diagnostics}"
+%       else:
+      return nil
+%       end
+%     end
+    default:
+      fatalError("Invalid index")
+    }
+  }
 }
 
 extension ${node.name}: CustomReflectable {

Sources/SwiftSyntax/SyntaxOtherNodes.swift

@@ -39,6 +39,10 @@ public struct UnknownSyntax: SyntaxProtocol, SyntaxHashable {
     let data = SyntaxData.forRoot(raw)
     self.init(data)
   }
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    return nil
+  }
 }
 
 extension UnknownSyntax: CustomReflectable {
@@ -192,6 +196,10 @@ public struct TokenSyntax: SyntaxProtocol, SyntaxHashable {
   public var totalLength: SourceLength {
     return raw.totalLength
   }
+
+  public func childNameForDiagnostics(_ index: SyntaxChildrenIndex) -> String? {
+    return nil
+  }
 }
 
 extension TokenSyntax: CustomReflectable {