Crystal: Doc generation and custom annotations issue

[anykeyh]

When a comment appears on top of attribute annotation, it won't be catch by crystal docs generation.

Example:

    class BillingProperties
      include JSON::Serializable

      # When set to true, Requester Pays is enabled for this bucket.
      @[JSON::Field(key: "requesterPays")]
      property requester_pays : Bool
    end

crystal docs example.cr

Obviously, changing the order of the lines appears to work, however I do think the readability of the code would be impacted, further more if the comment is multi-lines.

[asterite]

The actual issue is a comment on top of an annotation on top of a macro call which in turns defines a method. Comment on top of annotation on top of regular method works fine.

[asterite]

Actually, the issue is that property expands to several things and the docs aren't copied to all of them.

[bew]

I think that macros should not copy docs automatically, but add a way to insert the doc where we want, for ex:

macro foo
  {{ @doc }}
  def bar
  end
end

# some doc
foo

So we have precise control of where to doc is applied (and how)

[KimBurgess]

I've just been taking a look at this - the issue doesn't appear to be on the macro expansion side (that I can see). To narrow the scope a little though, the same behavior occurs with the getter macro.

In existing setup, the expansion is:

@var : Type

def var : Type
  @var
end

If this is flipped around to

def var : Type
  @var
end

@var : Type

the doc generation works as intended with an without annotations.

Is there a downside to flipping these around until the root issue can be resolved?

[Blacksmoke16]

Wouldn't that make the annotation apply to the method and not the ivar?

[straight-shoota]

There is already a mechanism implemented in the PropagateDocVisitor which duplicates the doc comment of a macro to the features defined in it. A result of this is that the doc comment of a property macro is assigned to both the setter and getter method it defines.
So that should be all good.

It seems the issue here is just that for some reason this somehow gets mixed up when an annotation is involved, as per https://github.com/crystal-lang/crystal/issues/6449#issuecomment-495419391

[KimBurgess]

@Blacksmoke16 the annotation applies to both the InstanceVar and Def(s) that these macros pump out.

[Blacksmoke16]

@Blacksmoke16 the annotation applies to both the InstanceVar and Def(s) that these macros pump out.

It only gets applied to the first type. See https://play.crystal-lang.org/#/r/7ut6.

[KimBurgess]

Ah you're absolutely right. I was thrown off by my quick test the with JSON::Field annotation - looks like the parser there loads values into all ivars where keys match, regardless of annotations.