[WIP] Support parsing @_package attribute

[stevapple]

This is a very, very early (partial) demo implementation of SwiftPM support for Swift scripts, which expects feedback from both the compiler & SwiftSyntax team as I'm new to implement a compiler-related feature.

[ahoppen]

The implementation looks good to me overall. I’ve got a few comments inline.

[ahoppen]

Do you think you could also add a few test cases for invalid Swift code? Ideas that come to my mind are

• misspelled argument labels
• missing commas or colons
• Using identifiers instead of string literals

[stevapple]

I didn't care of diagnostics for now, so some false syntax may be accepted by the current implementation. The error messages also seemed not helpful at all...

Anyway I'll do that some time. Thanks for your suggestion:)

[stevapple]

I adds some more valid test cases and I believe the parsing is robust enough now. It's time to dive into diagnostics...

[ahoppen]

Shouldn’t this also be in the if let label branch?

---

Open question:

Do we want to allow arbitrary expressions here? If we only want to allow e.g. "1.1.0" or "1.1.0"..<"1.2.0" it might make sense to create a new syntax node for that kind of version requirement.

[stevapple]

Well, the current syntax handles requirement like this:

• If the given location is a path, there should be no requirement; otherwise we always have one.
• If the label is from or exact, it accepts a semantic version in string literal;
• If the label is branch, it accepts a branch name in string literal;
• If the label is revision, it accepts a commit hash in string literal.
• If there's no label, it accepts either a range expression (eg. "1.0.0"..<"1.1.0") or a member access expression (eg. .upToNextMinor(from: "1.0.0")).

I haven't decided if we want to simplify the interface (making it less like PackageDescription), so we're now accepting arbitrary expressions here.

[stevapple]

Since this is only a draft, there're plenty of rooms for refactors, like:

• Extracting the logic of @_package into a separate file like @available does;
• Implementing fully semantical parsing logic including expression diagnostic and even semantics versioning;
• Introducing additional node types for package attribute;
• Consuming all parameters at a whole (just like a tuple) and implementing semantical error handling.

WDYT?

[ahoppen]

• Extracting the logic of @_package into a separate file like @available does;

I don’t think starting a new file makes sense for ~100 LOC. If things grow further,

• Implementing fully semantical parsing logic including expression diagnostic and even semantics versioning

What do you mean by “fully semantic parsing logic including expression diagnostics”.

• Introducing additional node types for package attribute;

There’s always a trade-off here between expressibility and trying to keep the syntax tree simple. The main consideration would be if there are invariants in the syntax tree that clients might care about but which are not expressed in the syntax tree at the moment (e.g. if two options are mutually exclusive). I don’t see anything like that at the moment, so maybe it’s fine as-is.

• Consuming all parameters at a whole (just like a tuple) and implementing semantical error handling.

My gut feeling is that we’ll save ourselves trouble by implementing as much as reasonably possible in the parser. We get a lot of diagnostics for free in the parser and whatever client consumes these nodes doesn’t need to worry about them. Also, eventually we might get automatic code completion based on the keywords in the syntax tree.

[stevapple]

Now I've refactored the support and it grows to ~120 loc. Parser implementation should look better with new nodes added.

After checking PackageDescription source, I found that wildcard requirement was deprecated in SwiftPM 5.6, so for a new feature it makes sense to drop the support. This means although the node is a plain Expr at the time, we can further assert it to be a range expression ( semantic-version-low ( '..<' | '...' ) semantic-version-high).

[ahoppen]

If we just allow semantic-version-low ( '..<' | '...' ) semantic-version-high, I think it would make sense to represent the version requirement in the syntax tree as three nodes VersionTuple, BinaryOperator and VersionTuple.

[stevapple]

VersionTuple doesn't fully mimic a semantic version (which is rather complex and better handed over to other tools). I’m going to use StringLiteralExpr instead. Thanks for your suggestions on BinaryOperator, but I didn’t have idea on how to diagnose wrong operator here🤔 Should that be handled elsewhere?

[ahoppen]

Handling the semantic version somewhere else sounds good to me. The binary operator could be anything. There isn’t any way to restrict the kinds of operators we allow in the SwiftSyntax tree.