diff --git a/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift b/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift index c1ad5793fed..db7175bd7e4 100644 --- a/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift +++ b/CodeGeneration/Sources/SyntaxSupport/ExperimentalFeatures.swift @@ -19,6 +19,7 @@ public enum ExperimentalFeature: String, CaseIterable { case nonescapableTypes case transferringArgsAndResults case borrowingSwitch + case trailingComma /// The name of the feature, which is used in the doc comment. public var featureName: String { @@ -35,6 +36,8 @@ public enum ExperimentalFeature: String, CaseIterable { return "TransferringArgsAndResults" case .borrowingSwitch: return "borrowing pattern matching" + case .trailingComma: + return "trailing comma" } } diff --git a/Sources/SwiftParser/Attributes.swift b/Sources/SwiftParser/Attributes.swift index 0d79d678a03..5fdb3b29060 100644 --- a/Sources/SwiftParser/Attributes.swift +++ b/Sources/SwiftParser/Attributes.swift @@ -372,7 +372,7 @@ extension Parser { } case nil: return parseAttribute(argumentMode: .customAttribute) { parser in - let arguments = parser.parseArgumentListElements(pattern: .none) + let arguments = parser.parseArgumentListElements(pattern: .none, allowTrailingComma: false) return .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena)) } } @@ -420,7 +420,11 @@ extension Parser { trailingComma: roleTrailingComma, arena: self.arena ) - let additionalArgs = self.parseArgumentListElements(pattern: .none, flavor: .attributeArguments) + let additionalArgs = self.parseArgumentListElements( + pattern: .none, + flavor: .attributeArguments, + allowTrailingComma: false + ) return [roleElement] + additionalArgs } } diff --git a/Sources/SwiftParser/Declarations.swift b/Sources/SwiftParser/Declarations.swift index d0f172b2ee9..31d9c86ba2a 100644 --- a/Sources/SwiftParser/Declarations.swift +++ b/Sources/SwiftParser/Declarations.swift @@ -2025,7 +2025,7 @@ extension Parser { let unexpectedBeforeRightParen: RawUnexpectedNodesSyntax? let rightParen: RawTokenSyntax? if leftParen != nil { - args = parseArgumentListElements(pattern: .none) + args = parseArgumentListElements(pattern: .none, allowTrailingComma: false) (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen) } else { args = [] diff --git a/Sources/SwiftParser/Expressions.swift b/Sources/SwiftParser/Expressions.swift index 06cc3f8dc13..c239ef16133 100644 --- a/Sources/SwiftParser/Expressions.swift +++ b/Sources/SwiftParser/Expressions.swift @@ -742,7 +742,11 @@ extension Parser { // If there is an expr-call-suffix, parse it and form a call. if let lparen = self.consume(if: TokenSpec(.leftParen, allowAtStartOfLine: false)) { - let args = self.parseArgumentListElements(pattern: pattern, flavor: flavor.callArgumentFlavor) + let args = self.parseArgumentListElements( + pattern: pattern, + flavor: flavor.callArgumentFlavor, + allowTrailingComma: experimentalFeatures.contains(.trailingComma) + ) let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen) // If we can parse trailing closures, do so. @@ -778,7 +782,7 @@ extension Parser { if self.at(.rightSquare) { args = [] } else { - args = self.parseArgumentListElements(pattern: pattern) + args = self.parseArgumentListElements(pattern: pattern, allowTrailingComma: false) } let (unexpectedBeforeRSquare, rsquare) = self.expect(.rightSquare) @@ -1011,7 +1015,7 @@ extension Parser { if self.at(.rightSquare) { args = [] } else { - args = self.parseArgumentListElements(pattern: pattern) + args = self.parseArgumentListElements(pattern: pattern, allowTrailingComma: false) } let (unexpectedBeforeRSquare, rsquare) = self.expect(.rightSquare) @@ -1306,7 +1310,7 @@ extension Parser { let unexpectedBeforeRightParen: RawUnexpectedNodesSyntax? let rightParen: RawTokenSyntax? if leftParen != nil { - args = parseArgumentListElements(pattern: pattern) + args = parseArgumentListElements(pattern: pattern, allowTrailingComma: false) (unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen) } else { args = [] @@ -1420,7 +1424,10 @@ extension Parser { /// Parse a tuple expression. mutating func parseTupleExpression(pattern: PatternContext) -> RawTupleExprSyntax { let (unexpectedBeforeLParen, lparen) = self.expect(.leftParen) - let elements = self.parseArgumentListElements(pattern: pattern) + let elements = self.parseArgumentListElements( + pattern: pattern, + allowTrailingComma: experimentalFeatures.contains(.trailingComma) + ) let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen) return RawTupleExprSyntax( unexpectedBeforeLParen, @@ -1862,7 +1869,8 @@ extension Parser { /// this will be a dedicated argument list type. mutating func parseArgumentListElements( pattern: PatternContext, - flavor: ExprFlavor = .basic + flavor: ExprFlavor = .basic, + allowTrailingComma: Bool ) -> [RawLabeledExprSyntax] { if let remainingTokens = remainingTokensIfMaximumNestingLevelReached() { return [ @@ -1921,9 +1929,13 @@ extension Parser { arena: self.arena ) ) - } while keepGoing != nil && self.hasProgressed(&loopProgress) + } while keepGoing != nil && !atArgumentListTerminator(allowTrailingComma) && self.hasProgressed(&loopProgress) return result } + + mutating func atArgumentListTerminator(_ allowTrailingComma: Bool) -> Bool { + return allowTrailingComma && self.at(.rightParen) + } } extension Parser { @@ -2101,9 +2113,7 @@ extension Parser { extension Parser { /// Parse an if statement/expression. - mutating func parseIfExpression( - ifHandle: RecoveryConsumptionHandle - ) -> RawIfExprSyntax { + mutating func parseIfExpression(ifHandle: RecoveryConsumptionHandle) -> RawIfExprSyntax { let (unexpectedBeforeIfKeyword, ifKeyword) = self.eat(ifHandle) let conditions: RawConditionElementListSyntax @@ -2120,7 +2130,7 @@ extension Parser { arena: self.arena ) } else { - conditions = self.parseConditionList() + conditions = self.parseConditionList(isGuardStatement: false) } let body = self.parseCodeBlock(introducer: ifKeyword) diff --git a/Sources/SwiftParser/Statements.swift b/Sources/SwiftParser/Statements.swift index aa1e22d0e3f..7e9ffd90aec 100644 --- a/Sources/SwiftParser/Statements.swift +++ b/Sources/SwiftParser/Statements.swift @@ -137,7 +137,7 @@ extension Parser { /// Parse a guard statement. mutating func parseGuardStatement(guardHandle: RecoveryConsumptionHandle) -> RawGuardStmtSyntax { let (unexpectedBeforeGuardKeyword, guardKeyword) = self.eat(guardHandle) - let conditions = self.parseConditionList() + let conditions = self.parseConditionList(isGuardStatement: true) let (unexpectedBeforeElseKeyword, elseKeyword) = self.expect(.keyword(.else)) let body = self.parseCodeBlock(introducer: guardKeyword) return RawGuardStmtSyntax( @@ -154,7 +154,7 @@ extension Parser { extension Parser { /// Parse a list of condition elements. - mutating func parseConditionList() -> RawConditionElementListSyntax { + mutating func parseConditionList(isGuardStatement: Bool) -> RawConditionElementListSyntax { // We have a simple comma separated list of clauses, but also need to handle // a variety of common errors situations (including migrating from Swift 2 // syntax). @@ -183,11 +183,25 @@ extension Parser { arena: self.arena ) ) - } while keepGoing != nil && self.hasProgressed(&loopProgress) + } while keepGoing != nil && !atConditionListTerminator(isGuardStatement: isGuardStatement) + && self.hasProgressed(&loopProgress) return RawConditionElementListSyntax(elements: elements, arena: self.arena) } + mutating func atConditionListTerminator(isGuardStatement: Bool) -> Bool { + guard experimentalFeatures.contains(.trailingComma) else { + return false + } + // Condition terminator is `else` for `guard` statements. + if isGuardStatement, self.at(.keyword(.else)) { + return true + } + // Condition terminator is start of statement body for `if` or `while` statements. + // Missing `else` is a common mistake for `guard` statements so we fall back to lookahead for a body. + return self.at(.leftBrace) && withLookahead({ $0.atStartOfConditionalStatementBody() }) + } + /// Parse a condition element. /// /// `lastBindingKind` will be used to get a correct fall back, when there is missing `var` or `let` in a `if` statement etc. @@ -498,7 +512,6 @@ extension Parser { mutating func parseWhileStatement(whileHandle: RecoveryConsumptionHandle) -> RawWhileStmtSyntax { let (unexpectedBeforeWhileKeyword, whileKeyword) = self.eat(whileHandle) let conditions: RawConditionElementListSyntax - if self.at(.leftBrace) { conditions = RawConditionElementListSyntax( elements: [ @@ -511,9 +524,11 @@ extension Parser { arena: self.arena ) } else { - conditions = self.parseConditionList() + conditions = self.parseConditionList(isGuardStatement: false) } + let body = self.parseCodeBlock(introducer: whileKeyword) + return RawWhileStmtSyntax( unexpectedBeforeWhileKeyword, whileKeyword: whileKeyword, @@ -1053,4 +1068,54 @@ extension Parser.Lookahead { } while lookahead.at(.poundIf, .poundElseif, .poundElse) && lookahead.hasProgressed(&loopProgress) return lookahead.atStartOfSwitchCase() } + + /// Returns `true` if the current token represents the start of an `if` or `while` statement body. + mutating func atStartOfConditionalStatementBody() -> Bool { + guard at(.leftBrace) else { + // Statement bodies always start with a '{'. If there is no '{', we can't be at the statement body. + return false + } + skipSingle() + if self.at(.endOfFile) { + // There's nothing else in the source file that could be the statement body, so this must be it. + return true + } + if self.at(.semicolon) { + // We can't have a semicolon between the condition and the statement body, so this must be the statement body. + return true + } + if self.at(.keyword(.else)) { + // If the current token is an `else` keyword, this must be the statement body of an `if` statement since conditions can't be followed by `else`. + return true + } + if self.at(.rightBrace, .rightParen) { + // A right brace or parenthesis cannot start a statement body, nor can the condition list continue afterwards. So, this must be the statement body. + // This covers cases like `if true, { if true, { } }` or `( if true, { print(0) } )`. While the latter is not valid code, it improves diagnostics. + return true + } + if self.atStartOfLine { + // If the current token is at the start of a line, it is most likely a statement body. The only exceptions are: + if self.at(.comma) { + // If newline begins with ',' it must be a condition trailing comma, so this can't be the statement body, e.g. + // if true, { true } + // , true { print("body") } + return false + } + if self.at(.binaryOperator) { + // If current token is a binary operator this can't be the statement body since an `if` expression can't be the left-hand side of an operator, e.g. + // if true, { true } + // != nil + // { + // print("body") + // } + return false + } + // Excluded the above exceptions, this must be the statement body. + return true + } else { + // If the current token isn't at the start of a line and isn't `EOF`, `;`, `else`, `)` or `}` this can't be the statement body. + return false + } + } + } diff --git a/Sources/SwiftParser/StringLiterals.swift b/Sources/SwiftParser/StringLiterals.swift index 7533f5a87f3..19dd10b66db 100644 --- a/Sources/SwiftParser/StringLiterals.swift +++ b/Sources/SwiftParser/StringLiterals.swift @@ -547,7 +547,7 @@ extension Parser { ) let leftParen = self.expectWithoutRecoveryOrLeadingTrivia(.leftParen) let expressions = RawLabeledExprListSyntax( - elements: self.parseArgumentListElements(pattern: .none), + elements: self.parseArgumentListElements(pattern: .none, allowTrailingComma: false), arena: self.arena ) diff --git a/Sources/SwiftParser/Types.swift b/Sources/SwiftParser/Types.swift index 9e730f5d96a..fd85963b968 100644 --- a/Sources/SwiftParser/Types.swift +++ b/Sources/SwiftParser/Types.swift @@ -1054,7 +1054,7 @@ extension Parser { } case nil: // Custom attribute return parseAttribute(argumentMode: .customAttribute) { parser in - let arguments = parser.parseArgumentListElements(pattern: .none) + let arguments = parser.parseArgumentListElements(pattern: .none, allowTrailingComma: false) return .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena)) } diff --git a/Sources/SwiftParser/generated/ExperimentalFeatures.swift b/Sources/SwiftParser/generated/ExperimentalFeatures.swift index 6fae76a97a2..f63e1fd1a09 100644 --- a/Sources/SwiftParser/generated/ExperimentalFeatures.swift +++ b/Sources/SwiftParser/generated/ExperimentalFeatures.swift @@ -41,4 +41,7 @@ extension Parser.ExperimentalFeatures { /// Whether to enable the parsing of borrowing pattern matching. public static let borrowingSwitch = Self (rawValue: 1 << 5) + + /// Whether to enable the parsing of trailing comma. + public static let trailingComma = Self (rawValue: 1 << 6) } diff --git a/Tests/SwiftParserTest/TrailingCommaTests.swift b/Tests/SwiftParserTest/TrailingCommaTests.swift new file mode 100644 index 00000000000..2f7ff467dc8 --- /dev/null +++ b/Tests/SwiftParserTest/TrailingCommaTests.swift @@ -0,0 +1,234 @@ +//===----------------------------------------------------------------------===// +// +// This source file is part of the Swift.org open source project +// +// Copyright (c) 2023 Apple Inc. and the Swift project authors +// Licensed under Apache License v2.0 with Runtime Library Exception +// +// See https://swift.org/LICENSE.txt for license information +// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors +// +//===----------------------------------------------------------------------===// + +@_spi(ExperimentalLanguageFeatures) import SwiftParser +import SwiftSyntax +import XCTest + +final class TrailingCommaTests: ParserTestCase { + override var experimentalFeatures: Parser.ExperimentalFeatures { [.trailingComma] } + + func testTuple() { + assertParse("(1, 2, 3,)") + + assertParse( + "(1️⃣,)", + diagnostics: [DiagnosticSpec(message: "expected value in tuple", fixIts: ["insert value"])], + fixedSource: "(<#expression#>,)" + ) + } + + func testArgumentList() { + assertParse("foo(1, 2, 3,)") + + assertParse( + "foo(1️⃣,)", + diagnostics: [DiagnosticSpec(message: "expected value in function call", fixIts: ["insert value"])], + fixedSource: "foo(<#expression#>,)" + ) + } + + func testParameterList() { + assertParse( + """ + func foo( + a: Int = 0, + b: Int = 0, + ) { + } + """ + ) + + assertParse( + "func foo(1️⃣,) { }", + diagnostics: [ + DiagnosticSpec( + message: "expected identifier, ':', and type in parameter", + fixIts: ["insert identifier, ':', and type"] + ) + ], + fixedSource: "func foo(<#identifier#>: <#type#>,) { }" + ) + } + + func testIfConditions() { + assertParse("if true, { }") + + assertParse("if true, { }; { }()") + + assertParse( + """ + if true, { print("if-body") } else { print("else-body") } + """ + ) + + assertParse( + """ + if true, { print("if-body") } else if true, { print("else-if-body") } { print("else-body") } + """ + ) + + assertParse("if true, { if true { { } } }") + + assertParse("{ if true, { print(0) } }") + + assertParse("( if true, { print(0) } )") + + assertParse( + """ + if true, { true }() { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax(condition: .expression("{ true }()"), trailingComma: nil), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { true }, { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ true }"), trailingComma: .commaToken()), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { true } { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ true }"), trailingComma: nil), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { print(0) } + { }() + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()) + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { true } + { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()) + ], + body: "{ true }" + ) + ) + + assertParse( + """ + if true, { true } + ,{ print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ true }"), trailingComma: .commaToken()), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, + { true }+++ { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ true }+++"), trailingComma: nil), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { (x: () -> Void) in true } != nil { print(0) } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ (x: () -> Void) in true } != nil"), trailingComma: nil), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + """ + if true, { (x: () -> Void) in true } + != nil + { + print(0) + } + """, + substructure: IfExprSyntax( + conditions: [ + ConditionElementSyntax.init(condition: .expression("true"), trailingComma: .commaToken()), + ConditionElementSyntax.init(condition: .expression("{ (x: () -> Void) in true } != nil"), trailingComma: nil), + ], + body: "{ print(0) }" + ) + ) + + assertParse( + "if 1️⃣, { }", + diagnostics: [DiagnosticSpec(message: "missing condition in 'if' statement")] + ) + } + + func testGuardConditions() { + assertParse("guard true, else { break }") + + assertParse( + "guard true, 1️⃣, else { return }", + diagnostics: [DiagnosticSpec(message: "expected expression in 'guard' statement", fixIts: ["insert expression"])], + fixedSource: "guard true, <#expression#>, else { return }" + ) + + assertParse( + "guard true, 1️⃣{ return }", + diagnostics: [DiagnosticSpec(message: "expected 'else' in 'guard' statement", fixIts: ["insert 'else'"])], + fixedSource: "guard true, else { return }" + ) + } + + func testWhileConditions() { + assertParse("while true, { print(0) }") + } +} diff --git a/Tests/SwiftParserTest/translated/InvalidTests.swift b/Tests/SwiftParserTest/translated/InvalidTests.swift index 8ee85a3104a..f971ad7dc15 100644 --- a/Tests/SwiftParserTest/translated/InvalidTests.swift +++ b/Tests/SwiftParserTest/translated/InvalidTests.swift @@ -369,27 +369,6 @@ final class InvalidTests: ParserTestCase { ) } - func testInvalid14() { - // https://github.com/apple/swift/issues/43313 - assertParse( - """ - do { - func f(_ a: Int, b: Int) {} - f(1, b: 2,1️⃣) - } - """, - diagnostics: [ - DiagnosticSpec(message: "expected value in function call", fixIts: ["insert value"]) - ], - fixedSource: """ - do { - func f(_ a: Int, b: Int) {} - f(1, b: 2, <#expression#>) - } - """ - ) - } - func testInvalid16a() { assertParse( """