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

[RFC] Accessing all annotations on an ivar/type/def #9322

Closed
Blacksmoke16 opened this issue May 19, 2020 · 2 comments · Fixed by #9326
Closed

[RFC] Accessing all annotations on an ivar/type/def #9322

Blacksmoke16 opened this issue May 19, 2020 · 2 comments · Fixed by #9326

Comments

@Blacksmoke16
Copy link
Member

Blacksmoke16 commented May 19, 2020

I been thinking about #6984 and thought of another use case that it would be better suited for.

For the serializer shard I been working on, I have a concept of PropertyMetadata. An array of these types is built out via a macro as a way to generically describe a property; such as its type, name, owning class, etc.

I also have a concept of ExclusionStrategies, which allow applying custom runtime logic to determine if a given property should be skipped. A common implementation I could see for this is defining some custom annotation, applying it to a property/class, and then using the presence of the annotation, or values within the annotation, as part of the exclusion strategy logic.

annotation SomeAnn; end

class User
  include ASR::Serializable  

  @[SomeAnn(priority: 1, privilege: :private)]
  property manager_id : Int32
end

However, given Crystal doesn't have a way to access annotations at runtime, nor is there a clean way to generically do it at compile time, I thought of a pretty decent solution. If there is a method to return all annotations applied to a given ivar/type/method, then I could include this information as part of the PropertyMetadata. For example, manager_id could be represented like:

<PropertyMetadata(Int32, user)
@name="manager_id",
@annotations={
  "SomeAnn" => {
    priority: 1,
    privilege: :private
  }
}>

Then the logic within the strategy could check for a specific annotation:

return false unless ann = metadata.annotations["SomeAnn"]?

return true if ann[:privilege] == :private

...

I think it also would play well for future annotation enhancements, such as being able to do ivar.annotations.select(ValidationAnnotation) or something along those lines.

WDYT?

@HertzDevil
Copy link
Contributor

A concern with Defs is that it should be possible to reconstruct every annotatable node from its parts (#3274 (comment)), so if any annotations are attached to a Def, there needs a way to iterate through all of them.

One problem is, the following:

@[Bar]
def foo
end

is parsed as:

Expressions.new([
  Annotation.new(Path.new(["Bar"])),
  Def.new("foo", [] of Arg),
] of ASTNode)

and not as a single Def node that contains the annotation. So perhaps such a macro method is not very useful in this scenario, because to reconstruct the Expressions we would have encountered the Annotation node itself anyway, and if we interpolate #annotations for the Def we would doubly apply every annotation. That said, it's not like such a macro method would do any harm as long as the parsing behaviour is documented properly.

For MetaVar and TypeNode I'm less sure because they are fictitious nodes.

The extension is simple; if we add Annotation#name : Path then it will be possible to do ivar.annotations.select &.name.resolve.== SomeAnnotation. (This macro method is also needed to reconstruct the Annotation node itself.)

@straight-shoota
Copy link
Member

Can we change the AST so that annotations belong to the node they annotate? It seems weird to have them just as previous expressions. I suppose this could be a breaking change, but perhaps we can keep the backwards-compatible API until 2.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants