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

Fix handling of attribute in enum #6286

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Commits on Aug 18, 2024

  1. fix(items): Fix handling of rustdoc and macro attributes in enum

    This fix was made thanks to the hint at [1].
    This was reported in issue rust-lang#5662 [2].
    
    Previously, a enum item containing an attribute (rustdoc or macro) would
    be considered multi-line, thus forcing the formatting strategy of all
    the items in the enum to be Vertical (i.e. multi-line formatting).
    
    When determining the formatting strategy for enum items, we should
    ignore the attributes. This is what we do in the `is_multi_line_variant`
    function. Or else, simply adding a rustdoc comment or a macro attribute
    would cause the formatting of the whole enum to change, which is not a
    desirable behavior.
    
    We will be adding tests in the following commits.
    
    - [1] rust-lang#5662 (comment)
    - [2] rust-lang#5662
    malikolivier committed Aug 18, 2024
    Configuration menu
    Copy the full SHA
    ce990c8 View commit details
    Browse the repository at this point in the history
  2. tests/target: Add failing test for rustdoc in enum

    Test case as reported in rust-lang#6280.
    This test case used to fail, but the code in the previous commit fixes
    it.
    
    We followed instructions on Contributing.md [1] to create a test case.
    
    `tests/target/rust-doc-in-enum/vertical-no-doc.rs` is being left unformatted
    (expected behavior), while `tests/target/rust-doc-in-enum/vertical-with-doc.rs`
    is formatted to `C { a: usize }` (unexpected behavior).
    
    The only different between the two samples is the `/// C` rustdoc added
    in `with-doc.rs`.
    This reproducing example is minimal: for example, after removing `d: usize`
    we do not reproduce the reported behavior.
    
    We found this issue while adding rustdocs to an existing project.
    We would expect that adding a line of documentation should not cause the
    formatting of the code to change.
    
    [1] https://github.com/rust-lang/rustfmt/blob/40f507526993651ad3b92eda89d5b1cebd0ed374/Contributing.md#L33
    malikolivier committed Aug 18, 2024
    Configuration menu
    Copy the full SHA
    6f204cd View commit details
    Browse the repository at this point in the history
  3. fix: Fix rustfmt formatting after previous commit fix

    The enums modified here were not properly formatted.
    The fix in the previous commit now formats them properly.
    
    -> Regardless of the presence of comments or macro attributes, if any of
    the enum items is multi-line, all enum items should be formatted as
    multi-line.
    malikolivier committed Aug 18, 2024
    Configuration menu
    Copy the full SHA
    757e73d View commit details
    Browse the repository at this point in the history
  4. Configuration menu
    Copy the full SHA
    df5b6d9 View commit details
    Browse the repository at this point in the history

Commits on Aug 19, 2024

  1. tests(attribute-in-enum): Add a test with a macro attribute for exhau…

    …stivity
    
    Current naive implementation fails with multi-line macro attributes.
    malikolivier committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    933e997 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    103d13e View commit details
    Browse the repository at this point in the history
  3. Revert some part of "fix: Fix rustfmt formatting after previous commi…

    …t fix"
    
    This reverts some part of commit 757e73d.
    
    The `enum.rs` should still be formatted with the default style edition.
    So no change should happen for those files.
    malikolivier committed Aug 19, 2024
    Configuration menu
    Copy the full SHA
    22b076c View commit details
    Browse the repository at this point in the history

Commits on Aug 20, 2024

  1. fix(items): Mention Edition 2021

    The project has been using `>= StyleEdition::Edition2024` to indicate new formatting,
    and it's easier to grep for `Edition2021` to find all of the older formatting points. [1]
    
    [1] rust-lang#6286 (comment)
    malikolivier committed Aug 20, 2024
    Configuration menu
    Copy the full SHA
    5601462 View commit details
    Browse the repository at this point in the history

Commits on Aug 21, 2024

  1. fix(items): Take into account multi-line outer macro attributes

    We properly handle multi-line attributes in front of an enum variant.
    
    There are several types of attributes [1], but the only attributes that
    can occur in this context are outer attributes like
    `#[repr(transparent)]`, `/// Example` or `/** Example */`.
    
    This commit deals with macro attributes like `#[repr(transparent)]`.
    We implement our own trivial macro attribute parser to exclude them from
    the variant definition, as we could not find a easy way of re-using
    existing parsing code (with the `syn` crate or `rustc_parse`).
    
    We will deal with outer documentation blocks in the next commit.
    
    [1] https://docs.rs/syn/2.0.75/syn/struct.Attribute.html
    malikolivier committed Aug 21, 2024
    Configuration menu
    Copy the full SHA
    277cb90 View commit details
    Browse the repository at this point in the history
  2. fix(items): Take into account outer documentation blocks

    We properly handle outer documentation blocks in front of an enum
    variant.
    
    With this commit, we believe we have handled all edge-cases regarding
    attributes in front of enum variants.
    malikolivier committed Aug 21, 2024
    Configuration menu
    Copy the full SHA
    3d1b831 View commit details
    Browse the repository at this point in the history

Commits on Sep 20, 2024

  1. let format_variant tell us if the variant is multi-line

    Instead of returning `Option<String>`, `format_variant` now returns
    `(Option<String>, bool).
    
    Maybe this could be `Option<(String, bool)>` instead though?
    ytmimi committed Sep 20, 2024
    Configuration menu
    Copy the full SHA
    7594e30 View commit details
    Browse the repository at this point in the history