Autoformat build! macros with new windows_fmt crate

[MarijnS95]

Closes #827

We convert the build! (and generate!) macro calls to use curly braces, so that we can then replace the call with a use something::{ that allows rustfmt to perform all its formatting tricks like it normally would to use trees, to clean up the blocks and make them look more like the use blocks in main.rs.

This is wrapped up in a PowerShell script and pipes the replaced result through rustfmt to not modify and/or leave garbled files on disk. The script is ran in the CI to enforce proper formatting at all times; contributors are encouraged to run the script themselves if formatting differs.

[MarijnS95]

Now we have an example of a failed CI run at https://github.com/microsoft/windows-rs/actions/runs/887995007, including the ::error:: annotation:

Follow the CI link to see the full diff output.

[kennykerr]

Sweet - will take a look soon!

[kennykerr]

I had a quick look and not knowing anything about PowerShell am a little concerned about what this actually does and how. It would also be great if I could run it locally to observe its behavior, but when I run it I get errors.

C:\git\windows-format>powershell scripts\Format-Build-Macro.ps1
Get-Content : Cannot find path 'C:\git\windows-format\build.rs' because it does not exist.
At C:\git\windows-format\scripts\Format-Build-Macro.ps1:15 char:18
+   $fmtResult = ((Get-Content -Raw $File).replace($Pattern, $shim) | & ...
+                  ~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (C:\git\windows-format\build.rs:String) [Get-Content], ItemNotFoundException
    + FullyQualifiedErrorId : PathNotFound,Microsoft.PowerShell.Commands.GetContentCommand

You cannot call a method on a null-valued expression.
At C:\git\windows-format\scripts\Format-Build-Macro.ps1:15 char:3
+   $fmtResult = ((Get-Content -Raw $File).replace($Pattern, $shim) | & ...
+   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [], RuntimeException
    + FullyQualifiedErrorId : InvokeMethodOnNull

Another option is to write a Rust command to perform this operation. I think that would ultimately be a better choice anyway e.g. cargo windows-fmt. I'd prefer to keep everything in Rust if at all possible. Then we don't have to worry about making sure some other toolchain/scripting environment is present/enabled or have general expertise in other languages. I imagine a Rust tool could pretty easily do this reformatting with the help of the syn crate or just regex. 😊 Anyway, that's something I might explore when I get a moment. It's worth a try.

[MarijnS95]

I had a quick look and not knowing anything about PowerShell am a little concerned about what this actually does and how.

That is unfortunate, I had assumed PowerShell to be the go-to scripting language for Microsoft projects, like how win32metadata exists almost exclusively of PS scripts (and some C#). Would it help if I document it with some comments?

It would also be great if I could run it locally to observe its behavior, but when I run it I get errors.

Definitely! It runs fine on the CI under Windows and has me curious what's going on on your system.

C:\git\windows-format>powershell scripts\Format-Build-Macro.ps1
Get-Content : Cannot find path 'C:\git\windows-format\build.rs' because it does not exist.
...

This makes zero sense. Get-ChildItem (gci) managed to find a build.rs file that doesn't actually exist, there's no build.rs in the root. Can you run gci -r -fi build.rs in PowerShell in this tree and report what it managed to find? Alternatively I can push some extra logging here.

Another option is to write a Rust command to perform this operation. I think that would ultimately be a better choice anyway e.g. cargo windows-fmt.

I'm not a fan of binaries that have to be installed first, but it seems the xtask "trick" perfectly suits this need. It works beautifully in other projects like rust-analyzer. More simple than that, this can be a simple extra binary crate that's ran in the tree, just like cargo r -p windows_bindings.

I'd prefer to keep everything in Rust if at all possible. Then we don't have to worry about making sure some other toolchain/scripting environment is present/enabled or have general expertise in other languages. I imagine a Rust tool could pretty easily do this reformatting with the help of the syn crate or just regex. Anyway, that's something I might explore when I get a moment. It's worth a try.

The syn crate would have worked perfectly to solve the lost bracketing issue when the macro consists of a single top-level namespace. However, there is no way to maintain formatting when replacing the substituted use XXX::{} back to the original macro - that's after all what windows-rs currently calls rustfmt for on emitted output. A regex works but can be used from PowerShell as well, though it is not necessary with the current implementation.

Let me know if you'd like to debug this script a little or jump to an extra Rust crate immediately.

[kennykerr]

I'm not a fan of binaries that have to be installed first

It could be something simpler like the the way the "bindings" crate is run. The simplest possible solution (in Rust) would be ideal.

[MarijnS95]

https://github.com/microsoft/windows-rs/pull/828/checks?check_run_id=2766964459 Awesome, the new Rust script is still working exactly as PowerShell 🎉

[kennykerr]

Looks great! Looking forward to seeing a failed build. 😉 The only thing is I'd like to remove all "author" names from toml files. The only ones that should remain are those required by "cargo publish" for crates.io but the rest including examples/tests/tools should just be removed entirely. GitHub does a fine job of tracking contributors and I'd prefer not to manage that in text format as well. So just a heads up that I'll probably scrub these when I get a moment.

[MarijnS95]

That's fair; I think most community driven projects (ie. winit and their repos) have something along the lines of authors = ["The Rust Windowing contributors"] :). Forgot to remove it after creating this crate with cargo new --bin fmt.

[MarijnS95]

And after re-pushing the CI succeeds, as expected. @kennykerr given #842 (comment), in what steps do you want to land this?

[kennykerr]

And after re-pushing the CI succeeds, as expected. @kennykerr given #842 (comment), in what steps do you want to land this?

Looks great! It looks like this takes care of it all so I'm happy to just merge this in.

[MarijnS95]

Looks great! It looks like this takes care of it all so I'm happy to just merge this in.

Works for me!