Skip to content

[5.9] Add diagnostic for missing conditional binding keyword #1493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 11, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 18 additions & 8 deletions Sources/SwiftParser/Statements.swift
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ extension Parser {
var keepGoing: RawTokenSyntax? = nil
var loopProgress = LoopProgressCondition()
repeat {
let condition = self.parseConditionElement()
let condition = self.parseConditionElement(lastBindingKind: elements.last?.condition.as(RawOptionalBindingConditionSyntax.self)?.bindingKeyword)
let unexpectedBeforeKeepGoing: RawUnexpectedNodesSyntax?
keepGoing = self.consume(if: .comma)
if keepGoing == nil, let andOperator = self.consumeIfContextualPunctuator("&&") {
Expand Down Expand Up @@ -219,15 +219,16 @@ extension Parser {
/// optional-binding-condition → 'let' pattern initializer? | 'var' pattern initializer? |
/// 'inout' pattern initializer?
@_spi(RawSyntax)
public mutating func parseConditionElement() -> RawConditionElementSyntax.Condition {
/// `lastBindingKind` will be used to get a correct fall back, when there is missing `var` or `let` in a `if` statement etc.
public mutating func parseConditionElement(lastBindingKind: RawTokenSyntax?) -> RawConditionElementSyntax.Condition {
// Parse a leading #available/#unavailable condition if present.
if self.at(.poundAvailableKeyword, .poundUnavailableKeyword) {
return self.parsePoundAvailableConditionElement()
}

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

// We're parsing a conditional binding.
precondition(self.at(.keyword(.let), .keyword(.var)) || self.at(.keyword(.inout), .keyword(.case)))
enum BindingKind {
case pattern(RawTokenSyntax, RawPatternSyntax)
case optional(RawTokenSyntax, RawPatternSyntax)
case optional(RawUnexpectedNodesSyntax?, RawTokenSyntax, RawPatternSyntax)
}

let kind: BindingKind
if let caseKeyword = self.consume(if: .keyword(.case)) {
let pattern = self.parseMatchingPattern(context: .matching)
kind = .pattern(caseKeyword, pattern)
} else {
let letOrVar = self.consumeAnyToken()
let unexpectedBeforeBindingKeyword: RawUnexpectedNodesSyntax?
let letOrVar: RawTokenSyntax

if self.at(.identifier), let lastBindingKind = lastBindingKind {
(unexpectedBeforeBindingKeyword, letOrVar) = self.expect(.keyword(.let), .keyword(.var), default: .keyword(Keyword(lastBindingKind.tokenText) ?? .let))
} else {
letOrVar = self.consume(if: TokenSpec.keyword(.let), .keyword(.var)) ?? self.missingToken(.let)
unexpectedBeforeBindingKeyword = nil
}

let pattern = self.parseMatchingPattern(context: .bindingIntroducer)
kind = .optional(letOrVar, pattern)
kind = .optional(unexpectedBeforeBindingKeyword, letOrVar, pattern)
}

// Now parse an optional type annotation.
Expand Down Expand Up @@ -289,9 +298,10 @@ extension Parser {
}

switch kind {
case let .optional(bindingKeyword, pattern):
case let .optional(unexpectedBeforeBindingKeyword, bindingKeyword, pattern):
return .optionalBinding(
RawOptionalBindingConditionSyntax(
unexpectedBeforeBindingKeyword,
bindingKeyword: bindingKeyword,
pattern: pattern,
typeAnnotation: annotation,
Expand Down
11 changes: 8 additions & 3 deletions Tests/SwiftParserTest/translated/RecoveryTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1841,13 +1841,18 @@ final class RecoveryTests: XCTestCase {
func testRecovery153() {
assertParse(
"""
if var y = x, z = x {
if var y = x, 1️⃣z = x {
z = y; y = z
}
""",
diagnostics: [
// TODO: Old parser expected error on line 1: expected 'var' in conditional, Fix-It replacements: 17 - 17 = 'var '
]
DiagnosticSpec(message: "expected 'var' in optional binding", fixIts: ["insert 'var'"])
],
fixedSource: """
if var y = x, var z = x {
z = y; y = z
}
"""
)
}

Expand Down