Skip to content

Commit be6fb9a

Browse files
committed
Add diagnostic for missing condition binding
1 parent 9ad7a48 commit be6fb9a

File tree

4 files changed

+55
-11
lines changed

4 files changed

+55
-11
lines changed

Sources/SwiftParser/Statements.swift

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ extension Parser {
186186
var keepGoing: RawTokenSyntax? = nil
187187
var loopProgress = LoopProgressCondition()
188188
repeat {
189-
let condition = self.parseConditionElement()
189+
let condition = self.parseConditionElement(lastBindingKind: elements.last?.condition.as(RawOptionalBindingConditionSyntax.self)?.bindingKeyword)
190190
let unexpectedBeforeKeepGoing: RawUnexpectedNodesSyntax?
191191
keepGoing = self.consume(if: .comma)
192192
if keepGoing == nil, let andOperator = self.consumeIfContextualPunctuator("&&") {
@@ -219,15 +219,15 @@ extension Parser {
219219
/// optional-binding-condition → 'let' pattern initializer? | 'var' pattern initializer? |
220220
/// 'inout' pattern initializer?
221221
@_spi(RawSyntax)
222-
public mutating func parseConditionElement() -> RawConditionElementSyntax.Condition {
222+
public mutating func parseConditionElement(lastBindingKind: RawTokenSyntax?) -> RawConditionElementSyntax.Condition {
223223
// Parse a leading #available/#unavailable condition if present.
224224
if self.at(.poundAvailableKeyword, .poundUnavailableKeyword) {
225225
return self.parsePoundAvailableConditionElement()
226226
}
227227

228228
// Parse the basic expression case. If we have a leading let, var, inout,
229229
// borrow, case keyword or an assignment, then we know this is a binding.
230-
guard self.at(.keyword(.let), .keyword(.var), .keyword(.case)) || self.at(.keyword(.inout)) else {
230+
guard self.at(.keyword(.let), .keyword(.var), .keyword(.case)) || self.at(.keyword(.inout)) || (lastBindingKind != nil && self.peek().rawTokenKind == .equal) else {
231231
// If we lack it, then this is theoretically a boolean condition.
232232
// However, we also need to handle migrating from Swift 2 syntax, in
233233
// which a comma followed by an expression could actually be a pattern
@@ -244,20 +244,20 @@ extension Parser {
244244
}
245245

246246
// We're parsing a conditional binding.
247-
precondition(self.at(.keyword(.let), .keyword(.var)) || self.at(.keyword(.inout), .keyword(.case)))
248247
enum BindingKind {
249248
case pattern(RawTokenSyntax, RawPatternSyntax)
250-
case optional(RawTokenSyntax, RawPatternSyntax)
249+
case optional(RawUnexpectedNodesSyntax?, RawTokenSyntax, RawPatternSyntax)
251250
}
252251

253252
let kind: BindingKind
254253
if let caseKeyword = self.consume(if: .keyword(.case)) {
255254
let pattern = self.parseMatchingPattern(context: .matching)
256255
kind = .pattern(caseKeyword, pattern)
257256
} else {
258-
let letOrVar = self.consumeAnyToken()
257+
let (unexpectedBeforeBindingKeyword, letOrVar) = self.expect(.keyword(.let), .keyword(.var), default: .keyword(Keyword(lastBindingKind!.tokenText)!))
258+
259259
let pattern = self.parseMatchingPattern(context: .bindingIntroducer)
260-
kind = .optional(letOrVar, pattern)
260+
kind = .optional(unexpectedBeforeBindingKeyword, letOrVar, pattern)
261261
}
262262

263263
// Now parse an optional type annotation.
@@ -289,9 +289,10 @@ extension Parser {
289289
}
290290

291291
switch kind {
292-
case let .optional(bindingKeyword, pattern):
292+
case let .optional(unexpectedBeforeBindingKeyword, bindingKeyword, pattern):
293293
return .optionalBinding(
294294
RawOptionalBindingConditionSyntax(
295+
unexpectedBeforeBindingKeyword,
295296
bindingKeyword: bindingKeyword,
296297
pattern: pattern,
297298
typeAnnotation: annotation,

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,28 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
935935
return .visitChildren
936936
}
937937

938+
public override func visit(_ node: OptionalBindingConditionSyntax) -> SyntaxVisitorContinueKind {
939+
if shouldSkip(node) {
940+
return .skipChildren
941+
}
942+
943+
if node.bindingKeyword.presence == .missing {
944+
addDiagnostic(
945+
node.bindingKeyword,
946+
ExpectedBindingKeyword(token: node.bindingKeyword),
947+
fixIts: [
948+
FixIt(
949+
message: InsertFixIt(tokenToBeInserted: node.bindingKeyword),
950+
changes: [.makePresent(node.bindingKeyword)]
951+
)
952+
],
953+
handledNodes: [node.bindingKeyword.id]
954+
)
955+
}
956+
957+
return .visitChildren
958+
}
959+
938960
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
939961
if shouldSkip(node) {
940962
return .skipChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,14 @@ public struct EffectsSpecifierAfterArrow: ParserError {
266266
}
267267
}
268268

269+
public struct ExpectedBindingKeyword: ParserError {
270+
let token: TokenSyntax
271+
272+
public var message: String {
273+
return "expected '\(token.text)' in conditional"
274+
}
275+
}
276+
269277
public struct ExtaneousCodeAtTopLevel: ParserError {
270278
public let extraneousCode: UnexpectedNodesSyntax
271279

@@ -536,6 +544,14 @@ extension FixItMessage where Self == StaticParserFixIt {
536544
}
537545
}
538546

547+
public struct InsertFixIt: ParserFixIt {
548+
public let tokenToBeInserted: TokenSyntax
549+
550+
public var message: String {
551+
"insert '\(tokenToBeInserted.text)'"
552+
}
553+
}
554+
539555
public struct MoveTokensAfterFixIt: ParserFixIt {
540556
/// The token that should be moved
541557
public let movedTokens: [TokenSyntax]

Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1839,13 +1839,18 @@ final class RecoveryTests: XCTestCase {
18391839
func testRecovery153() {
18401840
assertParse(
18411841
"""
1842-
if var y = x, z = x {
1842+
if var y = x, 1️⃣z = x {
18431843
z = y; y = z
18441844
}
18451845
""",
18461846
diagnostics: [
1847-
// TODO: Old parser expected error on line 1: expected 'var' in conditional, Fix-It replacements: 17 - 17 = 'var '
1848-
]
1847+
DiagnosticSpec(message: "expected 'var' in conditional", fixIts: ["insert 'var'"])
1848+
],
1849+
fixedSource: """
1850+
if var y = x, var z = x {
1851+
z = y; y = z
1852+
}
1853+
"""
18491854
)
18501855
}
18511856

0 commit comments

Comments
 (0)