Skip to content

Add a convenience init to ClosureCaptureSyntax#2127

Merged
ahoppen merged 2 commits into
swiftlang:mainfrom
natikgadzhi:syntax/convenience/closure-capture
Sep 2, 2023
Merged

Add a convenience init to ClosureCaptureSyntax#2127
ahoppen merged 2 commits into
swiftlang:mainfrom
natikgadzhi:syntax/convenience/closure-capture

Conversation

@natikgadzhi

Copy link
Copy Markdown
Contributor

Summary

This adds a convenience init to ClosureCaptureSyntax that adds an equal token if the provided name is not nil.

It's supposed to be used by developers, so it omits the unexpected tokens.

Related: #1984

@ahoppen ahoppen left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add an entry for this PR and #2112 to the new release notes file https://github.com/apple/swift-syntax/blob/main/Release%20Notes/510.md?

natikgadzhi added a commit to natikgadzhi/swift-syntax that referenced this pull request Aug 31, 2023
Provides release notes entries for:

- `ClosureCaptureSyntax.init` swiftlang#2127
- `EnumCaseParameterSyntax.init`
  swiftlang#2112
@natikgadzhi

Copy link
Copy Markdown
Contributor Author

@ahoppen done: 23c2098

@kimdv

kimdv commented Sep 1, 2023

Copy link
Copy Markdown
Contributor

@swift-ci please test

@Matejkob Matejkob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the updates—great work! My internal nitpick radar 📡 kicked in, so I've left a few comments. Not sure how helpful they are, but there you go! 🙈

Comment thread Release Notes/510.md Outdated
Comment on lines +11 to +18
- `ClosureCaptureSyntax.init(leadingTrivia: Trivia? = nil, specifier: ClosureCaptureSpecifierSyntax? = nil, name: TokenSyntax, equal: TokenSyntax = TokenSyntax.equalToken(), expression: some ExprSyntaxProtocol, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `ClosureCaptureSyntax` that takes a concrete `name` argument and automatically adds `equal = TokenSyntax.equalToken()` to it.
- Pull Request: https://github.com/apple/swift-syntax/pull/2127
- `EnumCaseParameterSyntax.init(leadingTrivia: Trivia? = nil, modifiers: DeclModifierListSyntax = [], firstName: TokenSyntax, secondName: TokenSyntax? = nil, colon: TokenSyntax = TokenSyntax.colonToken(), type: some TypeSyntaxProtocol, defaultValue: InitializerClauseSyntax? = nil, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `EnumCaseParameterSyntax` that takes a concrete `firstName` value and adds `colon = TokenSyntax.colonToken()` automatically to it.
- Pull Request: https://github.com/apple/swift-syntax/pull/2112

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The issue related to these changes contains important information about the new APIs. I would consider adding a link to it, as stated in the template below. What do you think about it?

Suggested change
- `ClosureCaptureSyntax.init(leadingTrivia: Trivia? = nil, specifier: ClosureCaptureSpecifierSyntax? = nil, name: TokenSyntax, equal: TokenSyntax = TokenSyntax.equalToken(), expression: some ExprSyntaxProtocol, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `ClosureCaptureSyntax` that takes a concrete `name` argument and automatically adds `equal = TokenSyntax.equalToken()` to it.
- Pull Request: https://github.com/apple/swift-syntax/pull/2127
- `EnumCaseParameterSyntax.init(leadingTrivia: Trivia? = nil, modifiers: DeclModifierListSyntax = [], firstName: TokenSyntax, secondName: TokenSyntax? = nil, colon: TokenSyntax = TokenSyntax.colonToken(), type: some TypeSyntaxProtocol, defaultValue: InitializerClauseSyntax? = nil, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `EnumCaseParameterSyntax` that takes a concrete `firstName` value and adds `colon = TokenSyntax.colonToken()` automatically to it.
- Pull Request: https://github.com/apple/swift-syntax/pull/2112
- `ClosureCaptureSyntax.init(leadingTrivia: Trivia? = nil, specifier: ClosureCaptureSpecifierSyntax? = nil, name: TokenSyntax, equal: TokenSyntax = TokenSyntax.equalToken(), expression: some ExprSyntaxProtocol, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `ClosureCaptureSyntax` that takes a concrete `name` argument and automatically adds `equal = TokenSyntax.equalToken()` to it.
- Issue: https://github.com/apple/swift-syntax/issues/1984
- Pull Request: https://github.com/apple/swift-syntax/pull/2127
- `EnumCaseParameterSyntax.init(leadingTrivia: Trivia? = nil, modifiers: DeclModifierListSyntax = [], firstName: TokenSyntax, secondName: TokenSyntax? = nil, colon: TokenSyntax = TokenSyntax.colonToken(), type: some TypeSyntaxProtocol, defaultValue: InitializerClauseSyntax? = nil, trailingComma: TokenSyntax? = nil, trailingTrivia: Trivia? = nil)`
- Description: Provides a convenience initializer for `EnumCaseParameterSyntax` that takes a concrete `firstName` value and adds `colon = TokenSyntax.colonToken()` automatically to it.
- Issue: https://github.com/apple/swift-syntax/issues/1984
- Pull Request: https://github.com/apple/swift-syntax/pull/2112

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Yes, I think linking the issue would be valuable.

Also, I think the full API signature isn’t that enlightening. What I would do instead is name the items

  • ClosureCaptureSyntax.init with a required name
  • EnumCaseParameterSyntax.init with a required firstName

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

How about incorporating release notes into the DocC documentation for SwiftSyntax package? With this approach, it would be really easy to add direct links to specific methods right in the code.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The problem is that we would need to decide which module to put the release notes in. If we put it in the SwiftSyntax module, you can’t link to any API in e.g. SwiftParser. Since the release notes are for the entire package and not one specific module, I think having them at a top-level makes sense for now.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I wasn't aware of this restriction that you can't link to any API from a different target. That's unfortunate. :/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

FWIW swiftlang/swift-docc#208 is tracking this restriction.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you both for the review! I'll clean up the changelog for now. Hopefully, I can get a generator-based version quickly enough and clean them up altogether in a few days.

Comment thread Sources/SwiftSyntax/Convenience.swift Outdated
Comment thread Sources/SwiftSyntax/Convenience.swift Outdated
**Summary**
This adds a convenience init to ClosureCaptureSyntax that adds equal
token if the provided `name` is not nil.

It's supposed to be used by developers, so it omits the unexpected
tokens.

Related: swiftlang#1984
natikgadzhi added a commit to natikgadzhi/swift-syntax that referenced this pull request Sep 1, 2023
Provides release notes entries for:

- `ClosureCaptureSyntax.init` swiftlang#2127
- `EnumCaseParameterSyntax.init`
  swiftlang#2112

Co-authored-by: Mateusz Bąk <bakmatthew@icloud.com>
@natikgadzhi natikgadzhi force-pushed the syntax/convenience/closure-capture branch from 4d6c9b0 to 440b6da Compare September 1, 2023 20:40
Provides release notes entries for:

- `ClosureCaptureSyntax.init` swiftlang#2127
- `EnumCaseParameterSyntax.init`
  swiftlang#2112

Co-authored-by: Mateusz Bąk <bakmatthew@icloud.com>
@natikgadzhi natikgadzhi force-pushed the syntax/convenience/closure-capture branch from 440b6da to a9acb00 Compare September 1, 2023 20:42
@natikgadzhi

Copy link
Copy Markdown
Contributor Author

Alright! Should be good to go for this PR.

@ahoppen

ahoppen commented Sep 1, 2023

Copy link
Copy Markdown
Member

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge September 1, 2023 20:51
@ahoppen

ahoppen commented Sep 1, 2023

Copy link
Copy Markdown
Member

@swift-ci Please test Windows

@ahoppen

ahoppen commented Sep 2, 2023

Copy link
Copy Markdown
Member

@swift-ci Please test macOS

@ahoppen

ahoppen commented Sep 2, 2023

Copy link
Copy Markdown
Member

@swift-ci Please test Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants