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

apply 2024 version sort algorithm to mods #6368

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

calebcartwright
Copy link
Member

This closes the loop on all the implementation work for rust-lang/rust#123800 by applying the 2024 version sorting algorithm to mod declarations.

In practice I feel like this will functionally be nearly a no-op with zero-minimal diff on codebases when applied due to mod naming conventions, but the algorithm is required per the style guide.

The diff of this PR is exceptionally minimal in reality, I just went overboard with tests since the reorder_modules option is a stable option, and added a lot of empty mod.rs files

r? @ytmimi

Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make the old behavior easier to grep for I think we should change < StyleEdition::Edition2024 to <= StyleEdition::Edition2021

@calebcartwright
Copy link
Member Author

To make the old behavior easier to grep for I think we should change < StyleEdition::Edition2024 to <= StyleEdition::Edition2021

What makes you think that's easier to grep? I feel like that's bordering on a "glass half full vs half empty" and it would really depend on the author, but regardless, I think that anyone that wants to have a high degree of confidence that they've found all the locations of a behavior split between two style editions is going to have to search for all the >/< combinations with those two editions

(i.e. even if we try to be diligent about consistently applying one vs. the other, I don't think we can or should safely assume that a singular search 100% identified all such cases)

to be clear I'm not opposed to one vs. the other, I just don't know if this is something we should block on

@ytmimi
Copy link
Contributor

ytmimi commented Oct 17, 2024

What makes you think that's easier to grep?

Just to be clear, I don't think we'd need to block on that, though that's currently how the version=One formatting is gated in the codebase, and I think it'll be easier if we're consistent.

If you run rg "(<=\s.*Edition2021)|(<\s.*Edition2024)" src you'll see that we don't use < StyleEdition::Edition2024 for the old version=One formatting.

src/chains.rs
301:                if nested && context.config.style_edition() <= StyleEdition::Edition2021 {

src/matches.rs
116:        let shape = if context.config.style_edition() <= StyleEdition::Edition2021 {
441:        let arrow_index = if context.config.style_edition() <= StyleEdition::Edition2021 {
479:                let semicolon = if context.config.style_edition() <= StyleEdition::Edition2021 {

src/items.rs
2174:                if context.config.style_edition() <= StyleEdition::Edition2021
2616:            if context.config.style_edition() <= StyleEdition::Edition2021
3136:    let preserve_newline = context.config.style_edition() <= StyleEdition::Edition2021;

src/patterns.rs
297:                if context.config.style_edition() <= StyleEdition::Edition2021 =>

src/types.rs
910:                if context.config.style_edition() <= StyleEdition::Edition2021
988:                let rw = if context.config.style_edition() <= StyleEdition::Edition2021 {
1233:        style_edition @ _ if style_edition <= StyleEdition::Edition2021 => {

Conversely, if you run rg "(\s>\s.*Edition2021)|(>=\s.*Edition2024)" src you won't see any places where we gate version=Two formatting using > StyleEdition::Edition2021.

src/visitor.rs
293:                    if self.config.style_edition() >= StyleEdition::Edition2024

src/utils.rs
599:                || (config.style_edition() >= StyleEdition::Edition2024
615:                    if config.style_edition() >= StyleEdition::Edition2024 =>
659:    } else if config.style_edition() >= StyleEdition::Edition2024 {

src/missed_spans.rs
250:        } else if self.config.style_edition() >= StyleEdition::Edition2024

src/macros.rs
1289:        if context.config.style_edition() >= StyleEdition::Edition2024 {

src/expr.rs
1294:            && context.config.style_edition() >= StyleEdition::Edition2024

src/items.rs
147:                let init_str = if style_edition >= StyleEdition::Edition2024 {
172:                let assign_str_with_else_kw = if style_edition >= StyleEdition::Edition2024 {
678:        let attrs_str = if context.config.style_edition() >= StyleEdition::Edition2024 {
972:    let shape = if context.config.style_edition() >= StyleEdition::Edition2024 {
2573:        if context.config.style_edition() >= StyleEdition::Edition2024 {
2649:            if context.config.style_edition() >= StyleEdition::Edition2024 {

src/imports.rs
926:                    if self.style_edition >= StyleEdition::Edition2024 {
936:                let (ia, ib) = if self.style_edition >= StyleEdition::Edition2024 {
942:                let ident_ord = if self.style_edition >= StyleEdition::Edition2024 {
968:                        if self.style_edition >= StyleEdition::Edition2024 {

src/overflow.rs
210:                if config.style_edition() >= StyleEdition::Edition2024 =>
506:                        && self.context.config.style_edition() >= StyleEdition::Edition2024 =>
701:        let force_single_line = if self.context.config.style_edition() >= StyleEdition::Edition2024

src/stmt.rs
102:            if context.config.style_edition() >= StyleEdition::Edition2024 && self.is_last_expr() {

src/closures.rs
468:        ast::ExprKind::Loop(..) if style_edition >= StyleEdition::Edition2024 => true,

@calebcartwright
Copy link
Member Author

Right, but my point is regardless of it being what we do currently, and even if we are consistent with that through the shipping of 2024 and never make any other 2021 <=> 2024 gated changes, if someone in the future finds themselves trying to find those gated changes (even if it's the two of us) I think it's highly unlikely we will remember the less than/greater than/etc. mechanism that was used and still end up having to search for all possible combinations.

I think if search-ability and consistency are something we really care about here, then we will need a much more structured way of doing that. We can't just hope/rely on unenforced historical consistency at code authorship time

Copy link
Contributor

@ytmimi ytmimi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally agree with the points you're making. I don't have any suggestions for a more structured approach to this. Just wanted to highlight this while I still remembered. Feel free to merge this as is.

@calebcartwright calebcartwright merged commit 2d049af into rust-lang:master Oct 18, 2024
26 checks passed
@calebcartwright calebcartwright deleted the version-sort-mods branch October 18, 2024 18:54
@calebcartwright calebcartwright added release-notes Needs an associated changelog entry and removed pr-not-reviewed labels Oct 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-2024-edition Style Edition 2024 p-high release-notes Needs an associated changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants