ysbaddaden/earl

`Earl::Artist(AbstractType)` raises `#call(AbstractType) must be defined`

Opened this issue · 3 comments

y8 commented

This example fails to compile with Error: method Bar#call(AbstractMessage) must be defined:

require "earl/artist"

abstract struct AbstractMessage
end

record FooMessage < AbstractMessage

module Foo
  macro included
    include Earl::Artist(AbstractMessage)
  end

  def call(event : FooMessage)
    p "event: #{event}"
  end
end

class Bar
  include Foo
end

bar = Bar.new
bar.spawn
bar.send FooMessage.new

sleep

But there is call(event : FooMessage) implementation in module Foo so it should not raise the error.

Oh, this may be my custom macro to circumvent a crystal limitation that's failing because FooMessage is a descendant of AbstractMessage while the macro expects an AbstractMessage.

See

earl/src/artist.cr

Lines 26 to 68 in 3f4c145

def call(message : M)
# this should be an abstract def, but if M is an union and the artist
# implements specific overloads for each type in the union this would fail
# to compile because crystal expects an explicit `call(Foo | Bar)` def to
# exist.
#
# instead of merely raising, we try to detect which overload exist to
# report a helpful message to the developer.
#
# See https://github.com/crystal-lang/crystal/issues/8232
{% if M.union? %}
{% types = [] of String %}
{% for t in M.union_types %}
{% for type in t.stringify.split(" | ") %}
{% types << type %}
{% end %}
{% end %}
{% for fn in @type.methods %}
{% if fn.name == "call" && fn.args.size >= 1 %}
{% types = types.reject do |type|
fn.args[0].restriction.stringify.split(" | ").includes?(type)
end %}
{% end %}
{% end %}
{% for ancestor in @type.ancestors %}
{% for fn in ancestor.methods %}
{% if fn.name == "call" && fn.args.size >= 1 %}
{% types = types.reject do |type|
fn.args[0].restriction.stringify.split(" | ").includes?(type)
end %}
{% end %}
{% end %}
{% end %}
{% raise "Error: method #{@type}#call(#{types.join(" | ").id}) must be defined" %}
{% else %}
{% raise "Error: method #{@type}#call(#{M}) must be defined" %}
{% end %}
end

If you want to have a look: The includes? are most likely at fault (they expect the exact type. Maybe the whole macro isn't working for different other edge cases...

y8 commented

Yeah, I've figured about this macro

The includes? are most likely at fault (they expect the exact type

In this case macro should look if particular class implements overloads for all AbstractMessage subclasses and raise pricing ones that are not implemented, right?

Yeah, for each possible type, the macro should check that there is an overload with that explicit type (in which case the type + all its descendant types are fully implemented) or if all its descendant types have an overload (in which case the type is fully implemented).

But that will start to be a bit complex (a type can have a tree of descendants) 😨