Fix SyntaxError when instrumenting hash with shorthand

scoutapp · Jan 19, 2024 · decd05c · decd05c
1 parent a49ea7b
commit decd05c
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 5 deletions.
diff --git a/lib/scout_apm/auto_instrument/rails.rb b/lib/scout_apm/auto_instrument/rails.rb
@@ -135,6 +135,27 @@ def on_send(node)
           wrap(node.location.expression, *instrument(node.location.expression.source, file_name, line))
         end
 
+        def on_hash(node)
+          node.children.each do |pair|
+            # Skip `pair` if we're sure it's not using the hash shorthand syntax
+            next if pair.type != :pair
+            key_node, value_node = pair.children
+            next unless key_node.type == :sym && value_node.type == :send
+            key = key_node.children[0]
+            next unless value_node.children[0].nil? && key == value_node.children[1]
+
+            # Extract useful metadata for instrumentation:
+            line = pair.location.line || 'line?'
+            # column = pair.location.column || 'column?' # not used
+            # method_name = key || '*unknown*' # not used
+            file_name = @source_rewriter.source_buffer.name
+
+            instrument_before, instrument_after = instrument(pair.location.expression.source, file_name, line)
+            replace(pair.loc.expression, "#{key}: #{instrument_before}#{key}#{instrument_after}")
+          end
+          super
+        end
+
         # def on_class(node)
         #   class_name = node.children[1]
         #

diff --git a/test/unit/auto_instrument/controller-instrumented.rb b/test/unit/auto_instrument/controller-instrumented.rb
@@ -40,10 +40,39 @@ def data
 
     ::ScoutApm::AutoInstrument("respond_with @clients.each(&formatter).join(\"\\n\"), :content_type => 'application/json; boundary=NL'",["ROOT/test/unit/auto_instrument/controller.rb:41:in `data'"]){respond_with @clients.each(&formatter).join("\n"), :content_type => 'application/json; boundary=NL'}
   end
-  
+
   def things
-    x = {}
+    x = {
+      static: "static",
+      shorthand: ::ScoutApm::AutoInstrument("shorthand:",["ROOT/test/unit/auto_instrument/controller.rb:47:in `things'"]){shorthand},
+      longhand: ::ScoutApm::AutoInstrument("longhand: longhand",["ROOT/test/unit/auto_instrument/controller.rb:48:in `things'"]){longhand},
+      longhand_different_key: ::ScoutApm::AutoInstrument("longhand",["ROOT/test/unit/auto_instrument/controller.rb:49:in `things'"]){longhand},
+      hash_rocket: ::ScoutApm::AutoInstrument(":hash_rocket => hash_rocket",["ROOT/test/unit/auto_instrument/controller.rb:50:in `things'"]){hash_rocket},
+      :hash_rocket_different_key => ::ScoutApm::AutoInstrument("hash_rocket",["ROOT/test/unit/auto_instrument/controller.rb:51:in `things'"]){hash_rocket},
+      non_nil_receiver: ::ScoutApm::AutoInstrument("non_nil_receiver.value",["ROOT/test/unit/auto_instrument/controller.rb:52:in `things'"]){non_nil_receiver.value},
+      nested: {
+        shorthand: ::ScoutApm::AutoInstrument("shorthand:",["ROOT/test/unit/auto_instrument/controller.rb:54:in `things'"]){shorthand},
+      }
+    }
     x[:this] ||= 'foo'
-    x[:that] &&= ::ScoutApm::AutoInstrument("'foo'.size",["ROOT/test/unit/auto_instrument/controller.rb:47:in `things'"]){'foo'.size}
+    x[:that] &&= ::ScoutApm::AutoInstrument("'foo'.size",["ROOT/test/unit/auto_instrument/controller.rb:58:in `things'"]){'foo'.size}
+  end
+
+  private
+
+  def shorthand
+    "shorthand"
+  end
+
+  def longhand
+    "longhand"
+  end
+
+  def hash_rocket
+    "hash_rocket"
+  end
+
+  def non_nil_receiver
+    ::ScoutApm::AutoInstrument("OpenStruct.new(value: \"value\")",["ROOT/test/unit/auto_instrument/controller.rb:76:in `non_nil_receiver'"]){OpenStruct.new(value: "value")}
   end
 end
diff --git a/test/unit/auto_instrument/controller.rb b/test/unit/auto_instrument/controller.rb
@@ -40,10 +40,39 @@ def data
 
     respond_with @clients.each(&formatter).join("\n"), :content_type => 'application/json; boundary=NL'
   end
-  
+
   def things
-    x = {}
+    x = {
+      static: "static",
+      shorthand:,
+      longhand: longhand,
+      longhand_different_key: longhand,
+      :hash_rocket => hash_rocket,
+      :hash_rocket_different_key => hash_rocket,
+      non_nil_receiver: non_nil_receiver.value,
+      nested: {
+        shorthand:,
+      }
+    }
     x[:this] ||= 'foo'
     x[:that] &&= 'foo'.size
   end
+
+  private
+
+  def shorthand
+    "shorthand"
+  end
+
+  def longhand
+    "longhand"
+  end
+
+  def hash_rocket
+    "hash_rocket"
+  end
+
+  def non_nil_receiver
+    OpenStruct.new(value: "value")
+  end
 end