From 6e6feafa548177e0b82ae91a60d1bab2aa57a896 Mon Sep 17 00:00:00 2001
From: Bradley Buda <bradleybuda@gmail.com>
Date: Thu, 4 Apr 2024 21:18:07 -0700
Subject: [PATCH] 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
---
 lib/syntax_tree/node.rb | 23 +++++++++++++++--------
 test/fixtures/assoc.rb  |  4 ++++
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb
index 3b676552..4f3c99d9 100644
--- a/lib/syntax_tree/node.rb
+++ b/lib/syntax_tree/node.rb
@@ -1553,6 +1553,9 @@ def ===(other)
 
     def format_contents(q)
       (q.parent || HashKeyFormatter::Identity.new).format_key(q, key)
+
+      # TODO: if value is +nil+ but we have chosen Rocket formatting, we need to format key as a value
+
       return unless value
 
       if key.comments.empty? && AssignFormatting.skip_indent?(value)
@@ -1784,17 +1787,21 @@ def format_key(q, key)
     end
 
     def self.for(container)
+      # First check for assocs where the value is nil; that means it has been
+      # omitted. In this case we have to match the existing formatting because
+      # standardizing would potentially break the code. For example:
+      #
+      #     { first:, "second" => "value" }
+      #
+      container.assocs.each do |assoc|
+        if assoc.value.nil?
+          return Identity.new
+        end
+      end
+
       container.assocs.each do |assoc|
         if assoc.is_a?(AssocSplat)
           # Splat nodes do not impact the formatting choice.
-        elsif assoc.value.nil?
-          # If the value is nil, then it has been omitted. In this case we have
-          # to match the existing formatting because standardizing would
-          # potentially break the code. For example:
-          #
-          #     { first:, "second" => "value" }
-          #
-          return Identity.new
         else
           # Otherwise, we need to check the type of the key. If it's a label or
           # dynamic symbol, we can use labels. If it's a symbol literal then it
diff --git a/test/fixtures/assoc.rb b/test/fixtures/assoc.rb
index 0fc60e6f..df8bb938 100644
--- a/test/fixtures/assoc.rb
+++ b/test/fixtures/assoc.rb
@@ -48,3 +48,7 @@
 { "foo #{bar}": "baz" }
 %
 { "foo=": "baz" }
+%
+{ bar => 1, baz: }
+%
+{ baz:, bar => 1 }