Introduce `then` statements #2000

hamishknight · 2023-08-04T20:49:03Z

These allow multi-statement if/switch expression branches that can produce a value at the end by saying then <expr>. This is gated behind an experimental feature option pending evolution discussion.

Sources/SwiftSyntax/generated/syntaxNodes/SyntaxStmtNodes.swift

ahoppen

Implementation looks pretty good to me. I mostly have concerns with source breakage. I left a bunch of comments with whatever came to my mind.

Tests/SwiftParserTest/StatementTests.swift

ahoppen · 2023-08-04T21:46:44Z

Tests/SwiftParserTest/StatementTests.swift

+  func testThenStmt5() {
+    assertParse(
+      """
+      then (1, 2)


I’m sure it’s been discussed somewhere but I missed it: What are the disambiguation rules here. This could be a function call as well.

The space right now IIUC. Ie. then(1, 2) -> function call, then (1, 2) -> then statement.

ahoppen · 2023-08-04T21:47:10Z

Tests/SwiftParserTest/StatementTests.swift

+  func testThenStmt7() {
+    assertParse(
+      """
+      then []


Similarly here, before the feature, this would be a subscript call to then.

Indeed, I don't expect writing functions and subscripts with an intermediate space is a common case tho.

ahoppen · 2023-08-04T21:47:43Z

Tests/SwiftParserTest/StatementTests.swift

+  func testThenStmt13() {
+    assertParse(
+      """
+      then .foo


And here. Why isn’t this a member access on then?

I think the answer to basically all of these is "the whitespace". There's also:

then .foo

and I'm not sure if that one is a statement or member access with the rules as is.

Currently that's a statement, I'm hoping that then is a fairly uncommon name for a variable, but if not we'll probably need an extra rule that says the following expression must be on the same line for it to be treated as a statement

You can also disambiguate with (then), but that's pretty awful

ahoppen · 2023-08-04T21:48:21Z

Tests/SwiftParserTest/StatementTests.swift

+    // This is a function call.
+    assertParse(
+      """
+      then {}


Can you have then followed by a closure?

then ({}) was the answer there I believe

Yup, producing a closure from a then should a fairly uncommon thing to do anyways

Sources/SwiftParser/Statements.swift

Sources/SwiftParser/TokenSpecSet.swift

Sources/SwiftParser/Statements.swift

ahoppen · 2023-08-04T22:18:05Z

Sources/SwiftParser/Statements.swift

+    case .leftParen, .leftSquare, .period:
+      // These are handled based on whether there is trivia between the 'then'
+      // and the token. If so, it's a 'then' statement. Otherwise it should
+      // be treated as an expression, e.g `then(...)`, `then[...]`, `then.foo`.
+      return !self.currentToken.trailingTriviaText.isEmpty || !next.leadingTriviaText.isEmpty


AFAICT, the following would be interpreted as a ThenStmt but I feel like it would be fairly common to write

then .filter { ... } .map { ... }

hamishknight · 2023-08-04T22:38:30Z

Thanks for the review! I'll respond Monday since it's getting late here :)

hamishknight · 2023-08-09T12:16:29Z

For now I've left the heuristic without the newline rule, I want to do some source compatibility testing to see how often that actually happens in code

hamishknight · 2023-08-09T12:18:33Z

@ahoppen FYI the above force push diff is the correct diff of changes, I can separate out into a separate commit if you prefer tho

Sources/SwiftParser/Parser.swift

hamishknight · 2023-08-09T12:27:40Z

error: no type named 'ThenStmtSyntax' in module 'SwiftSyntax'
  case thenStmt(SwiftSyntax.ThenStmtSyntax)
                            ^

Gahhhh Swift 5.7 doesn't support SPI enum cases :( It was fixed in Swift 5.8

CodeGeneration/Sources/SyntaxSupport/Node.swift

CodeGeneration/Sources/SyntaxSupport/StmtNodes.swift

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftsyntax/SwiftSyntaxDoccIndex.swift

Sources/SwiftParser/Expressions.swift

Sources/SwiftParser/Parser.swift

Sources/SwiftParser/Statements.swift

Tests/SwiftParserTest/ThenStatementTests.swift

ahoppen

As discussed offline, I think it would be good to split this PR into a preliminary PR that introduces the experimental language feature infrastructure and then the actual PR that introduces the then statements.

hamishknight · 2023-08-10T11:36:55Z

Updated for feedback, and dropped the SPI from the syntax node for the time being as it's not supported.

These allow multi-statement `if`/`switch` expression branches that can produce a value at the end by saying `then <expr>`. This is gated behind an experimental feature option pending evolution discussion.

hamishknight · 2023-09-01T13:35:51Z

swiftlang/swift#67454

@swift-ci please test

hamishknight requested a review from ahoppen as a code owner August 4, 2023 20:49

hamishknight commented Aug 4, 2023

View reviewed changes

Sources/SwiftSyntax/generated/syntaxNodes/SyntaxStmtNodes.swift Outdated Show resolved Hide resolved

hamishknight mentioned this pull request Aug 4, 2023

Introduce then statements swiftlang/swift#67454

Merged

ahoppen reviewed Aug 4, 2023

View reviewed changes

hamishknight force-pushed the then branch from 6f898ae to f7e69a3 Compare August 4, 2023 22:38

hamishknight force-pushed the then branch 2 times, most recently from 2e588a3 to bff1bb2 Compare August 9, 2023 12:12

hamishknight requested a review from ahoppen August 9, 2023 12:13

hamishknight commented Aug 9, 2023

View reviewed changes

Sources/SwiftParser/Parser.swift Outdated Show resolved Hide resolved

ahoppen reviewed Aug 9, 2023

View reviewed changes

hamishknight force-pushed the then branch 2 times, most recently from 3570163 to 9faa43d Compare August 10, 2023 11:34

hamishknight requested a review from ahoppen August 10, 2023 11:37

hamishknight force-pushed the then branch from 9faa43d to ace2831 Compare August 10, 2023 21:17

hamishknight force-pushed the then branch 3 times, most recently from b6dcdb7 to 31b9bff Compare August 24, 2023 11:09

hamishknight force-pushed the then branch 2 times, most recently from fc87d5d to f8060fb Compare August 31, 2023 10:18

hamishknight added 3 commits September 1, 2023 14:32

Introduce then statements

94de133

These allow multi-statement `if`/`switch` expression branches that can produce a value at the end by saying `then <expr>`. This is gated behind an experimental feature option pending evolution discussion.

Avoid marking ThenStmtSyntax experimental for now

61fb200

[generated] Re-generate code

33ed2d6

hamishknight force-pushed the then branch from f8060fb to 33ed2d6 Compare September 1, 2023 13:35

hamishknight merged commit 2504b3f into swiftlang:main Sep 1, 2023

hamishknight deleted the then branch September 1, 2023 21:32

Introduce then statements #2000

Introduce then statements #2000

Uh oh!

Conversation

hamishknight commented Aug 4, 2023

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamishknight commented Aug 4, 2023

Uh oh!

hamishknight commented Aug 9, 2023

Uh oh!

hamishknight commented Aug 9, 2023

Uh oh!

Uh oh!

hamishknight commented Aug 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

hamishknight commented Aug 10, 2023

Uh oh!

hamishknight commented Sep 1, 2023

Uh oh!

Uh oh!

Introduce `then` statements #2000

Introduce `then` statements #2000

hamishknight commented Aug 9, 2023 •

edited

Loading