Some changes occurred in src/tools/rustfmt.

cc @calebcartwright

r? @estebank

(rust-highfive has picked a reviewer for you, use r? to override)

There is still one case that this arguably doesn't handle perfectly, namely this:

/* hey
whazzup */ fn main() {
    Command::new("git outta here");
}

where this still suggests:

2 | whazzup */ use std::process::Command;
  |                                      

(but that suggestion isn't wrong, and I expect that case to be absurdly rare, so I think its reasonable to just go ahead with this approach.)

re: The rustfmt test failure, I'd think a fmt/tidy run would sort it.

That particular test is a bootstrapping sort for rustfmt which runs rustfmt against itself. Technically that means in this repository that test runs with a slightly different version of rustfmt than what x.py has pinned, so let me know if there's anything strange/conflicting with the formatting there.

Edit: Disregard the above. Believe manual formatting on the subtree is required when editing in tree, in part because of those differences between the two repos in how formatting is executed

Nice!

There is a duplicate use placement finder at find_use_placement. Is it feasible to keep the implementations in sync?

Can you also update use_suggestion_placement.rs to remove the FIXME? Essentially something like:

--- a/src/test/ui/resolve/use_suggestion_placement.rs
    b/src/test/ui/resolve/use_suggestion_placement.rs
@@ -10,11  10,7 @@ mod m {
 }

 mod foo {
-    // FIXME: UsePlacementFinder is broken because active attributes are
-    // removed, and thus the `derive` attribute here is not in the AST.
-    // An inert attribute should work, though.
-    // #[derive(Debug)]
-    #[allow(warnings)]
     #[derive(Debug)]
     pub struct Foo;

     // test whether the use suggestion isn't

There is a duplicate use placement finder at find_use_placement. Is it feasible to keep the implementations in sync?

Oh, I don't know! I didn't know about that, but it seems obvious that I should try to do that.

Can you also update use_suggestion_placement.rs to remove the FIXME? Essentially something like:

Will do!

Nice!

There is a duplicate use placement finder at find_use_placement. Is it feasible to keep the implementations in sync?

Okay, I looked into doing this.

Unfortunately, the code I'm adding here is using the AST, but by the time typeck runs, the AST is long gone.

I don't want to block this PR on figuring out the best way to bring these two parts into sync. (E.g. one could thread down the same span information that I attached to the AST.) I will add a fixme at least, though.

@calebcartwright can you explain this part of the rustfmt formatting error to me? (see below)

Mismatch at src/parse/parser.rs:113:
         let result = catch_unwind(AssertUnwindSafe(|| {
             let mut parser = new_parser_from_file(sess.inner(), path, Some(span));
             match parser.parse_mod(&TokenKind::Eof) {
-                Ok((a,
                     a,

                     ast::ModSpans {
                         inner_span,
                         inner_span,

namely, why on earth is the diff saying that its removing Ok(( from the text?

Was there a line that got removed in the report? I would have perhaps expected to see:

             match parser.parse_mod(&TokenKind::Eof) {
-                Ok((a,
                 Ok((
                     a,
                     i,

It does look a bit odd @pnkfelix, I haven't noticed it before. Worth noting though that these tests are using an internal, rudimentary diff emitter because of where/how the test is constructed, and it's not the same emitter and output for standard end user invocations of rustfmt.

May take a look at this if we get time, but not too concerned given it's only seen by the small audience of those modifying rustfmt source. Here's what an end user would see and what I would expect:

             match parser.parse_mod(&TokenKind::Eof) {
-                Ok((a, i, ast::ModSpans { inner_span, inject_use_span: _ })) => Some((a, i, inner_span)),
                 Ok((
                     a,
                     i,
                     ast::ModSpans {
                         inner_span,
                         inject_use_span: _,
                     },
                 )) => Some((a, i, inner_span)),

@bors r=ekuber rollup

📌 Commit 8f4c6b0 has been approved by ekuber

The follow up work to keep the suggestion span in the hir might be a good first-timer issue.

Filed as #94941

⌛ Testing commit 8f4c6b0 with merge 95561b3...

☀️ Test successful - checks-actions
Approved by: ekuber
Pushing 95561b3 to master...

Finished benchmarking commit (95561b3): comparison url.

Summary: This benchmark run did not return any relevant results.

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression