Add `StyleEdition` enum and `StyleEditionDefault` trait #5933

ytmimi · 2023-10-06T21:17:34Z

Note This does not add the style_edition config option to rustfmt. The StyleEdition enum will eventually be used to allow users to configure style_edition, but for now it's added so we can introduce the the StyleEditionDefault trait.

This PR is an alternative to #5854 that doesn't use proc macros.

r? @calebcartwright

calebcartwright · 2023-10-07T20:52:06Z

Thanks @ytmimi!

I am still planning to try to review this over the weekend. However, also wanted to ensure you'd seen this @fee1-dead given the thoughts you shared on #5854 (though I know you do not have much review capacity currently either)

fee1-dead · 2023-10-17T06:55:36Z

src/config/options.rs

+/// [the style guide]: https://doc.rust-lang.org/nightly/style-guide/
+#[config_type]
+pub enum StyleEdition {
+    #[value = "2015"]


Can this config value be a number? would be nice if it did, but it would probably be inconsistent with other tools. (IIRC I saw a discussion around whether cargo can also have edition option as a number)

I don't see why not, but if I'm missing something please let me know. If you happen to find the link for that discussion in cargo I'll be sure to give it a read. I also want to note that we already allow edition to be a number.

rustfmt/src/config/options.rs

Lines 417 to 436 in 041f113

/// The edition of the syntax and semntics of code (RFC 2052).

#[config_type]

pub enum Edition {

#[value = "2015"]

#[doc_hint = "2015"]

/// Edition 2015.

Edition2015,

#[value = "2018"]

#[doc_hint = "2018"]

/// Edition 2018.

Edition2018,

#[value = "2021"]

#[doc_hint = "2021"]

/// Edition 2021.

Edition2021,

#[value = "2024"]

#[doc_hint = "2024"]

/// Edition 2024.

Edition2024,

}

fee1-dead · 2023-10-17T06:57:18Z

src/config/style_edition.rs

+            }
+        }
+    };
+    ($ty:ident, $config_ty:ty, $default_2015:expr, $default_2024:expr) => {


This seems a bit arbitrary and hard to understand just given the macro call and parameters without reading the macro source. Can we change it so that it is more like match arms? E.g. something like

style_edition_default!(Foo, usize, Edition2024 => 1000, _ => 100)

I think this is a good suggestion! I'll update the macro so that it's easier to understand what's going on by looking at the call.

fee1-dead · 2023-10-17T07:00:18Z

src/config/style_edition.rs

+#[macro_export]
+macro_rules! style_edition_default {
+    ($ty:ident, $config_ty:ty, $default:expr) => {
+        impl crate::config::style_edition::StyleEditionDefault for $ty {


This is very minor but in case we want to have multiple crates changing all the crate::* to $crate:* would save some time when moving it later.

Suggested change

impl crate::config::style_edition::StyleEditionDefault for $ty {

impl $crate::config::style_edition::StyleEditionDefault for $ty {

fee1-dead · 2023-10-17T07:03:08Z

Also please fix the typo when you next push ("triat" -> "trait")

ytmimi · 2023-10-28T20:22:34Z

@fee1-dead thanks again for your initial review. I incorporated your suggestions, and fixed the typo that you caught! Please let me know if there's anything else that you'd like me to change and I'll get on that as soon as I can!

fee1-dead

LGTM

calebcartwright · 2023-11-12T23:08:58Z

src/config/style_edition.rs

+    ($ty:ident, $config_ty:ty, Edition2024 => $default_2024:expr, _ => $default_2015:expr) => {
+        impl $crate::config::style_edition::StyleEditionDefault for $ty {
+            type ConfigType = $config_ty;
+
+            fn style_edition_default(
+                style_edition: $crate::config::StyleEdition,
+            ) -> Self::ConfigType {
+                match style_edition {
+                    $crate::config::StyleEdition::Edition2015
+                    | $crate::config::StyleEdition::Edition2018
+                    | $crate::config::StyleEdition::Edition2021 => $default_2015,


I wonder if we should make the first three editions roll up to 2021 as the default 🤔 ?

The style edition RFC and subsequent style team policies have established that in cases where an older edition supports syntax that did not exist/did not have rules in the corresponding version of the style guide/edition then the rules from the oldest style edition that does prescribe rules should be used.

Functionally I do not believe it makes a difference here one way or the other, it's almost a code readability thing

I wonder if we should make the first three editions roll up to 2021 as the default 🤔 ?

Are you suggesting we roll up 2015, 2018, and 2021 enum variants like this:

enum StyleEdition { /// There is no difference between the style in the 2015, 2018, and 2021 editions /// so they're rolled up to 2021 Edition2021, Edition2024, }

**Note** This does not add the `style_edition` config option to rustfmt. The `StyleEdition` enum will eventually be used to allow users to configure the `style_edition`, but for now it's added so we can introduce the the `StyleEditionDefault` trait.

ytmimi · 2023-12-09T22:20:22Z

@calebcartwright thanks for getting this merged!

rustbot added the pr-not-reviewed label Oct 6, 2023

ytmimi force-pushed the alternative_style_edition_default_trait branch from 3d8a56a to 84d2d6b Compare October 6, 2023 21:18

ytmimi mentioned this pull request Oct 8, 2023

Impl StyleEditionDefault trait for all configs #5937

Merged

ytmimi mentioned this pull request Oct 16, 2023

Add StyleEditionDefault trait, StyleEdition enum, and #[style_edition] proc macro #5854

Closed

fee1-dead reviewed Oct 17, 2023

View reviewed changes

ytmimi changed the title ~~Add StyleEdition enum and StyleEditionDefault triat~~ Add StyleEdition enum and StyleEditionDefault trait Oct 28, 2023

ytmimi force-pushed the alternative_style_edition_default_trait branch from 84d2d6b to 10ce986 Compare October 28, 2023 20:19

ytmimi requested a review from fee1-dead October 28, 2023 20:21

fee1-dead approved these changes Oct 29, 2023

View reviewed changes

calebcartwright reviewed Nov 12, 2023

View reviewed changes

ytmimi force-pushed the alternative_style_edition_default_trait branch from 10ce986 to 2492f70 Compare December 5, 2023 00:18

ytmimi force-pushed the alternative_style_edition_default_trait branch from 2492f70 to 02479b9 Compare December 5, 2023 00:20

calebcartwright removed the pr-not-reviewed label Dec 9, 2023

calebcartwright merged commit 2174e60 into rust-lang:master Dec 9, 2023
27 checks passed

ytmimi deleted the alternative_style_edition_default_trait branch December 9, 2023 22:20

ytmimi mentioned this pull request Apr 11, 2024

Tracking Issue for Rust 2024: rustfmt style editions rust-lang/rust#123799

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `StyleEdition` enum and `StyleEditionDefault` trait #5933

Add `StyleEdition` enum and `StyleEditionDefault` trait #5933

ytmimi commented Oct 6, 2023 •

edited

Loading

calebcartwright commented Oct 7, 2023

fee1-dead Oct 17, 2023

ytmimi Oct 28, 2023

fee1-dead Oct 17, 2023

ytmimi Oct 28, 2023

fee1-dead Oct 17, 2023

fee1-dead commented Oct 17, 2023

ytmimi commented Oct 28, 2023

fee1-dead left a comment

calebcartwright Nov 12, 2023

ytmimi Nov 15, 2023

ytmimi commented Dec 9, 2023

	/// The edition of the syntax and semntics of code (RFC 2052).
	#[config_type]
	pub enum Edition {
	#[value = "2015"]
	#[doc_hint = "2015"]
	/// Edition 2015.
	Edition2015,
	#[value = "2018"]
	#[doc_hint = "2018"]
	/// Edition 2018.
	Edition2018,
	#[value = "2021"]
	#[doc_hint = "2021"]
	/// Edition 2021.
	Edition2021,
	#[value = "2024"]
	#[doc_hint = "2024"]
	/// Edition 2024.
	Edition2024,
	}

	impl crate::config::style_edition::StyleEditionDefault for $ty {
	impl $crate::config::style_edition::StyleEditionDefault for $ty {

Add StyleEdition enum and StyleEditionDefault trait #5933

Add StyleEdition enum and StyleEditionDefault trait #5933

Conversation

ytmimi commented Oct 6, 2023 • edited Loading

calebcartwright commented Oct 7, 2023

fee1-dead Oct 17, 2023

Choose a reason for hiding this comment

ytmimi Oct 28, 2023

Choose a reason for hiding this comment

fee1-dead Oct 17, 2023

Choose a reason for hiding this comment

ytmimi Oct 28, 2023

Choose a reason for hiding this comment

fee1-dead Oct 17, 2023

Choose a reason for hiding this comment

fee1-dead commented Oct 17, 2023

ytmimi commented Oct 28, 2023

fee1-dead left a comment

Choose a reason for hiding this comment

calebcartwright Nov 12, 2023

Choose a reason for hiding this comment

ytmimi Nov 15, 2023

Choose a reason for hiding this comment

ytmimi commented Dec 9, 2023

Add `StyleEdition` enum and `StyleEditionDefault` trait #5933

Add `StyleEdition` enum and `StyleEditionDefault` trait #5933

ytmimi commented Oct 6, 2023 •

edited

Loading