Multiple arg types is kinda messed up

[xendk]

Trying to use the new multiple arg methods, I'm running into problems that might kill the multiple args idea. Take this:

      route context do |r, response, session|
        r.on "snapshot", String do |_, snapshot_id|
          snapshot = @project.snapshots[snapshot_id]

Which results in Error: expected argument #1 to 'Appocular::Storage::YAML(Appocular::Snapshot)#[]' to be String, not (Bool | String).

A bit of testing shows that the type of the block parameters is actually the union of the types returned by the called #match? methods (but only the ones that's actually called). Turns out that Tuple#map is just a macro that constructs a new tuple by inlining the block code. But the return type of self#[] would be the union of the types of the union, which in turn makes the return type of our block in on to the union of the return types of the match? methods.

Of course one can do as(Whatever) in one's on blocks, but I think it kinda ruins the DSL.

One solution that might work is making the multiple arg #is/#on into macros that unroll into nested single argument calls. That ought to make the compiler capable of pinpointing the exact type at each level, but I don't know if such a macro is possible.

What's your thoughts?

[jgaskins]

I feel like there's a way to do this because this works as we would expect:

tuple = {1, "hello", nil}
value = tuple[0]
pp typeof(value) # => Int32
value = tuple[1]
pp typeof(value) # => String
value = tuple[2]
pp typeof(value) # => Nil

But that could just be because I'm using numeric literals because passing a variable gives the union:

tuple = {1, "hello", nil}
tuple.size.times do |i|
  value = tuple[i]
  pp typeof(value)
end

(Int32 | String | Nil)
(Int32 | String | Nil)
(Int32 | String | Nil)

The weird thing,to me, is that the code that Tuple#map generates calls them with numeric literals:

Tuple.new(
  (yield self[0]),

  (yield self[1]),

  (yield self[2]),
)

So it seems like it would work, but I think because the variable yielded to the block is the union of all of the types, the block result becomes a union as well. So maybe we need to sort this out with a macro rather than a method.

[jgaskins]

With a bit of indirection and screwing around with macros, I was able to get this to work in a quick proof-of-concept:

require "uuid"

Request.new("/foo/bar").on "foo", String do |foo, string|
  pp foo: typeof(foo), string: typeof(string)
end

class Request
  property path : String

  def initialize(@path)
  end

  def on(*segments)
    on segments do |captures|
      yield captures
    end
  end

  def on(segments : Tuple(*T)) forall T
    {% begin %}
      captures = {
        {% for i in 0...T.size %}
          begin
            matcher = segments[{{i}}]
            if (match = %r(\A/?[^/]+).match path.sub(%r(\A/), "")) && (%result{i} = match?(match[0], matcher))
              self.path = path.sub(%r(\A/?#{match[0]}), "")
              %result{i}
            end
          end,
        {% end %}
      }

      if captures.any?(&.nil?)
        return
      else
        yield({
          {% for i in 0...T.size %}
            captures[{{i}}].not_nil!,
          {% end %}
        })
      end
      {% debug %}
    {% end %}
  end

  def match?(segment : String, matcher)
    matcher === segment
  end

  {% for type in %w[Int UInt] %}
    {% for size in %w[8 16 32 64 128] %}
      def match?(segment : String, matcher : {{type.id}}{{size.id}}.class)
        segment.to_{{type[0..0].downcase.id}}{{size.id}}?
      end
    {% end %}
  {% end %}

  def match?(segment : String, matcher : Symbol | String.class)
    segment
  end

  def match?(segment : String, matcher : UUID.class)
    UUID.parse? segment
  end

  def match?(segment : String, matcher : Regex)
    matcher.match segment
  end
end

It doesn't work completely yet (some variables are bleeding over for subsequent matchers), but the types are being constrained properly.

[jgaskins]

Got it, it was because I was setting self.path inside the macro. This works because I'm setting it as a local variable and only setting self.path if and when we're about to yield:

require "uuid"

%w[
  /foo/bar
  /posts/c31b6980-e0fb-439a-af0b-b13b8224fec5
].each do |path|
  pp r = Request.new(path)

  r.on "foo", String do |foo, string|
    pp({ {"foo", String} => {foo: typeof(foo), string: typeof(string)} })
  end

  r.on "posts", UUID do |posts, post_id|
    pp({ {"posts", UUID} => {posts: typeof(posts), post_id: typeof(post_id)} })
  end
end

class Request
  # ...

  def on(segments : Tuple(*T)) forall T
    {% begin %}
      path = self.path
      captures = {
        {% for i in 0...T.size %}
          begin
            %matcher{i} = segments[{{i}}]
            if (%match{i} = %r(\A/?[^/]+).match path.sub(%r(\A/), "")) && (%result{i} = match?(%match{i}[0], %matcher{i}))
              path = path.sub(%r(\A/?#{%match{i}[0]}), "")
              %result{i}
            end
          end,
        {% end %}
      }

      if captures.any?(&.nil?)
        return
      else
        self.path = path
        yield({
          {% for i in 0...T.size %}
            captures[{{i}}].not_nil!,
          {% end %}
        })
      end
    {% end %}
  end

  # ...
end

[xendk]

Trying to hack it into the code I get:

Showing last frame. Use --error-trace for full trace.

There was a problem expanding macro 'macro_139768752891200'

Code in src/route.cr:130:9

 130 | {% begin %}
       ^
Called macro defined in src/route.cr:130:9

 130 | {% begin %}

Which expanded to:

 > 12 |             
 > 13 |               begin
 > 14 |                 __temp_2400 = segments[1]
                                              ^
Error: index out of bounds for Tuple(Tuple(String, String)) (1 not in -1..0)

On the first multiple arg test, which I don't get. The double tuple (hehe) seems wrong to me, but how it ends up there?

[jgaskins]

I got it to compile but it does fail a couple specs:

  1) Armature::Route matches requests to dynamic routes using symbols
     Failure/Error: match.should eq "bar"

       Expected: "bar"
            got: {"bar"}

     # spec/route_spec.cr:99

  2) Armature::Route matches requests to dynamic routes with named types
     Failure/Error: match.should be_a Int64

       Expected {123} (Tuple(Int64 | Regex::MatchData | String | UUID)) to be a Int64

     # spec/route_spec.cr:144

I think this one is because it's yielding a tuple containing a single value. When your block accepts 2 arguments, the compiler knows to splat the arguments, but it's ambiguous with a 1-element tuple.

[jgaskins]

I got it working in #5 by splatting the tuple explicitly like we were doing before. 😄