Skip to content
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

Fix CodeGenerationFormat to correctly handle trailing trivia when formatting list elements #2864

Conversation

roopekv
Copy link
Contributor

@roopekv roopekv commented Sep 30, 2024

This fixes 2 issues with how CodeGenerationFormat handles trailing trivia when formatting lists to be separated by a newline. These issues don't manifest in the current codebase, but the fixes in this PR will prevent them from appearing in the future. Examples are provided below.

Change CodeGenerationFormat to keep whitespace after comma if it precedes other trivia on the same line

Example

DeclSyntax(
  """
  public static func \(tokenSpec.funcDeclName)(
    _ value: Keyword, // EXAMPLE <--
    leadingTrivia: Trivia = [],
    trailingTrivia: Trivia = [],
    presence: SourcePresence = .present
  ) -> TokenSyntax {
    // ...
  }
  """
)

Before (missing space between , and //)

public static func keyword(
  _ value: Keyword,// EXAMPLE <--
  leadingTrivia: Trivia = [],
  trailingTrivia: Trivia = [],
  presence: SourcePresence = .present
) -> TokenSyntax {
  // ...
}

After

public static func keyword(
  _ value: Keyword, // EXAMPLE <--
  leadingTrivia: Trivia = [],
  trailingTrivia: Trivia = [],
  presence: SourcePresence = .present
) -> TokenSyntax {
  // ...
}

Another example

DeclSyntax(
  """
  func verify<Node: RawSyntaxNodeProtocol>(_ raw: RawSyntax?, /* EXAMPLE <-- */ as _: Node.Type, file: StaticString = #fileID, line: UInt = #line) -> ValidationError? {
    // ...
  }
  """
)

Before (missing space between , and /*)

func verify<Node: RawSyntaxNodeProtocol>(
  _ raw: RawSyntax?,/* EXAMPLE <-- */ 
  as _: Node.Type,
  file: StaticString = #fileID,
  line: UInt = #line
) -> ValidationError? {
  // ...
}

After

func verify<Node: RawSyntaxNodeProtocol>(
  _ raw: RawSyntax?, /* EXAMPLE <-- */ 
  as _: Node.Type,
  file: StaticString = #fileID,
  line: UInt = #line
) -> ValidationError? {
  // ...
}

Fix CodeGenerationFormat to keep existing trailing trivia when adding a newline after the last element of a list

Example

DeclSyntax(
  """
  func verify<Node: RawSyntaxNodeProtocol>(_ raw: RawSyntax?, as _: Node.Type, file: StaticString = #fileID, line: UInt = #line /* EXAMPLE <-- */) -> ValidationError? {
    // ...
  }
  """
)

Before (missing /* EXAMPLE <-- */)

func verify<Node: RawSyntaxNodeProtocol>(
  _ raw: RawSyntax?,
  as _: Node.Type,
  file: StaticString = #fileID,
  line: UInt = #line
) -> ValidationError? {
  // ...
}

After

func verify<Node: RawSyntaxNodeProtocol>(
  _ raw: RawSyntax?,
  as _: Node.Type,
  file: StaticString = #fileID,
  line: UInt = #line /* EXAMPLE <-- */
) -> ValidationError? {
  // ...
}

…ng a newline after the last element of a list
@roopekv roopekv force-pushed the code-gen-keep-whitespace-after-comma-if-it-precedes-other-trivia branch from 1d83190 to 7c4f03a Compare September 30, 2024 22:34
@ahoppen
Copy link
Member

ahoppen commented Sep 30, 2024

@swift-ci Please test

@roopekv
Copy link
Contributor Author

roopekv commented Nov 6, 2024

Could this be merged?

@roopekv roopekv requested a review from ahoppen November 6, 2024 19:06
@ahoppen
Copy link
Member

ahoppen commented Nov 7, 2024

Thanks for the ping and sorry, I forgot about this PR. I’m not sure why CI failed and we don’t have the logs anymore. Let me re-trigger it and then we can see.

@swift-ci Please test

@roopekv
Copy link
Contributor Author

roopekv commented Nov 11, 2024

No problem, anything I can do to get that Windows test to pass?

@ahoppen
Copy link
Member

ahoppen commented Nov 12, 2024

Looks like the Windows failure was because it picked up swiftlang/swift#77149 but not #2859. Let’s try again.

@swift-ci Please test Windows

@ahoppen
Copy link
Member

ahoppen commented Nov 12, 2024

@swift-ci Please test Windows

@ahoppen ahoppen merged commit 39a57d4 into swiftlang:main Nov 12, 2024
3 checks passed
@roopekv roopekv deleted the code-gen-keep-whitespace-after-comma-if-it-precedes-other-trivia branch November 13, 2024 17:25
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.

None yet

2 participants