Merge pull request #1450 from kimdv/kimdv/add-diagnostic-for-wrong-inheritance

Add diagnostic for wrong inheritance

Author: kimdv

Sources/SwiftParser/Nominals.swift

@@ -240,7 +240,7 @@ extension Parser {
     }
 
     let inheritance: RawTypeInheritanceClauseSyntax?
-    if self.at(.colon) {
+    if self.at(.colon) || self.isAtPythonStyleInheritanceClause() {
       inheritance = self.parseInheritance()
     } else {
       inheritance = nil
@@ -273,7 +273,20 @@ extension Parser {
   /// Parse an inheritance clause.
   @_spi(RawSyntax)
   public mutating func parseInheritance() -> RawTypeInheritanceClauseSyntax {
-    let (unexpectedBeforeColon, colon) = self.expect(.colon)
+    let unexpectedBeforeColon: RawUnexpectedNodesSyntax?
+    let colon: RawTokenSyntax
+
+    let isPythonStyleInheritanceClause: Bool
+    // Parse python style inheritance clause and replace parentheses with a colon
+    if let leftParen = self.consume(if: .leftParen) {
+      unexpectedBeforeColon = RawUnexpectedNodesSyntax([leftParen], arena: self.arena)
+      colon = missingToken(.colon)
+      isPythonStyleInheritanceClause = true
+    } else {
+      (unexpectedBeforeColon, colon) = self.expect(.colon)
+      isPythonStyleInheritanceClause = false
+    }
+
     var elements = [RawInheritedTypeSyntax]()
     do {
       var keepGoing: RawTokenSyntax? = nil
@@ -301,10 +314,21 @@ extension Parser {
         )
       } while keepGoing != nil && loopProgress.evaluate(currentToken)
     }
+
+    let unexpectedAfterInheritedTypeCollection: RawUnexpectedNodesSyntax?
+
+    // If it is a Python style inheritance clause, then consume a right paren if there is one.
+    if isPythonStyleInheritanceClause, let rightParen = self.consume(if: .rightParen) {
+      unexpectedAfterInheritedTypeCollection = RawUnexpectedNodesSyntax(elements: [RawSyntax(rightParen)], arena: self.arena)
+    } else {
+      unexpectedAfterInheritedTypeCollection = nil
+    }
+
     return RawTypeInheritanceClauseSyntax(
       unexpectedBeforeColon,
       colon: colon,
       inheritedTypeCollection: RawInheritedTypeListSyntax(elements: elements, arena: self.arena),
+      unexpectedAfterInheritedTypeCollection,
       arena: self.arena
     )
   }
@@ -347,3 +371,14 @@ extension Parser {
     )
   }
 }
+
+extension Parser {
+  private mutating func isAtPythonStyleInheritanceClause() -> Bool {
+    guard self.at(.leftParen) else { return false }
+    return self.withLookahead {
+      $0.consume(if: .leftParen)
+      guard $0.canParseType() else { return false }
+      return $0.at(.rightParen, .keyword(.where), .leftBrace) || $0.at(.eof)
+    }
+  }
+}

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -1207,6 +1207,51 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return handleEffectSpecifiers(node)
   }
 
+  public override func visit(_ node: TypeInheritanceClauseSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if let unexpected = node.unexpectedBeforeColon,
+      let leftParen = unexpected.onlyToken(where: { $0.tokenKind == .leftParen })
+    {
+
+      var handledNodes: [SyntaxIdentifier] = [
+        leftParen.id,
+        node.colon.id,
+      ]
+
+      var changes: [FixIt.MultiNodeChange] = [
+        .makePresent(node.colon),
+        .makeMissing(unexpected),
+      ]
+
+      var replaceTokens = [leftParen]
+
+      if let rightParen = node.unexpectedAfterInheritedTypeCollection?.onlyToken(where: { $0.tokenKind == .rightParen }) {
+        handledNodes += [rightParen.id]
+        changes += [
+          .makeMissing(rightParen)
+        ]
+
+        replaceTokens += [rightParen]
+      }
+
+      addDiagnostic(
+        unexpected,
+        .expectedColonClass,
+        fixIts: [
+          FixIt(
+            message: ReplaceTokensFixIt(replaceTokens: replaceTokens, replacements: [.colonToken()]),
+            changes: changes
+          )
+        ],
+        handledNodes: handledNodes
+      )
+    }
+    return .visitChildren
+  }
+
   public override func visit(_ node: TypeInitializerClauseSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -125,6 +125,9 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var escapedNewlineAtLatlineOfMultiLineStringLiteralNotAllowed: Self {
     .init("escaped newline at the last line of a multi-line string literal is not allowed")
   }
+  public static var expectedColonClass: Self {
+    .init("expected ':' to begin inheritance clause")
+  }
   public static var expectedExpressionAfterTry: Self {
     .init("expected expression after 'try'")
   }

Tests/SwiftParserTest/translated/RecoveryTests.swift

@@ -1551,9 +1551,14 @@ final class RecoveryTests: XCTestCase {
       class WrongInheritanceClause11️⃣(Int) {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 34 - 35 = ''
-        DiagnosticSpec(message: "unexpected code '(Int)' in class")
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '()' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause1: Int {}
+        """
     )
   }
 
@@ -1563,9 +1568,14 @@ final class RecoveryTests: XCTestCase {
       class WrongInheritanceClause21️⃣(Base2<Int>) {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 41 - 42 = ''
-        DiagnosticSpec(message: "unexpected code '(Base2<Int>)' in class")
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '()' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause2: Base2<Int>{}
+        """
     )
   }
 
@@ -1575,9 +1585,14 @@ final class RecoveryTests: XCTestCase {
       class WrongInheritanceClause3<T>1️⃣(SubModule.Base1) where T:AnyObject {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': ', 49 - 50 = ''
-        DiagnosticSpec(message: "unexpected code '(SubModule.Base1) where T:AnyObject' in class")
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '()' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause3<T>: SubModule.Base1 where T:AnyObject {}
+        """
     )
   }
 
@@ -1587,9 +1602,14 @@ final class RecoveryTests: XCTestCase {
       class WrongInheritanceClause41️⃣(SubModule.Base2<Int>) {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 51 - 52 = ''
-        DiagnosticSpec(message: "unexpected code '(SubModule.Base2<Int>)' in class")
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '()' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause4: SubModule.Base2<Int>{}
+        """
     )
   }
 
@@ -1599,47 +1619,55 @@ final class RecoveryTests: XCTestCase {
       class WrongInheritanceClause5<T>1️⃣(SubModule.Base2<Int>) where T:AnyObject {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': ', 54 - 55 = ''
-        DiagnosticSpec(message: "unexpected code '(SubModule.Base2<Int>) where T:AnyObject' in class")
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '()' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause5<T>: SubModule.Base2<Int>where T:AnyObject {}
+        """
     )
   }
 
   func testRecovery130() {
     assertParse(
       """
-      class WrongInheritanceClause61️⃣(Int 2️⃣{}3️⃣
+      class WrongInheritanceClause61️⃣(Int {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': '
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in class"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end class"),
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '(' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause6: Int {}
+        """
     )
   }
 
   func testRecovery131() {
     assertParse(
       """
-      class WrongInheritanceClause7<T>1️⃣(Int 2️⃣where T:AnyObject {}
+      class WrongInheritanceClause7<T>1️⃣(Int where T:AnyObject {}
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': '
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in class"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected '}' to end class"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'where T:AnyObject {}' at top level"),
-      ]
+        DiagnosticSpec(
+          message: "expected ':' to begin inheritance clause",
+          fixIts: ["replace '(' with ':'"]
+        )
+      ],
+      fixedSource: """
+        class WrongInheritanceClause7<T>: Int where T:AnyObject {}
+        """
     )
   }
 
   func testRecovery132() {
+    // <rdar://problem/18502220> [swift-crashes 078] parser crash on invalid cast in sequence expr
     assertParse(
       """
-      // <rdar://problem/18502220> [swift-crashes 078] parser crash on invalid cast in sequence expr
       Base=1 as Base=1
       """
     )