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

Iterate over ivar's annotations/annotation keys #6984

Closed
Blacksmoke16 opened this issue Oct 23, 2018 · 6 comments
Closed

Iterate over ivar's annotations/annotation keys #6984

Blacksmoke16 opened this issue Oct 23, 2018 · 6 comments

Comments

@Blacksmoke16
Copy link
Member

Currently an ivar's annotation can only be accessed manually, one by one, where the name has to be known. Being able get an array of annotations on an ivar would dramatically simplify this.

On a similar note the only way to get a value from an annotation is knowing the name of the key. Having a method to return an array of keys on the annotation would further simply things.

Could look something like:

{% for ivar in @type.instance_vars %}
  {% for annotation in ivar.annotations %}
    {{annotation.keys.map { |k| annotation[k] }.join(',').id}}
  {% end %}
{% end %}

Based on #6980 (comment)

@bew
Copy link
Contributor

bew commented Oct 23, 2018

Whats your usecase?

@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented Oct 23, 2018

@bew I would be using this for https://github.com/Blacksmoke16/CrSerializer.

I'm working on changing the syntax to like:

@[Serialize::Assertions::NotBlank(message: "Password should not be blank")]
@[Serialize::Assertions::Size(range: 0..50, min_message: "Password is too short", max_message: "Password is too long")]
property password : String

Currently i'm having to define a hash that contains the annotation name and fields. like:

TYPES = {
    NotBlank => [] of Symbol,
    Size => [:range, :min_message, :max_message]
  }

{% for ivar in @type.instance_vars %}
  {% for t, v in TYPES %}
    {% ann = ivar.annotation(t.resolve) %}
      {{t.id}}Assertion.new({{v.map { |f| ann[f]}.join(',').id}}) # new up assertion class
  {% end %}
{% end %}

Having this would drastically reduce the complexity, and make it easier to use. The ideal solution would be something like

{% for ivar in @type.instance_vars %}
  {% for annotation in ivar.annotations %}
    {{annotation.name.id}}Assertion.new({{annotation.keys.map { |f| "#{f.id}: #{annotation[f]}"}.join(',').id}})
  {% end %}
{% end %}

@asterite
Copy link
Member

That won't work because you are assuming the only annotations an ivar will have are the ones you are interested in.

@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented Oct 23, 2018

Would have to have some logic to make sure the current annotation is of a given name or something yea. I don't think there is a name property on an annotation, so that wouldn't be doable atm.

@RX14
Copy link
Contributor

RX14 commented Oct 24, 2018

@Blacksmoke16 By the time you add in the required {% if TYPES.includes? annotation %} to your ideal solution they're about the same complexity and code.

@Blacksmoke16
Copy link
Member Author

Blacksmoke16 commented Oct 24, 2018

Ideally would go with like {% if annotation.name.starts_with? "MyAnnotation" %} to remove the need for the mapping hash. Also would remove the need for a macro to add user defined annotations to the mapping.

Was unsure of the effort to implement this, if its too much can go ahead and close, just an idea i had working with this a lot.

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

No branches or pull requests

4 participants