krzysztofzablocki / Sourcery

Meta-programming for Swift, stop writing boilerplate code.
http://merowing.info
MIT License
7.59k stars 605 forks source link

Templates currently can't express all logic #22

Closed conradev closed 7 years ago

conradev commented 7 years ago

I have been trying to do things like this:

{% for type in types.implementing.AutoEquatable %}
extension {{ type.name }}: Equatable {}

func == (lhs: {{ type.name }}, rhs: {{ type.name }}) -> Bool {
    {% for variable in type.storedVariables %}
    {% if variable.type implements Equatable or variable.type implements AutoEquatable %}
    if lhs.{{ variable.name }} != rhs.{{ variable.name }} { return false }
    {% endif %}
    {% endfor %}
    return true
}
{% endfor %}

or this:

{% for enum in types.enums where enum.name beginswith Gallery %}
extension {{ enum.name }} {
  static var count: Int { return {{ enum.cases.count }} }
}
{% endfor %}

and while operators like where ... beginswith and if ... implements are certainly possible to add to the template language, won't this project be hindered by the fact that the template language is not Turing-complete? Would it be possible to implement a library to build a Swift AST from Swift?

krzysztofzablocki commented 7 years ago

Stencil implementation we have here is not yet tweaked for more native code generation, we are actually splitting SwiftGen Stencil related extension into separate framework for Swift code generation and both tools will be using it. There are plenty of more useful nodes coming like set, map and function call that will significantly extend what's doable to express here.

But even so, I'm keeping the Parser, Models and Generator separate, because I've been experimenting with some other ideas, like generating code via native Swift by just calling swiftc and feeding it all model types as definitions. Is that something you'd like to explore, or think could be a better approach?

Right now I'm focusing most of my time on improving the Parsing and Model reflections, both functionality from templates you are showing we can support with small changes to Models and a few extra nodes, for example we could modify the first sample by the following:

{% for type in types.implementing.AutoEquatable %}
extension {{ type.name }}: Equatable {}

func == (lhs: {{ type.name }}, rhs: {{ type.name }}) -> Bool {
    {% for variable in type.storedVariables %}
    {% if variable.type.implements.Equatable or variable.type.implements.AutoEquatable %}
    if lhs.{{ variable.name }} != rhs.{{ variable.name }} { return false }
    {% endif %}
    {% endfor %}
    return true
}
{% endfor %}

fwiw. for second example I really prefer using phantom protocols as it's less error prone upon refactoring e.g. protocol AutoCountableEnum {} and just extending your type with it

ilyapuchka commented 7 years ago

Actually or is already supported by Stencil 0.7.0. {% for %} tag also now supports filter, but it looks like there is no standard filter like beginesWith, so we will need to add it. Then it will look like this I think:

{% for enum in types.enums|beginswith:"Gallery" %}
extension {{ enum.name }} {
  static var count: Int { return {{ enum.cases.count }} }
}
{% endfor %}

Btw @krzysztofzablocki do you think we should just add our custom filters as soon as we need them or contribute to Stencil instead? Would be cool if Sourcery will drive improvements in other projects 😀

krzysztofzablocki commented 7 years ago

@ilyapuchka we want to extract part of @alisoftware SwiftGen stencil extensions into separate framework and link both SwiftGen and Sourcery to it, then we can add things that make sense for Swift code-generation and both tools can leverage them.

Edit: https://github.com/AliSoftware/SwiftGen/issues/240, not sure if we can send a PR to SwiftGen extracting parts of it or does Oliver wants to wait for some PR's to get merged / new big release first, @alisoftware what do you prefer ?

If wait, then I think in the meantime we can take some nodes from SwiftGen and embeed them into Sourcery and switch to framework later on

krzysztofzablocki commented 7 years ago

@kzaher pretty much what I was doing that, parser / command line works without changes, and I just code-generate Type definitions as the plumbing code and then it's prefixed between your template code and compiled via swiftc

I find Stencil templates more readable now after writing few of them rather than using pure swift via prints, what's your pain point with Stencil templates?

AliSoftware commented 7 years ago

What if I have some functionality or extensions that I want also reuse across templates, can I write simply import MyXXXExtensions, I guess not

Actually you soon will be able to. Stencil contains a {% import xxx %} tag to import other templates already, and when we finish merging the Stencil tags added by SwiftGen into a dedicated framework that Sourcery can use too, it will also have tags to define functions/macros and call them later.

krzysztofzablocki commented 7 years ago

@kzaher you should be using --watch daemon when writing templates as it allows you to write them in real-time and side-by-side with generated code + it recovers from template mistakes that lead to crashes in normal mode.

it's not types.enums.implementing.AutoEquatable, the types reflection is offering top level filter so it's types.implementing.AutoEquatable, to support types.enums.implementing.AutoEquatable we'd need to tweak the reflection type a little which sounds it might be useful. On current version you'd have to do something like:

{% for type in types.implementing.AutoEquatable %} {% if type.kind == "enum" %}
// Logic
{% endif %} {% endfor %}

Edit: I exposed accessLevel as string in 0.4.7 so you should be able to render it properly now

kzaher commented 7 years ago

Hi @krzysztofzablocki ,

can you maybe share with me what is the best way to autoimplement Hashable and Equatable for enum. That's the last thing I need.

{% for type in types.implementing.AutoEquatable %} {% if type.kind == "enum" %}
    extension {{ type.name }} {
        public static func == (lhs: {{ type.name }}, rhs: {{ type.name }}) -> Bool {
            switch (lhs, rhs) {
            {% for case in type.cases %}
            {% if case.associatedValues.count == 0 %}
                case ({{ case.name }}, {{ case.name }}): return true
            {% else %}
                case let ({{ case.name }}({% for associatedValue in case.associatedValues %}lhs_{{ associatedValue.name }}, {% endfor %}), {{ case.name }}({% for associatedValue in case.associatedValues %}rhs_{{ associatedValue.name }}, {% endfor %}):
                {% for associatedValue in case.associatedValues %} if lhs_{{ associatedValue.name }} != rhs_{{ associatedValue.name }} { return false }
                {% endfor %}
                return true
            {% endif %}
            {% endfor %}
            }
        }
    }
{% endif %} {% endfor %}

This is my best attempt. I've found join and capitalize. The problem with my solution is that I need to do values.map { "lhs\($0)" }.joined(separator: ", ").

If I use for construct then I don't know how to remove trailing ,. If I use join, I don't know how to do "lhs\($0)".

krzysztofzablocki commented 7 years ago

once we have the extra nodes from @AliSoftware framework it will be simpler since we'll have map node :) for now I use simple separate statements, here is the body of the comparsion loop I've been using in NYT app:

func == (lhs: {{ type.name }}, rhs: {{ type.name }}) -> Bool {
    switch (lhs, rhs) {
        {% for case in type.cases %}
        {% if case.hasAssociatedValue %} case (.{{ case.name }}(let lhs), .{{ case.name }}(let rhs)): {% else %} case (.{{ case.name }}, .{{ case.name }}): {% endif %}
            {% ifnot case.hasAssociatedValue %} return true {% else %}
                {% if case.associatedValues.count == 1 %}
                    return lhs == rhs
                {% else %}
                    {% for associated in case.associatedValues %} if lhs.{{ associated.name }} != rhs.{{ associated.name }} { return false }
                    {% endfor %} return true
                    {% endif %}
            {% endif %}

        {% endfor %}
        default: return false
    }
}

It generates stuff like this (which is actually more convienent when working on production code and having to debug why things arent equal because you can step over them wheres we can't with && and ||):

case (.LargeImageInsetBanner(let lhs), .LargeImageInsetBanner(let rhs)): 
  if lhs.cellData != rhs.cellData { return false }
  if lhs.promotionalImageCrop != rhs.promotionalImageCrop { return false }
  if lhs.promotionalImageCredit != rhs.promotionalImageCredit { return false }
  return true
case (.Advertisement, .Advertisement): 
  return true 
kzaher commented 7 years ago

Thnx,

how about for Hashable. I've found this hacky way with 0.

return 0 {% for associatedValue in case.associatedValues %} ^ {{ associatedValue.name }}.hash {% endfor %}

... but would like to remove it. Is there any way to do this?

krzysztofzablocki commented 7 years ago

Word of caution: using ^ for hashing is not going to work well, I made that mistake in the past and we had bugs that were really hard to track, you need to use something else like djb2 or alternative algo, we based our on boost::hash_combine implementation, read more here: http://stackoverflow.com/a/27952689

Once we include the aformentioned map node this should be simple to get rid of, right now I also had the 0 prefix ¯\(ツ)/¯

kzaher commented 7 years ago

Thnx for the tip.

I guess I could rewrite hash to be something like:

{% for type in types.implementing.EnumAutoEquatable %}
// Enum auto hashable
extension {{ type.name }}: Hashable {
    public var hashValue: Int {
        switch self {
        {% for case in type.cases %}
        {% if case.associatedValues.count == 0 %}
            case .{{ case.name }}:
                return 1
        {% else %}
        {% if case.associatedValues.count == 1 %}
            case let .{{ case.name }}(value):
                var hash: Int = 5381
                {% for associatedValue in case.associatedValues %}hash = ((hash << 5) &+ hash) &+ value.hashValue
                {% endfor %}
                return hash
        {% else %}
            case let .{{ case.name }}(values):
                var hash: Int = 5381
                {% for associatedValue in case.associatedValues %}hash = ((hash << 5) &+ hash) &+ values.{{ associatedValue.name }}.hashValue
                {% endfor %}
                return hash
        {% endif %}
        {% endif %}
        {% endfor %}
        }
    }
}

{% endfor %}

Is there any way I could get associatedValue index so I can encode particular enum case ordinal in hashValue?

Is there something like enumerated?

AliSoftware commented 7 years ago

@kzaher inside a {% for %} block, Stencil gives you access to a forloop dictionary variable containing the sub variables forloop.first (true if first iteration), forloop.last (true if last iteration) andforloop.counter (containing the current iteration number)

You can thus use {{ forloop.counter }} to get your associated values index while iterating over them. You can also use things like {% if not forloop.last %}, {% end if %} to only add a comma if it's not the last iteration. That should allow you to solve your problem until we integrate the {% map %} node in a future version.

kzaher commented 7 years ago

Thnx

krzysztofzablocki commented 7 years ago

0.5.2 has experimental support for Swift templates which means we have access to Turing-Complete language, thanks @kzaher