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

[Macros] Fix column calculation in MacroExpansionContext for plugins #2656

Merged
merged 1 commit into from
May 20, 2024

Conversation

rintaro
Copy link
Member

@rintaro rintaro commented May 17, 2024

When retriving a location for a node at the first line, column calculation was incorrect. 'column' of 'PluginMessage.Syntax' is 1-based, so we need to '-1' when using it as a offset.

@rintaro
Copy link
Member Author

rintaro commented May 17, 2024

@swift-ci Please test


// MARK: - SourceLocationMacros

runSourceLoctionMacrosPlayground()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it make sense to also have an XCTest test for this that will fail if this breaks instead of just a print in the macro examples?

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately XCTest case's expansion (assertMacroExpansion) doesn't use this code path.

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, no worries

When retriving a location for a node at the first line, column
calculation was incorrect. 'column' of 'PluginMessage.Syntax' is
1-based, so we need to '-1' when using it as a offset.
@rintaro rintaro force-pushed the macros-expansioncontext-column branch from 35cfbc6 to 9867998 Compare May 17, 2024 21:00
@rintaro
Copy link
Member Author

rintaro commented May 17, 2024

@swift-ci Please test

@ahoppen
Copy link
Collaborator

ahoppen commented May 17, 2024

@swift-ci Please test Windows

@rintaro rintaro merged commit 6d47f0a into apple:main May 20, 2024
3 checks passed
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