fix(formatter): estimate call-arg and attached-table line widths from stable columns

crates/emmylua_formatter/src/formatter/expr.rs

@@ -23,8 +23,9 @@ use super::sequence::{
 };
 use super::spacing::{SpaceRule, space_around_binary_op};
 use super::trivia::{
-    count_blank_lines_before, has_non_trivia_before_on_same_line_tokenwise,
-    node_has_direct_comment_child, source_line_prefix_width, trailing_gap_requests_alignment,
+    attached_source_prefix_width, count_blank_lines_before,
+    has_non_trivia_before_on_same_line_tokenwise, node_has_direct_comment_child,
+    source_line_prefix_width, trailing_gap_requests_alignment,
 };
 
 #[derive(Clone, Copy, Default)]
@@ -1509,7 +1510,7 @@ fn format_table_expr(ctx: &FormatContext, plan: &FormatPlan, expr: &LuaTableExpr
                 let mut flat_layout = layout;
                 flat_layout.strategy = ExpandStrategy::Never;
                 let flat_docs = format_delimited_sequence(ctx, flat_layout);
-                if ir::ir_flat_width(&flat_docs) + source_line_prefix_width(expr.syntax())
+                if ir::ir_flat_width(&flat_docs) + attached_source_prefix_width(expr.syntax())
                     <= ctx.config.layout.max_line_width
                 {
                     flat_docs
@@ -1568,7 +1569,7 @@ fn format_table_expr(ctx: &FormatContext, plan: &FormatPlan, expr: &LuaTableExpr
         let mut flat_layout = layout.clone();
         flat_layout.strategy = ExpandStrategy::Never;
         let flat_docs = format_delimited_sequence(ctx, flat_layout);
-        if ir::ir_flat_width(&flat_docs) + source_line_prefix_width(expr.syntax())
+        if ir::ir_flat_width(&flat_docs) + attached_source_prefix_width(expr.syntax())
             <= ctx.config.layout.max_line_width
         {
             return flat_docs;

crates/emmylua_formatter/src/formatter/layout/mod.rs

@@ -320,10 +320,10 @@ fn analyze_call_arg_list_layout(ctx: &FormatContext, args: &LuaCallArgList, plan
         is_multiline_block_like_expr(arg)
             || matches!(arg, LuaExpr::TableExpr(table) if table.syntax().text().contains_char('\n'))
     });
-    let first_line_prefix_width = arg_exprs
-        .first()
-        .map(|arg| source_line_prefix_width(arg.syntax()))
-        .unwrap_or(0);
+    // The candidates are measured from the opening `(`, so the prefix is that token's column. The first
+    // argument's column would depend on this list's own previous layout (on the `(` line or below it),
+    // and the next pass could then choose differently (#1302).
+    let first_line_prefix_width = source_line_prefix_width(args.syntax());
     let first_arg_multiline_block = arg_exprs.first().is_some_and(is_multiline_block_like_expr);
     let first_arg_multiline_table = matches!(arg_exprs.first(), Some(LuaExpr::TableExpr(table)) if table.syntax().text().contains_char('\n'));
     let mut single_inline_block_arg_index = None;

crates/emmylua_formatter/src/formatter/trivia.rs

@@ -1,4 +1,4 @@
-use emmylua_parser::{LuaKind, LuaSyntaxKind, LuaSyntaxNode, LuaTokenKind};
+use emmylua_parser::{LuaKind, LuaSyntaxKind, LuaSyntaxNode, LuaSyntaxToken, LuaTokenKind};
 use rowan::TextRange;
 
 /// Count how many blank lines appear before a node.
@@ -55,10 +55,32 @@ pub fn has_non_trivia_before_on_same_line_tokenwise(node: &LuaSyntaxNode) -> boo
 }
 
 pub fn source_line_prefix_width(node: &LuaSyntaxNode) -> usize {
+    node.first_token()
+        .map_or(0, |token| source_token_prefix_width(&token))
+}
+
+/// Where `node` (a table's `{`) will start when it follows `=` or `return`, which the formatter always attaches
+/// it to: that token's source column, its width and a space. The node's own source column would depend on
+/// whether it sat on that token's line, and the next pass could then decide differently (#1302). After any
+/// other token (`,`, `(`) the node may go on a line of its own, so its own column is kept.
+pub fn attached_source_prefix_width(node: &LuaSyntaxNode) -> usize {
+    let mut previous = node.first_token().and_then(|token| token.prev_token());
+    while let Some(token) = previous {
+        match token.kind().to_token() {
+            LuaTokenKind::TkWhitespace | LuaTokenKind::TkEndOfLine => previous = token.prev_token(),
+            LuaTokenKind::TkAssign | LuaTokenKind::TkReturn => {
+                return source_token_prefix_width(&token) + token.text().chars().count() + 1;
+            }
+            _ => break,
+        }
+    }
+    source_line_prefix_width(node)
+}
+
+/// The source column of `token`: the width of its line before it.
+fn source_token_prefix_width(token: &LuaSyntaxToken) -> usize {
     let mut width = 0usize;
-    let Some(mut token) = node.first_token() else {
-        return 0;
-    };
+    let mut token = token.clone();
 
     while let Some(prev) = token.prev_token() {
         let text = prev.text();

crates/emmylua_formatter/src/test/expression_tests.rs

@@ -996,7 +996,7 @@ end, 4, 5, 6, 7, 8, 9, 10
     fn test_non_first_multiline_callback_tail_refills_after_end() {
         let config = LuaFormatConfig {
             layout: LayoutConfig {
-                max_line_width: 28,
+                max_line_width: 27,
                 ..Default::default()
             },
             ..Default::default()

crates/emmylua_formatter/src/test/misc_tests.rs

@@ -2,7 +2,7 @@
 mod tests {
     use crate::{
         SourceText, assert_format,
-        config::{LayoutConfig, LuaFormatConfig},
+        config::{IndentConfig, IndentKind, LayoutConfig, LuaFormatConfig},
         reformat_lua_code,
     };
     use emmylua_parser::LuaLanguageLevel;
@@ -832,4 +832,96 @@ end
 
         assert_eq!(first, second);
     }
+
+    // ========== width estimates independent of the source layout (#1302) ==========
+
+    /// Format `text`, then format the result again.
+    fn format_twice(text: &str, config: &LuaFormatConfig) -> (String, String) {
+        let level = LuaLanguageLevel::default();
+        let first = reformat_lua_code(&SourceText { text, level }, config);
+        let second = reformat_lua_code(
+            &SourceText {
+                text: &first,
+                level,
+            },
+            config,
+        );
+        (first, second)
+    }
+
+    fn two_space_config(max_line_width: usize) -> LuaFormatConfig {
+        LuaFormatConfig {
+            indent: IndentConfig {
+                kind: IndentKind::Space,
+                width: 2,
+            },
+            layout: LayoutConfig {
+                max_line_width,
+                ..Default::default()
+            },
+            ..Default::default()
+        }
+    }
+
+    #[test]
+    fn test_call_args_layout_does_not_depend_on_first_arg_source_column() {
+        // The packed layout's first line is exactly 120 columns. Its first argument sits right after the
+        // `(` there, and on a line of its own in the expanded layout; estimating the line from that column
+        // made each layout format into the other forever.
+        let config = two_space_config(120);
+        let packed = r#"function M.evidence(rows)
+  local facts = Seq.from(rows)
+    :map(function(row)
+      local p = row[2]
+      return Seq.concat({ p.tail ~= '' and p.tail or '[no name]', 'ft=' .. (p.filetype ~= '' and p.filetype or '-') }, {
+        'lines=' .. p.lines
+      }, p.marks ~= nil and { 'marks=' .. p.marks } or {}, p.modified and { 'MODIFIED' } or {}
+      ):join '  '
+    end)
+    :totable()
+end
+"#;
+        let expanded = r#"function M.evidence(rows)
+  local facts = Seq.from(rows)
+    :map(function(row)
+      local p = row[2]
+      return Seq.concat(
+        { p.tail ~= '' and p.tail or '[no name]', 'ft=' .. (p.filetype ~= '' and p.filetype or '-') },
+        {
+          'lines=' .. p.lines
+        },
+        p.marks ~= nil and { 'marks=' .. p.marks } or {},
+        p.modified and { 'MODIFIED' } or {}
+      ):join '  '
+    end)
+    :totable()
+end
+"#;
+        let (from_packed, again) = format_twice(packed, &config);
+        assert_eq!(from_packed, again);
+        let (from_expanded, again) = format_twice(expanded, &config);
+        assert_eq!(from_expanded, again);
+        assert_eq!(from_packed, from_expanded);
+        assert!(
+            from_packed.lines().all(|line| line.len() <= 120),
+            "{from_packed}"
+        );
+    }
+
+    #[test]
+    fn test_table_after_assign_layout_does_not_depend_on_source_line() {
+        // Flat, the statement is 123 columns. With the table on its own line in the source, its column there
+        // (2) was taken for its column in the output (14), so it was joined flat past the limit.
+        let config = two_space_config(120);
+        let fields = "{ red = '31', green = '32', yellow = '33', blue = '34', magenta = '35', cyan = '36', dim = '90', bold = '1' }";
+        let (same_line, again) = format_twice(&format!("local CODES = {fields}\n"), &config);
+        assert_eq!(same_line, again);
+        let (next_line, again) = format_twice(&format!("local CODES =\n  {fields}\n"), &config);
+        assert_eq!(next_line, again);
+        assert_eq!(same_line, next_line);
+        assert!(
+            next_line.lines().all(|line| line.len() <= 120),
+            "{next_line}"
+        );
+    }
 }