From 05e5cd6d04fc9944654ac615115dd416bbc642fe Mon Sep 17 00:00:00 2001 From: Jean-Philippe Date: Wed, 25 Feb 2026 11:26:57 -0500 Subject: [PATCH] Preserve keyword argument semantics through AST transformation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The upstream parser builder emits :hash nodes for both bare keyword arguments and explicit hash literals. Unparser uses node type to decide whether to emit braces, so kwargs were round-tripping as positional hashes — breaking Ruby 3.0+ strict keyword/positional separation. KwargsBuilder overrides associate to emit :kwargs nodes when no brace tokens are present, which Unparser already handles correctly. Co-authored-by: Cursor --- lib/ast_transform/kwargs_builder.rb | 22 +++++ lib/ast_transform/transformer.rb | 7 +- test/ast_transform/kwargs_builder_test.rb | 98 +++++++++++++++++++++++ test/ast_transform/transformation_test.rb | 42 ++++++++++ 4 files changed, 165 insertions(+), 4 deletions(-) create mode 100644 lib/ast_transform/kwargs_builder.rb create mode 100644 test/ast_transform/kwargs_builder_test.rb diff --git a/lib/ast_transform/kwargs_builder.rb b/lib/ast_transform/kwargs_builder.rb new file mode 100644 index 0000000..db5abb5 --- /dev/null +++ b/lib/ast_transform/kwargs_builder.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require 'prism/translation/parser' + +module ASTTransform + # Extends the default Prism parser builder to distinguish keyword arguments + # from hash literals in the AST. + # + # The upstream builder always emits :hash nodes for both `foo(bar: 1)` and + # `foo({ bar: 1 })`. Unparser uses the node type to decide whether to emit + # braces: :hash gets `{}`, :kwargs does not. Since Ruby 3.0+ treats these as + # semantically different (strict keyword/positional separation), we need the + # AST to preserve the distinction. + class KwargsBuilder < Prism::Translation::Parser::Builder + def associate(begin_t, pairs, end_t) + node = super + return node unless begin_t.nil? && end_t.nil? + + node.updated(:kwargs) + end + end +end diff --git a/lib/ast_transform/transformer.rb b/lib/ast_transform/transformer.rb index 4d53ea1..14ea5c7 100644 --- a/lib/ast_transform/transformer.rb +++ b/lib/ast_transform/transformer.rb @@ -2,6 +2,7 @@ require 'prism' require 'prism/translation/parser' require 'unparser' +require 'ast_transform/kwargs_builder' require 'ast_transform/source_map' module ASTTransform @@ -9,10 +10,8 @@ class Transformer # Constructs a new Transformer instance. # # @param transformations [Array] The transformations to be run. - # @param builder [Prism::Translation::Parser::Builder] The AST Node builder. - def initialize(*transformations, builder: Prism::Translation::Parser::Builder.new) + def initialize(*transformations) @transformations = transformations - @builder = builder end # Builds the AST for the given +source+. @@ -100,7 +99,7 @@ def create_buffer(source, file_path) def parser @parser&.reset - @parser ||= Prism::Translation::Parser.new(@builder) + @parser ||= Prism::Translation::Parser.new(ASTTransform::KwargsBuilder.new) end def register_source_map(source_file_path, transformed_file_path, transformed_ast, transformed_source) diff --git a/test/ast_transform/kwargs_builder_test.rb b/test/ast_transform/kwargs_builder_test.rb new file mode 100644 index 0000000..2adac97 --- /dev/null +++ b/test/ast_transform/kwargs_builder_test.rb @@ -0,0 +1,98 @@ +# frozen_string_literal: true + +require 'test_helper' +require 'ast_transform/kwargs_builder' + +module ASTTransform + class KwargsBuilderTest < Minitest::Test + extend ASTTransform::Declarative + + def setup + @builder = ASTTransform::KwargsBuilder.new + @parser = Prism::Translation::Parser.new(@builder) + end + + test "#associate emits :kwargs for bare keyword arguments" do + ast = parse('foo(bar: 1, baz: 2)') + node = find_node(ast, :kwargs) + + refute_nil node, "expected a :kwargs node for bare keyword arguments" + assert_equal :kwargs, node.type + end + + test "#associate emits :hash for explicit hash with braces" do + ast = parse('foo({ bar: 1, baz: 2 })') + hash_node = find_node(ast, :hash) + + refute_nil hash_node, "expected a :hash node for explicit hash" + assert_equal :hash, hash_node.type + assert_nil find_node(ast, :kwargs) + end + + test "#associate emits :kwargs for keyword arguments mixed with positional arguments" do + ast = parse('foo("hello", bar: 1)') + node = find_node(ast, :kwargs) + + refute_nil node, "expected a :kwargs node for keyword arguments" + assert_equal :kwargs, node.type + end + + test "#associate emits :hash for standalone hash literals" do + ast = parse('x = { a: 1, b: 2 }') + hash_node = find_node(ast, :hash) + + refute_nil hash_node, "expected a :hash node for hash literal" + assert_equal :hash, hash_node.type + assert_nil find_node(ast, :kwargs) + end + + test "#associate preserves keyword argument pairs" do + ast = parse('foo(bar: 1, baz: 2)') + node = find_node(ast, :kwargs) + + assert_equal 2, node.children.length + assert_equal :pair, node.children[0].type + assert_equal :pair, node.children[1].type + + assert_equal :bar, node.children[0].children[0].children[0] + assert_equal :baz, node.children[1].children[0].children[0] + end + + test "#associate emits :kwargs for keyword arguments in constructor calls" do + ast = parse('Foo.new(bar: 1, baz: 2)') + node = find_node(ast, :kwargs) + + refute_nil node, "expected a :kwargs node in constructor call" + assert_equal :kwargs, node.type + end + + test "#associate emits :kwargs for double-splat keyword arguments" do + ast = parse('foo(**opts)') + node = find_node(ast, :kwargs) + + refute_nil node, "expected a :kwargs node for double-splat" + end + + private + + def parse(source) + buffer = Parser::Source::Buffer.new('test') + buffer.source = source + @parser.parse(buffer) + end + + def find_node(ast, type) + return ast if ast.type == type + return nil unless ast.respond_to?(:children) + + ast.children.each do |child| + next unless child.is_a?(Parser::AST::Node) + + found = find_node(child, type) + return found if found + end + + nil + end + end +end diff --git a/test/ast_transform/transformation_test.rb b/test/ast_transform/transformation_test.rb index dbfc8c0..451bd7b 100644 --- a/test/ast_transform/transformation_test.rb +++ b/test/ast_transform/transformation_test.rb @@ -189,6 +189,48 @@ class Bar assert_equal expected, transform(source, @transformation) end + test "transform! preserves keyword arguments in method calls" do + source = <<~HEREDOC + transform!(ASTTransform::TransformationTest::ClassNamePrefixerTransformation) + class Foo + def setup + @obj = MyClass.new(bar: 1, baz: 2) + end + end + HEREDOC + + expected = <<~HEREDOC + class PrefixFoo + def setup + @obj = MyClass.new(bar: 1, baz: 2) + end + end + HEREDOC + + assert_equal expected, transform(source, @transformation) + end + + test "transform! preserves keyword arguments mixed with positional arguments" do + source = <<~HEREDOC + transform!(ASTTransform::TransformationTest::ClassNamePrefixerTransformation) + class Foo + def call + method("hello", bar: 1) + end + end + HEREDOC + + expected = <<~HEREDOC + class PrefixFoo + def call + method("hello", bar: 1) + end + end + HEREDOC + + assert_equal expected, transform(source, @transformation) + end + test "transform! runs the transformation on a constant assignment node" do source = <<~HEREDOC transform!(ASTTransform::TransformationTest::FooTransformation)