Local variables whose name is a keyword

[akimd]

Hi Markus,

I have a function that takes a named argument named return. The usual trick to capture it in modern Rubies (shorter and faster that looking into binding) is {return:}[:return].

Unfortunately, unparser simply prints what parser gave, so it turns a "local variable" named return into an actual return keyword.

require 'parser/current'
require 'unparser'

ast = Unparser.parse(<<~RUBY)
  def foo(return:)
    puts({return:}[:return])
  end
RUBY
p ast
puts Unparser.unparse(ast)

$ bundle exec /tmp/unparser.rb
s(:def, :foo,
  s(:args,
    s(:kwarg, :return)),
  s(:send, nil, :puts,
    s(:index,
      s(:hash,
        s(:pair,
          s(:sym, :return),
          s(:lvar, :return))),
      s(:sym, :return))))
def foo(return:)
  puts({ return: return }[:return])
end

Of course the generated file then fails to run properly.

As far as I'm concerned, this is not urgent: I used binding for this particular case.

Cheers!

[mbj]

@akimd thats an easy fix, I think I can source a list of all keywords form the parser gem and filter the emitter through that. Should be able to get this done this weekend.

Curious if you can potentially consider making your org sponsor my work here? I know you could in the past ;)

[mbj]

@akimd I've released unparser-0.6.14 with a fix for this case. Note it required also #369 to make it long term maintainable.

[akimd]

Hi Markus,

Thanks a lot for fixing this issue so fast.

Please note that this is not enough, you don't handle the case of a method call.

require 'parser/current'
require 'unparser'

ast = Unparser.parse(<<~RUBY)
  class Foo
    def return = 'return'
    
    def foo
      puts({return:}[:return])
    end
  end
RUBY
puts Unparser.unparse(ast)

class Foo
  def return
    "return"
  end

  def foo
    puts({ return: return }[:return])
  end
end

Curious if you can potentially consider making your org sponsor my work here?

I've been answered "no, not this time". Sorry :/

Cheers!

[mbj]

@akimd that fix should also be easy, this weekend. OT: I've got quite extensive corpus tests here and none of them found this case. Whats your domain to find this interesting cases?

[akimd]

We're building asts using kwargs, so it's quite frequent to be willing to use arguments such as then: or else:.

We also have tools that takes some piece of Ruby, and rewrite it to match some requirements. It is based on parser + transformation + unparser. This is a part I don't have access to.

When you pipe the first ones into that step, you get the bugs I reported.

The last case I reported (self.return) is not something we have, it's just the second case I know where {foo:} must not be "expanded".

[mbj]

Minimal reproduction:

bundle exec bin/unparser --verbose -e 'foo(return:)'
(string)
Original-Source:
foo(return:)
Generated-Source:
foo(return: return)
Original-Node:
(send nil :foo
  (kwargs
    (pair
      (sym :return)
      (send nil :return))))
Generated-Node:
(send nil :foo
  (kwargs
    (pair
      (sym :return)
      (return))))
Node-Diff:
@@ -1,6 +1,6 @@
 (send nil :foo
   (kwargs
     (pair
       (sym :return)
-      (send nil :return))))
+      (return))))

Error: (string)

[mbj]

@akimd fix is here: #372

[mbj]

@akimd Fix released as unparser-0.6.15.

[mbj]

@akimd

We also have tools that takes some piece of Ruby, and rewrite it to match some requirements. It is based on parser + transformation + unparser. This is a part I don't have access to.

I recommend to always use Unparser.unparse_validate which is a bit slower but makes sure that unparser did produce source that round trips nicely. This will catch these kinds of bugs earlier. It It returns an Either type you can unwrap via .from_right.

So:

source = Unparser.unparse_validate(ast).from_right { |validation_error| raise(validation_error.inspect) }

This will re-parse the generated source before returning the source. And only make the source available if it could be parsed again to the same AST.

[mbj]

I've been answered "no, not this time". Sorry :/

np, ty for trying.