From 731e46ac127b7db82327aea7b83ddcd30a27cfc0 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Tue, 25 Apr 2023 20:32:32 +0200 Subject: [PATCH] Merge pull request #1450 from kimdv/kimdv/add-diagnostic-for-wrong-inheritance Add diagnostic for wrong inheritance --- Sources/SwiftParser/Nominals.swift | 39 +++++++- .../ParseDiagnosticsGenerator.swift | 45 ++++++++++ .../ParserDiagnosticMessages.swift | 3 + .../translated/RecoveryTests.swift | 90 ++++++++++++------- 4 files changed, 144 insertions(+), 33 deletions(-) diff --git a/Sources/SwiftParser/Nominals.swift b/Sources/SwiftParser/Nominals.swift index 636b1c1fe25..7429c868903 100644 --- a/Sources/SwiftParser/Nominals.swift +++ b/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) + } + } +} diff --git a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift index 1db5b02763d..27c26906c8e 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/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 diff --git a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift b/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift index d73d2ee5b1a..600d6789949 100644 --- a/Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift +++ b/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'") } diff --git a/Tests/SwiftParserTest/translated/RecoveryTests.swift b/Tests/SwiftParserTest/translated/RecoveryTests.swift index d768064ee74..01b65a11226 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/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) {} """, 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)' in class") - ] + DiagnosticSpec( + message: "expected ':' to begin inheritance clause", + fixIts: ["replace '()' with ':'"] + ) + ], + fixedSource: """ + class WrongInheritanceClause2: Base2{} + """ ) } @@ -1575,9 +1585,14 @@ final class RecoveryTests: XCTestCase { class WrongInheritanceClause31️⃣(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: SubModule.Base1 where T:AnyObject {} + """ ) } @@ -1587,9 +1602,14 @@ final class RecoveryTests: XCTestCase { class WrongInheritanceClause41️⃣(SubModule.Base2) {} """, 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)' in class") - ] + DiagnosticSpec( + message: "expected ':' to begin inheritance clause", + fixIts: ["replace '()' with ':'"] + ) + ], + fixedSource: """ + class WrongInheritanceClause4: SubModule.Base2{} + """ ) } @@ -1599,47 +1619,55 @@ final class RecoveryTests: XCTestCase { class WrongInheritanceClause51️⃣(SubModule.Base2) 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) where T:AnyObject' in class") - ] + DiagnosticSpec( + message: "expected ':' to begin inheritance clause", + fixIts: ["replace '()' with ':'"] + ) + ], + fixedSource: """ + class WrongInheritanceClause5: SubModule.Base2where 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 WrongInheritanceClause71️⃣(Int 2️⃣where T:AnyObject {} + class WrongInheritanceClause71️⃣(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: Int where T:AnyObject {} + """ ) } func testRecovery132() { + // [swift-crashes 078] parser crash on invalid cast in sequence expr assertParse( """ - // [swift-crashes 078] parser crash on invalid cast in sequence expr Base=1 as Base=1 """ )