Remove method_missing macro hook

[straight-shoota]

This request is extracted from #4490 to keep the issue more focused.

method_missing was copied over from Ruby but has quite some problems in a statically typed language, which particularly emerge when used in combination with responds_to?: responds_to? will report false for a method defined by method_missing unless it is invoked somewhere else in code, because responds_to? has no way of knowing which methods might be implemented through method_missing unless the method call is actually initialized. This also means that such methods will not be included in the documentation.

There are a number of use cases for method_missing and it is actively used in many Crystal projects (c.f. uses in Crystal repositories on Github - though many are still using the signature from pre-0.18.0). These use cases however are actually quite different and in some cases macros might be an equal or even better approach.

• Transparent delegation (mostly through forward_missing_to) is probably one of the most important use cases. This can be replaced by either a direct call to delegate (with a predefined set of delegated methods) or a macro which dynamically delegates all methods of the receiving object's class.

  delegate {% for method in WrappedClass.methods %}:{{ method.name.id }}, {% end %} to: @wrapped

  In combination with the finished macro, this provides nearly the same functionality as forward_missing_to and works well with responds_to?. This would not include method defined in a finished hook which is invoked after the on in the delegator class, but I'd consider this negligible. A simple delegator macro should replace forward_missing_to in the stdlib. It would also be nice if delegate could optionally accept a block or proc to wrap delegated methods for example in a try or rescue block or modify the return value.

• Dynamic accessors are also quite popular (for example for easily accessible configuration), an example of a simplified macro hook:

macro method_missing(call)
  @store[{{call.name.stringify}}]
end

This is merely a nice frontend for a Hash (or an other backing data structure) so instead of config["conf_key"] you can write config.conf_key. I don't think this adds a real benefit. As an alternative, a custom struct or class would also provide proper type restriction instead of always returning a union type if the store contains values with different types.

• In some cases it is just used as syntactic sugar to make method calls look a bit nicer.

  macro method_missing(call)
  log({{call.name.stringify}}, {{*args}})
  end

  This does not add much value IMHO and could be replaced by more explicit code.

• Dynamic DSL-methods which transform a string into a data value, like ActiveRecord's find_by_first_name_and_email(first_name, email) which is essentially equal to find_by(first_name: first_name, email: email). I did not find any evidence, that this is actively used in Crystal, but it is possible. This could probably be achieved by creating all possible combinations of method names through macro loops.

As a sidenote, the availability of method_missing might be a good selling point to Ruby developers.

[paulcsmith]

Thanks for putting this together in such a well written issue.

I like method_missing for this use case and I haven't found an alternative that I like better. So I'd be for keeping method_missing (unless there is some other way I haven't thought of to get this functionality:

abstract class BaseBox
  private getter form

  macro form(form_class)
    @form = {{ form_class }}.new
  end

  macro inherited
    form {{ @type.name.gsub(/Box/, "::BaseForm").id }}
  end

  macro method_missing(call)
    form.{{ call.name }}.value = {{ call.args.first }}
    self
  end

  def save
    form.save
  end
end

This is so that you can easily create factories and makes it easy to chain methods.

class UserBox
  def admin
    role_id find_admin_role_somehow.id

    # Instead of...
    form.role_id.value = find_admin_role_somehow.id
    self
  end
end

UserBox.new.admin.save

I could see that some people think this is too magic, but I've found it to be super helpful in this case and I would be sad to not be able to do it.

[straight-shoota]

I don't really get what your example is about (seems like there are a few bits missing), but it's essentially a delegation.

You could probably implement this with a custom macro similar to what delegate does:

{% for method in form_class %}
def {{method.id}}(val)
  form.{{method.id}}.value = val
  self
end
{% end %}

[paulcsmith]

Yeah there is a lot missing in the example, but you seemed to get it because I think that example will work. I'll give it a try. Thanks!

[Papierkorb]

Please don't break the language. method_missing is an integral part of writing delegators for the decorator pattern without having to manually delegate each and every method onwards. Using method_missing through the forward_missing_to makes it a delight to write such delegator classes, encouraging maintainable business logic.

Use-case example

Say you have a bunch of models: User with name and password fields, and Article with body and write_key. In these two models, the password nor the write_key fields should be accessible to the (internet) public, they must not be exposed through JSON.

Thus, for this the following example ensures that upon calling #to_json, our models can't expose these fields, while allowing usual access to other fields. Also useful as upgrade path, so that old code can remain working while you're working on improving the code base.

class JsonDecorator(T) # Generic JSON decorator
  property inner : T # The inner object
  forward_missing_to @inner # And forward all other methods on to @inner.
  # This is the magic that let's us easily write this kind of decorator without fuzz,
  # without cluttering this code with tons of delegator methods (Like Java requires),
  # and without requiring the user to use an editor/IDE to generate these as to not break their hands writing all of that mess.

  property json_exclude : Array(Symbol)

  def initialize(@inner, @json_exclude)
  end

  # And now, we can easily implement our special #to_json, which actually just calls the
  # inner to_json method but with our default argument for exclusion.
  def to_json
    @inner.to_json(exclude: @json_exclude)
  end
end

Note: The code above most likely violates the JSON API and is only meant as show-case example.

Using this technique, it becomes trivial to safely generate a JSON document of this, without being able to forget to pass anything special, and without cluttering each original model with extra logic. It also allows to dynamically change the behaviour of the inner model: For example, the owner of an Article may be allowed to see the restricted write_key. When the owner now accesses the article resource, the web application may choose to either not add write_key to the exclusion list, or not use that decorator at all. The code using the found article model instance doesn't care: It'll work just fine with both.

With regards to https://github.com/crystal-lang/crystal/issues/4701#issuecomment-314551722, this makes such code harder to write for no real gain. It's trivial right now to easily write more special rules if needed - And that's a feature, not a bug. A language is there to serve me, it however can expect me to know what's right. We don't need another Go lang.

[straight-shoota]

As I have written in the initial post, with method_missing removed, there should be a delegator macro to replace forward_missing_to. I am confident that this can be achieved with the remaining macro language with only minor differences to the current behavior from a usage perspective. This way objects will play nice with responds_to? and generate API documentation for delegated methods. In your example, an instance of JsonDecorator would report responds_to?(:write_key) as false unless this method is invoked somewhere.

You example is in a way special, that the methods provided by method_missing depend on the generic type T. I don't think a type's instance methods should be mutated based on a generic type argument. In my opinion that's abusing generics in a way they're not meant to be. Therefore, without method_missing, this would not be possible. I'd actually consider this another reason for method_missing to be removed. It find it quite strange that JsonDecorator(String) and JsonDecorator(Int32) would have completely different instance methods - and don't even fully report which they are responding to!

That doesn't mean that your use case would be completely impossible, it needs to be implemented in a different way.

[mvz]

If I understand correctly the main problem with method_missing stated here is that respond_to? gives the wrong results. This exact problem is why in Ruby it is recommended to also implement respond_to_missing?. So, instead of dropping method_missing and all its foreseen and unforeseen uses, why not attempt to solve the respond_to? problem in some way?

I don't think a type's instance methods should be mutated based on a generic type argument.

Isn't that exactly how you'd expect a generic decorator to behave?

[straight-shoota]

I don't think a type's instance methods should be mutated based on a generic type argument.

Isn't that exactly how you'd expect a generic decorator to behave?

Yes, it is. But I am not sure this should be in a static typed language at all. Besides, I can't think of many usecases where you would really need this and can't accomplish with other features of Crystal.

And even if we'd agree to have generic decorators with variable interfaces, that would not necessarily require method_missing? but can surely be implemented in a different way (maybe a special case in the compiler or some macro-fu). We're basically talking about two individual features:

• Having method_missing? for arbitrary purposes
• Having generic decorators

Regarding your suggestion: I don't think respond_to_missing? is of much help because it would be difficult to ensure consistency. And it would not add discoverability for type inspection or API doc generator.

[Papierkorb]

I still don't see the problem. Is bad method_missing usage so prevalent that it's an issue? There are a few select users of it. I especially don't get the "doesn't fit" argument: It works, really good at that, is controllable, doesn't incur any performance penalty, and is first and foremost perfectly type safe.

I also want a powerful language. A language which let's me try something new, and a language which trusts me to not do something totally stupid - as something totally stupid can be done in any language, regardless of the feature set. If I didn't want that, I would've chosen Go-lang.

[mvz]

We're basically talking about two individual features:

• Having method_missing? for arbitrary purposes
• Having generic decorators

Yes, and for the case of the second feature I'm arguing with your assertion that a generic decorator should not vary its set of instance methods based on its type parameter in a statically typed language (regardless of how that variation is implemented). If it cannot do this, it severely limits the set of types it can usefully decorate.

Regarding your suggestion: I don't think respond_to_missing? is of much help because it would be difficult to ensure consistency. And it would not add discoverability for type inspection or API doc generator.

Consistency could be ensured by ensuring respond_to_missing? is called before method_missing. Whether callable methods can be enumerated for human inspection may or may not be useful depending on the use case so there is no real reason to enforce something there.

Allowing objects to respond to arbitrary messages lies at the core of the object-oriented programming paradigm that Crystal inherited from Smalltalk via Ruby. It is a great strength of Crystal that it can combine this with compile-time enforced type safety.

[straight-shoota]

But what if respond_to_missing? says "yes" but method_missing does not actually respond? Or the other way around? You'll always have duplicated code and that will lead to consistency problems.

Being able to send arbitrary messages is nice, I know. But what for? In Crystal we have great macro features, so you can just batch generate a bunch of methods instead of putting them together on demand. Apart from that there are only very limited use-cases left out and I still can't think of a compelling one that would legitimize having this feature in the language. But that's just my opinion. I'm glad there are people who disagree ;)

[zw963]

Not complete reading comments, but please keep this good part(come from Ruby) as much as possible.

when you use method_missing macro, you should respect the risk and various potential impacts, and point it out in document is useful.