Skip to content

Removes duplicate trivia from UnsafeExprSyntax #3043

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 1 commit into from
Apr 9, 2025

Conversation

TTOzzi
Copy link
Member

@TTOzzi TTOzzi commented Apr 5, 2025

When creating UnsafeExprSyntax, the leadingTrivia passed as an argument from unsafeExpr.leadingTrivia is already included in unsafeExpr.unsafeKeyword, which causes the trivia to be duplicated if it exists. To fix this, I modified the code to avoid passing leadingTrivia as an argument. (Similarly, the same applies to trailingTrivia.)

I couldn’t find a suitable way to directly test UnsafeExprSyntax created via OperatorTable, so I verified the change by integrating my local modifications into swift-format.
Please let me know if there’s a better way to test this.

@TTOzzi TTOzzi requested a review from DougGregor as a code owner April 5, 2025 11:29
@TTOzzi TTOzzi force-pushed the unsafe-leading-trivia branch from bd6fdcd to a1d0b83 Compare April 5, 2025 13:02
@TTOzzi TTOzzi changed the title Removes duplicate leading trivia from UnsafeExprSyntax Removes duplicate trivia from UnsafeExprSyntax Apr 5, 2025
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.

Thanks for the fix @TTOzzi.

You should be able to test this using

func testTriviaAroundUnsafe() throws {
  let original = ExprSyntax("/*leading*/ unsafe a + b")
  let folded = try OperatorTable.standardOperators.foldAll(original)
  XCTAssertEqual(original.description, folded.description)
}

@TTOzzi TTOzzi force-pushed the unsafe-leading-trivia branch from a1d0b83 to 1637566 Compare April 8, 2025 15:07
@TTOzzi
Copy link
Member Author

TTOzzi commented Apr 8, 2025

Thanks for the fix @TTOzzi.

You should be able to test this using

func testTriviaAroundUnsafe() throws {
  let original = ExprSyntax("/*leading*/ unsafe a + b")
  let folded = try OperatorTable.standardOperators.foldAll(original)
  XCTAssertEqual(original.description, folded.description)
}

Thank you for letting me know! 🙇
I've added test cases for the changes.

@TTOzzi TTOzzi requested a review from ahoppen April 8, 2025 15:07
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.

Thanks for the fix!

@ahoppen
Copy link
Member

ahoppen commented Apr 8, 2025

@swift-ci Please test

@ahoppen ahoppen merged commit 367883a into swiftlang:main Apr 9, 2025
26 checks passed
@TTOzzi TTOzzi deleted the unsafe-leading-trivia branch April 11, 2025 19:34
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