Skip to content

Fix missing diagnostics for multiple expressions on the same line in closures, switches, and getters #1453

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 2 commits into from
Mar 27, 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
12 changes: 3 additions & 9 deletions Sources/SwiftParser/Declarations.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1768,20 +1768,14 @@ extension Parser {
)
}

var body = [RawCodeBlockItemSyntax]()
var codeBlockProgress = LoopProgressCondition()
while !self.at(.rightBrace),
let newItem = self.parseCodeBlockItem(),
codeBlockProgress.evaluate(currentToken)
{
body.append(newItem)
}
let body = parseCodeBlockItemList(until: { $0.at(.rightBrace) })

let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
return .getter(
RawCodeBlockSyntax(
unexpectedBeforeLBrace,
leftBrace: lbrace,
statements: RawCodeBlockItemListSyntax(elements: body, arena: self.arena),
statements: body,
unexpectedBeforeRBrace,
rightBrace: rbrace,
arena: self.arena
Expand Down
23 changes: 5 additions & 18 deletions Sources/SwiftParser/Expressions.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1710,21 +1710,15 @@ extension Parser {
let signature = self.parseClosureSignatureIfPresent()

// Parse the body.
var elements = [RawCodeBlockItemSyntax]()
do {
var loopProgress = LoopProgressCondition()
while !self.at(.rightBrace), let newItem = self.parseCodeBlockItem(), loopProgress.evaluate(currentToken) {
elements.append(newItem)
}
}
let elements = parseCodeBlockItemList(until: { $0.at(.rightBrace) })

// Parse the closing '}'.
let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
return RawClosureExprSyntax(
unexpectedBeforeLBrace,
leftBrace: lbrace,
signature: signature,
statements: RawCodeBlockItemListSyntax(elements: elements, arena: arena),
statements: elements,
unexpectedBeforeRBrace,
rightBrace: rbrace,
arena: self.arena
Expand Down Expand Up @@ -2350,16 +2344,9 @@ extension Parser {
}

mutating func parseSwitchCaseBody() -> RawCodeBlockItemListSyntax {
var items = [RawCodeBlockItemSyntax]()
var loopProgress = LoopProgressCondition()
while !self.at(.rightBrace) && !self.at(.poundEndifKeyword, .poundElseifKeyword, .poundElseKeyword)
&& !self.withLookahead({ $0.isStartOfConditionalSwitchCases() }),
let newItem = self.parseCodeBlockItem(),
loopProgress.evaluate(currentToken)
{
items.append(newItem)
}
return RawCodeBlockItemListSyntax(elements: items, arena: self.arena)
parseCodeBlockItemList(until: {
$0.at(.rightBrace) || $0.at(.poundEndifKeyword, .poundElseifKeyword, .poundElseKeyword) || $0.withLookahead({ $0.isStartOfConditionalSwitchCases() })
})
}

/// Parse a single switch case clause.
Expand Down
10 changes: 5 additions & 5 deletions Sources/SwiftParser/TopLevel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ extension Parser {
}

extension Parser {
mutating func parseCodeBlockItemList(isAtTopLevel: Bool, allowInitDecl: Bool = true, stopCondition: (inout Parser) -> Bool) -> RawCodeBlockItemListSyntax {
mutating func parseCodeBlockItemList(isAtTopLevel: Bool = false, allowInitDecl: Bool = true, until stopCondition: (inout Parser) -> Bool) -> RawCodeBlockItemListSyntax {
var elements = [RawCodeBlockItemSyntax]()
var loopProgress = LoopProgressCondition()
while !stopCondition(&self), loopProgress.evaluate(currentToken) {
Expand Down Expand Up @@ -89,7 +89,7 @@ extension Parser {
///
/// top-level-declaration → statements?
mutating func parseTopLevelCodeBlockItems() -> RawCodeBlockItemListSyntax {
return parseCodeBlockItemList(isAtTopLevel: true, stopCondition: { _ in false })
return parseCodeBlockItemList(isAtTopLevel: true, until: { _ in false })
}

/// The optional form of `parseCodeBlock` that checks to see if the parser has
Expand All @@ -116,7 +116,7 @@ extension Parser {
/// indented to close this code block or a surrounding context. See `expectRightBrace`.
mutating func parseCodeBlock(introducer: RawTokenSyntax? = nil, allowInitDecl: Bool = true) -> RawCodeBlockSyntax {
let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
let itemList = parseCodeBlockItemList(isAtTopLevel: false, allowInitDecl: allowInitDecl, stopCondition: { $0.at(.rightBrace) })
let itemList = parseCodeBlockItemList(allowInitDecl: allowInitDecl, until: { $0.at(.rightBrace) })
let (unexpectedBeforeRBrace, rbrace) = self.expectRightBrace(leftBrace: lbrace, introducer: introducer)

return .init(
Expand Down Expand Up @@ -148,7 +148,7 @@ extension Parser {
/// statement → compiler-control-statement
/// statements → statement statements?
@_spi(RawSyntax)
public mutating func parseCodeBlockItem(isAtTopLevel: Bool = false, allowInitDecl: Bool = true) -> RawCodeBlockItemSyntax? {
public mutating func parseCodeBlockItem(isAtTopLevel: Bool, allowInitDecl: Bool) -> RawCodeBlockItemSyntax? {
if let remainingTokens = remainingTokensIfMaximumNestingLevelReached() {
return RawCodeBlockItemSyntax(
remainingTokens,
Expand Down Expand Up @@ -229,7 +229,7 @@ extension Parser {
// If config of attributes is parsed as part of declaration parsing as it
// doesn't constitute its own code block item.
let directive = self.parsePoundIfDirective { (parser, _) in
parser.parseCodeBlockItem()
parser.parseCodeBlockItem(isAtTopLevel: isAtTopLevel, allowInitDecl: allowInitDecl)
} addSemicolonIfNeeded: { lastElement, newItemAtStartOfLine, parser in
if lastElement.semicolon == nil && !newItemAtStartOfLine {
return RawCodeBlockItemSyntax(
Expand Down
81 changes: 81 additions & 0 deletions Tests/SwiftParserTest/ExpressionTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1830,6 +1830,87 @@ final class StatementExpressionTests: XCTestCase {
)
}

func testConsecutiveStatements1() {
assertParse(
"{a1️⃣ b2️⃣ c}",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'"),
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'"),
]
)
}

func testConsecutiveStatements2() {
assertParse(
"switch x {case y: a1️⃣ b2️⃣ c}",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'"),
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'"),
]
)
}

func testConsecutiveStatements3() {
assertParse(
"""
var i: Int { a1️⃣ b2️⃣ c }
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'"),
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'"),
]
)
}

func testConsecutiveStatements4() {
assertParse(
"""
var i: Int { get {a1️⃣ b} set {c2️⃣ d} }
""",
diagnostics: [
DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive statements on a line must be separated by ';'"),
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'"),
]
)
}

func testInitCallInPoundIf() {
// Make sure we parse 'init()' as an expr, not a decl.
assertParse(
"""
class C {
init() {
#if true
init()
#endif
}
}
""",
substructure: Syntax(
FunctionCallExprSyntax(
calledExpression: IdentifierExprSyntax(identifier: .keyword(.init("init")!)),
leftParen: .leftParenToken(),
argumentList: TupleExprElementListSyntax([]),
rightParen: .rightParenToken()
)
)
)
}

func testUnexpectedCloseBraceInPoundIf() {
assertParse(
"""
#if true
1️⃣}
class C {}
#endif
""",
diagnostics: [
DiagnosticSpec(message: "unexpected brace before class")
]
)
}

func testStringLiteralAfterKeyPath() {
assertParse(
#"""
Expand Down
5 changes: 3 additions & 2 deletions Tests/SwiftParserTest/translated/InvalidTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ final class InvalidTests: XCTestCase {
1️⃣let y = "foo"
switch y {
case "bar":
blah blah // ignored
blah2️⃣ blah // ignored
}
case "baz":
break
Expand All @@ -174,7 +174,8 @@ final class InvalidTests: XCTestCase {
}
"""#,
diagnostics: [
DiagnosticSpec(message: "all statements inside a switch must be covered by a 'case' or 'default' label")
DiagnosticSpec(locationMarker: "1️⃣", message: "all statements inside a switch must be covered by a 'case' or 'default' label"),
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive statements on a line must be separated by ';'"),
]
)
}
Expand Down