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

cargo fmt stopped working since 1.44.0 #4266

Closed
xStrom opened this issue Jun 19, 2020 · 7 comments · Fixed by #4268
Closed

cargo fmt stopped working since 1.44.0 #4266

xStrom opened this issue Jun 19, 2020 · 7 comments · Fixed by #4268
Labels
bug Panic, non-idempotency, invalid code, etc.

Comments

@xStrom
Copy link

xStrom commented Jun 19, 2020

I'm trying to format code for the druid project.
Everything was working fine up until Rust 1.43.1 / rustfmt 1.4.12-stable (a828ffe 2020-03-11).

It no longer works properly with:
Rust 1.44.0 / rustfmt 1.4.14-stable (e417356 2020-04-21)
Rust 1.44.1 / rustfmt 1.4.16-stable (939e164 2020-06-11)

It does not seem to be platform dependant, the issue exists at minimum both on Windows 7 and macOS Catalina, and not just my machines.

The druid workspace consists of several packages including druid-shell which is the package that is problematic.

If I run cargo fmt inside the druid-shell package root then it works.

The problem is with using cargo fmt or cargo fmt --all in the workspace directory.
That used to also work, but now works only partially on druid-shell.

Some of the modules get formatted while others don't.
It seems to have something to do with the cfg-if macro.

A file of interest is druid-shell/src/platform/mod.rs, which has the following code:

cfg_if::cfg_if! {
    if #[cfg(target_os = "windows")] {
        mod windows;
        pub use windows::*;
    } else if #[cfg(target_os = "macos")] {
        mod mac;
        pub use mac::*;
    } else if #[cfg(all(feature = "x11", target_os = "linux"))] {
        mod x11;
        pub use x11::*;
    } else if #[cfg(target_os = "linux")] {
        mod gtk;
        pub use self::gtk::*;
    } else if #[cfg(target_arch = "wasm32")] {
        mod web;
        pub use web::*;
    }
}

If I delete that macro and replace it with a simple:

mod windows;
pub use windows::*;

Then the windows module gets formatted when I run cargo fmt in the workspace directory. However if the cfg-if macro is there, then none of these platform modules get formatted. Again, this is a recent regression and previously all those platform modules got formatted.

Any insight would be appreciated.

@topecongiro topecongiro added the bug Panic, non-idempotency, invalid code, etc. label Jun 19, 2020
@topecongiro
Copy link
Contributor

MWE:

rustfmt -cr \
        druid/examples/web/src/lib.rs \
        druid-shell/src/lib.rs

However, the below works correctly 🤔

rustfmt -cr \
        druid-shell/src/lib.rs

@calebcartwright
Copy link
Member

I'm not sure we've ever fully supported this scenario (see #3253), even though I've convinced myself on a few occasions that we were previously close 😆

Also FWIW, I'm able to reproduce the issue with rustfmt 1.4.12 in identical cfg_if scenarios

@calebcartwright
Copy link
Member

calebcartwright commented Jun 19, 2020

It's also fixed in rustfmt 1.4.18 and on master/rustfmt 2.0.

rustfmt 1.4.14 was the first rustfmt release that had to incorporate the fallout from the upstream parse/expansion split in rustc, and there were a few parser/mod resolution bugs that snuck through in rustfmt 1.4.14 and 1.4.16 that I'm guessing may be the culprit here.

(rustfmt 1.4.18 will hit nightly once rust-lang/rust/pull/73506 lands)

Disregard ☝️

@topecongiro
Copy link
Contributor

@calebcartwright I don't think this is fixed on the master; I can still reproduce this locally using the master branch.


This seems the problem:

lazy_static! {
static ref CFG_IF: Symbol = Symbol::intern("cfg_if");
}

The CFG_IF is somehow changed to //! after formatting druid/examples/web/src/lib.rs. I have no idea why 🤷

@calebcartwright
Copy link
Member

calebcartwright commented Jun 19, 2020

well that's puzzling 🤔 I'll check again with 2.0

I'm pretty confident that 1.4.18 is fine though (confident, but wrong!)

$ cargo fmt --version
rustfmt 1.4.18-nightly (c1e9b7b8 2020-06-13)
$ pwd
/home/caleb/dev/druid
$ cargo fmt -- --check
Diff in /home/caleb/dev/druid/druid-shell/src/window.rs at line 14:
 
 //! Platform independent window types.
 
-    use std::any::Any;
+use std::any::Any;
 use std::time::Duration;
 
 use _crate::application::Application;_

@topecongiro
Copy link
Contributor

@calebcartwright You are modifying the wrong file 😉 You should change one of the files under druid-shell/src/platform/ other than mod.rs.

@calebcartwright
Copy link
Member

crikey 🤦‍♂️

sorry for the noise! I started looking at this earlier but had to stop, and assumed I had the right file up when I came back 😆

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

Successfully merging a pull request may close this issue.

3 participants