RubyGateway icon indicating copy to clipboard operation
RubyGateway copied to clipboard

[WIP] Add dynamic callable and member lookup

Open toastersocks opened this issue 5 years ago • 7 comments

This adds dynamic callable and member lookup. I uncommented the commented out tests in TestDynamic.swift and they pass. Is this something you'd be open to merging? I can add the dynamic call syntax to a section in the README.md if you'd like.

toastersocks avatar Oct 10 '20 23:10 toastersocks

Codecov Report

Merging #27 into master will decrease coverage by 1.01%. The diff coverage is 36.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #27      +/-   ##
==========================================
- Coverage   97.85%   96.84%   -1.02%     
==========================================
  Files          25       25              
  Lines        1775     1805      +30     
==========================================
+ Hits         1737     1748      +11     
- Misses         38       57      +19     
Impacted Files Coverage Δ
Sources/RubyGateway/RbGateway.swift 95.87% <0.00%> (-2.02%) :arrow_down:
Sources/RubyGateway/RbObject.swift 80.76% <39.28%> (-11.39%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update dc75929...e55c083. Read the comment docs.

codecov-io avatar Oct 10 '20 23:10 codecov-io

Hmm, I'll add a test for the @dynamicCallable code that's not currently covered.

toastersocks avatar Oct 10 '20 23:10 toastersocks

Thanks for the PR. I would like to include some kind of support for this but I've never quite worked out a satisfactory model.

The part of this PR's approach that goes wrong is to do with methods with default arguments. Minimally consider:

class C
  def f(a = 2)
    a
  end
end

So C.new.f(1) is 1.

But because the approach 'eagerly' evaluates f before it sees the (1) it evaluates C.new.f(2)(1) instead.

I think my least bad solution to this ambiguity was to drop the eager dynamic member lookup part, forcing users to always write explicit method calls (). So C.new.f would yield some new 'partial method call' type that would support dynamic callable. I remember thinking it should be a separate type -- rather than an RbObject for the method for example -- to catch accidental misuse at compile-time. This design is unsatisfactory because it prevents the idiomatic Ruby omission of unnecessary (), though it is probably an improvement over the current call("methodname") pattern.

Perhaps your fresh eyes can see a better way through this!

johnfairh avatar Oct 11 '20 15:10 johnfairh

Oh, right. It didn't even occur to me that methods with default arguments would behave differently. Hmm, yeah I see what you mean about it not being ideal to have to put parentheses on every Ruby method that idiomatic Ruby treats more like properties. It does seem more "Swifty" to me though. As does getting a method reference back without the parens that you could potentially call later with parens. My first approach months ago was to use a separate type for callables, but I abandoned it because I didn't like the extra complexity, and I realized I could get a method reference using the Ruby runtime and still return a 'RbObject'. My Ruby knowledge is pretty basic. Maybe we can use the Ruby runtime to check for default arguments? This would keep things nicer at the call site, allowing you to call methods that take no arguments without parens. I'll look into this more.

toastersocks avatar Oct 12 '20 02:10 toastersocks

Actually, thinking about this more, It would add a lot of idiosyncrasy in that you'd have to only use parens to call a method if all its arguments have default values. Also some methods without parens would give you a method reference, and some would give you the result of the method, without a very good way to know beforehand without carefully checking the method signature and keeping these non-Ruby/non-Swift rules in mind. As a user, I think I would favor the consistency and more Swift-like behavior of a method name without parens gives you a method reference, and to get the result, you need the parens. As I said, my Ruby is pretty basic, so I don't know if this would make things too cumbersome with a lot of common Ruby APIs.

toastersocks avatar Oct 12 '20 02:10 toastersocks

And, I just realized this doesn't handle named parameters at all... I'm going to flesh this out more. Thanks for looking at this.

toastersocks avatar Oct 12 '20 02:10 toastersocks

favor [...] a method name without parens gives you a method reference

I agree with this. I think it's important that this type not be RbObject (it could wrap an RbObject & support RbObjectConvertible) so that the Swift type system prevents users from forgetting to add the parens, which would be a natural mistake coming from Ruby / porting Ruby code.

e: thinking about it, RbObjectConvertible compliance is probably not good for the same reason, too easy to accidentally use as an argument.

named parameters

Yes it should just be boilerplate to add the call variants once the object model is sound.

johnfairh avatar Oct 12 '20 09:10 johnfairh