Crystal: Annotation Not Set on Property Definition

[toddsundsted]

Code like the following does not add the Perishable annotation to the @test instance variable:

class Test
  annotation Perisable
  end

  @[Perisable]
  property test : String? { "test" }

  def initialize
    {% puts @type.instance_vars.map(&.name) %}
    {% puts @type.instance_vars.select(&.annotation(Perisable)).map(&.name) %}
  end
end

Test.new.inspect

Running this example produces:

$ crystal run /tmp/test.cr
[test]
[]

Expanding the property macro shows that when property is used with a block, the setter macro is used to generate the setter, which results in a secondary definition of @test. The secondary definition does not get the annotation:

$ crystal tool expand /tmp/test.cr -c /tmp/test.cr:6:3
[test]
[]
1 expansion found
expansion 1:
   property(test : String | ::Nil) do
     "test"
   end

# expand macro 'property' (/usr/local/Cellar/crystal/0.34.0/src/object.cr:277:3)
~> setter(test : String | ::Nil)
   @test : String | ::Nil | ::Nil
   def test : String | ::Nil | ::Nil
     if (  value = @test).nil?
       @test = ("test")
     else
       value
     end
   end

# expand macro 'setter' (/usr/local/Cellar/crystal/0.34.0/src/object.cr:277:3)
~> @test : String | ::Nil
   def test=(test : String | ::Nil)
     @test = test
   end
   @test : String | ::Nil | ::Nil
   def test : String | ::Nil | ::Nil
     if value = @test.nil?
       @test = "test"
     else
       value
     end
   end

You can see the code here.

The property? macro handles this correctly, and generates its own setter. Likewise, simple assignment does not have this problem, nor does attaching the annotation to the instance variable directly.

$ crystal -v
Crystal 0.34.0 (2020-04-07)

LLVM: 10.0.0
Default target: x86_64-apple-macosx

[asterite]

Reduced:

macro foo
  1 # remove this
  @test : String?
end

class Test
  annotation Perisable
  end

  @[Perisable]
  foo

  def initialize
    {% puts @type.instance_vars.map(&.name) %}
    {% puts @type.instance_vars.select(&.annotation(Perisable)).map(&.name) %}
  end
end

Test.new

It seems an annotation on top of a macro is only being applied to the first expression in the case of a multi-expression. I'll try to fix this.

[asterite]

Actually, I'm not sure the annotation should be applied to all the expressions. Imagine generating an instance var and a lot of methods, it doesn't make sense for all of them to have the annotation. So I think just applying it to the first one is fine. We need to fix the macro.

[jhass]

maybe a macro should have access to its annotations as a kind of pseudo argument so it has more liberty in placing them? Not sure what good interface for this would be, {{@annotations}}?

[asterite]

There's another bug:

class Test
  annotation Perisable
  end

  @[Perisable]
  @test : String | ::Nil?

  @test : String | ::Nil?

  def initialize
    {% puts @type.instance_vars.map(&.name) %}
    {% puts @type.instance_vars.select(&.annotation(Perisable)).map(&.name) %}
  end
end

Test.new.inspect

If you repeat the instance var declaration, the annotation is lost.

I was trying to fix it by moving the type declaration in the generated macro code above the generated setter, but the setter redeclares the instance var so the annotation is lost.

[toddsundsted]

the property? case works simply by defining its own setter after the declaration and the getter.

[toddsundsted]

that, at least , makes property and property? work consistently. (without regard to other edge cases.)