From f844afa90f066a7de1d2e1da1bcae74dbd45d11a Mon Sep 17 00:00:00 2001 From: denk Date: Tue, 3 Nov 2026 18:52:46 +0300 Subject: [PATCH] Fix assignment width check for multiline variable lists The check deciding whether an assignment fits the column width measured the variable list by its total length. When the list itself spans multiple lines, the total length is far larger than what actually sits on the line with the equals token, so the assigned function's parameters were expanded unnecessarily. The same applied to Luau type specifiers and Lua 5.4 attributes, which are also appended to the variable list before the equals token. Measure the last line of the variable list instead, which is what the equals token and the expression are appended to: let singleline_shape = shape.take_last_line(&strip_leading_trivia(&var_list)) + 3 // 3 = " = " + strip_trailing_trivia(&expr_list).to_string().len(); This is the same approach the TableConstructor branch of function_args_multiline_heuristic uses for the equivalent width check. Adds regression tests for each affected path, plus a guard test asserting that the parameters are still expanded when they genuinely do not fit. --- CHANGELOG.md | 4 + src/formatters/assignment.rs | 90 +++++++++------- tests/tests.rs | 201 +++++++++++++++++++++++++++++++++++ 3 files changed, 258 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ac6dc18..98ca9f1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- Fixed the parameters of an assigned function being expanded unnecessarily when the variable list, type specifier or attribute spans multiple lines + ## [2.6.0] - 2026-10-10 ### Added diff --git a/src/formatters/assignment.rs b/src/formatters/assignment.rs index 3983e7c4..5c55e5a5 100644 --- a/src/formatters/assignment.rs +++ b/src/formatters/assignment.rs @@ -367,15 +367,15 @@ pub fn format_assignment_no_trivia( equal_token = prepend_newline_indent(ctx, &equal_token, shape); } - // Test the assignment to see if its over width - let singleline_shape = shape - + (strip_leading_trivia(&var_list).to_string().len() - + 3 - + strip_trailing_trivia(&expr_list).to_string().len()); + // Test the assignment to see if its over width. The variable list may span multiple lines + // (e.g. a multiline table constructor), so only its last line is followed by the equals token. + let singleline_shape = shape.take_last_line(&strip_leading_trivia(&var_list)) + + 3 // 3 = " = " + + strip_trailing_trivia(&expr_list).to_string().len(); if contains_comments || singleline_shape.over_budget() { // We won't attempt anything else with the var_list. Format it normally var_list = try_format_punctuated(ctx, assignment.variables(), shape, format_var, Some(1)); - let shape = shape + (strip_leading_trivia(&var_list).to_string().len() + 3); + let shape = shape.take_last_line(&strip_leading_trivia(&var_list)) + 3; // 3 = " = " let (new_expr_list, new_equal_token) = attempt_assignment_tactics(ctx, assignment.expressions(), shape, equal_token); @@ -489,20 +489,25 @@ pub fn format_local_assignment_no_trivia( .map(|x| x.map(|type_specifier| format_type_specifier(ctx, type_specifier, shape))) .collect(); + // Attributes and type specifiers follow the name list, before the equals token. Only their + // last line shares a line with the equals token, so collect their text and measure the + // last line of the whole variable list rather than summing up lengths. #[allow(unused_mut)] - let mut type_specifier_len = 0; + let mut var_list_suffix = String::new(); #[cfg(feature = "lua54")] - { - type_specifier_len += attributes.iter().fold(0, |acc, x| { - acc + x.as_ref().map_or(0, |y| y.to_string().len()) - }); - } + var_list_suffix.extend( + attributes + .iter() + .flatten() + .map(|attribute| attribute.to_string()), + ); #[cfg(feature = "luau")] - { - type_specifier_len += type_specifiers.iter().fold(0, |acc, x| { - acc + x.as_ref().map_or(0, |y| y.to_string().len()) - }); - } + var_list_suffix.extend( + type_specifiers + .iter() + .flatten() + .map(|specifier| specifier.to_string()), + ); #[cfg(feature = "luau")] let var_list_ends_with_comments = match type_specifiers.last() { @@ -525,13 +530,14 @@ pub fn format_local_assignment_no_trivia( equal_token = prepend_newline_indent(ctx, &equal_token, shape); } - // Test the assignment to see if its over width - let singleline_shape = shape - + (strip_leading_trivia(&name_list).to_string().len() - + 6 // 6 = "local " - + 3 // 3 = " = " - + type_specifier_len - + strip_trailing_trivia(&expr_list).to_string().len()); + // Test the assignment to see if its over width. The name list, attributes and type + // specifiers may all span multiple lines, so only their last line is followed by the + // equals token. + let var_list = format!("{}{}", strip_leading_trivia(&name_list), var_list_suffix); + let singleline_shape = (shape + 6) // 6 = "local " + .take_last_line(&var_list) + + 3 // 3 = " = " + + strip_trailing_trivia(&expr_list).to_string().len(); if contains_comments || singleline_shape.over_budget() { // We won't attempt anything else with the name_list. Format it normally @@ -542,8 +548,9 @@ pub fn format_local_assignment_no_trivia( format_token_reference, Some(1), ); - let shape = shape - + (strip_leading_trivia(&name_list).to_string().len() + 6 + 3 + type_specifier_len); + let shape = (shape + 6) // 6 = "local " + .take_last_line(&var_list) + + 3; // 3 = " = " let (new_expr_list, new_equal_token) = attempt_assignment_tactics(ctx, assignment.expressions(), shape, equal_token); @@ -621,9 +628,17 @@ pub fn format_const_assignment_no_trivia( .map(|x| x.map(|type_specifier| format_type_specifier(ctx, type_specifier, shape))) .collect(); - let type_specifier_len = type_specifiers.iter().fold(0, |acc, x| { - acc + x.as_ref().map_or(0, |y| y.to_string().len()) - }); + // Type specifiers follow the name list, before the equals token. Only their last line shares a + // line with the equals token, so append their text to the name list and measure the last line. + let var_list = format!( + "{}{}", + strip_leading_trivia(&name_list), + type_specifiers + .iter() + .flatten() + .map(|specifier| specifier.to_string()) + .collect::() + ); let var_list_ends_with_comments = match type_specifiers.last() { Some(Some(specifier)) => { @@ -641,12 +656,12 @@ pub fn format_const_assignment_no_trivia( equal_token = prepend_newline_indent(ctx, &equal_token, shape); } - let singleline_shape = shape - + (strip_leading_trivia(&name_list).to_string().len() - + 6 // 6 = "const " - + 3 // 3 = " = " - + type_specifier_len - + strip_trailing_trivia(&expr_list).to_string().len()); + // The name list and type specifiers may span multiple lines (e.g. due to comments), so only + // their last line is followed by the equals token. + let singleline_shape = (shape + 6) // 6 = "const " + .take_last_line(&var_list) + + 3 // 3 = " = " + + strip_trailing_trivia(&expr_list).to_string().len(); if contains_comments || singleline_shape.over_budget() { name_list = try_format_punctuated( @@ -656,8 +671,9 @@ pub fn format_const_assignment_no_trivia( format_token_reference, Some(1), ); - let shape = shape - + (strip_leading_trivia(&name_list).to_string().len() + 6 + 3 + type_specifier_len); + let shape = (shape + 6) // 6 = "const " + .take_last_line(&var_list) + + 3; // 3 = " = " let (new_expr_list, new_equal_token) = attempt_assignment_tactics(ctx, assignment.expressions(), shape, equal_token); diff --git a/tests/tests.rs b/tests/tests.rs index 908a390f..2962a18b 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -263,6 +263,207 @@ end) ); } +#[test] +fn test_assignment_var_list_spanning_multiple_lines() { + // The variable list spans multiple lines (a multiline table constructor passed to a function + // call), so only its last line (`}).g`) shares a line with the equals token. The width check + // must measure that line, not the total length of the variable list: otherwise the function + // parameters are incorrectly expanded onto multiple lines once the table grows. + insta::assert_snapshot!( + format_code( + r#"f("s", { + { a = 1 }, + { b = 2 }, + { c = 3 }, +}).g = function(h) + return h +end +"#, + Config { + column_width: 40, + ..Config::default() + }, + None, + OutputVerification::None + ) + .unwrap(), + @r###" + f("s", { + { a = 1 }, + { b = 2 }, + { c = 3 }, + }).g = function(h) + return h + end + "### + ); +} + +#[test] +fn test_assignment_var_list_spanning_multiple_lines_still_expands_when_over_budget() { + // Same shape as above, but the parameters genuinely do not fit after the last line of the + // variable list, so they must still be expanded. This guards the fix from being too permissive. + insta::assert_snapshot!( + format_code( + r#"f("s", { + { a = 1 }, +}).g = function(first_argument, second_argument) + return first_argument +end +"#, + Config { + column_width: 40, + ..Config::default() + }, + None, + OutputVerification::None + ) + .unwrap(), + @r###" + f("s", { + { a = 1 }, + }).g = function( + first_argument, + second_argument + ) + return first_argument + end + "### + ); +} + +#[test] +fn test_local_assignment_name_list_spanning_multiple_lines() { + // The name list spans multiple lines because of the comment, so only its last line + // (`variable_06, variable_07`) shares a line with the equals token. Measuring the total + // length of the name list instead would expand the function parameters unnecessarily. + insta::assert_snapshot!( + format_code( + r#"local variable_01, variable_02, variable_03, variable_04, variable_05, -- note + variable_06, variable_07 = function(h) + return h +end +"#, + Config::default(), + None, + OutputVerification::None + ) + .unwrap(), + @r###" + local variable_01, + variable_02, + variable_03, + variable_04, + variable_05, -- note + variable_06, + variable_07 = function(h) + return h + end + "### + ); +} + +#[test] +#[cfg(feature = "luau")] +fn test_const_assignment_name_list_spanning_multiple_lines() { + // Same as above, for Luau `const` declarations. + insta::assert_snapshot!( + format_code( + r#"const a, -- note + b = function(h) + return h +end, 2 +"#, + Config { + syntax: LuaVersion::Luau, + column_width: 40, + ..Config::default() + }, + None, + OutputVerification::None + ) + .unwrap(), + @r###" + const a, -- note + b = function(h) + return h + end, 2 + "### + ); +} + +#[test] +#[cfg(feature = "luau")] +fn test_local_assignment_multiline_type_specifier() { + // The type specifier spans multiple lines, so only its last line (`}`) shares a line with the + // equals token. Its total length must not be counted, or the function parameters get expanded + // unnecessarily. + insta::assert_snapshot!( + format_code( + r#"local x: { + a: string, + b: string, + c: string, +} = function(h) + return h +end +"#, + Config { + syntax: LuaVersion::Luau, + column_width: 40, + ..Config::default() + }, + None, + OutputVerification::None + ) + .unwrap(), + @r###" + local x: { + a: string, + b: string, + c: string, + } = function(h) + return h + end + "### + ); +} + +#[test] +#[cfg(feature = "luau")] +fn test_const_assignment_multiline_type_specifier() { + // Same as above, for Luau `const` declarations. + insta::assert_snapshot!( + format_code( + r#"const x: { + a: string, + b: string, + c: string, +} = function(h) + return h +end +"#, + Config { + syntax: LuaVersion::Luau, + column_width: 40, + ..Config::default() + }, + None, + OutputVerification::None + ) + .unwrap(), + @r###" + const x: { + a: string, + b: string, + c: string, + } = function(h) + return h + end + "### + ); +} + #[test] fn test_preserve_block_newline_gaps() { insta::glob!("inputs-preserve-block-newline-gaps/*.lua", |path| {