-
Notifications
You must be signed in to change notification settings - Fork 928
Add StyleEdition
enum and StyleEditionDefault
trait
#5933
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
use crate::config::StyleEdition; | ||
|
||
/// Defines the default value for the given style edition | ||
pub(crate) trait StyleEditionDefault { | ||
type ConfigType; | ||
fn style_edition_default(style_edition: StyleEdition) -> Self::ConfigType; | ||
} | ||
|
||
/// macro to help implement `StyleEditionDefault` for config options | ||
#[macro_export] | ||
macro_rules! style_edition_default { | ||
($ty:ident, $config_ty:ty, _ => $default:expr) => { | ||
impl $crate::config::style_edition::StyleEditionDefault for $ty { | ||
type ConfigType = $config_ty; | ||
|
||
fn style_edition_default(_: $crate::config::StyleEdition) -> Self::ConfigType { | ||
$default | ||
} | ||
} | ||
}; | ||
($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, | ||
Comment on lines
+21
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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,
} |
||
$crate::config::StyleEdition::Edition2024 => $default_2024, | ||
} | ||
} | ||
} | ||
}; | ||
} | ||
|
||
#[cfg(test)] | ||
mod test { | ||
use super::*; | ||
use crate::config::StyleEdition; | ||
|
||
#[test] | ||
fn test_impl_default_style_edition_struct_for_all_editions() { | ||
struct Unit; | ||
style_edition_default!(Unit, usize, _ => 100); | ||
|
||
// regardless of the style edition used the value will always return 100 | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2015), 100); | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2018), 100); | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2021), 100); | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2024), 100); | ||
} | ||
|
||
#[test] | ||
fn test_impl_default_style_edition_for_old_and_new_editions() { | ||
struct Unit; | ||
style_edition_default!(Unit, usize, Edition2024 => 50, _ => 100); | ||
|
||
// style edition 2015-2021 are all the same | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2015), 100); | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2018), 100); | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2021), 100); | ||
|
||
// style edition 2024 | ||
assert_eq!(Unit::style_edition_default(StyleEdition::Edition2024), 50); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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