Reformat issue on call arguments

[ematipico]

A regression has been found when using the best_fitting! macro.

The following code hits a reformat issue:

// Before
deepCopyAndAsyncMapLeavesC(
  { source: sourceValue, destination: destination[sourceKey] },
  1337,
  { valueMapper, overwriteExistingKeys }
)

// First pass
deepCopyAndAsyncMapLeavesC({
	source: sourceValue,
	destination: destination[sourceKey],
}, 1337, { valueMapper, overwriteExistingKeys });

// Second pass
deepCopyAndAsyncMapLeavesC(
	{
		source: sourceValue,
		destination: destination[sourceKey],
	},
	1337,
	{ valueMapper, overwriteExistingKeys },
);

[MichaReiser]

Prettier's formatting as reference:

deepCopyAndAsyncMapLeavesC(
  { source: sourceValue, destination: destination[sourceKey] },
  1337,
  { valueMapper, overwriteExistingKeys }
);

So it might be that we're just picking the "incorrect" layout.

[ematipico]

Prettier's formatting as reference:

deepCopyAndAsyncMapLeavesC(
  { source: sourceValue, destination: destination[sourceKey] },
  1337,
  { valueMapper, overwriteExistingKeys }
);

So it might be that we're just picking the "incorrect" layout.

Myself and @yassere triaged the issue time ago, and we found out that the issue is due to the fact that our best_fitting logic differs from prettier:

tools/crates/rome_formatter/src/macros.rs

Lines 244 to 245 in 9e22611

	/// Unlike in Prettier, the printer will try to fit **only the first variant** in [`Flat`] mode,
	/// then it will try to fit the rest of the variants in [`Expanded`] mode.

[github-actions]

This issue is stale because it has been open 14 days with no activity.

[ematipico]

This seems to be still valid issue

[MichaReiser]

I think the issue here is another one. I haven't looked very deeply but the call arguments formatter emits two different IRs for the first and second formatting.

First:

  "deepCopyAndAsyncMapLeavesC",
  best_fitting([
    [
      <interned 0> "(",
      group([
        <interned 1> [
          "{",
          group([
            indent([
              soft_line_break_or_space,
              group([
                group(["source"]),
                ":",
                group(
                  [indent([soft_line_break_or_space])],
                  { id: #assignment_like-1 }
                ),
                line_suffix_boundary,
                indent_if_group_breaks(
                  ["sourceValue"],
                  { group-id: #assignment_like-1 }
              ]),
              ",",
              soft_line_break_or_space,
              group([
                group(["destination"]),
                ":",
                group([
                  indent([
                    soft_line_break_or_space,
                    "destination",
                    group([
                      "[",
                      soft_line_break,
                      indent([soft_line_break, "sourceKey"]),
                      soft_line_break,
                      "]"
                    ])
                  ])
                ])
              ]),
              if_group_breaks([","])
            ]),
            soft_line_break_or_space
          ]),
          "}",
          ","
        ],
        soft_line_break_or_space,
        <interned 2> ["1337", ","],
        soft_line_break_or_space,
        <interned 3> [
          "{",
          group([
            indent([
              soft_line_break_or_space,
              "valueMapper",
              ",",
              soft_line_break_or_space,
              "overwriteExistingKeys",
              if_group_breaks([","])
            ]),
            soft_line_break_or_space
          ]),
          "}"
        ],
        if_group_breaks([","])
      ]),
      <interned 4> ")"
    ],
    [
      <ref interned *0>,
      <ref interned *1>,
      " ",
      <ref interned *2>,
      " ",
      <ref interned *3>,
      expand_parent,
      <ref interned *4>
    ],
    [
      <ref interned *0>,
      group([
        indent([
          soft_line_break,
          <ref interned *1>,
          soft_line_break_or_space,
          <ref interned *2>,
          soft_line_break_or_space,
          <ref interned *3>,
          if_group_breaks([","]),
          soft_line_break
        ]),
        soft_line_break,
        <ref interned *4>
      ])
    ]
  ]),
  ";",
  empty_line,

Second

"deepCopyAndAsyncMapLeavesC",
  <interned 5> "(",
  group([
    indent([
      soft_line_break,
      <interned 6> [
        "{",
        indent([
          hard_line_break,
          group([
            group(["source"]),
            ":",
            group(
              [indent([soft_line_break_or_space])],
              { id: #assignment_like-2 }
            ),
            line_suffix_boundary,
            indent_if_group_breaks(
              ["sourceValue"],
              { group-id: #assignment_like-2 }
          ]),
          ",",
          soft_line_break_or_space,
          group([
            group(["destination"]),
            ":",
            group([
              indent([
                soft_line_break_or_space,
                "destination",
                group([
                  "[",
                  soft_line_break,
                  indent([soft_line_break, "sourceKey"]),
                  soft_line_break,
                  "]"
                ])
              ])
            ])
          ]),
          if_group_breaks([","])
        ]),
        hard_line_break,
        "}",
        ","
      ],
      soft_line_break_or_space,
      <interned 7> ["1337", ","],
      soft_line_break_or_space,
      <interned 8> [
        "{",
        group([
          indent([
            soft_line_break_or_space,
            "valueMapper",
            ",",
            soft_line_break_or_space,
            "overwriteExistingKeys",
            if_group_breaks([","])
          ]),
          soft_line_break_or_space
        ]),
        "}"
      ],
      if_group_breaks([","]),
      soft_line_break
    ]),
    soft_line_break,
    <interned 9> ")"
  ]),
  ";",
  hard_line_break

One difference that could be of importance is that object in the first version uses soft line breaks where the second version uses hard line breaks (always forcing the group to break, even if it otherwise would fit):

"{",
          group([
            indent([
              soft_line_break_or_space,
              group([
                group(["source"]),
                ":",
                group(
                  [indent([soft_line_break_or_space])],
                  { id: #assignment_like-1 }
                ),
                line_suffix_boundary,
                indent_if_group_breaks(
                  ["sourceValue"],
                  { group-id: #assignment_like-1 }
              ]),
              ",",
              soft_line_break_or_space,
              group([
                group(["destination"]),
                ":",
                group([
                  indent([
                    soft_line_break_or_space,
                    "destination",
                    group([
                      "[",
                      soft_line_break,
                      indent([soft_line_break, "sourceKey"]),
                      soft_line_break,
                      "]"
                    ])
                  ])
                ])
              ]),
              if_group_breaks([","])
            ]),
            soft_line_break_or_space
          ]),
          "}",

vs

"{",
        indent([
          hard_line_break,
          group([
            group(["source"]),
            ":",
            group(
              [indent([soft_line_break_or_space])],
              { id: #assignment_like-2 }
            ),
            line_suffix_boundary,
            indent_if_group_breaks(
              ["sourceValue"],
              { group-id: #assignment_like-2 }
          ]),
          ",",
          soft_line_break_or_space,
          group([
            group(["destination"]),
            ":",
            group([
              indent([
                soft_line_break_or_space,
                "destination",
                group([
                  "[",
                  soft_line_break,
                  indent([soft_line_break, "sourceKey"]),
                  soft_line_break,
                  "]"
                ])
              ])
            ])
          ]),
          if_group_breaks([","])
        ]),
        hard_line_break,
        "}",

Notice the hard_line_break at the beginning and end.

By the way, Prettier has the same issue.

To sum up. This isn't an issue with the printer but an issue with our formatting rules.