From c86b6926ab7bbf47740f36abe62a9698dcc96a5e 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 | 83 ++++++++++++++++++++++++-----------------
 test/fixtures/assoc.rb  |  4 ++
 2 files changed, 53 insertions(+), 34 deletions(-)

diff --git a/lib/syntax_tree/node.rb b/lib/syntax_tree/node.rb
index 5a92a5a7..4d148f35 100644
--- a/lib/syntax_tree/node.rb
+++ b/lib/syntax_tree/node.rb
@@ -1783,45 +1783,60 @@ def format_key(q, key)
       end
     end
 
-    def self.for(container)
-      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
-          # needs to match a certain pattern to be used as a label. If it's
-          # anything else, then we need to use hash rockets.
-          case assoc.key
-          when Label, DynaSymbol
-            # Here labels can be used.
-          when SymbolLiteral
-            # When attempting to convert a hash rocket into a hash label,
-            # you need to take care because only certain patterns are
-            # allowed. Ruby source says that they have to match keyword
-            # arguments to methods, but don't specify what that is. After
-            # some experimentation, it looks like it's:
-            value = assoc.key.value.value
-
-            if !value.match?(/^[_A-Za-z]/) || value.end_with?("=")
-              return Rockets.new
-            end
+    class << self
+      def for(container)
+        (assocs = container.assocs).each_with_index do |assoc, index|
+          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
-            # If the value is anything else, we have to use hash rockets.
-            return Rockets.new
+            # 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 needs to match a certain pattern to be used as a label. If
+            # it's anything else, then we need to use hash rockets.
+            case assoc.key
+            when Label, DynaSymbol
+              # Here labels can be used.
+            when SymbolLiteral
+              # When attempting to convert a hash rocket into a hash label,
+              # you need to take care because only certain patterns are
+              # allowed. Ruby source says that they have to match keyword
+              # arguments to methods, but don't specify what that is. After
+              # some experimentation, it looks like it's:
+              value = assoc.key.value.value
+
+              if !value.match?(/^[_A-Za-z]/) || value.end_with?("=")
+                if omitted_value?(assocs[(index + 1)..])
+                  return Identity.new
+                else
+                  return Rockets.new
+                end
+              end
+            else
+              if omitted_value?(assocs[(index + 1)..])
+                return Identity.new
+              else
+                return Rockets.new
+              end
+            end
           end
         end
+
+        Labels.new
       end
 
-      Labels.new
+      private
+
+      def omitted_value?(assocs)
+        assocs.any? { |assoc| !assoc.is_a?(AssocSplat) && assoc.value.nil? }
+      end
     end
   end
 
diff --git a/test/fixtures/assoc.rb b/test/fixtures/assoc.rb
index 0fc60e6f..83a4887a 100644
--- a/test/fixtures/assoc.rb
+++ b/test/fixtures/assoc.rb
@@ -48,3 +48,7 @@
 { "foo #{bar}": "baz" }
 %
 { "foo=": "baz" }
+% # >= 3.1.0
+{ bar => 1, baz: }
+% # >= 3.1.0
+{ baz:, bar => 1 }