Skip to content

Commit c5b61fb

Browse files
committed
Assoc uses Identity formatter if any value is nil
The existing assoc formatter has logic to identify the case where a value is nil (in a Ruby-3.1 style hash) and preserve the existing formatting. For example: `{ first:, "second" => "value" }` is correctly left as-is. However, this logic only worked if the first assoc in the container had the nil value - if a later assoc had a nil value, the Identity formatter might not be chosen which could cause the formatter to generate invalid Ruby code. As an example, this code: `{ "first" => "value", second: }` would be turned into `{ "first" => "value", :second => }`. This patch pulls the nil value check up to the top of `HashKeyFormatter.for` to ensure it takes precendence over any other formatting selections. The fixtures have been updated to cover both cases (nil value in first position, nil value in last position). Fixes #446
1 parent 8dfae19 commit c5b61fb

File tree

2 files changed

+16
-8
lines changed

2 files changed

+16
-8
lines changed

Diff for: lib/syntax_tree/node.rb

+12-8
Original file line numberDiff line numberDiff line change
@@ -1784,17 +1784,21 @@ def format_key(q, key)
17841784
end
17851785

17861786
def self.for(container)
1787+
# First check for assocs where the value is nil; that means it has been
1788+
# omitted. In this case we have to match the existing formatting because
1789+
# standardizing would potentially break the code. For example:
1790+
#
1791+
# { first:, "second" => "value" }
1792+
#
1793+
container.assocs.each do |assoc|
1794+
if assoc.value.nil?
1795+
return Identity.new
1796+
end
1797+
end
1798+
17871799
container.assocs.each do |assoc|
17881800
if assoc.is_a?(AssocSplat)
17891801
# Splat nodes do not impact the formatting choice.
1790-
elsif assoc.value.nil?
1791-
# If the value is nil, then it has been omitted. In this case we have
1792-
# to match the existing formatting because standardizing would
1793-
# potentially break the code. For example:
1794-
#
1795-
# { first:, "second" => "value" }
1796-
#
1797-
return Identity.new
17981802
else
17991803
# Otherwise, we need to check the type of the key. If it's a label or
18001804
# dynamic symbol, we can use labels. If it's a symbol literal then it

Diff for: test/fixtures/assoc.rb

+4
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,7 @@
4848
{ "foo #{bar}": "baz" }
4949
%
5050
{ "foo=": "baz" }
51+
%
52+
{ bar => 1, baz: }
53+
%
54+
{ baz:, bar => 1 }

0 commit comments

Comments
 (0)