-
Notifications
You must be signed in to change notification settings - Fork 974
Bugfix/comment duplication #5913
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
Bugfix/comment duplication #5913
Conversation
calebcartwright
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! You've definitely identified the root cause, as well as the proper underlying technique to address the issue.
However, this is such a common type of occurrence that we've got existing utilities that can and should be reused here instead. Details linked below, but basically using our span() instead of the AST struct's span field will cover the inline logic you originally proposed
Lines 43 to 55 in da7f678
| macro_rules! implement_spanned { | |
| ($this:ty) => { | |
| impl Spanned for $this { | |
| fn span(&self) -> Span { | |
| span_with_attrs!(self) | |
| } | |
| } | |
| }; | |
| } | |
| // Implement `Spanned` for structs with `attrs` field. | |
| implement_spanned!(ast::AssocItem); | |
| implement_spanned!(ast::Expr); |
Co-authored-by: Caleb Cartwright <[email protected]>
|
That did the trick 😄 |
ytmimi
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes @calebcartwright asked for. I think we're good to go here!
|
Thanks for fixing this and for your first contribution! |
Fixes #5871
The issue was in the
rewrite_parenfunction.Since the span on expressions doesn't include attributes and attributes comes before the expression, this caused the attribute to be included in
pre_commentsince the end of the comment span wassubexpr.span.lo().I changed the logic to use the span of the first attribute, if there are any attributes present. If not, default to the expression span.