From 83b707d37a2f6b463d7e06907867547023dc58cd Mon Sep 17 00:00:00 2001 From: Sidney Cammeresi Date: Mon, 7 Oct 2024 08:50:32 -0700 Subject: [PATCH] Clamp width settings to max_width when entering macro scope Within a macro scope, max_width is reduced, which can trigger warnings if it is reduced below some other width setting (e.g. struct_lit_width.) Width settings were already being clamped to max_width, but only after the warning fired. A macro scope now clamps the setting to max_width before the warning. --- src/config/config_type.rs | 68 ++++++++++++++++++++++++++------------- src/config/mod.rs | 23 ++++++++++--- src/macros.rs | 2 +- 3 files changed, 66 insertions(+), 27 deletions(-) diff --git a/src/config/config_type.rs b/src/config/config_type.rs index afac8d3fcf9..bb4412e70de 100644 --- a/src/config/config_type.rs +++ b/src/config/config_type.rs @@ -65,6 +65,13 @@ impl ConfigType for IgnoreList { } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[allow(unreachable_pub)] +pub enum Clamp { + Yes, + No, +} + macro_rules! create_config { // Options passed into the macro. // @@ -79,6 +86,7 @@ macro_rules! create_config { use serde::{Deserialize, Serialize}; use $crate::config::style_edition::StyleEditionDefault; + use $crate::config::config_type::Clamp; #[derive(Clone)] #[allow(unreachable_pub)] @@ -112,13 +120,16 @@ macro_rules! create_config { // with `config.set_option(false)` if we ever get a stable/usable // `concat_idents!()`. #[allow(unreachable_pub)] - pub struct ConfigSetter<'a>(&'a mut Config); + pub struct ConfigSetter<'a> { + config: &'a mut Config, + clamp: Clamp, + } impl<'a> ConfigSetter<'a> { $( #[allow(unreachable_pub)] pub fn $i(&mut self, value: <$ty as StyleEditionDefault>::ConfigType) { - (self.0).$i.2 = value; + self.config.$i.2 = value; match stringify!($i) { "max_width" | "use_small_heuristics" @@ -129,11 +140,11 @@ macro_rules! create_config { | "struct_lit_width" | "struct_variant_width" | "array_width" - | "chain_width" => self.0.set_heuristics(), - "merge_imports" => self.0.set_merge_imports(), - "fn_args_layout" => self.0.set_fn_args_layout(), - "hide_parse_errors" => self.0.set_hide_parse_errors(), - "version" => self.0.set_version(), + | "chain_width" => self.config.set_heuristics(self.clamp), + "merge_imports" => self.config.set_merge_imports(), + "fn_args_layout" => self.config.set_fn_args_layout(), + "hide_parse_errors" => self.config.set_hide_parse_errors(), + "version" => self.config.set_version(), &_ => (), } } @@ -159,7 +170,7 @@ macro_rules! create_config { | "struct_lit_width" | "struct_variant_width" | "array_width" - | "chain_width" => self.0.set_heuristics(), + | "chain_width" => self.0.set_heuristics(Clamp::No), "merge_imports" => self.0.set_merge_imports(), "fn_args_layout" => self.0.set_fn_args_layout(), "hide_parse_errors" => self.0.set_hide_parse_errors(), @@ -226,7 +237,18 @@ macro_rules! create_config { #[allow(unreachable_pub)] pub fn set(&mut self) -> ConfigSetter<'_> { - ConfigSetter(self) + ConfigSetter { + config: self, + clamp: Clamp::No, + } + } + + #[allow(unreachable_pub)] + pub fn set_clamp(&mut self) -> ConfigSetter<'_> { + ConfigSetter { + config: self, + clamp: Clamp::Yes, + } } #[allow(unreachable_pub)] @@ -256,7 +278,7 @@ macro_rules! create_config { } } )+ - self.set_heuristics(); + self.set_heuristics(Clamp::No); self.set_ignore(dir); self.set_merge_imports(); self.set_fn_args_layout(); @@ -359,7 +381,7 @@ macro_rules! create_config { | "struct_lit_width" | "struct_variant_width" | "array_width" - | "chain_width" => self.set_heuristics(), + | "chain_width" => self.set_heuristics(Clamp::No), "merge_imports" => self.set_merge_imports(), "fn_args_layout" => self.set_fn_args_layout(), "hide_parse_errors" => self.set_hide_parse_errors(), @@ -423,7 +445,7 @@ macro_rules! create_config { )+ } - fn set_width_heuristics(&mut self, heuristics: WidthHeuristics) { + fn set_width_heuristics(&mut self, heuristics: WidthHeuristics, clamp: Clamp) { let max_width = self.max_width.2; let get_width_value = | was_set: bool, @@ -431,18 +453,19 @@ macro_rules! create_config { heuristic_value: usize, config_key: &str, | -> usize { - if !was_set { - return heuristic_value; - } - if override_value > max_width { + let value = if !was_set { + heuristic_value + } else { + override_value + }; + if clamp == Clamp::No && value > max_width { eprintln!( "`{0}` cannot have a value that exceeds `max_width`. \ `{0}` will be set to the same value as `max_width`", config_key, ); - return max_width; } - override_value + value.min(max_width) }; let fn_call_width = get_width_value( @@ -510,13 +533,14 @@ macro_rules! create_config { self.single_line_let_else_max_width.2 = single_line_let_else_max_width; } - fn set_heuristics(&mut self) { + fn set_heuristics(&mut self, clamp: Clamp) { let max_width = self.max_width.2; match self.use_small_heuristics.2 { Heuristics::Default => - self.set_width_heuristics(WidthHeuristics::scaled(max_width)), - Heuristics::Max => self.set_width_heuristics(WidthHeuristics::set(max_width)), - Heuristics::Off => self.set_width_heuristics(WidthHeuristics::null()), + self.set_width_heuristics(WidthHeuristics::scaled(max_width), clamp), + Heuristics::Max => + self.set_width_heuristics(WidthHeuristics::set(max_width), clamp), + Heuristics::Off => self.set_width_heuristics(WidthHeuristics::null(), clamp), }; } diff --git a/src/config/mod.rs b/src/config/mod.rs index ea257c0a8ba..d1d20aadaf0 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -680,6 +680,21 @@ mod test { assert_eq!(config.was_set().verbose(), false); } + #[test] + fn test_clamp_to_max_width() { + let toml = r#" + max_width = 100 + struct_lit_width = 200 + "#; + let mut config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); + assert_eq!(config.struct_lit_width(), config.max_width()); + + // simulate entering a macro scope + let new = config.max_width() - 4; + config.set_clamp().max_width(new); + assert_eq!(config.struct_lit_width(), config.max_width()); + } + const PRINT_DOCS_STABLE_OPTION: &str = "stable_option Default: false"; const PRINT_DOCS_UNSTABLE_OPTION: &str = "unstable_option Default: false (unstable)"; const PRINT_DOCS_PARTIALLY_UNSTABLE_OPTION: &str = @@ -1049,10 +1064,10 @@ make_backup = false max_width = 100 "#; let config = Config::from_toml(toml, Path::new("./rustfmt.toml")).unwrap(); - assert_eq!(config.array_width(), usize::MAX); - assert_eq!(config.attr_fn_like_width(), usize::MAX); - assert_eq!(config.chain_width(), usize::MAX); - assert_eq!(config.fn_call_width(), usize::MAX); + assert_eq!(config.array_width(), 100); + assert_eq!(config.attr_fn_like_width(), 100); + assert_eq!(config.chain_width(), 100); + assert_eq!(config.fn_call_width(), 100); assert_eq!(config.single_line_if_else_max_width(), 0); assert_eq!(config.struct_lit_width(), 0); assert_eq!(config.struct_variant_width(), 0); diff --git a/src/macros.rs b/src/macros.rs index 5a35e115d8f..b5205948d2f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1329,7 +1329,7 @@ impl MacroBranch { shape.indent.block_indent(&config) }; let new_width = config.max_width() - body_indent.width(); - config.set().max_width(new_width); + config.set_clamp().max_width(new_width); // First try to format as items, then as statements. let new_body_snippet = match crate::format_snippet(&body_str, &config, true) {