Developer experience, cargo fmt

[pacak]

What does this PR do

Right now cargo fmt is broken, CI admits it calling format on each individual file: cargo fmt --all -- --check **/*.rs. Second commit shows that CI was not actually enforcing formatting across all the files. This breaks workflow of writing code as is and letting your editor to deal with formatting on save or hotkey.

The problem is caused by the feature! macro. rustfmt does not perform any macro expansion and ignores everything inside of any unknown macro, in fact nothing inside of the feature! macro blocks is formatted. This includes scanning for child modules. feature! macro is used to lock some parts of the library behind features as well as explicitly document that in rustdoc using nightly feature doc_cfg.

This PR proposes a potential alternative - I replaced some of the feature! macro invocation with plain #[cfg(feature ...)] invocation and implicit feature documentation using a different nightly feature: doc_auto_cfg. Downside (or an upside) is that documentation will contain the actual combination of feature flags and target platforms, for example features for dir from dir becomes Non-Redox and dir.

If we agree that this is a way forward - I'll remove all the remaining uses of feature! macro and remove it. It might cause some merge conflicts.

Checklist:

• I have read CONTRIBUTING.md
• I have written necessary tests and rustdoc comments
• A change log has been added if this PR modifies nix's API

[SteveLauC]

The problem is caused by the feature! macro. rustfmt does not perform any macro expansion and ignores everything inside of any unknown macro,

Yeah, this is indeed annoying!

This PR proposes a potential alternative - I replaced some of the feature! macro invocation with plain #[cfg(feature ...)] invocation

I am not the person who added this macro, but I think the point of using this macro is to avoid repeated cfg attribute invocation.

implicit feature documentation using a different nightly feature: doc_auto_cfg.

This is not something we want as discussed here: #1611 (comment)

[SteveLauC]

cargo fmt --all -- --check **/*.rs. Second commit shows that CI was not actually enforcing formatting across all the files. This breaks workflow of writing code as is and letting your editor to deal with formatting on save or hotkey.

Would setting the option --all **/*.rs for your format command in your IDE work? 🤔

[pacak]

Would setting the option --all **/*.rs for your format command in your IDE work? 🤔

This doesn't solve all the problems:

1. This PR made it so rustfmt sees that formatting in src/net/if_.rs was broken and cargo fmt managed to fix it.
2. from rustfmt point of view - everything inside unknown macro invocations is just token trees it doesn't know how to format.

Plus as I said in #2506 - I don't want to make any changes to my global configuration and figuring how to make it local only is still fairly low priority.

This is not something we want as discussed here: #1611 (comment)

Hmmm... Banners are verbose, but I think they are accurate. At least in the docs I generated I saw this: net and (linux_android or bsd or solarish) and I'm on Linux (not android).

This is an unstable feature and if there are problems with how it works - it's probably a good idea to at least mention it here: rust-lang/rfcs#3631

[asomers]

Yes, as @SteveLauC said we don't want to use doc_auto_cfg. That would make the docs worse than they are now. And many more people need to read our docs than format our source code, so we should optimize for their experiences.

[SteveLauC]

This is an unstable feature and if there are problems with how it works - it's probably a good idea to at least mention it here: rust-lang/rfcs#3631

Looks like we can combine doc_auto_cfg and doc_cfg_hide, e.g., the banner of function foo:

#![cfg_attr(docsrs, feature(doc_auto_cfg))]
#![cfg_attr(docsrs, feature(doc_cfg_hide))]
#![doc(cfg_hide(target_os = "macos"))]

#[cfg(any(target_os = "macos", feature = "foo"))]
pub fn add(left: u64, right: u64) -> u64 {
    left + right
}

will be "available on feature foo" rather than "available on macOS and feature foo" because I hide the target_os = "macos" at the crate level.

---

Regarding this PR, I still think the point of macro feature!() is to relieve us of writing repeated cfg(xxx), so removing it does not seem good. Though I do agree that it makes formatting hard and the developer experience bad if you have something like "fmt-on-save" open.