Adjust non-empty tuple struct span to start before fields #5015
Conversation
src/items.rs
Outdated
| let span_start_before_tuple_fields = mk_sp( | ||
| context.snippet_provider.span_before_last(span, "("), | ||
| span.hi(), | ||
| ); |
There was a problem hiding this comment.
We don't want to try to search for the last occurrence, as the presence of fields and various generics could produce different specious span derivations (there could be other opening parens that come after the one we want in this context).
What we really need to do is make sure we're passing the correct subset of the span with the lo position being advanced far enough. To be safe, we should probably check for the presence of generics and use the hi position of that span if present, otherwise using the high of the ident (the identifier/name, e.g. AStar and BStar in our tests) should be fine:
| let span_start_before_tuple_fields = mk_sp( | |
| context.snippet_provider.span_before_last(span, "("), | |
| span.hi(), | |
| ); | |
| let lo = if let Some(generics) = struct_parts.generics { | |
| generics.span.hi() | |
| } else { | |
| struct_parts.ident.span.hi() | |
| }; |
src/items.rs
Outdated
| result = overflow::rewrite_with_parens( | ||
| context, | ||
| &result, | ||
| fields.iter(), | ||
| shape, | ||
| span, | ||
| span_start_before_tuple_fields, |
There was a problem hiding this comment.
| span_start_before_tuple_fields, | |
| mk_sp(lo, span.hi()), |
There was a problem hiding this comment.
Thanks for pointing this out. I didn't even realize that struct_parts had all the information I needed to adjust the span correctly. I definitely feel like I'm learning more about the project with each PR!
Resolves 5011
Tuple structs with visibility modifiers and comments before the first
field were incorrectly formatted. Comments would duplicate part of the
visibility modifier and struct name.
When trying to parse the tuple fields the ``items::Context`` searches
for the opening '(', but because the visibility modifier introduces
another '(' -- for example ``pub(crate)`` -- the parsing gets messed up.
Now the span is adjusted to start after the struct identifier, or after
any generics. Adjusting the span in this way ensures that the
``items::Contex`` will correctly find the tuple fields.
|
Just wanted to let you know that I made the changes you suggested! |
|
Excellent, thank you! |
Resolves #5011
Tuple structs with visibility modifiers and comments before the first
field were incorrectly formatted. Comments would duplicate part of the
visibility modifier and struct name.
When trying to parse the tuple fields the
items::Contextsearchesfor the opening '(', but because the visibility modifier introduces
another '(' -- for example
pub(crate)-- the parsing gets messed up.Now the span is updated to start right before the tuple struct's opening
'(' so that the
items::Contexwill always find the tuple fields.