Skip to content

Expand trailing comma implementation #2689

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.

Sign up for GitHub

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
Aug 16, 2024
Merged

Expand trailing comma implementation #2689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1db511e
Expand trailing comma implementation
mateusrodriguesxyz Jun 6, 2024
1d11c43
Ungate SE0439 accepted parts
mateusrodriguesxyz Aug 12, 2024
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
5 changes: 4 additions & 1 deletion Sources/SwiftParser/Attributes.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,10 @@ extension Parser {
}
case nil:
return parseAttribute(argumentMode: .customAttribute) { parser in
let arguments = parser.parseArgumentListElements(pattern: .none, allowTrailingComma: false)
let arguments = parser.parseArgumentListElements(
pattern: .none,
allowTrailingComma: true
)
return .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena))
}
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/SwiftParser/Availability.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ extension Parser {
arena: self.arena
)
)
} while keepGoing != nil && self.hasProgressed(&availabilityArgumentProgress)
} while keepGoing != nil
&& self.hasProgressed(&availabilityArgumentProgress)
}

return RawAvailabilityArgumentListSyntax(elements: elements, arena: self.arena)
Expand Down
17 changes: 14 additions & 3 deletions Sources/SwiftParser/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ extension Parser {
arena: self.arena
)
)
} while keepGoing != nil && self.hasProgressed(&loopProgress)
} while keepGoing != nil && !atGenericParametersListTerminator() && self.hasProgressed(&loopProgress)
}

let whereClause: RawGenericWhereClauseSyntax?
Expand All @@ -519,6 +519,10 @@ extension Parser {
)
}

mutating func atGenericParametersListTerminator() -> Bool {
return self.at(prefix: ">")
}

mutating func parseGenericWhereClause() -> RawGenericWhereClauseSyntax {
let (unexpectedBeforeWhereKeyword, whereKeyword) = self.expect(.keyword(.where))

Expand Down Expand Up @@ -700,7 +704,7 @@ extension Parser {
arena: self.arena
)
)
} while keepGoing != nil && self.hasProgressed(&loopProgress)
} while keepGoing != nil && !self.atWhereClauseListTerminator() && self.hasProgressed(&loopProgress)
}

return RawGenericWhereClauseSyntax(
Expand All @@ -710,6 +714,10 @@ extension Parser {
arena: self.arena
)
}

mutating func atWhereClauseListTerminator() -> Bool {
return self.at(.leftBrace)
}
}

extension Parser {
Expand Down Expand Up @@ -2029,7 +2037,10 @@ extension Parser {
let unexpectedBeforeRightParen: RawUnexpectedNodesSyntax?
let rightParen: RawTokenSyntax?
if leftParen != nil {
args = parseArgumentListElements(pattern: .none, allowTrailingComma: false)
args = parseArgumentListElements(
pattern: .none,
allowTrailingComma: true
)
(unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
} else {
args = []
Expand Down
33 changes: 25 additions & 8 deletions Sources/SwiftParser/Expressions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,7 +749,7 @@ extension Parser {
let args = self.parseArgumentListElements(
pattern: pattern,
flavor: flavor.callArgumentFlavor,
allowTrailingComma: experimentalFeatures.contains(.trailingComma)
allowTrailingComma: true
)
let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen)

Expand Down Expand Up @@ -786,7 +786,10 @@ extension Parser {
if self.at(.rightSquare) {
args = []
} else {
args = self.parseArgumentListElements(pattern: pattern, allowTrailingComma: false)
args = self.parseArgumentListElements(
pattern: pattern,
allowTrailingComma: true
)
}
let (unexpectedBeforeRSquare, rsquare) = self.expect(.rightSquare)

Expand Down Expand Up @@ -1019,7 +1022,10 @@ extension Parser {
if self.at(.rightSquare) {
args = []
} else {
args = self.parseArgumentListElements(pattern: pattern, allowTrailingComma: false)
args = self.parseArgumentListElements(
pattern: pattern,
allowTrailingComma: true
)
}
let (unexpectedBeforeRSquare, rsquare) = self.expect(.rightSquare)

Expand Down Expand Up @@ -1314,7 +1320,10 @@ extension Parser {
let unexpectedBeforeRightParen: RawUnexpectedNodesSyntax?
let rightParen: RawTokenSyntax?
if leftParen != nil {
args = parseArgumentListElements(pattern: pattern, allowTrailingComma: false)
args = parseArgumentListElements(
pattern: pattern,
allowTrailingComma: true
)
(unexpectedBeforeRightParen, rightParen) = self.expect(.rightParen)
} else {
args = []
Expand Down Expand Up @@ -1430,7 +1439,7 @@ extension Parser {
let (unexpectedBeforeLParen, lparen) = self.expect(.leftParen)
let elements = self.parseArgumentListElements(
pattern: pattern,
allowTrailingComma: experimentalFeatures.contains(.trailingComma)
allowTrailingComma: true
)
let (unexpectedBeforeRParen, rparen) = self.expect(.rightParen)
return RawTupleExprSyntax(
Expand Down Expand Up @@ -1740,7 +1749,7 @@ extension Parser {
arena: self.arena
)
)
} while keepGoing != nil && self.hasProgressed(&loopProgress)
} while keepGoing != nil && !self.atCaptureListTerminator() && self.hasProgressed(&loopProgress)
}
// We were promised a right square bracket, so we're going to get it.
var unexpectedNodes = [RawSyntax]()
Expand Down Expand Up @@ -1827,6 +1836,10 @@ extension Parser {
)
}

mutating func atCaptureListTerminator() -> Bool {
return self.at(.rightSquare)
}

mutating func parseClosureCaptureSpecifiers() -> RawClosureCaptureSpecifierSyntax? {
// Check for the strength specifier: "weak", "unowned", or
// "unowned(safe/unsafe)".
Expand Down Expand Up @@ -1938,7 +1951,7 @@ extension Parser {
}

mutating func atArgumentListTerminator(_ allowTrailingComma: Bool) -> Bool {
return allowTrailingComma && self.at(.rightParen)
return allowTrailingComma && (self.at(.rightParen) || self.at(.rightSquare))
}
}

Expand Down Expand Up @@ -2383,7 +2396,7 @@ extension Parser {
} else {
unexpectedPrePatternCase = nil
}
} while keepGoing != nil && self.hasProgressed(&loopProgress)
} while keepGoing != nil && !self.atSwitchCaseListTerminator() && self.hasProgressed(&loopProgress)
}
let (unexpectedBeforeColon, colon) = self.expect(.colon)
return RawSwitchCaseLabelSyntax(
Expand All @@ -2396,6 +2409,10 @@ extension Parser {
)
}

mutating func atSwitchCaseListTerminator() -> Bool {
return self.experimentalFeatures.contains(.trailingComma) && self.at(.colon)
}

/// Parse a switch case with a 'default' label.
mutating func parseSwitchDefaultLabel(
_ handle: RecoveryConsumptionHandle
Expand Down
6 changes: 5 additions & 1 deletion Sources/SwiftParser/Nominals.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ extension Parser {
arena: self.arena
)
)
} while keepGoing != nil && self.hasProgressed(&loopProgress)
} while keepGoing != nil && !self.atInheritanceListTerminator() && self.hasProgressed(&loopProgress)
}

let unexpectedAfterInheritedTypeCollection: RawUnexpectedNodesSyntax?
Expand All @@ -339,6 +339,10 @@ extension Parser {
)
}

mutating func atInheritanceListTerminator() -> Bool {
return self.experimentalFeatures.contains(.trailingComma) && (self.at(.leftBrace) || self.at(.keyword(.where)))
}

Comment on lines +342 to +345
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this doesn’t work in general because associatedtype can also have an inheritance clause and it is not followed by { or where.

Eg.

associatedtype Iterator: IteratorProtocol,

would probably fail to parse wit this change.

I think we should limit the trailing comma to the types where it is guaranteed to be followed by where or { (ie. everything except associatedtype).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahoppen I believe it's just a matter of update the proposal text mentioning that trailing comma won't work for inheritance clause of an associatedtype, right? The same for where clauses for protocol definition. In general, make it clear that trailing comma will only be allowed when there's a clear terminator.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m only speaking as a reviewer of this PR here but I think it would make sense to exclude trailing commas for these situations without a clear terminator from the evolution proposal for now.

There might be ways of getting them right, but doing so is more complicated than the cases that have a clear terminator. Highlighting these situations in a Future directions section would make sense, I think. That way your proposal can focus on the majority of cases and if we do choose to extend trailing commas eg. case lists, the corner cases will get the attention they deserve.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the proposal text highlighting these cases. Thanks for catching them.

mutating func parsePrimaryAssociatedTypes() -> RawPrimaryAssociatedTypeClauseSyntax {
let langle = self.consumePrefix("<", as: .leftAngle)
var associatedTypes = [RawPrimaryAssociatedTypeSyntax]()
Expand Down
5 changes: 4 additions & 1 deletion Sources/SwiftParser/StringLiterals.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -551,7 +551,10 @@ extension Parser {
)
let leftParen = self.expectWithoutRecoveryOrLeadingTrivia(.leftParen)
let expressions = RawLabeledExprListSyntax(
elements: self.parseArgumentListElements(pattern: .none, allowTrailingComma: false),
elements: self.parseArgumentListElements(
pattern: .none,
allowTrailingComma: true
),
arena: self.arena
)

Expand Down
5 changes: 4 additions & 1 deletion Sources/SwiftParser/Types.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1025,7 +1025,10 @@ extension Parser {
}
case nil: // Custom attribute
return parseAttribute(argumentMode: .customAttribute) { parser in
let arguments = parser.parseArgumentListElements(pattern: .none, allowTrailingComma: false)
let arguments = parser.parseArgumentListElements(
pattern: .none,
allowTrailingComma: true
)
return .argumentList(RawLabeledExprListSyntax(elements: arguments, arena: parser.arena))
}

Expand Down
Loading