Tracking issue: implement versioning

[nrc]

As specified in rust-lang/rfcs#2437

cc #2614

[topecongiro]

So now that rustfmt 1.0 is availble on nightly and will be soon on stable, I think that we need to consider seriously about how we are going to implement versioning. The 'easiest' implementation would be to if guard the new formatting changes after 1.0:

// If we are to change this code,
format_something();

// we have to do something like this:
if rustmft_format_version() >= 1.1 {
    new_way_to_format_something();
} else {
    format_something();
}

However this should be the last resort as it will mess up the code base. Any idea on how to implement this cleanly?

Also what kind of formatting changes need to be blocked between different versions?

[nrc]

Yes, I have been thinking a bit about this and how to implement it. I was thinking we would add another option for the version and then there is a relatively simple rule that every formatting change has to be gated on an option (either a regular option or the version one). We only need to care about the major version number (i.e., 1 or 2, not 1.0 vs 1.1).

Ideally we would make version-dependent changes at a high level so as not to introduce too many checks. I think we should also be triaging bug fixes with the cost of the fix (in terms of clean code) in mind.

Also once we release 2.0, we can delete all 1.x code, so thinking long term, it is not so bad.

[topecongiro]

Hmm, did things changed from the rfc 2437? Your comment and the initial implementation seems to differ from the original proposal.

[topecongiro]

Ah, I see what you meant. So we only offer two options to users w.r.t. formatting, either using the latest stable at that point (1.0, 2.0, ...) or the absolute latest.

[scampi]

I overlooked this issue.

From what you said @topecongiro in your last comment, then probably a more appropriate change for https://github.com/rust-lang/rustfmt/pull/3250/files#diff-ab15584b47bdf72834e0d64a4cc5f17fR417 would have been something like:

                if context.config.version() == Version::One {
                    ""
                } else if semicolon_for_expr(context, body) {
                    ";"
                };

This is what you did say #3229 (comment) but I had not this conversation in mind...
Should I fix this in a new PR ? Sorry for the back and forth!

[topecongiro]

@scampi I agree that the expression is a bit obscure, but not so much that it requires refactoring. If you are willing to do an extra work here, then the PR is greatly appreciated 😃

[topecongiro]

Closing in favor of #3383.