Skip to content

Commit 4988ddd

Browse files
kimdvkimdv
committed
Add diagnostic for wrong inheritance
1 parent 94120e0 commit 4988ddd

File tree

7 files changed

+172
-36
lines changed

7 files changed

+172
-36
lines changed

‎Sources/SwiftCompilerPluginMessageHandling/Diagnostics.swift

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ extension PluginMessage.Diagnostic {
8585
PluginMessage.Diagnostic.FixIt(
8686
message: $0.message.message,
8787
changes: $0.changes.compactMap {
88-
let range: SourceManager.SourceRange?
89-
let text: String
88+
var range: SourceManager.SourceRange?
89+
var text: String?
9090
switch $0 {
9191
case .replace(let oldNode, let newNode):
9292
range = sourceManager.range(
@@ -109,8 +109,17 @@ extension PluginMessage.Diagnostic {
109109
to: .afterTrailingTrivia
110110
)
111111
text = newTrivia.description
112+
case .ensuringTrailingSpace(let token):
113+
if !token.trailingTrivia.contains(where: { $0.isSpaceOrTab }) {
114+
range = sourceManager.range(
115+
of: Syntax(token),
116+
from: .beforeTrailingTrivia,
117+
to: .afterTrailingTrivia
118+
)
119+
text = token.trailingTrivia.appending(.space).description
120+
}
112121
}
113-
guard let range = range else {
122+
guard let range = range, let text = text else {
114123
return nil
115124
}
116125
return .init(
@@ -126,3 +135,14 @@ extension PluginMessage.Diagnostic {
126135
}
127136
}
128137
}
138+
139+
extension TriviaPiece {
140+
var isSpaceOrTab: Bool {
141+
switch self {
142+
case .spaces, .tabs:
143+
return true
144+
default:
145+
return false
146+
}
147+
}
148+
}

‎Sources/SwiftDiagnostics/FixIt.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ public struct FixIt {
3232
case replaceLeadingTrivia(token: TokenSyntax, newTrivia: Trivia)
3333
/// Replace the trailing trivia on the given token
3434
case replaceTrailingTrivia(token: TokenSyntax, newTrivia: Trivia)
35+
/// Ensures the token have a trailing space.
36+
case ensuringTrailingSpace(token: TokenSyntax)
3537
}
3638

3739
/// A description of what this Fix-It performs.

‎Sources/SwiftParser/Nominals.swift

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ extension Parser {
240240
}
241241

242242
let inheritance: RawTypeInheritanceClauseSyntax?
243-
if self.at(.colon) {
243+
if self.at(.colon) || self.isAtPythonStyleInheritanceClause() {
244244
inheritance = self.parseInheritance()
245245
} else {
246246
inheritance = nil
@@ -273,7 +273,17 @@ extension Parser {
273273
/// Parse an inheritance clause.
274274
@_spi(RawSyntax)
275275
public mutating func parseInheritance() -> RawTypeInheritanceClauseSyntax {
276-
let (unexpectedBeforeColon, colon) = self.expect(.colon)
276+
let unexpectedBeforeColon: RawUnexpectedNodesSyntax?
277+
let colon: RawTokenSyntax
278+
279+
// Parse python style inheritance clause and replace parentheses with a colon
280+
if let leftParen = self.consume(if: .leftParen) {
281+
unexpectedBeforeColon = RawUnexpectedNodesSyntax([leftParen], arena: self.arena)
282+
colon = missingToken(.colon)
283+
} else {
284+
(unexpectedBeforeColon, colon) = self.expect(.colon)
285+
}
286+
277287
var elements = [RawInheritedTypeSyntax]()
278288
do {
279289
var keepGoing: RawTokenSyntax? = nil
@@ -306,10 +316,22 @@ extension Parser {
306316
)
307317
} while keepGoing != nil && loopProgress.evaluate(currentToken)
308318
}
319+
320+
let unexpectedAfterInheritedTypeCollection: RawUnexpectedNodesSyntax?
321+
322+
// If it contains a left paren, then it might be a python style inheritance clause.
323+
// Left paren have been replaced with colon
324+
if unexpectedBeforeColon?.containsToken(where: { $0.tokenKind == .leftParen }) == true, let rightBrace = self.consume(if: .rightParen) {
325+
unexpectedAfterInheritedTypeCollection = RawUnexpectedNodesSyntax(elements: [RawSyntax(rightBrace)], arena: self.arena)
326+
} else {
327+
unexpectedAfterInheritedTypeCollection = nil
328+
}
329+
309330
return RawTypeInheritanceClauseSyntax(
310331
unexpectedBeforeColon,
311332
colon: colon,
312333
inheritedTypeCollection: RawInheritedTypeListSyntax(elements: elements, arena: self.arena),
334+
unexpectedAfterInheritedTypeCollection,
313335
arena: self.arena
314336
)
315337
}
@@ -352,3 +374,14 @@ extension Parser {
352374
)
353375
}
354376
}
377+
378+
extension Parser {
379+
private mutating func isAtPythonStyleInheritanceClause() -> Bool {
380+
guard self.at(.leftParen) else { return false }
381+
return self.withLookahead {
382+
$0.consume(if: .leftParen)
383+
guard $0.canParseType() else { return false }
384+
return $0.at(.rightParen, .keyword(.where), .leftBrace) || $0.at(.eof)
385+
}
386+
}
387+
}

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1269,6 +1269,54 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
12691269
return handleEffectSpecifiers(node)
12701270
}
12711271

1272+
public override func visit(_ node: TypeInheritanceClauseSyntax) -> SyntaxVisitorContinueKind {
1273+
if shouldSkip(node) {
1274+
return .skipChildren
1275+
}
1276+
1277+
if let unexpected = node.unexpectedBeforeColon,
1278+
let leftParen = unexpected.onlyToken(where: { $0.tokenKind == .leftParen })
1279+
{
1280+
1281+
var handledNodes: [SyntaxIdentifier] = [
1282+
leftParen.id,
1283+
node.colon.id,
1284+
]
1285+
1286+
var changes: [FixIt.MultiNodeChange] = [
1287+
.makePresent(node.colon, trailingTrivia: .space),
1288+
.makeMissing(unexpected),
1289+
]
1290+
1291+
var replaceTokens = [leftParen]
1292+
1293+
if let rightParen = node.unexpectedAfterInheritedTypeCollection?.onlyToken(where: { $0.tokenKind == .rightParen }),
1294+
let lastToken = node.inheritedTypeCollection.lastToken(viewMode: .sourceAccurate)
1295+
{
1296+
handledNodes += [rightParen.id]
1297+
changes += [
1298+
.makeMissing(rightParen),
1299+
FixIt.MultiNodeChange(.ensuringTrailingSpace(token: lastToken)),
1300+
]
1301+
1302+
replaceTokens += [rightParen]
1303+
}
1304+
1305+
addDiagnostic(
1306+
unexpected,
1307+
.expectedColonClass,
1308+
fixIts: [
1309+
FixIt(
1310+
message: ReplaceTokensFixIt(replaceTokens: replaceTokens, replacements: [.colonToken()]),
1311+
changes: changes
1312+
)
1313+
],
1314+
handledNodes: handledNodes
1315+
)
1316+
}
1317+
return .visitChildren
1318+
}
1319+
12721320
public override func visit(_ node: TypeInitializerClauseSyntax) -> SyntaxVisitorContinueKind {
12731321
if shouldSkip(node) {
12741322
return .skipChildren

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,9 @@ extension DiagnosticMessage where Self == StaticParserError {
125125
public static var escapedNewlineAtLatlineOfMultiLineStringLiteralNotAllowed: Self {
126126
.init("escaped newline at the last line of a multi-line string literal is not allowed")
127127
}
128+
public static var expectedColonClass: Self {
129+
.init("expected ':' to begin inheritance clause")
130+
}
128131
public static var expectedExpressionAfterTry: Self {
129132
.init("expected expression after 'try'")
130133
}

‎Tests/SwiftParserTest/Assertions.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -308,6 +308,8 @@ class FixItApplier: SyntaxRewriter {
308308
modifiedNode = node.with(\.leadingTrivia, newTrivia)
309309
case .replaceTrailingTrivia(token: let changedNode, newTrivia: let newTrivia) where changedNode.id == node.id:
310310
modifiedNode = node.with(\.trailingTrivia, newTrivia)
311+
case .ensuringTrailingSpace(token: let changedNode) where changedNode.id == node.id:
312+
modifiedNode = changedNode.with(\.trailingTrivia, node.trailingTrivia + .space)
311313
default:
312314
break
313315
}

‎Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 59 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1570,9 +1570,14 @@ final class RecoveryTests: XCTestCase {
15701570
class WrongInheritanceClause11️⃣(Int) {}
15711571
""",
15721572
diagnostics: [
1573-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 34 - 35 = ''
1574-
DiagnosticSpec(message: "unexpected code '(Int)' in class")
1575-
]
1573+
DiagnosticSpec(
1574+
message: "expected ':' to begin inheritance clause",
1575+
fixIts: ["replace '()' with ':'"]
1576+
)
1577+
],
1578+
fixedSource: """
1579+
class WrongInheritanceClause1: Int {}
1580+
"""
15761581
)
15771582
}
15781583

@@ -1582,9 +1587,14 @@ final class RecoveryTests: XCTestCase {
15821587
class WrongInheritanceClause21️⃣(Base2<Int>) {}
15831588
""",
15841589
diagnostics: [
1585-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 41 - 42 = ''
1586-
DiagnosticSpec(message: "unexpected code '(Base2<Int>)' in class")
1587-
]
1590+
DiagnosticSpec(
1591+
message: "expected ':' to begin inheritance clause",
1592+
fixIts: ["replace '()' with ':'"]
1593+
)
1594+
],
1595+
fixedSource: """
1596+
class WrongInheritanceClause2: Base2<Int> {}
1597+
"""
15881598
)
15891599
}
15901600

@@ -1594,9 +1604,14 @@ final class RecoveryTests: XCTestCase {
15941604
class WrongInheritanceClause3<T>1️⃣(SubModule.Base1) where T:AnyObject {}
15951605
""",
15961606
diagnostics: [
1597-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': ', 49 - 50 = ''
1598-
DiagnosticSpec(message: "unexpected code '(SubModule.Base1) where T:AnyObject' in class")
1599-
]
1607+
DiagnosticSpec(
1608+
message: "expected ':' to begin inheritance clause",
1609+
fixIts: ["replace '()' with ':'"]
1610+
)
1611+
],
1612+
fixedSource: """
1613+
class WrongInheritanceClause3<T>: SubModule.Base1 where T:AnyObject {}
1614+
"""
16001615
)
16011616
}
16021617

@@ -1606,9 +1621,14 @@ final class RecoveryTests: XCTestCase {
16061621
class WrongInheritanceClause41️⃣(SubModule.Base2<Int>) {}
16071622
""",
16081623
diagnostics: [
1609-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': ', 51 - 52 = ''
1610-
DiagnosticSpec(message: "unexpected code '(SubModule.Base2<Int>)' in class")
1611-
]
1624+
DiagnosticSpec(
1625+
message: "expected ':' to begin inheritance clause",
1626+
fixIts: ["replace '()' with ':'"]
1627+
)
1628+
],
1629+
fixedSource: """
1630+
class WrongInheritanceClause4: SubModule.Base2<Int> {}
1631+
"""
16121632
)
16131633
}
16141634

@@ -1618,47 +1638,55 @@ final class RecoveryTests: XCTestCase {
16181638
class WrongInheritanceClause5<T>1️⃣(SubModule.Base2<Int>) where T:AnyObject {}
16191639
""",
16201640
diagnostics: [
1621-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': ', 54 - 55 = ''
1622-
DiagnosticSpec(message: "unexpected code '(SubModule.Base2<Int>) where T:AnyObject' in class")
1623-
]
1641+
DiagnosticSpec(
1642+
message: "expected ':' to begin inheritance clause",
1643+
fixIts: ["replace '()' with ':'"]
1644+
)
1645+
],
1646+
fixedSource: """
1647+
class WrongInheritanceClause5<T>: SubModule.Base2<Int> where T:AnyObject {}
1648+
"""
16241649
)
16251650
}
16261651

16271652
func testRecovery130() {
16281653
assertParse(
16291654
"""
1630-
class WrongInheritanceClause61️⃣(Int 2️⃣{}3️⃣
1655+
class WrongInheritanceClause61️⃣(Int {}
16311656
""",
16321657
diagnostics: [
1633-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 30 - 31 = ': '
1634-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in class", fixIts: ["insert '{'"]),
1635-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable", fixIts: ["insert 'var'"]),
1636-
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern", fixIts: ["insert ')'"]),
1637-
DiagnosticSpec(locationMarker: "3️⃣", message: "expected '}' to end class", fixIts: ["insert '}'"]),
1638-
]
1658+
DiagnosticSpec(
1659+
message: "expected ':' to begin inheritance clause",
1660+
fixIts: ["replace '(' with ':'"]
1661+
)
1662+
],
1663+
fixedSource: """
1664+
class WrongInheritanceClause6: Int {}
1665+
"""
16391666
)
16401667
}
16411668

16421669
func testRecovery131() {
16431670
assertParse(
16441671
"""
1645-
class WrongInheritanceClause7<T>1️⃣(Int 2️⃣where T:AnyObject {}
1672+
class WrongInheritanceClause7<T>1️⃣(Int where T:AnyObject {}
16461673
""",
16471674
diagnostics: [
1648-
// TODO: Old parser expected error on line 1: expected ':' to begin inheritance clause, Fix-It replacements: 33 - 34 = ': '
1649-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected '{' in class", fixIts: ["insert '{'"]),
1650-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'var' in variable", fixIts: ["insert 'var'"]),
1651-
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ')' to end tuple pattern", fixIts: ["insert ')'"]),
1652-
DiagnosticSpec(locationMarker: "2️⃣", message: "expected '}' to end class", fixIts: ["insert '}'"]),
1653-
DiagnosticSpec(locationMarker: "2️⃣", message: "extraneous code 'where T:AnyObject {}' at top level"),
1654-
]
1675+
DiagnosticSpec(
1676+
message: "expected ':' to begin inheritance clause",
1677+
fixIts: ["replace '(' with ':'"]
1678+
)
1679+
],
1680+
fixedSource: """
1681+
class WrongInheritanceClause7<T>: Int where T:AnyObject {}
1682+
"""
16551683
)
16561684
}
16571685

16581686
func testRecovery132() {
1687+
// <rdar://problem/18502220> [swift-crashes 078] parser crash on invalid cast in sequence expr
16591688
assertParse(
16601689
"""
1661-
// <rdar://problem/18502220> [swift-crashes 078] parser crash on invalid cast in sequence expr
16621690
Base=1 as Base=1
16631691
"""
16641692
)

0 commit comments

Comments
 (0)