Skip to content

Include code comments before expectations which are preceded by try/await in recorded issues #1092

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 3 commits into from
Apr 24, 2025

Conversation

stmontgomery
Copy link
Contributor

This fixes an issue where code comments placed before an expectation like #expect() which has effect introducer keywords like try or await are ignored, and ensures they are included in the recorded issue if the expectation fails.

Consider this example of two failing expectations:

// Uh oh!
#expect(try x() == 2)

// Uh oh!
try #expect(x() == 2)

Prior to this PR, if x() returned a value other than 2, there would be two issues recorded, but the second one would not have the comment “Uh oh!” because from the macro’s perspective, that code comment was on the try keyword and it could only see trivia associated with #expect().

Now, with the recent swift-syntax fix from swiftlang/swift-syntax#3037, the try keyword and its associated trivia can be included and this bug can be fixed. We recently adopted a new-enough swift-syntax in #1069, so the only fix needed is to adopt lexicalContext for this new purpose in our macro.

Checklist:

  • Code and documentation should follow the style of the Style Guide.
  • If public symbols are renamed or modified, DocC references should be updated.

Sorry, something went wrong.

@stmontgomery stmontgomery added bug 🪲 Something isn't working issue-handling Related to Issue handling within the testing library macros 🔭 Related to Swift macros such as @Test or #expect labels Apr 23, 2025
@stmontgomery stmontgomery added this to the Swift 6.x milestone Apr 23, 2025
@stmontgomery stmontgomery self-assigned this Apr 23, 2025
@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

cc @hamishknight

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
…wait in recorded issues
@stmontgomery stmontgomery force-pushed the expect-effect-comments branch from 2eb4b3b to 47951ff Compare April 24, 2025 02:20
@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

@swift-ci please test

@stmontgomery
Copy link
Contributor Author

The macOS and Linux CI failures are expected to be resolved by swiftlang/swift#80830, but the release of a new main snapshot toolchain is currently blocked for unrelated reasons. Proceeding to merge.

@stmontgomery stmontgomery merged commit b5fa554 into swiftlang:main Apr 24, 2025
1 of 3 checks passed
@stmontgomery stmontgomery deleted the expect-effect-comments branch April 24, 2025 15:46
@stmontgomery stmontgomery modified the milestones: Swift 6.x, Swift 6.2 Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🪲 Something isn't working issue-handling Related to Issue handling within the testing library macros 🔭 Related to Swift macros such as @Test or #expect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants