Skip to content

Add trailing trivia and with{Leading,Trailing}Trivia to buildable nodes #667

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 2, 2022

Conversation

fwcd
Copy link
Member

@fwcd fwcd commented Aug 31, 2022

Based on #666.

This PR adds a trailingTrivia field to buildable SwiftSyntaxBuilder nodes, along with withLeadingTrivia/withTrailingTrivia functions.

To do:

  • Add trailingTrivia to buildable nodes
  • Add leadingTrivia and trailingTrivia to buildable collection nodes
  • Add withLeadingTrivia and withTrailingTrivia
  • Add unit tests

For a future PR:

  • Add protocol(s) for withLeadingTrivia/withTrailingTrivia (e.g. HasLeadingTrivia/HasTrailingTrivia or just a combined HasTrivia) that buildable nodes, collection nodes and e.g. tokens could conform to

@fwcd
Copy link
Member Author

fwcd commented Aug 31, 2022

@swift-ci please test

@ahoppen
Copy link
Member

ahoppen commented Aug 31, 2022

What do you intend to use trailingTrivia for?

@fwcd
Copy link
Member Author

fwcd commented Aug 31, 2022

My use case is fixing an interaction between CustomAttribute and VariableDecl:

VariableDecl(
  attributes: [CustomAttribute("Test") { TupleExprElement(expression: "a") }],
  modifiers: [Token.public],
  .let,
  name: "test",
  type: "Int"
)

generates

@Test(a)public  let test: Int

The issue is that letOrVarKeyword gets a leading trivia based on whether attributeList is nil, this is not correct when modifiers are set as shown in the example above. Therefore my thinking was to attach a trailing space trivia to the attribute list instead, similar to how tokens use trailing spaces rather than leading ones.

@ahoppen
Copy link
Member

ahoppen commented Aug 31, 2022

Thanks for the explanation. That makes sense 👍

@fwcd fwcd force-pushed the fwcd/trailing-trivia branch from 2e5780f to e36232f Compare August 31, 2022 08:57
@fwcd
Copy link
Member Author

fwcd commented Aug 31, 2022

@swift-ci please test

@fwcd fwcd marked this pull request as ready for review August 31, 2022 09:00
@fwcd fwcd requested a review from ahoppen as a code owner August 31, 2022 09:00
@fwcd fwcd force-pushed the fwcd/trailing-trivia branch from e36232f to a2b08a3 Compare August 31, 2022 10:32
@fwcd
Copy link
Member Author

fwcd commented Aug 31, 2022

@swift-ci please test

@fwcd fwcd force-pushed the fwcd/trailing-trivia branch from a2b08a3 to 63a7ace Compare September 1, 2022 18:35
@fwcd
Copy link
Member Author

fwcd commented Sep 1, 2022

@swift-ci please test

Copy link
Member

@ahoppen ahoppen left a comment

Choose a reason for hiding this comment

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

LGTM

@ahoppen
Copy link
Member

ahoppen commented Sep 2, 2022

@swift-ci Please test macOS

@fwcd fwcd merged commit ce5be96 into swiftlang:main Sep 2, 2022
@fwcd fwcd deleted the fwcd/trailing-trivia branch September 2, 2022 08:23
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.

2 participants