Skip to content

"left behind trailing whitespace" in long patterns #5559

Open
@RReverser

Description

@RReverser

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,
) {
}

Activity

ytmimi

ytmimi commented on Oct 4, 2022

@ytmimi
Contributor

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)
}

sammysheep

sammysheep commented on Oct 11, 2022

@sammysheep
calebcartwright

calebcartwright commented on Oct 12, 2022

@calebcartwright
YpeKingma

YpeKingma commented on Nov 6, 2022

@YpeKingma
YpeKingma

YpeKingma commented on Nov 6, 2022

@YpeKingma
ytmimi

ytmimi commented on Nov 7, 2022

@ytmimi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @RReverser@sammysheep@calebcartwright@ytmimi@YpeKingma

        Issue actions

          "left behind trailing whitespace" in long patterns · Issue #5559 · rust-lang/rustfmt