Invalid syntax produced if "in" is a subset of a variable name between for ... in.

[SimonWoodburyForget]

If the subset of a variable name is "in" inside of the for-loop's for ... in section and requires formatting with a comment around the in-keyword, an invalid output is produced.

For this input:

fn main() {
    for variable_in_here /* ... */ in 0..1 {}
    for variable_in_here in /* ... */ 0..1 {}
    for variable_in_here
    in /* ... */ 0..1 {}
    for variable_in_here
    /* ... */ in 0..1 {}
}

The output becomes invalid syntax:

fn main() {
    for variable_in_here in _here /* ... */ in 0..1 {}
    for variable_in_here in _here in /* ... */ 0..1 {}
    for variable_in_here in
        _here
        in /* ... */
        0..1
    {}
    for variable_in_here in
        _here
        /* ... */ in
        0..1
    {}
}

If the characters "in" are removed from the input, the output will remain valid syntax:

fn main() {
    for variable__here /* ... */ in 0..1 {}
    for variable__here in /* ... */ 0..1 {}
    for variable__here in /* ... */ 0..1 {}
    for variable__here
    /* ... */ in 0..1 {}
}

---

$ rustfmt --version
rustfmt 1.4.37-stable (a178d03 2021-07-26)

[calebcartwright]

Thank you for the report! These types of cases are almost always related to some bad span derivations on rustfmt's part when it needs to figure out where to look for comments in between those elements explicitly represented in the AST.

I wouldn't be surprised if we can't yet handle some of those comments, but the absolute worst case scenario should be emitting the original contents associated with the span of the range expression, and not emitting invalid code and carrying on happily, so this definitely a bug.

For anyone interested/willing in working on this, note that we use an internal ControlFlow construct to format certain kinds of expressions to increase reuse/simplify formatting for things that get formatted similarly, and that construct also includes what's referred to as a "connector" which is used in cases where there's something between a pattern and an expression.

The "connector" here would be the in keyword, which isn't explicitly represented in the ForLoop expression node, so what seems to be happening here is that rustfmt is trying to find the span between the pattern and keyword (between variable_in_here and in) in a buggy way that's using too early a starting point (i.e. it's starting from the beginning of the pattern span or even the entire expression span and looking for the first non-commented in, which is incorrectly finding the in inside the pattern ident (variable_in_here).

As a starting point I'd suggest taking a look in https://github.com/rust-lang/rustfmt/blob/master/src/expr.rs for controlflow rewriting that's working with the connector. Buggy span derivation code is most likely somewhere in there, but even if not you should be able to find it be following the calls from there.

[ytmimi]

I've tracked down the issue to ControlFlow::rewrite_pat_expr, and I'll submit a PR a little later. Specifically the Issue has to do with how the comment_lo is calcualted. In the case of a for loop the connector is " in".

rustfmt/src/expr.rs

Lines 823 to 826 in f0f449d

	let comments_lo = context
	.snippet_provider
	.span_after(self.span, self.connector.trim());
	let comments_span = mk_sp(comments_lo, expr.span.lo());

Case 1)

We write a for loop where the variable contains the trimmed connector in:

             Calculated Comment Span
             -----------------------
               |                 |
               V                 V
for variable_in_here /* ... */ in 0..1 {}

Case 2)

we write a for loop where the variable name doesn't contain the word in:

             Span is the whitespace after "in"
             --------------------------------
                              |
                              V
for variable_here /* ... */ in 0..1 {}

which actually leads to rustfmt bailing since it would have removed the comment

error[internal]: not formatted because a comment would be lost
 --> \\?\C:\Users\ytmimi\Product\RUST_PROJECTS\rustfmt\issue_5009_simple.rs:3
  |
3 |     for variable_here /* ... */ in 0..1 {}
  |
  = note: set `error_on_unformatted = false` to suppress the warning against comments or string literals

warning: rustfmt has failed to format. See previous 1 errors.

Proposed solution

Updated the comment_lo derivation to use context.snippet_provider.span_after_last instead of context.snippet_provider.span_after. This solution leads to the same behavior as Case 2) where rustfmt refuses to reformat the line because it doesn't want to remove a comment.

let comments_lo = context
    .snippet_provider
    .span_after_last(self.span, self.connector.trim());

Maybe we can do better than this? but then we'd probably have to consider comments in various different spots:

for /* ... */ x in 0..1 {}
for x /* ... */ in 0..1 {}
for x in /* ... */ 0..1 {} // currently only comments in here are considered