From b2a371c4e58d59592a8e3a729ecd7fde8e0b6740 Mon Sep 17 00:00:00 2001 From: Kim de Vos Date: Sun, 23 Apr 2023 21:41:31 +0200 Subject: [PATCH] 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 a41c628c872..8a0a36129ec 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 @@ -304,10 +317,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 ) } @@ -350,3 +374,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 f127f835d83..913e5109dba 100644 --- a/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift +++ b/Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift @@ -1286,6 +1286,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 8dbd814f369..19a6b185454 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 85ddc4b0b29..eee8a0d36ad 100644 --- a/Tests/SwiftParserTest/translated/RecoveryTests.swift +++ b/Tests/SwiftParserTest/translated/RecoveryTests.swift @@ -1521,9 +1521,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 {} + """ ) } @@ -1533,9 +1538,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{} + """ ) } @@ -1545,9 +1555,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 {} + """ ) } @@ -1557,9 +1572,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{} + """ ) } @@ -1569,47 +1589,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", fixIts: ["insert '{'"]), - DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable", fixIts: ["insert 'var'"]), - DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern", fixIts: ["insert ')'"]), - DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end class", fixIts: ["insert '}'"]), - ] + 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", fixIts: ["insert '{'"]), - DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable", fixIts: ["insert 'var'"]), - DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern", fixIts: ["insert ')'"]), - DiagnosticSpec(locationMarker: "2️⃣", message: "expected '}' to end class", fixIts: ["insert '}'"]), - 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 """ )