luckyframework / lucky

A full-featured Crystal web framework that catches bugs for you, runs incredibly fast, and helps you write code that lasts.
https://luckyframework.org
MIT License
2.59k stars 156 forks source link

memoize doesn't work when there's an external name #1814

Closed jwoertink closed 1 year ago

jwoertink commented 1 year ago

This code throws an undefined variable "thing" error:

memoize def needed?(for thing : Whatever) : Bool
  thing.does_stuff?
end

needed?(for: thing)
robacarp commented 1 year ago

Yeah, I've run into this myself. I feel like I've tried to capture the internal/external stuff in macros before and it didn't work...but that may have been 452 crystal versions ago...

jwoertink commented 1 year ago

Turns out, if you use arg.name it'll default to the external name... You have to use arg.internal_name which will either use the internal, or the regular arg name...

class Test
  macro memoize(method_def)
    {% puts method_def.name %}
    {% for arg in method_def.args %}
        {% puts arg.internal_name %}
      {% end %}
  end

  memoize def call(x y : String) : String
    "done"
  end

  memoize def run : String
    ""
  end

  memoize def exec(x : String) : String
    ""
  end
end

That would need to be updated everywhere we use arg.name in this file.

https://github.com/luckyframework/lucky/blob/22fcb187c2045a36823a03310d1ed68aec5d8f0b/src/lucky/memoizable.cr#L22