fmt no longer finds modules inside inline modules

[heftig]

Problem
cargo fmt no longer processes modules that are referenced in an inline mod.

Steps

crate tree:

.
├── Cargo.toml
└── src
    ├── bar
    │   └── baz.rs
    ├── foo
    │   └── qux.rs
    ├── foo.rs
    └── main.rs

src/main.rs:

fn main() {
    println!("Hello, world!");
}

mod foo;
mod bar {
    mod baz;
}

src/foo.rs: mod qux;
src/foo/qux.rs: empty
src/bar/baz.rs: empty

Stable is fine:

> cargo +stable fmt -- -v
Formatting /home/jan/x/src/bar/baz.rs
Formatting /home/jan/x/src/foo/qux.rs
Formatting /home/jan/x/src/foo.rs
Formatting /home/jan/x/src/main.rs

Nightly is broken:

> cargo +nightly fmt -- -v
Formatting /home/jan/x/src/foo/qux.rs
Formatting /home/jan/x/src/foo.rs
Formatting /home/jan/x/src/main.rs

Notes

Output of cargo version:

• Nightly is broken: cargo 1.54.0-nightly (44456677b 2021-06-12)
• Stable is fine: cargo 1.52.0 (69767412a 2021-04-21)

Rust via rustup 1.24.3, Arch Linux.

[heftig]

Beta (cargo 1.53.0-beta (4369396ce 2021-04-27)) is affected as well.

> cargo +beta fmt -- -v
Formatting /home/jan/x/src/foo/qux.rs
Formatting /home/jan/x/src/foo.rs
Formatting /home/jan/x/src/main.rs

[calebcartwright]

We haven't really had any changes on our end, much less anything that touched the mod resolver. Could you please run cargo fmt --version just to rule out the possibility of an older install being used

[heftig]

> cargo fmt --version         
rustfmt 1.4.36-stable (7de6968 2021-02-07)

> cargo +stable fmt --version
rustfmt 1.4.36-stable (7de6968 2021-02-07)

> cargo +beta fmt --version  
rustfmt 1.4.37-beta (2a3635d 2021-05-04)

> cargo +nightly fmt --version
rustfmt 1.4.37-nightly (a85f584 2021-06-16)

[calebcartwright]

Thanks! Have a theory, will test it out later today

[ehuss]

Bisecting was a bit tricky since it was just a "rustfmt update", but 612e8d5 looks suspicious. Let us know if you need any help.

[calebcartwright]

thanks @ehuss

in the ast prior to any kind of expansion, the bar mod item here would have a loaded/inline::yes kind wouldn't it?

mod bar {
    mod baz;
}

if so it's probably an issue here. i'm stuck at work for several more hours but if anyone wants to take a pass at it in the interim i'd probably start looking here on that first arm

rustfmt/src/modules.rs

Lines 320 to 325 in 6495024

	match (sub_mod.ast_mod_kind, sub_mod.items) {
	(Some(Cow::Borrowed(ast::ModKind::Loaded(items, ast::Inline::No, _))), _) => {
	self.visit_mod_from_ast(&items)
	}
	(Some(Cow::Owned(..)), Cow::Owned(items)) => self.visit_mod_outside_ast(items),
	(_, _) => Ok(()),

[ehuss]

Yea, from my understanding of rust-lang/rust@4a88165, it is loaded/inline::Yes. So maybe something like this could work:

--- a/src/modules.rs
+++ b/src/modules.rs
@@ -318,7 +318,7 @@ impl<'ast, 'sess, 'c> ModResolver<'ast, 'sess> {
             self.directory = directory;
         }
         match (sub_mod.ast_mod_kind, sub_mod.items) {
-            (Some(Cow::Borrowed(ast::ModKind::Loaded(items, ast::Inline::No, _))), _) => {
+            (Some(Cow::Borrowed(ast::ModKind::Loaded(items, ast::Inline::Yes, _))), _) => {
                 self.visit_mod_from_ast(&items)
             }
             (Some(Cow::Owned(..)), Cow::Owned(items)) => self.visit_mod_outside_ast(items),

However, from what I can tell, rustfmt does not format post-expansion loaded modules. I'm not too familiar with rustfmt, but is it known or intended that does not work? For example:

macro_rules! foo {
    () => {
        mod foo;
    };
}

foo! {}

Here, foo.rs will never get formatted because it is only loaded after expansion. That's maybe unrelated to the issue here, just curious.

[calebcartwright]

However, from what I can tell, rustfmt does not format post-expansion loaded modules. I'm not too familiar with rustfmt, but is it known or intended that does not work? For example:
That's maybe unrelated to the issue here, just curious.

You are correct and yes it's known/expected. There's a few exceptions, such as cfg_if, where we have special case handling to use the tokens and attempt to parse out mod items, but otherwise such post-expansion mods and their associated files don't get formatted.

Probably a couple options for extending support for those types of cases, but I'd expect it to be a sizeable undertaking that'd be a relatively low priority compared to some other items on our backlog.

[calebcartwright]

The fix has been applied in the rustc repo and should be in the latest nightly so going to close

[pnkfelix]

If this affects beta or stable versions (i.e. not just nightly) then should it remain open (at least until there's a decision about whether to backport the thing to beta/stable)? That is the policy we use for rustc, but I don't know if the rustfmt team uses a different policy.

or maybe the point is that the real bug needs to be tracked on the rustc repo, not the rustfmt repo... hmm.

[calebcartwright]

If this affects beta or stable versions (i.e. not just nightly) then should it remain open (at least until there's a decision about whether to backport the thing to beta/stable)? That is the policy we use for rustc, but I don't know if the rustfmt team uses a different policy.

We've always closed our issues as they are fixed in source, and don't base our issue status on release/channel propagation upstream. In my opinion backport nominations/status are far better handled in rustc and I don't want to attempt to try to duplicate/mirror that here.

Although I'd personally encourage proceeding with the backport, that's a separate concern from the bug being fixed, and whether or not backporting happens is obviously a decision outside our control.

or maybe the point is that the real bug needs to be tracked on the rustc repo, not the rustfmt repo... hmm.

No, development and maintenance of rustfmt happens here and our issues need to continue to be managed here as well. I'm not sure if it makes sense to have a separate tracking-like issue in rustc for the backporting piece, but I don't want to bifurcate our issue tracker across two repos.