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

Style evolution #3338

Merged
merged 13 commits into from
Dec 5, 2022
Merged

Style evolution #3338

merged 13 commits into from
Dec 5, 2022

Conversation

joshtriplett
Copy link
Member

@joshtriplett joshtriplett commented Oct 30, 2022

This RFC defines a mechanism for evolving the default Rust style over time
without breaking backwards compatibility, via the Rust edition mechanism.

The current Rust style, as defined in the Rust Style Guide and as implemented
by rustfmt, has some stability expectations associated with it. In particular,
many projects implement continuous integration hooks that verify the style of
Rust code (such as with cargo fmt --check), and changes to the style would
break the CI of such projects, in addition to causing churn.

This document proposes to evolve the current Rust style, without breaking
backwards compatibility, by tying style evolution to Rust edition. Code in Rust
2015, 2018, or 2021 will use the existing default style. Code in future
editions (Rust 2024 and onwards) may use a new style edition.

This RFC only defines the mechanism by which we evolve the Rust style; this RFC
does not define any specific style changes. Future RFCs or style-guide PRs
will define future style editions. This RFC does not propose or define any
specific future style editions or other formatting changes.

Rendered

@faptc

This comment was marked as resolved.

@ehuss ehuss added the T-style Relevant to the style team, which will review and decide on the RFC. label Oct 30, 2022
@ehuss
Copy link
Contributor

ehuss commented Oct 30, 2022

Can you provide a few examples of what might change across editions, and what can change without them (if any)? I would imagine many of the current options in rustfmt.toml would be fair game for an edition? But what about something like new syntax? For example, let-else is hitting stable this week without formatting (AFAIK). Does that mean we have to wait a few years for that to be fixed? What about bug fixes? (And there are different classes of bug fixes, such as it clearly not working as intended, or an oversight, or a change of opinion, etc.)

Is anything not explicitly specified in the style guide fair game to change at any time? If there is a mistake in the style guide, is it stuck until the next edition? Is it possible to add new content to the style guide without an edition, or is it frozen when the edition launches?

(Sorry for flood of questions. I think they are all asking the same thing in different ways.)

@joshtriplett
Copy link
Member Author

At least at the moment, we're talking about adding formatting for let-else and let chains without waiting for an edition.

I would expect bug fixes to be treated similarly to rustc: if it can be fixed without breaking anything we go ahead and do it, if it can't then we wait.

cc @calebcartwright

@joshtriplett
Copy link
Member Author

Can you provide a few examples of what might change across editions, and what can change without them (if any)?

Concretely, I would expect that the only thing we can do without an edition would be adding formatting to something currently unformatted, and even then we should try to start providing formatting for new constructs by the time they stabilize.

@afetisov
Copy link

My understanding is that part of the reason people want stable formatting is to avoid spurious (and sometimes quite disruptive, like when indentation changes) modification of their files, to simplify git blame and diffs.

A syntax change over an edition would negate those benefits, potentially leading to huge irrelevant changes in an edition-update commit. That's not desirable.

The surface syntax of editions can also potentially diverge significantly. In principle, each edition has the syntax of an entirely different language with the same low-level semantics (even though in practice the differences are kept minimal). How would rustfmt deal with the case where the code edition and the style edition differ in some important place, e.g. if the parsing of some ambiguous construct changes between editions?

@joshtriplett
Copy link
Member Author

@afetisov rustfmt already has to handle all of its configuration options in every edition; style editions would change the defaults for those options.

@joshtriplett
Copy link
Member Author

A syntax change over an edition would negate those benefits, potentially leading to huge irrelevant changes in an edition-update commit. That's not desirable.

I covered that in the RFC; that's what the formatting edition mechanism helps with.

Copy link
Member

@yaahc yaahc left a comment

Choose a reason for hiding this comment

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

overall looks great.

Once this is approved I think it might serve as a good opportunity to start working on a living policy document for the style team. The answers to the questions @ehuss asked also seem like good content to include in our policy docs.

Concretely, I would expect that the only thing we can do without an edition would be adding formatting to something currently unformatted, and even then we should try to start providing formatting for new constructs by the time they stabilize.

In the long run, I believe we should strive to always have the formatting ready before the constructs stabilize and adhere to stricter guidelines against changing formatting on stable constructs within the same edition, but in the short run, I'm hoping the churn will be minimal enough to be acceptable and understood given that we're still setting everything up.

text/3338-style-evolution.md Outdated Show resolved Hide resolved
@yaahc
Copy link
Member

yaahc commented Nov 1, 2022

Actually another thought I just had. I think it might be important to include a feedback plan for how we will evolve the style editions. I'm thinking something along the lines like have the next style edition be available on nightly as soon as the previous edition is stabilized and put out periodic calls for testing whenever we make significant changes or additions to the next style edition's formatting. We could also have tracking issues for the experimental formatting settings in the edition that we can use for a feedback loop rather than leaving fmt-rfcs open for long enough to try to gather all the feedback ahead of time. That way we can get feedback on the way the formatting is working out in practice and hopefully catch issues long before they become permanent.

@afetisov
Copy link

afetisov commented Nov 1, 2022

@joshtriplett that's what the formatting edition mechanism helps with.

Sorry, but I don't understand. How would the formatting edition help you if the syntax of two editions is different to the point of a major incompatibility (which is quite likely 20 years in the future)?

I also don't understand how it is supposed to solve the churn problem. It seems that the proposed RFC explicitly exists so that it's possible to make (potentially arbitrary) breaking changes to the formatting, gating it on a style edition. Are you suggesting that the project should update its core edition, but stay on an outdated syntax edition? In that case, either it will update some time in the future (which doesn't solve the churn, just delays it), or its stuck on an old edition indefinitely (which I expect is undesirable, and may hit the edition incompatibility mentioned above).

Correct me if I'm wrong: the proposed mechanism exists so that the projects may update their language edition and style editions in separate steps, but it's not a goal to support edition=2015 style on an edition=2045 Rust (and vice versa).

If that's just a proposal for a gradual upgrade mechanism, then I guess it's fine. But I still think there should be more details on the handling of wildly divergent lang&style editions (because someone will certainly do it).

@Nemo157
Copy link
Member

Nemo157 commented Nov 1, 2022

Correct me if I'm wrong: the proposed mechanism exists so that the projects may update their language edition and style editions in separate steps, but it's not a goal to support edition=2015 style on an edition=2045 Rust (and vice versa).

If that's just a proposal for a gradual upgrade mechanism, then I guess it's fine. But I still think there should be more details on the handling of wildly divergent lang&style editions (because someone will certainly do it).

That seems to be covered in the RFC:

Note that rustfmt may not necessarily support all combinations of Rust edition and style edition; in particular, it may not support using a style edition that differs by more than one step from the Rust edition.

Co-authored-by: Jane Losare-Lusby <jlusby42@gmail.com>
@joshtriplett
Copy link
Member Author

joshtriplett commented Nov 1, 2022

@yaahc wrote:

Actually another thought I just had. I think it might be important to include a feedback plan for how we will evolve the style editions. I'm thinking something along the lines like have the next style edition be available on nightly as soon as the previous edition is stabilized and put out periodic calls for testing whenever we make significant changes or additions to the next style edition's formatting. We could also have tracking issues for the experimental formatting settings in the edition that we can use for a feedback loop rather than leaving fmt-rfcs open for long enough to try to gather all the feedback ahead of time. That way we can get feedback on the way the formatting is working out in practice and hopefully catch issues long before they become permanent.

That's a really good idea! You're right, new style editions should start out nightly only, and get stabilized later. I've updated the RFC accordingly.

@joshtriplett
Copy link
Member Author

joshtriplett commented Nov 1, 2022

@afetisov wrote:

@joshtriplett

that's what the formatting edition mechanism helps with.

Sorry, but I don't understand. How would the formatting edition help you if the syntax of two editions is different to the point of a major incompatibility (which is quite likely 20 years in the future)?

See @Nemo157's comment about not necessarily permitting excessive skew between style edition and Rust edition. I wouldn't expect style edition 2015 to work with Rust edition 2060.

I also don't understand how it is supposed to solve the churn problem. It seems that the proposed RFC explicitly exists so that it's possible to make (potentially arbitrary) breaking changes to the formatting, gating it on a style edition.

The expectation is that people can separate changes to the formatting and other changes for the edition, so that they don't get intermixed in the same commit. You can cargo fmt --style-edition 2024 and commit the result, then migrate to the new Rust edition and commit the result.

Changing style editions will still result in a reformat. As mentioned in the style team blog post, "We don't plan to make any earth-shattering style changes; the look and feel of Rust will remain largely the same." As a design goal, I'm hoping that the diff for a move from style edition 2021 to style edition 2024 will primarily make people go "oh that's much better".

But I still think there should be more details on the handling of wildly divergent lang&style editions (because someone will certainly do it).

I would expect the handling to be an error message.

@calebcartwright
Copy link
Member

I completely understand the thinking behind some of these questions and comments, particularly because rustfmt is the surface through which users typically "experience" formatting. As such I'll try to address them all, but I do want to make a few meta points and somewhat preemptively caution against digressing the conversation away from the Style Guide.

First I want to underscore that the style guide and rustfmt are separate entities owned by separate teams. rustfmt's applicability in this context comes through a relatively narrow lens due to rustfmt being a core consumer of the style guide. rustfmt has various constraints around formatting stability, which were noted in RFC #3309 that re-chartered the Style Team and additional details can be found back in https://rust-lang.github.io/rfcs/2437-rustfmt-stability.html (it's dated, but still applicable with the gist being that rustfmt can't change the way it formats a construct that older versions of rustfmt were formatting differently). It's those rustfmt stability constraints that transitively impose some level of constraints on how the style guide can be evolved.

There's an established need to define a mechanism through which we can evolve the style guide, and to be able to do so in a manner that does not force rustfmt to violate any of its constraints. All we're doing here is proposing such a mechanism via what's tantamount to a versioning scheme for the style guide itself. We've opted to use what I'll call EditionVer over other versioning schemes like SemVer or CalVer for the sake of familiarity and encouraging/making it easier for the community to update over time. However, it really is just versioning for the style guide and is unrelated to the underlying edition utilized for parsing (that's why the proposal allows for different values for the two)

Will respond to some of the other items raised in separate comments, but the last thing I'll note in this one is that IMO the following items are off topic/unrelated to what we're trying to propose here:

  • characterizations of formatting changes (we've intentionally separated the evolution mechanism from any specific changes)
  • anything purely in the domain of rustfmt (e.g. rustfmt bug fix policy)
  • process changes around relationship between formatting and language feature/syntax stabilization (though this is an important topic the style team will be discussing in parallel)

@calebcartwright
Copy link
Member

calebcartwright commented Nov 3, 2022

Can you provide a few examples of what might change across editions, and what can change without them (if any)?

Technically an edition boundary could be subject to any formatting change, but in principle we don't expect there to be many changes nor that they will be frequent (for reference, as proposed the 2015-2021 "editions" of the style guide would be identical).

The types of things I'd anticipate changing across editions would be cases where the community wants a position on something that wasn't previously considered, e.g. whether empty match arms should be () or {} (refs rust-lang/style-team#147), and where adding such a position to the style guide would force rustfmt to modify a construct it was already formatting differently. Another would be cases where, with the benefit of hindsight, better formatting/style approaches have been realized.

The goal is to be able to make formatting improvements that are hopefully mostly non-controversial, but we will of course be cognizant of the impact any formatting changes will have.

The types of things that can be changed in the style guide without requiring an edition change are going to inherently be fairly minimal. Due to rustfmt constraints they will largely boil down to cases where rustfmt isn't applying formatting and/or the rustfmt output can at least partially driven off the input (e.g. changing text from always requiring the removal of braces, to allowing braces to be kept certain contexts if braces were already present in the input code - https://github.com/rust-dev-tools/fmt-rfcs/pull/159/files)

@calebcartwright
Copy link
Member

I would imagine many of the current options in rustfmt.toml would be fair game for an edition?

Yes, though there's two angles to this. Most (though not all) of the config options rustfmt supports are provided explicitly to allow users to do something different than what the style guide prescribes. Certain types of changes to the style guide would indeed necessitate that corresponding rustfmt options have their default value changed when using that style edition.

Additionally, rustfmt supports options that have no associated style guide prescriptions, simply to offer users more flexibility. This angle is more rustfmt-oriented and not strictly related to the style guide evolution mechanism, but yes it's also possible that we (speaking for rustfmt team) may change the defaults of such options as well as part of a style edition change

@calebcartwright
Copy link
Member

For example, let-else is hitting stable this week without formatting (AFAIK). Does that mean we have to wait a few years for that to be fixed?

No, at least not necessarily, but this does tie into a still unanswered question. The Style Guide currently provides no prescriptions for let-else statements, and as such rustfmt isn't applying any formatting when it encounters them (it "leaves them alone" by emitting the contents of the associated span). Even absent this proposal, if rules for let-else were added to the existing style guide then rustfmt would implement those rules and apply the very first formatting for that construct.

Obviously we want to get rules defined for new lang constructs more expeditiously, but that's another problem for another day.

How we handle representing rules for new syntax in older style editions is a detail the style team is still discussing and which I don't think we've resolved yet. I'm not sure whether that's something we feel we need to have conclusively resolved as part of this RFC, but I think the below phrase from the current RFC text is intended to give us some wiggle room to address the specifics later:

Note that archived versions of the style guide may not necessarily document formatting for newer Rust constructs that did not exist at the time that version of the style guide was archived.

@clarfonthey
Copy link
Contributor

@afetisov rustfmt already has to handle all of its configuration options in every edition; style editions would change the defaults for those options.

I feel like this is kind of at odds with the RFC text, and should be explained further. For example, the RFC mentions potentially not supporting all possible combinations of editions and style editions, but rules are always stable regardless of edition.

Are rules changed across editions, or must they always work if they're stable? For example, can we remove rules on certain editions (e.g. if they become incompatible with the style for that edition), can we gate certain options or rules on editions? Or must the rules simply always work regardless of what edition we're on, and the behaviour of rules must not change for any given setting to them?

To me, it seems fair to simply allow this in full, since rustfmt by its nature doesn't affect anything beyond the current crate, and it makes sense to allow the flexibility of changing things up if the style truly requires it. But this isn't mentioned at all in the RFC and is probably a good idea.

@nikomatsakis
Copy link
Contributor

Big fan of this change.

@joshtriplett
Copy link
Member Author

@calebcartwright wrote:

I feel like this is kind of at odds with the RFC text, and should be explained further. For example, the RFC mentions potentially not supporting all possible combinations of editions and style editions, but rules are always stable regardless of edition.

I was attempting to use that as an example of rustfmt supporting styles over multiple editions; I didn't intend it as prescriptive, since a style edition could modify something that isn't a rustfmt configuration option.

Are rules changed across editions, or must they always work if they're stable? For example, can we remove rules on certain editions (e.g. if they become incompatible with the style for that edition), can we gate certain options or rules on editions?

This is a great point. I added a note that rustfmt need not support all configuration options on new style editions, to provide leeway for obsoleting or deprecating such options if a new style edition makes them no longer relevant.

@the8472
Copy link
Member

the8472 commented Nov 10, 2022

My understanding is that part of the reason people want stable formatting is to avoid spurious (and sometimes quite disruptive, like when indentation changes) modification of their files, to simplify git blame and diffs.

Format-the-world and similar changes should be put into separate commits which can then be ignored by blame by having a file listing files to ignore. Add the commit hashes to .git-blame-ignore-revs and then git config --local blame.ignoreRevsFile .git-blame-ignore-revs

Github supports this too

@yaahc
Copy link
Member

yaahc commented Nov 16, 2022

@rfcbot merge

@rfcbot
Copy link
Collaborator

rfcbot commented Nov 16, 2022

Team member @yaahc has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. labels Nov 16, 2022
@joshtriplett
Copy link
Member Author

joshtriplett commented Nov 16, 2022

@the8472 Captured that in the RFC, thank you! I did have that in mind for reasons people would want to separate formatting changes into a separate commit, but hadn't thought to mention it in the RFC.

@rfcbot rfcbot added final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. and removed proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. labels Nov 17, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 17, 2022

🔔 This is now entering its final comment period, as per the review above. 🔔

@ytmimi
Copy link
Contributor

ytmimi commented Nov 21, 2022

Would --style-edition replace the version config?

@joshtriplett
Copy link
Member Author

joshtriplett commented Nov 22, 2022

@ytmimi That's up to the rustfmt team, not the style team or this RFC. (The style team determines the default style(s), not any other configuration offered by rustfmt.) The indications I've heard suggest that rustfmt does want to deprecate version = "Two" in favor of this and not stabilize it, and assess what style changes from version = "Two" that we'll want to incorporate in a style edition.

@ytmimi
Copy link
Contributor

ytmimi commented Nov 22, 2022

Got it, thanks for the clarification. I asked because I think it would be confusing to have both --style-edition and version as configuration options, and I'm not sure what benefit the version config would provide if we add --style-edition.

@rfcbot rfcbot added finished-final-comment-period The final comment period is finished for this RFC. and removed final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. labels Nov 27, 2022
@rfcbot
Copy link
Collaborator

rfcbot commented Nov 27, 2022

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

@joshtriplett joshtriplett merged commit 9653bae into rust-lang:master Dec 5, 2022
@joshtriplett joshtriplett deleted the style-evolution branch December 5, 2022 21:57
@joshtriplett
Copy link
Member Author

The RFC has been merged! The tracking issue for this RFC is rust-lang/rust#105336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
disposition-merge This RFC is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this RFC. T-style Relevant to the style team, which will review and decide on the RFC. to-announce
Projects
None yet
Development

Successfully merging this pull request may close these issues.