Make CodeBlockItemListSyntax expressible by string literal #1426

StevenWong12 · 2023-03-21T03:36:48Z

This PR is to enable creating a CodeBlockItemList from one multi-line string, which is mentioned in #1165.

BTW, I also noticed that there are two linked TODOs in ConvenienceInitializers.swift and SyntaxExpressibleByStringInterpolationConformances.swift

// TODO: This should be inlined in SyntaxParseable.init(stringInterpolation:),
// but is currently used in `ConvenienceInitializers.swift`.
// See the corresponding TODO there.
func performParse<SyntaxType: SyntaxProtocol>(

// TODO: We should split FunctionParameter into separate nodes
extension FunctionParameterSyntax {

I'm a little confused about We should split FunctionParameter into separate nodes. Some examples or elaborations would be of much help!

Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift

ahoppen · 2023-03-21T16:05:15Z

As I mentioned in #1165 (comment), I would prefer to make CodeBlockItemSyntax expressible by string literal instead of CodeBlockItemListSyntax. I think that generalizes better because you can then still use result builder constructs such as loops to create the final CodeBlockItemListSyntax.

Also, regarding the implementation, instead of adding parserCodeBlockItemListForBuilder you should make CodeBlockItemSyntax conform to SyntaxParsable in Parser+Entry.swift.

I'm a little confused about We should split FunctionParameter into separate nodes. Some examples or elaborations would be of much help!

We currently use FunctionParameterSyntax for a variety of parameters like function parameters, enum parameters and closure parameters and they all have different needs. Closure parameters for example don’t require a type and enum parameters no parameter label. Thus all the children of FunctionParameterSyntax are optional. If we had a separate node for each of these uses we could get rid of ParameterSubject and make the children that are required for each of these parameter kinds non-optional.

StevenWong12 · 2023-03-22T07:59:11Z

Hi @ahoppen, thanks for your comments!

Sorry for misunderstanding your comments in #1165. I have updated the codes according to your suggestions. To prevent the undefined behavior of parsing a code block item, I added an assertion in the Parser+Entry.swift. I wonder whether this is reasonable?

As for FunctionParameterSyntax, I would like to make a try to separate it into different nodes. Should I create an issue for that to follow up?

Thanks in advance!

ahoppen · 2023-03-22T21:19:11Z

Sources/SwiftParser/generated/Parser+Entry.swift

+extension CodeBlockItemSyntax: SyntaxParseable {
+  public static func parse(from parser: inout Parser) -> Self {
+    guard let node = parser.parseCodeBlockItem() else {
+      assertionFailure("Invalid code block item")
+      return Syntax(raw: RawSyntax(parser.parseCodeBlockItem()!)).cast(Self.self)
+    }
+    let raw = RawSyntax(parser.parseRemainder(into:node))
+    return Syntax(raw: raw).cast(Self.self)
+  }
+}
+


I think this will get overridden once you re-generate this file using the following command.

swift run --package-path CodeGeneration generate-swiftsyntax Sources

You will need to do this by adjusting CodeGeneration/Sources/ParserEntryFile.swift, probably by providing a parserFunction to CodeBlockItem in CommonNodes.swift

That makes sense! I added a parserFunction to CodeBlockItem since it is also used in SyntaxExpressibleByStringInterpolationConformancesFile.swift

ahoppen · 2023-03-23T15:21:40Z

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftparser/ParserEntryFile.swift

+          extension \(raw: node.name): SyntaxParseable {
+            public static func parse(from parser: inout Parser) -> Self {
+              guard let node = parser.\(raw: parserFunction)() else {
+                assertionFailure("Invalid code block item")


We should not crash when parsing an invalid code block item. Instead, we should form an invalid syntax node. What you could do instead when parseCodeBlockItem returns nil is to create a CodeBlockItemSyntax that contains a MissingExprSyntax – or any of the other Missing*Syntax nodes, I don’t have a strong preference here.

Also, I would prefer not to add a special-case in this code generation loop. Instead, what I think would be the better solution is to have a function parseNonOptionalCodeBlockItem, which does what I described above, at the bottom of Parser+Entry.swift and then use that as the parserFunction.

Awesome, thanks for your suggestions!

I've updated the codes and added corresponding test cases. If there is anything I can do better for this PR, please feel free to let me know.

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftparser/ParserEntryFile.swift

ahoppen

I just noticed one missing space, otherwise LGTM.

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftparser/ParserEntryFile.swift

ahoppen · 2023-05-05T19:38:52Z

@swift-ci Please test

ahoppen · 2023-05-05T19:39:11Z

@swift-ci Please test Windows

ahoppen · 2023-05-06T01:06:05Z

Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift

+      """
+
+    XCTAssertTrue(codeBlockItem.is(CodeBlockItemSyntax.self))
+    AssertStringsEqualWithDiff(


This function has been converted to lowercase. Could you update it and make sure that tests build and pass locally?

Ohhh...I selected swift-parser-cli as the scheme and it just skipped SwiftSyntaxBuilderTest. Sorry for that.

Already made these lowercase.😉

kimdv · 2023-05-06T06:48:01Z

@swift-ci please test

kimdv · 2023-05-06T06:48:38Z

@swift-ci please test Windows

StevenWong12 · 2023-05-06T09:01:32Z

I think there is something wrong with the Windows CI. Could you please trigger that again? @kimdv

kimdv · 2023-05-06T09:04:53Z

@swift-ci please test Windows

StevenWong12 · 2023-05-06T09:12:36Z

Thank you 😁

StevenWong12 requested a review from ahoppen as a code owner March 21, 2023 03:36

kimdv reviewed Mar 21, 2023

View reviewed changes

Tests/SwiftSyntaxBuilderTest/StringInterpolationTests.swift Outdated Show resolved Hide resolved

StevenWong12 requested review from kimdv and removed request for ahoppen March 21, 2023 07:22

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from bc01576 to c37c1a4 Compare March 22, 2023 07:49

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from c37c1a4 to cf128b1 Compare March 22, 2023 08:00

ahoppen reviewed Mar 22, 2023

View reviewed changes

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from 11c1980 to a812313 Compare March 23, 2023 09:47

ahoppen reviewed Mar 23, 2023

View reviewed changes

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from a812313 to d9f155e Compare March 24, 2023 08:03

StevenWong12 requested review from ahoppen and removed request for kimdv March 24, 2023 08:04

ahoppen reviewed Apr 28, 2023

View reviewed changes

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftparser/ParserEntryFile.swift Outdated Show resolved Hide resolved

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from d9f155e to 5b5eb4f Compare May 4, 2023 08:45

StevenWong12 requested a review from ahoppen May 4, 2023 08:49

ahoppen approved these changes May 4, 2023

View reviewed changes

CodeGeneration/Sources/generate-swiftsyntax/templates/swiftparser/ParserEntryFile.swift Outdated Show resolved Hide resolved

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from 5b5eb4f to d2a71ae Compare May 5, 2023 05:01

ahoppen enabled auto-merge May 5, 2023 19:38

ahoppen reviewed May 6, 2023

View reviewed changes

Make CodeBlockItemSyntax expressible by string

e182ac6

auto-merge was automatically disabled May 6, 2023 02:11
Head branch was pushed to by a user without write access

StevenWong12 force-pushed the enable_multi_line_codeblocklist branch from d2a71ae to e182ac6 Compare May 6, 2023 02:11

ahoppen merged commit 6b71c97 into swiftlang:main May 8, 2023

bnbarham mentioned this pull request Jun 7, 2023

[5.9] Make CodeBlockItemSyntax expressible by string #1740

Merged

Make CodeBlockItemListSyntax expressible by string literal #1426

Make CodeBlockItemListSyntax expressible by string literal #1426

Uh oh!

Conversation

StevenWong12 commented Mar 21, 2023

Uh oh!

Uh oh!

ahoppen commented Mar 21, 2023

Uh oh!

StevenWong12 commented Mar 22, 2023

Uh oh!

ahoppen Mar 22, 2023

Choose a reason for hiding this comment

Uh oh!

StevenWong12 Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

ahoppen Mar 23, 2023

Choose a reason for hiding this comment

Uh oh!

StevenWong12 Mar 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahoppen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahoppen commented May 5, 2023

Uh oh!

ahoppen commented May 5, 2023

Uh oh!

ahoppen May 6, 2023

Choose a reason for hiding this comment

Uh oh!

StevenWong12 May 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kimdv commented May 6, 2023

Uh oh!

kimdv commented May 6, 2023

Uh oh!

StevenWong12 commented May 6, 2023

Uh oh!

kimdv commented May 6, 2023

Uh oh!

StevenWong12 commented May 6, 2023

Uh oh!

Uh oh!

StevenWong12 May 6, 2023 •

edited

Loading