Make examples runnable

[SimplyDanny]

• Add main method to both examples.
• Provide package defining executables for each example.
• Update examples to use SwiftParser.

[fwcd]

The medium-term plan was for the examples to be migrated to Swift 5.7 snippets, which would let users run the examples using swift run <snippet> without the need for main methods or full-blown targets. Essentially, this 'migration' only involves renaming the folder from Examples to Snippets to have SPM pick up on these scripts:

• Build snippets in the build script #531

This was already implemented in #477, but reverted due to an Xcode issue, and therefore currently put on hold.

cc @ahoppen

[ahoppen]

Thanks for the idea but I agree with @fwcd that we should be using SwiftPM’s snippets to make sure the examples compile one they are available in Xcode. That way we don’t spam the targets in Package.swift. Also, I think the primary used case for these example should be to show how the API can be used and not a set of scripts should be easy to run.

[SimplyDanny]

I accept the decision to keep everything as it is, but I also have some thoughts I would like to share:

1. In their current state, the examples do not work anymore. Some outdated APIs are used.
2. Even though the intention was not to provide small scripts, the AddOneToIntegerLiterals example parses command line arguments.
3. It is natural for a user discovering the project to run the examples and experiment with them. Quite some manual work is required (basically writing the Package.swift) to do so.
4. SwiftPM's snippets might be perfectly suitable. As long as their application here is on hold, there is no benefit of being a perfect fit. Better have something easily usable than waiting for the perfect solution.
5. The main Package.swift is not touched at all. There is a new manifest solely for the examples that only references the main manifest.

[fwcd]

I think the central idea is that these scripts are really intended to be more of a documentation on how to quickly get started using the API rather than standalone programs. So ideally they'd have as little noise as possible around them (main methods, argument parsing, ...).

I definitely agree that being able to run them directly can be very useful and from what I understand we also do want to have them as part of the build pipeline, e.g. to make sure that they stay in sync with the (rapidly evolving) API.

Snippets let us do all of this without the added complexity of another package layer. For CodeGeneration an extra package was necessary to address specific problems with bootstrapping, but here I believe we should keep it simple and hopefully get the Xcode issues sorted out soon.

[fwcd]

Regarding the outdated APIs: Fixes are definitely welcomed there, I think especially the Format part has undergone some changes recently, but I haven't tracked it super closely, so @ahoppen probably has a better idea of how the API is to be used idiomatically.

[ahoppen]

Oh, sorry, I somehow missed that you also adjusted these scripts, I inferred too much from the PR title, sorry.

• The change to use SwiftParser and the typo fixes are definitely good changes that I would like to merge.
• IMHO the change to use @main adds noise to these examples that draws attention away from the important bits

[SimplyDanny]

Created #929 to address the accepted changes. I'm still not happy with the examples not being easily executable. Hope the issue Xcode is having with the Snippets will be resolved shortly.

On the other hand, why not just name the folder back to Snippets? That would make them runnable at least with SwiftPM. Or is a Snippets folder already causing problems in Xcode? This comment assumes the rename happened inadvertently. Did it?

[ahoppen]

Created #929 to address the accepted changes. I'm still not happy with the examples not being easily executable. Hope the issue Xcode is having with the Snippets will be resolved shortly.

What would your use case of having these example being runnable be? I can’t really see any but I might be missing something. What I would like to avoid is that these examples start becoming a grab-bag of scripts based on SwiftSyntax that people rely on in production use cases.

On the other hand, why not just name the folder back to Snippets? That would make them runnable at least with SwiftPM. Or is a Snippets folder already causing problems in Xcode? This comment assumes the rename happened inadvertently. Did it?

Yes, exactly, if you rename the folder to Snippets, building the package in Xcode fails with

Expressions are not allowed at the top level

in the snippets

[SimplyDanny]

What would your use case of having these example being runnable be? I can’t really see any but I might be missing something. What I would like to avoid is that these examples start becoming a grab-bag of scripts based on SwiftSyntax that people rely on in production use cases.

I see two: First off, you can easily run them to verify they still compile and produce reasonable results, in other words, they are not outdated.

Secondly, people discovering the project can readily use them to get an idea on how the stuff works. They may change their content experimenting a bit with the API. I expect most of the people to write their own Package.swift to do so if they don't give up right away. You might take a guess what I've done when preparing and verifying #929. 😉

[ahoppen]

Thinking over it for a few days, maybe I was wrong to close this. I think we should move to snippets as soon as they are available but in the meantime a separate package is probably right after all.

If you want to re-open this, could you modify build-script.py to build the examples package? That way CI would ensure that the examples always build. I hope that the changes in this repo should be fairly straight-forward. You will need to also add the package to contents.xcworkspacedata in the apple/swift repo. Note that this file is not really an Xcode workspace, it’s just a file that specifies the SwiftPM packages that should get built in CI (it’s for the --multiroot-data-file option to make sure we don’t need to build a package twice if two packages depend on it).

[fwcd]

Sorry if my comments came across a bit harsh.

As long as we keep the examples simple scripts, having a package manifest for them probably doesn't hurt, that's true. Ultimately making them snippets would be nice, but being able to run them now benefits everyone and also makes it easier to test against regressions.

[SimplyDanny]

No worries! Happy I was able to convince you in the end. 😉

I've updated my branch incorporating the newly added example, letting the build script know about the examples and rebasing it onto main. Could you please re-open the PR? I'm not able to do that.

I guess, the changes in apple/swift can be done as soon as this PR is merged?

[ahoppen]

I've updated my branch incorporating the newly added example, letting the build script know about the examples and rebasing it onto main. Could you please re-open the PR? I'm not able to do that.

I can’t re-open it either. It says ”The runnable-examples branch was force-pushed or recreated.”. Could you just open a new PR?

I guess, the changes in apple/swift can be done as soon as this PR is merged?

Sure.

[SimplyDanny]

I can’t re-open it either. It says ”The runnable-examples branch was force-pushed or recreated.”. Could you just open a new PR?

Ah, I should not have updated the branch. Created #1007.

[fwcd]

You could have force-pushed the old commit, reopened and then force-pushed back the new commit :D But yeah, a new PR works too