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

"left behind trailing whitespace" in long patterns #5559

Open
RReverser opened this issue Oct 4, 2022 · 6 comments
Open

"left behind trailing whitespace" in long patterns #5559

RReverser opened this issue Oct 4, 2022 · 6 comments
Labels
a-matches match arms, patterns, blocks, etc e-trailing whitespace error[internal]: left behind trailing whitespace p-low poor-formatting

Comments

@RReverser
Copy link
Contributor

Code like this:

fn put_telescope_synctocoordinates(
    PutTelescopeSynctocoordinatesPathParams {
                    
                        device_number,
                    
                }: PutTelescopeSynctocoordinatesPathParams,
) {
}

triggers "left behind trailing whitespace".

I suppose it's because rustfmt tries to put entire param - both pattern and type - on the same line, fails in doing so as that exceeds the max width, and gives up. Instead, it should probably fallback to something like

fn put_telescope_synctocoordinates(
    PutTelescopeSynctocoordinatesPathParams {
      device_number,
    }: PutTelescopeSynctocoordinatesPathParams,
) {
}
@ytmimi
Copy link
Contributor

ytmimi commented Oct 4, 2022

Thanks for the report!.

I believe your assessment is correct and we're trying to put the pattern and the type on the same line. After looking into it I think what's happening is there's enough space to write the pattern on a single line so we format it on one line, but there isn't enough space to put both the pattern and the type on one line so rewriting fails, which leads to returning the span unchanged. I did some digging to try and pin that down:

starting at rewrite_fn_base, we eventually call rewrite_params

rustfmt/src/items.rs

Lines 2287 to 2296 in ef91154

let param_str = rewrite_params(
context,
&fd.inputs,
one_line_budget,
multi_line_budget,
indent,
param_indent,
params_span,
fd.c_variadic(),
)?;

rewrite_params then calls the Rewrite impl for ast::Param, and if it fails to format the param we just return the original span unchanged, which explains the left behind trailing whitespace issue.

rustfmt/src/items.rs

Lines 2595 to 2597 in ef91154

param
.rewrite(context, Shape::legacy(multi_line_budget, param_indent))
.or_else(|| Some(context.snippet(param.span()).to_owned()))

For anyone interested in taking this on I'd start looking at the is_named_param if-else block in the ast::Param Rewrite impl.

rustfmt/src/items.rs

Lines 2035 to 2088 in ef91154

} else if is_named_param(self) {
let param_name = &self
.pat
.rewrite(context, Shape::legacy(shape.width, shape.indent))?;
let mut result = combine_strs_with_missing_comments(
context,
&param_attrs_result,
param_name,
span,
shape,
!has_multiple_attr_lines && !has_doc_comments,
)?;
if !is_empty_infer(&*self.ty, self.pat.span) {
let (before_comment, after_comment) =
get_missing_param_comments(context, self.pat.span, self.ty.span, shape);
result.push_str(&before_comment);
result.push_str(colon_spaces(context.config));
result.push_str(&after_comment);
let overhead = last_line_width(&result);
let max_width = shape.width.checked_sub(overhead)?;
if let Some(ty_str) = self
.ty
.rewrite(context, Shape::legacy(max_width, shape.indent))
{
result.push_str(&ty_str);
} else {
let prev_str = if param_attrs_result.is_empty() {
param_attrs_result
} else {
param_attrs_result + &shape.to_string_with_newline(context.config)
};
result = combine_strs_with_missing_comments(
context,
&prev_str,
param_name,
span,
shape,
!has_multiple_attr_lines,
)?;
result.push_str(&before_comment);
result.push_str(colon_spaces(context.config));
result.push_str(&after_comment);
let overhead = last_line_width(&result);
let max_width = shape.width.checked_sub(overhead)?;
let ty_str = self
.ty
.rewrite(context, Shape::legacy(max_width, shape.indent))?;
result.push_str(&ty_str);
}
}
Some(result)

I'd assume we'll also need to make some changes to rewrite_struct_pat, which gets called as a result of rewriting a struct pattern (the self.pat.rewrite call above Line 2036).

rustfmt/src/patterns.rs

Lines 258 to 260 in ef91154

PatKind::Struct(ref qself, ref path, ref fields, ellipsis) => {
rewrite_struct_pat(qself, path, fields, ellipsis, self.span, context, shape)
}

@ytmimi ytmimi added poor-formatting p-low a-matches match arms, patterns, blocks, etc e-trailing whitespace error[internal]: left behind trailing whitespace labels Oct 4, 2022
@sammysheep

This comment was marked as off-topic.

@calebcartwright

This comment was marked as off-topic.

@YpeKingma

This comment was marked as off-topic.

@YpeKingma

This comment was marked as off-topic.

@ytmimi

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-matches match arms, patterns, blocks, etc e-trailing whitespace error[internal]: left behind trailing whitespace p-low poor-formatting
Projects
None yet
Development

No branches or pull requests

5 participants