Emit diagnostics for missing nodes

Author: ahoppen

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -114,8 +114,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
@@ -138,7 +170,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         Syntax(node.unexpectedBetweenWhereClauseAndBody),
         Syntax(unexpectedCondition)
       ] as [Syntax?]).compactMap({ $0 }))
-      markNodesAsHandled(node.inKeyword.id, unexpectedCondition.id)
+      markNodesAsHandled(node.inKeyword.id, node.sequenceExpr.id, unexpectedCondition.id)
     }
     return .visitChildren
   }

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -16,12 +16,26 @@ import SwiftSyntax
 let diagnosticDomain: String = "SwiftParser"
 
 extension SyntaxProtocol {
-  var nodeTypeNameForDiagnostics: String? {
+  /// Return a name of this syntax node that can be used to describe it in
+  /// diagnostics.
+  /// Nodes that mostly exist for the syntax tree's structure and don't have a
+  /// correspondence in the source code that's meeingful to the user by default
+  /// use the name of the parent node that encloses it. Pass `false` to `inherit`
+  /// to prevent this name inheritance.
+  /// 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 let parent = self.parent {
-      return parent.nodeTypeNameForDiagnostics
+    if inherit {
+      if let parent = self.parent {
+        return parent.nodeTypeNameForDiagnostics(inherit: inherit, allowSourceFile: allowSourceFile)
+      }
     }
     return nil
   }
@@ -114,11 +128,42 @@ public struct ExtaneousCodeAtTopLevel: 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 parent = missingNode.parent,
+          let parentTypeName = parent.nodeTypeNameForDiagnostics(inherit: false) {
+        message += " of \(parentTypeName)"
+        hasNamedParent = true
+      }
+    } else {
+      message = "Expected \(missingNode.nodeTypeNameForDiagnostics() ?? "syntax")"
+      if let lastChild = missingNode.lastToken(viewMode: .fixedUp), lastChild.presence == .present {
+        message += " after '\(lastChild.text)'"
+      } 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
 
   public var message: String {
-    guard let parent = missingToken.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics else {
+    guard let parent = missingToken.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics() else {
       return "Expected '\(missingToken.text)'"
     }
     switch missingToken.tokenKind {
@@ -141,7 +186,7 @@ public struct UnexpectedNodesError: ParserError {
   public let unexpectedNodes: UnexpectedNodesSyntax
 
   public var message: String {
-    let parentTypeName = unexpectedNodes.parent?.nodeTypeNameForDiagnostics
+    let parentTypeName = unexpectedNodes.parent?.nodeTypeNameForDiagnostics()
     let shortContent = unexpectedNodes.contentForDiagnosticsIfShortSingleLine
     switch (parentTypeName, shortContent) {
     case (let parentTypeName?, let shortContent?):

Tests/SwiftParserTest/Attributes.swift

@@ -11,6 +11,7 @@ final class AttributeTests: XCTestCase {
       }
       """,
       diagnostics: [
+        DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected declaration"),
         DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected 'for' in attribute argument"),
         DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
         DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ')' to end attribute"),
@@ -27,7 +28,9 @@ final class AttributeTests: XCTestCase {
       """,
       diagnostics: [
         DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in '@differentiable' argument"),
+        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected parameters of '@differentiable' argument"),
         DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '=' in same type requirement"),
+        DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected right-hand type of same type requirement"),
         DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ')' to end attribute"),
       ]
     )
@@ -36,21 +39,23 @@ final class AttributeTests: XCTestCase {
   func testMissingClosingParenToAttribute() {
     AssertParse(
       """
-      @_specialize(e#^DIAG_1^#
+      @_specialize(e#^DIAG^#
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
-        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ')' to end attribute"),
+        DiagnosticSpec(message: "Expected declaration"),
+        DiagnosticSpec(message: "Expected ':' in attribute argument"),
+        DiagnosticSpec(message: "Expected ')' to end attribute"),
       ]
     )
   }
 
   func testMultipleInvalidSpecializeParams() {
     AssertParse(
       """
-      @_specialize(e#^DIAG_1^#, exported#^DIAG_2^#)
+      @_specialize(e#^DIAG_1^#, exported#^DIAG_2^#)#^DIAG_3^#
       """,
       diagnostics: [
+        DiagnosticSpec(locationMarker: "DIAG_3", message: "Expected declaration after ')'"),
         DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
         DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ':' in attribute argument"),
         DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected 'false' in attribute argument"),

Tests/SwiftParserTest/Declarations.swift

@@ -41,6 +41,7 @@ final class DeclarationTests: XCTestCase {
                   DiagnosticSpec(locationMarker: "DIAG1", message: "Expected '' in function"),
                   DiagnosticSpec(locationMarker: "DIAG1", message: "Expected argument list in function declaration"),
                   DiagnosticSpec(locationMarker: "DIAG2", message: "Expected '=' in same type requirement"),
+                  DiagnosticSpec(locationMarker: "DIAG2", message: "Expected right-hand type of same type requirement"),
                 ])
   }
 
@@ -77,13 +78,15 @@ final class DeclarationTests: XCTestCase {
     AssertParse("class T where t#^DIAG^#",
                 diagnostics: [
                   DiagnosticSpec(message: "Expected '=' in same type requirement"),
+                  DiagnosticSpec(message: "Expected right-hand type of same type requirement"),
                   DiagnosticSpec(message: "Expected '{' to start class"),
                   DiagnosticSpec(message: "Expected '}' to end class"),
                 ])
     AssertParse("class B<#^DIAG_1^#where g#^DIAG_2^#",
                 diagnostics: [
                   DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected '>' to end generic parameter clause"),
                   DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '=' in same type requirement"),
+                  DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected right-hand type of same type requirement"),
                   DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '{' to start class"),
                   DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '}' to end class"),
                 ])
@@ -147,7 +150,8 @@ final class DeclarationTests: XCTestCase {
     AssertParse(
       "_ = foo/* */?.description#^DIAG^#",
       diagnostics: [
-        DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression")
+        DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression"),
+        DiagnosticSpec(message: "Expected expression"),
       ]
     )
 
@@ -396,9 +400,12 @@ final class DeclarationTests: XCTestCase {
     AssertParse(
       """
       struct a {
-        public
+        public#^DIAG^#
       }
-      """
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "Expected declaration after 'public' in struct")
+      ]
     )
   }
 
@@ -489,7 +496,12 @@ final class DeclarationTests: XCTestCase {
 
   func testExtraneousRightBraceRecovery() {
     AssertParse(
-      "class ABC { let def = ghi(jkl: mno) } #^DIAG^#}",
+      """
+      class ABC {
+        let def = ghi(jkl: mno)
+      }
+      #^DIAG^#}
+      """,
       diagnostics: [
         DiagnosticSpec(message: "Extraneous '}' at top level")
       ]
@@ -504,8 +516,8 @@ final class DeclarationTests: XCTestCase {
       }
       """,
       diagnostics: [
-        // FIXME: This diagnostic should be more contextual
-        DiagnosticSpec(message: "Expected '->' in return clause")
+        DiagnosticSpec(message: "Expected '->' in subscript"),
+        DiagnosticSpec(message: "Expected return type in subscript"),
       ]
     )
   }
@@ -552,16 +564,13 @@ final class DeclarationTests: XCTestCase {
   func testExpressionMember() {
     AssertParse(
       """
-      struct S {
-        #^DIAG^#/ ###line 25 "line-directive.swift"
+      struct S {#^EXPECTED_DECL^#
+        #^UNEXPECTED_TEXT^#/ ###line 25 "line-directive.swift"
       }
       """,
       diagnostics: [
-        DiagnosticSpec(
-          message: """
-            Unexpected text '/ ###line 25 "line-directive.swift"' found in struct
-            """
-        )
+        DiagnosticSpec(locationMarker: "EXPECTED_DECL", message: "Expected declaration after '{' in struct"),
+        DiagnosticSpec(locationMarker: "UNEXPECTED_TEXT", message: #"Unexpected text '/ ###line 25 "line-directive.swift"' found in struct"#)
       ]
     )
   }
@@ -735,12 +744,17 @@ final class DeclarationTests: XCTestCase {
   func testMalforedStruct() {
     AssertParse(
       """
-      struct n#^OPENINGBRACES^##if@#^ENDIF^##^CLOSINGBRACES^#
+      struct n#^OPENING_BRACE^#
+      #if#^AFTER_POUND_IF^#
+      @#^END^#
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "OPENINGBRACES", message: "Expected '{' to start struct"),
-        DiagnosticSpec(locationMarker: "ENDIF", message: "Expected '#endif' in conditional compilation block"),
-        DiagnosticSpec(locationMarker: "CLOSINGBRACES", message: "Expected '}' to end struct")
+        DiagnosticSpec(locationMarker: "OPENING_BRACE", message: "Expected '{' to start struct"),
+        DiagnosticSpec(locationMarker: "AFTER_POUND_IF", message: "Expected condition of conditional compilation clause"),
+        DiagnosticSpec(locationMarker: "END", message: "Expected declaration after '@' in conditional compilation clause"),
+        DiagnosticSpec(locationMarker: "END", message: "Expected name of attribute"),
+        DiagnosticSpec(locationMarker: "END", message: "Expected '#endif' in conditional compilation block"),
+        DiagnosticSpec(locationMarker: "END", message: "Expected '}' to end struct")
       ]
     )
   }