Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid identifier naming collision in getter, setter, and property macros #15239

Conversation

jgaskins
Copy link
Contributor

TL;DR

Currently, you can't refer to an identifier called value inside a getter, setter, and property blocks due to that identifier colliding with one defined inside the macro. This commit uses a safe variable name to avoid that collision.

Details

Given the following code:

struct Foo
  getter value = "bar"
  getter baz : String do
    value.upcase
  end
end

Foo.new.baz

I get this error:

In foo.cr:8:9

 8 | Foo.new.baz
             ^--
Error: instantiating 'Foo#baz()'


In foo.cr:4:11

 4 | value.upcase
           ^-----
Error: undefined method 'upcase' for Nil

Nil trace:

  foo.cr:4

        value.upcase


  macro getter (in expanded macro: macro_4331394032:117):10

                if (value = @baz).nil?


  macro getter (in expanded macro: macro_4331394032:117):10

                if (value = @baz).nil?


  macro getter (in expanded macro: macro_4331394032:117):7

              @baz : String?

This didn't make sense because my value method is guaranteed to return a String at compile time. One thing I noticed was that changing the getter baz block to self.value.upcase worked, which pointed to a naming collision.

I did find local variables named value in those macros, and this PR updates them with the % sigil. With this PR, that example code runs as expected.

Previously, using a local variable called `value` or calling a method
with that name without an explicit `self` receiver would collide with
the one defined in the `getter`, `setter`, and `property` blocks. This
commit uses a safe variable name to avoid that collision.
@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:macros labels Nov 30, 2024
@RX14 RX14 added this to the 1.14.1 milestone Dec 1, 2024
@RX14 RX14 merged commit 62638f4 into crystal-lang:master Dec 1, 2024
68 of 69 checks passed
@jgaskins jgaskins deleted the allow-using-value-variable-in-getter-blocks branch December 1, 2024 17:25
@straight-shoota straight-shoota modified the milestones: 1.14.1, 1.15.0 Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:macros
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants