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

Comment is indented incorrectly #4506

Closed
camelid opened this issue Oct 30, 2020 · 5 comments
Closed

Comment is indented incorrectly #4506

camelid opened this issue Oct 30, 2020 · 5 comments
Labels
bug Panic, non-idempotency, invalid code, etc.

Comments

@camelid
Copy link
Member

camelid commented Oct 30, 2020

Describe the bug

The comment at the end of the first outer if is indented incorrectly by rustfmt. It can be fixed by adding a statement after it, but is not fixed with a statement before it.

To Reproduce

Here's a snippet of source code that causes the issue (it's from something I'm working on for rust-lang/triagebot) [playground]:

pub fn parse<'a>(input: &mut Tokenizer<'a>) -> Result<Option<Self>, Error<'a>> {
    let mut toks = input.clone();

    if toks.eat_token(Token::Word("modify"))? && toks.eat_token(Token::Word("labels"))? {
        if toks.eat_token(Token::Colon)? {
            // ate the token
        } else if toks.eat_token(Token::Word("to"))? {
            // optionally eat the colon after to, e.g.:
            // @rustbot modify labels to: -S-waiting-on-author, +S-waiting-on-review
            toks.eat_token(Token::Colon)?;
        } else {
            // It's okay if there's no to or colon, we can just eat labels
            // afterwards.
        }
        1 + 2;
    // continue
    } else if toks.eat_token(Token::Word("label"))? {
        // continue
    } else {
        return Ok(None);
    }

    if let Some(Token::Word("to")) = toks.peek_token()? {
        return Err(toks.error(ParseError::MisleadingTo));
    }
    // start parsing deltas
    let mut deltas = Vec::new();
    loop {
        deltas.push(LabelDelta::parse(&mut toks)?);

        // optional `, and` separator
        if let Some(Token::Comma) = toks.peek_token()? {
            toks.next_token()?;
        }
        if let Some(Token::Word("and")) = toks.peek_token()? {
            toks.next_token()?;
        }

        if let Some(Token::Semi) | Some(Token::Dot) | Some(Token::EndOfLine) = toks.peek_token()? {
            toks.next_token()?;
            *input = toks;
            return Ok(Some(RelabelCommand(deltas)));
        }
    }
}

Expected behavior

The comment should be indented four more spaces.

Meta

  • rustfmt version: rustfmt 1.4.20-stable (48f6c32 2020-08-09) and playground rustfmt
  • From where did you install rustfmt?: rustup and on the playground
  • How do you run rustfmt: VS Code Rust plugin, cargo fmt, and on the playground
@camelid camelid added the bug Panic, non-idempotency, invalid code, etc. label Oct 30, 2020
@camelid
Copy link
Member Author

camelid commented Oct 30, 2020

Note that it works fine with doc comments.

@calebcartwright
Copy link
Member

Closing as duplicate of #4120

For background, some authors intend for the comment to be associated to the following else/else if and want it indented accordingly while in other cases it's truly intended to be a member of the containing block with the same indentation as other block members. Based on earlier bug reports for the former, rustfmt categorically, and incorrectly, was configured to always indent a trailing comment at this level.

Instead of always defaulting to one (or the other), we've updated rustfmt to take the author's original comment placement into account when determining indentation for such a trailing comma so that we can preserve that intent.

This has already been implemented in the rustfmt source but not yet backported to a 1.x release.

@camelid
Copy link
Member Author

camelid commented Nov 1, 2020

Great! Glad to know it’s fixed. Any timeline on when it will be backported to 1.x?

@calebcartwright
Copy link
Member

I was hoping to pull it into the next one (1.4.23) but probably not going to happen due to the broken tool state and the desire to get that fix (and a few other high priority items) out as quickly as possible. The following 1.4.24 release is more likely

@calebcartwright
Copy link
Member

We had a couple quick interim releases to remove some blockers in other workstreams, but this change will be available in v1.4.26 whenever it hits nightly.

Note that rustfmt won't modify the existing indentation, but if you re-indent that // continue comment within the if block then rustfmt will honor that indentation level going forward

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Panic, non-idempotency, invalid code, etc.
Projects
None yet
Development

No branches or pull requests

2 participants