Clamp width settings to max_width when entering macro scope

rs-sac · rs-sac · commit 83b707d37a2f · 2024-10-14T14:05:13.000-07:00
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.
diff --git 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,26 +445,27 @@ 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,
                     override_value: usize,
                     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
@@ -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 <boolean> Default: false";
     const PRINT_DOCS_UNSTABLE_OPTION: &str = "unstable_option <boolean> 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
@@ -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) {
