Skip to content
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

Allow trailing comma in comma-separated lists #2344

Merged
merged 12 commits into from
Jul 1, 2024

Conversation

mateusrodriguesxyz
Copy link
Contributor

@jessbennett
Copy link

Where are your GitHub checks?

@rjmccall rjmccall added the LSG Contains topics under the domain of the Language Steering Group label Apr 1, 2024
@mateusrodriguesxyz mateusrodriguesxyz marked this pull request as ready for review April 23, 2024 13:38
```

> [!NOTE]
> A similar proposal was [rejected](https://forums.swift.org/t/rejected-se-0084-allow-trailing-commas-in-parameter-lists-and-tuples/2777) back in 2016 for Swift 3. It's been 8 years since that, the swift language has evolved a lot, some changes highlighted above as motivation, and the code style that "puts the terminating right parenthesis on a line following the arguments to that call" has been widely adopted by community, swift standard library codebase, swift-format, docc documentation and Xcode. Therefore, not encourage or endorse this code style doesn't hold true anymore nor is a reason for rejection.
Copy link
Contributor

Choose a reason for hiding this comment

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

SE-0084 explicitly proposed not allowing trailing comma in single-element tuples.

So that this proposal text meets the standard of describing the solution with sufficient detail that someone else can implement it just by reading the text, I think this proposal should spell out explicitly the grammar changes proposed, which includes calling out whether trailing comma is allowed in single-element tuples, as well as zero-element tuples and parameter lists—e.g., is (,) equivalent to () (aka Void)?


### Allow trailing comma anywhere there's a comma-separated list

Although this proposal focuses on the most requested use cases for trailing comma, there's other places with comma-separated list and the restriction could be consistently lifted for all of these.
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing from this list are variable bindings, e.g., var a, b, c.

Also worth calling out explicitly (in the proposed solution) is whether this proposal will support var (a, b,) = (1, 2,), or whether only the right-hand side is allowed to have a trailing comma—in other words, whether a tuple pattern counts as a "tuple" under the proposed solution.

default:
...
}
```
Copy link
Contributor

Choose a reason for hiding this comment

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

One of the justifications offered (which you should feel free to include in the text here) for expanding support for trailing commas is growing developer expectation, with many popular languages offering support. As folks have replied on the pitch thread, many languages either choose to stop at arrays/lists or support trailing commas basically everywhere that commas are used as delimiters.

Is there a principle or justification that underlies this proposal expanding support to include tuples and arguments, and control flow conditions other than switch, but not these future directions? If so, can you articulate it here?

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 think I'm pro adding everywhere that is possible but I chose the ones that I personally considered more demanded for scope reason. I did considered add more during the implementation review but @ahoppen rightly pointed that the review had already dragged too long so we went with just the proposed here. And to be honest, this has been more time-consuming than I expected so if possible I would prefer to not expand the proposal. If this is approved and there is a desire to expand, I would be willing to make another proposal expanding to more places.


## Source compatibility

This change is purely additive and has no impact on existing code.
Copy link
Contributor

Choose a reason for hiding this comment

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

This, or the above section, might be a good place to include your findings regarding the implications for parsing condition lists that you've mentioned in the pitch thread. Even if how they're parsed is formally never ambiguous to the compiler, if there are scenarios in which a reader could be confused/misled due to how trailing commas are parsed in condition lists, it'd be good to record for posterity what the intended behavior is.

@mateusrodriguesxyz
Copy link
Contributor Author

@xwu I have updated proposed solution and source compatibility sections to address your feedbacks.

@mateusrodriguesxyz mateusrodriguesxyz changed the title Allow trailing comma in tuples, arguments and if/guard/while conditions Allow trailing comma in comma-separated lists Jun 4, 2024
Copy link
Contributor

@xwu xwu left a comment

Choose a reason for hiding this comment

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

Editorial suggestions. Feel free, of course, to adopt or reject any as the proposal author, but my hope is that the suggested changes help readers to provide useful feedback during review and better understand what's proposed and what's not.


## Introduction

This proposal aims to allow the use of trailing commas, currently restricted to array and dictionary literals, in more comma-separated lists.
Copy link
Contributor

@xwu xwu Jun 29, 2024

Choose a reason for hiding this comment

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

I'd suggest being more concrete about "more." Since it's not helpful or practical to list them all here individually, you could perhaps instead give the design principles that distinguish those places where you're proposing to add support and those where you're not (for example: where there are terminators that enable unambiguous parsing). You could use what you've written later in the proposal text to that effect.

]
```

Using trailing commas makes it easy to add, remove, reorder and comment in/out elements, without the need to add or delete the comma while doing any of these manipulations.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Using trailing commas makes it easy to add, remove, reorder and comment in/out elements, without the need to add or delete the comma while doing any of these manipulations.
Swift's support for trailing commas in array and dictionary literals makes it as easy to append, remove, reorder, or comment out the last element as any other element.

To be precise, the benefits have to do with the last element of a list, not other elements. So perhaps something like this? Also, it is a bit pedantic, but one can try to use a trailing comma but if it's not supported then it's not easy...

Comment on lines 30 to 47
Consider the following SwiftUI modifier:

```swift
func frame(
width: CGFloat? = nil,
height: CGFloat? = nil,
alignment: Alignment = .center
) -> some View
```

`frame(width:)`, `frame(height:)`, `frame(width:alignment:)`, `frame(height:alignment:)`, `frame(width:height:)`, `frame(width:height:alignment:)` are all valid calls but you can't easily switch between `frame(width:)` and `frame(width:alignment:)` by commenting in/out `alignment` without adding/removing the trailing comma.

```swift
.frame(
width: 500,
// alignment: .leading
) // ❌ Unexpected ',' separator
```
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggestion—I'd start the shift from your first example of something already supported (array literals) to the present motivation with a sentence to introduce this shift, like: "Other comma-separated lists in the language could also benefit from the flexibility enabled by trailing commas." You have a sentence at the end of this section that probably would work here.

It's permitted certainly to mention SwiftUI as an example, but if there's an example that applies more broadly than Apple platforms and app development, it's arguably preferable. For example, you could choose an API from the standard library, swift-algorithms, etc.

If you choose a well known enough API, you also wouldn't have to provide the declaration and list all the overloads: just the call site and the error that results would be sufficient.

) // ❌ Unexpected ',' separator
```

The introduction of [parameter packs](https://github.com/apple/swift-evolution/blob/main/proposals/0393-parameter-packs.md) allows more APIs that are comma-separated lists at call site and would benefit from trailing comma.
Copy link
Contributor

@xwu xwu Jun 29, 2024

Choose a reason for hiding this comment

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

Before this sentence, I would move the acknowledgment you have written below about SE-0084. You can write that a narrower proposal was reviewed and rejected, but since that time the language has evolved substantially that change the basis for rejection.

That would include not just evolution in prevailing code style, but also the point here that we have introduced parameter packs, but also the following point that we have introduced code generation tools. The more points you give here the more clearly you demonstrate why this proposal is reviewable even though a narrower one was previously rejected.

Comment on lines 51 to 61
```swift
extension [S] {
func sorted<each T: Comparable>(_ keyPath: repeat KeyPath<S, each T>) { }
}

arrayOfS.sorted(
\.a,
\.b,
// \.c
) // ❌ Unexpected ',' separator
```
Copy link
Contributor

Choose a reason for hiding this comment

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

It isn't necessary to demonstrate how parameter packs work in this proposal, I think.

{ print("something") }
```

Currently the parser uses the last comma to determine that whatever follows is the last condition, so `{ return true }` is a condition and `{ print("something") }` is the if body.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Currently the parser uses the last comma to determine that whatever follows is the last condition, so `{ return true }` is a condition and `{ print("something") }` is the if body.
Currently the parser uses the last comma to determine that whatever follows is the last condition, so `{ return true }` is a condition and `{ print("something") }` is the `if` body.

```

Currently the parser uses the last comma to determine that whatever follows is the last condition, so `{ return true }` is a condition and `{ print("something") }` is the if body.
To allow trailing comma the proposed solution is to change the parser to terminate de condition list before the first block that is a valid if body, so `{ return true }` will the parsed as the if body.
Copy link
Contributor

@xwu xwu Jun 29, 2024

Choose a reason for hiding this comment

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

Suggested change
To allow trailing comma the proposed solution is to change the parser to terminate de condition list before the first block that is a valid if body, so `{ return true }` will the parsed as the if body.
With trailing comma support, the parser will terminate the condition list before the first block that is a valid `if` body, so `{ return true }` will be parsed as the `if` body and `{ print("something") }` will be parsed as an unused closure expression.

"blue"
)
```
This was even [proposed](https://forums.swift.org/t/se-0257-eliding-commas-from-multiline-expression-lists/22889/188) and returned to revision back in 2019.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This was even [proposed](https://forums.swift.org/t/se-0257-eliding-commas-from-multiline-expression-lists/22889/188) and returned to revision back in 2019.
This was even [proposed](https://forums.swift.org/t/se-0257-eliding-commas-from-multiline-expression-lists/22889/188) and returned for revision back in 2019.

```
This was even [proposed](https://forums.swift.org/t/se-0257-eliding-commas-from-multiline-expression-lists/22889/188) and returned to revision back in 2019.

Even though both approach are not mutually exclusive, this proposal is about consistently extend an existing behavior in the language while eliding comma is a more serious change to the language.
Copy link
Contributor

@xwu xwu Jun 29, 2024

Choose a reason for hiding this comment

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

Suggested change
Even though both approach are not mutually exclusive, this proposal is about consistently extend an existing behavior in the language while eliding comma is a more serious change to the language.
The two approaches are not mutually exclusive. There remain unresolved questions about how the language can accommodate elided commas, and adopting this proposal does not prevent that approach from being considered in the future.

Just a suggestion. In part, because I don't think your intention here is to give off the impression that the alternative considered might actually be better but just too hard. Instead, I understand you to mean that a less invasive change to the language that adequately addresses the motivation is actually better.


### Eliding commas

A different approach to address the exact same motivation is to allow the comma between two expressions to be elided when they are separated by a newline.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
A different approach to address the exact same motivation is to allow the comma between two expressions to be elided when they are separated by a newline.
A different approach to address similar motivations is to allow the comma between two expressions to be elided when they are separated by a newline.

They don't have to be the "exact same" motivation, or at least it's not necessary for you to argue that they're the exact same motivations here.

@mateusrodriguesxyz
Copy link
Contributor Author

@xwu I've just commited a updated version of the proposal text addressing your suggestions.

Copy link
Contributor

@xwu xwu left a comment

Choose a reason for hiding this comment

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

Fantastic. Minor grammar edits and one point of clarification. I think otherwise we are ready to go.

proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
proposals/nnnn-trailing-comma-lists.md Outdated Show resolved Hide resolved
@xwu xwu merged commit bc3e3fa into swiftlang:main Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LSG Contains topics under the domain of the Language Steering Group
Projects
None yet
4 participants