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

Add finished macro hook #3612

Merged
merged 1 commit into from
Dec 1, 2016
Merged

Add finished macro hook #3612

merged 1 commit into from
Dec 1, 2016

Conversation

asterite
Copy link
Member

@asterite asterite commented Nov 30, 2016

Fixes #3604 , fixes #2987

This adds a new macro hook: finished. I chose that name because it's a past participle and similar/compatible with included, inherited, extended. This macro can be thought as if it's placed at the end of the program, though scope is preserved.

For example, to define a base JSON mapping and let users extend it with custom (typed) attributes, one would do:

require "json"

class Foo
  # This is our base mapping.
  # Remember that in Crystal constants aren't typed unless used
  # in a program. If we only use them at compile time, we can
  # use them to remember and gather data at compile time.
  MAPPING = {
    x: Int32,
    y: Int32,
  }

  # We tell users to use this for extending JSON attributes
  macro extra_attributes(**values)
    # We mutate MAPPING at compile time
    {% for key, value in values %}
      {% MAPPING[key] = value %}
    {% end %}
  end

  # This runs once the bottom of the program is reached.
  # Scope is preserved, so MAPPING refers to the above constant.
  macro finished
    JSON.mapping({{MAPPING}})
  end
end

# One extension
class Foo
  extra_attributes z: Int32?
end

# Another extension
class Foo
  extra_attributes w: Int32?
end

# All of these work
p Foo.from_json(%<{"x": 1, "y": 2}>)
p Foo.from_json(%<{"x": 1, "y": 2, "z": 3}>)
p Foo.from_json(%<{"x": 1, "y": 2, "z": 3, "w": 4}>)

In fact, with this some APIs could be defined more nicely. Imagine a mapping to a DB model:

class ModelBase
  macro inherited
    # Here we capture all properties. The `of Nil => Nil` is needed for syntax,
    # but the contents will never be used at runtime, so no problem
    PROPERTIES = {} of Nil => Nil

    macro finished
      process_properties
    end
  end

  macro attribute(decl)
    {% PROPERTIES[decl.var] = decl.type %}
  end

  macro process_properties
    # Here we process properties to define methods, instance variables, etc.
    {{ puts PROPERTIES }}
  end
end

class User < ModelBase
  attribute name : String
  attribute age : Int32
end

# Without finished one would probably need to specify all attributes in one go:
# class User < ModelBase
#   attributes({
#     name : String,
#     age : Int32
#   })
# end

In fact, we could change JSON.mapping with this "additive" behaviour, though we probably won't do it because mappings are usually pretty well defined and "closed".

@kostya
Copy link
Contributor

kostya commented Nov 30, 2016

if it would be like this:

class Bla < Foo
  extra_attributes z: Int32?
end

it finished both classes?, schema of Foo, would be x,y and Bla: x,y,z?

@bcardiff
Copy link
Member

So the following code prints 2 and not 1, right?

class Foo
  def bar
    1
  end

  macro finished
    def bar
      2
    end
  end
end

puts Foo.new.bar # => 1 or 2 ?

It there an order guaranteed for iterating the finished macros across types?

@asterite
Copy link
Member Author

asterite commented Nov 30, 2016

@kostya There's only one finished macro and only one MAPPING. The mapping created for Foo and Bla will be the same. This doesn't cover extending a class and extending the mapping.

@bcardiff It prints 2. finished macros are executed sequentially.

@Sija
Copy link
Contributor

Sija commented Nov 30, 2016

@asterite Would it be sensible/possible to add super covering extending classes?

@bcardiff
Copy link
Member

I think the extend+subclass might be covered if macro inherited is combined with macro finished. Am i right?

@asterite
Copy link
Member Author

I think the whole mapping must be repeated in the subclass. The problem is that both parsing and generating JSON for an object happens inside {...}, so calling super would duplicate those calls.

I'll try to see how extending JSON mapping can be done, I tried several ways and they all fail because some macro power is missing... (though the original use case of this PR wasn't this one)

@asterite asterite added this to the 0.20.1 milestone Dec 1, 2016
@asterite asterite merged commit 258be5b into master Dec 1, 2016
@alex-fedorov
Copy link

Great feature! Thank you

@samueleaton
Copy link
Contributor

Where are these kind of things documented?

@asterite
Copy link
Member Author

It's not documented because it's experimental. I'm not sure it was a good choice yet.

@samueleaton
Copy link
Contributor

I wonder if the good people over at kemal know that its experimental :) https://github.com/kemalcr/kemal/pull/339/files#diff-855bbab0979fea823de24052246e6982R10

@asterite
Copy link
Member Author

Well, the whole language is experimental for that matter, until we reach 1.0

I don't quite understand the purpose of that store property, though...

@sdogruyol
Copy link
Member

@asterite it's for storing objects between Handlers and filters. We use HTTP::Context for that

@asterite
Copy link
Member Author

@sdogruyol Why not add individual properties to HTTP::Context as users need them?

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

Successfully merging this pull request may close these issues.

macro shared state Additive JSON mapping?
7 participants