rustfmt: Format cfg_select! rust-lang/rust#154202

The current implementation leaves empty cfg_select! calls written with () or [] delimiters unchanged so that comments aren't accidentally removed. That's why these examples aren't properly indented.

Suggestion: could you leave this comment in the test too? (um, slightly funny for rustfmt tests because well, comments can be formatting-meaningful themselves)

A pre-existing issue with trailing comments in rustfmt. I wanted to explicitly capture the current behavior in a test case.

Through some testing I found that in style_edition <= 2021 this example won't be formatted at all because the predicate can't be formatted within max_width, but with style_edition >= 2024 that's not an issue.

Not the best place for a comment to begin with IMO, but I wanted to call out that comments within the predicate aren't correctly handled in style_edition <= 2021. This is how the current example is formatted:

cfg_select! {
    any(
        true, /* comment */
        true, true, // true,
        true,
    ) => {}

    not(false // comment) => {}

    any(false // comment) => "any",
}

Clearly that's wrong and produces invalid code, but fixing this seems like it's outisde the scope of this PR. since it only impacts style_edition <= 2021.

Maybe this is something we can address later?

I agree, I think we shouldn't try to bundle that in this PR. We probably should open an issue in rustfmt/r-l/r to track this as a known-bug though.

For completeness it might be good to test at all expansion sites. e.g. types, patterns and several items in a trait, impl or extern block. We have some basic examples here:

Thanks for pointing that out. I'll add some extra test cases!

Just added some extra test cases

Question: just to check my understanding,

cfg_select! parsing needs to be implemented in rustfmt right now
because there's no good way to call rustc_attr_parsing::parse_cfg_select.

I assume this is because rustfmt can't / don't want to (transitively) depend on things beyond parsing/AST? I.e. rustc_hir?

Suggestion: add a backlink to the reference re. cfg_select!:

//! See <https://doc.rust-lang.org/nightly/reference/conditional-compilation.html#the-cfg_select-macro> for grammar.

Suggestion: maybe leave a quick TL;DR example for this:

/// LHS predicate of a `cfg_select!` arm.
pub(crate) enum CfgSelectFormatPredicate {
    /// Example: the `unix` in `unix => {}`. Notably, outer or inner attributes are not permitted.
    Cfg(ast::MetaItemInner),
    /// `_` in `_ => {}`.
    Wildcard(Span),
}

Re. outer/inner attributes, counter-example:

cfg_select! {
    #[cfg(true)]
    _ => {
        fn main() {}
    }
}

Suggestion: maybe some comments for "visual" purposes.

/// Each `$predicate => $production` arm in `cfg_select!`.
pub(crate) struct CfgSelectArm {
    /// The `$predicate` part.
    pub(crate) predicate: CfgSelectFormatPredicate,
    /// Span of `=>`.
    pub(crate) arrow: Token,
    /// The RHS `$production` expression.
    pub(crate) expr: Box<ast::Expr>,
    /// `cfg_select!` arms `$production`s can be optionally `,` terminated, like `match` arms. The `,` is not needed when `$production` is itself braced `{}`.
    pub(crate) trailing_comma: Option<Span>,
}

Question: could you double-check if the following cross-crate macro expansion case

// another_crate.rs
macro_rules! garbage {
    () => {
        fn main() {}
    }
}

// main.rs
cfg_select! {
    _ => {
        garbage!();
    }
}

produces a span that can be used for formatting?

Remark: hm, I think this will work for 99% of use cases, but a user can re-export the std cfg_select! under another name (rustfmt AFAIK cannot use namers information, so I believe this is indeed the upper bound of what rustfmt can and should do), i.e.

// `cfg_select` available from core prelude.
// Re-export that name as another in macro namespace.
use cfg_select as i_want_to_trick_rustfmt;

i_want_to_trick_rustfmt! {
    _ => {
        fn main() {}
    }
}

Nit (typo): cfg_select!

Question: could you remind me how multiple (>= 1, >= 2) whitespace lines are intended to be handled? Should we add a test case for e.g.

// Original `{}` delimiters, multiple whitespace lines
cfg_select! { // opening brace comment



}

Question: does/should the above interact with the max whitespace lines config option? (Sorry I don't immediately recall what that one was called.)

Question: this should also a "known bug" right? Should we also include a follow-up issue to track this?

Suggestion: there's two cases of additional test coverage that we may want to add:

1. Max-width and when to prefer , versus {}.
   This was originally discussed in #145233 where it was decided that we also permit unbraced RHS productions when the RHS is an expression, that <expr>, versus {...} is picked based on max width IIUC. Should we add a test cases for non-default max-width versus default max-width and its influence on whether , or {} is picked?
2. Nested cfg_select!. For better or worse, you can also write

   cfg_select! {
       _ => cfg_select! [
           _ => {
               // I don't know why you would write this,
               // but you can, so... 🤷 
           }
       ]
   }

Suggestion: should we add some coverage for how certain inline comments are handled?

    match x {
        (/* 1 */ cfg_select! /* 2 */  {
            unix => Some(n),
            _ => None,
        } /* 3 */) => true,
    }

I'm less fussed about them, since I know certain inline comments gets dropped (long-standing known issues). But might still be good to have coverage to detect changes in case a fix is attempted.

Backlinks / context for myself (and future travellers):

Metadata

• Library feature: cfg_select
• std docs: https://doc.rust-lang.org/nightly/std/macro.cfg_select.html
• Tracking issue: #115585
• Style team discussion: rust-lang/style-team#201
• Reference update PR: rust-lang/reference#2103

History

Initial style team discussions

• The arms must be wrapped in braces, so rustfmt will have to ensure to not remove those.

Follow-up: unbraced expressions are permitted

PR: #145233