Skip to content

Fix macro expansion diagnostic for multi-binding variable declarations in MacroSystem #2139

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
Sep 2, 2023

Conversation

gohanlon
Copy link
Contributor

@gohanlon gohanlon commented Sep 1, 2023

When there are no macro attributes attached to the visited VariableDeclSyntax node, proceed without performing any additional checks or transformations.

  • Addressed the erroneous triggering of MacroApplicationError.accessorMacroOnVariableWithMultipleBindings diagnostic for nested multi-binding variable declarations.
  • Added corresponding tests to validate the fix.
  • Introduce fileprivate NoOpMemberMacro, extracted from testCommentAroundeAttachedMacro and used in 2 new tests.

Fixes #2133
rdar://114836887

@gohanlon gohanlon requested a review from ahoppen as a code owner September 1, 2023 20:55
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.

Thank you. Looks great!

@ahoppen
Copy link
Member

ahoppen commented Sep 1, 2023

@swift-ci Please test

@ahoppen ahoppen enabled auto-merge September 1, 2023 20:58
@ahoppen
Copy link
Member

ahoppen commented Sep 1, 2023

Would you mind also creating a second PR that targets the package-release/509 branch? That way we can get this fix into the swift-syntax 509.0.0 release.

gohanlon added a commit to gohanlon/swift-syntax that referenced this pull request Sep 1, 2023
---

Fix macro expansion diagnostic for multi-binding variable declarations in MacroSystem

When there are no macro attributes attached to the visited `VariableDeclSyntax` node, proceed without performing any additional checks or transformations.

- Addressed the erroneous triggering of `MacroApplicationError.accessorMacroOnVariableWithMultipleBindings` diagnostic for nested multi-binding variable declarations.
- Added corresponding tests to validate the fix.
- Introduce fileprivate `NoOpMemberMacro`, extracted from `testCommentAroundeAttachedMacro` and used in 2 new tests.

Fixes swiftlang#2133
rdar://114836887
gohanlon added a commit to gohanlon/swift-syntax that referenced this pull request Sep 1, 2023
---

Fix macro expansion diagnostic for multi-binding variable declarations in MacroSystem

When there are no macro attributes attached to the visited `VariableDeclSyntax` node, proceed without performing any additional checks or transformations.

- Addressed the erroneous triggering of `MacroApplicationError.accessorMacroOnVariableWithMultipleBindings` diagnostic for nested multi-binding variable declarations.
- Added corresponding tests to validate the fix.
- Introduce fileprivate `NoOpMemberMacro`, extracted from `testCommentAroundeAttachedMacro` and used in 2 new tests.

Fixes swiftlang#2133
rdar://114836887
@gohanlon
Copy link
Contributor Author

gohanlon commented Sep 1, 2023

Would you mind also creating a second PR that targets the package-release/509 branch? That way we can get this fix into the swift-syntax 509.0.0 release.

Done, see #2141.

@ahoppen
Copy link
Member

ahoppen commented Sep 2, 2023

It looks like your changes are not formatted using swift-format. Could you run format.py to format them? Both in this PR and in #2141?

gohanlon added a commit to gohanlon/swift-syntax that referenced this pull request Sep 2, 2023
---

Fix macro expansion diagnostic for multi-binding variable declarations in MacroSystem

When there are no macro attributes attached to the visited `VariableDeclSyntax` node, proceed without performing any additional checks or transformations.

- Addressed the erroneous triggering of `MacroApplicationError.accessorMacroOnVariableWithMultipleBindings` diagnostic for nested multi-binding variable declarations.
- Added corresponding tests to validate the fix.
- Introduce fileprivate `NoOpMemberMacro`, extracted from `testCommentAroundeAttachedMacro` and used in 2 new tests.

Fixes swiftlang#2133
rdar://114836887
…s in MacroSystem

When there are no macro attributes attached to the visited `VariableDeclSyntax` node, proceed without performing any additional checks or transformations.

- Addressed the erroneous triggering of `MacroApplicationError.accessorMacroOnVariableWithMultipleBindings` diagnostic for nested multi-binding variable declarations.
- Added corresponding tests to validate the fix.
- Introduce fileprivate `NoOpMemberMacro`, extracted from `testCommentAroundeAttachedMacro` and used in 2 new tests.

Fixes swiftlang#2133
rdar://114836887
auto-merge was automatically disabled September 2, 2023 05:17

Head branch was pushed to by a user without write access

@gohanlon gohanlon force-pushed the fix-leaky-diagnostic branch from 983c3d9 to b8e3122 Compare September 2, 2023 05:17
@ahoppen ahoppen enabled auto-merge September 2, 2023 05:18
@gohanlon
Copy link
Contributor Author

gohanlon commented Sep 2, 2023

It looks like your changes are not formatted using swift-format. Could you run format.py to format them? Both in this PR and in #2141?

I reformatted with format.py and amended both this PR and #2141.

@ahoppen
Copy link
Member

ahoppen commented Sep 2, 2023

Thank you!

@ahoppen
Copy link
Member

ahoppen commented Sep 2, 2023

@swift-ci Please test

@ahoppen ahoppen merged commit 13f113e into swiftlang:main Sep 2, 2023
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.

Diagnostic triggered for every multi-binding variable declaration
2 participants