No indication of which method is used when two libraries have the same method in a re-opened class.

[MistressRemilia]

Let's say I have a program that depends on two Shards, Shard1 and Shard2. In Shard1, the String class is reopened and a new method called some_method is added. Shard2 has the same thing, a re-opened String class with a method called some_method. The order that I require the shards seems to dictate which version of some_method is used.

I didn't find anything about this in the docs, or a previously opened issue, but my searching skillz may be a bit lacking 😅

• What aspect of the language would you like to see improved?
  A warning/note message stating that one library overrode the method introduced in another library would be helpful, or explicit documentation about this. Or both.

• What are the reasons?
  If I wasn't aware that this depended on the order of requires, in a complex program, this could be quite confusing without indication of which method the compiler is choosing.

• Include practical examples to illustrate your points.
  There's a very small project to illustrate this here: https://gitlab.com/RemiliaScarlet/reopened-class-example

[asterite]

Hi!

Redefining a method with another one is the language's nature. It works like that in Ruby too. And, like in Ruby, you get no indication about when that happens.

I'm... not sure there's anything actionable here.

[Blacksmoke16]

Of course a solution to this is to just not randomly monkey patch common types like that. Maybe there would be room for introducing the final keyword in Crystal that would allow a shard developer to mark a type such that it can't be reopened, thus preventing this issue in the first place. While powerful, monkey patching can be dangerous and should be used responsibly imo.

Definitely not saying we should make common types like String final however.

[straight-shoota]

Crystal considers the entire codebase a single unit and there is no compartmentalization between code from different libraries, or stdlib. To the compiler, it's all the same and it can't really tell whether something defined in location A conflicts with the same thing in location B.

Of course, the compiler knows when a method definition shadows another one. But that's normal and often expected behaviour. It shouldn't cause a warning.
And I suppose it would be the same if the compiler could differentiate between different library sources: It would still be unclear whether an override is intended or accidental. An answer to this could be an @[Overrides] annotation or something similar (see #1647).

[straight-shoota]

I came to think of a practical enhancement that would make it easier to deal with such issues.
The main problem associated with this is that you call a method (such as String#someMethod in the example) with a signature that you expect to match a specific def. But when the code executes, you don't see the effect of that def.
I've found myself in this situtation and I assume @MistressRemilia might have had a similar experience. It can be very confusing.

Note that this is not just about different shards implementing a method with the same name, but also overloads of a method in the same library with different signatures etc. When it's a complete shadowing like in the example case, the actual target def can be determined statically. When there are multiple matching defs which are dispatched at runtime, it gets even more complicated.

You might be looking for bugs in other parts of the implementation before you realize that perhaps the call goes to some other def. And even when you've realized that, it can be tricky to figure out which def actually gets called.

The crystal tool implementations can be useful for that. It prints the target defs of a call. Using it on the example repo, shows the actuall target def:

$ crystal tool implementations src/main.cr -c src/main.cr:4:7
1 implementation found
/home/johannes/src/crystal-lang/crystal/reopened-class-example/lib/shard1/src/shard1.cr:2:3

For cases such as this example, it can be a big help. I can easily see which def gets called and can jump to the location. That should be enough to get you going.

But this tool is far from easy to use.

It would be really nice if we could have a macro for this, which would make it super easy to use when you just put it in the code at the call you want to inspect:

tell_me_where_this_ends_up "".someMethod # => /home/johannes/src/crystal-lang/crystal/reopened-class-example/lib/shard1/src/shard1.cr:2:3

This should be easy to do, the code already exists and should be possible to hook into the macro language. Inspecting implementations at compile time would be super nice.

Now with dynamic dispatch, it is much more complicated to figure out the actual target def. We'd need some enhanced run time reflection for that.
It could be relatively simple to implement in the interpreter. Maybe we can try that?

For native code, I'm not sure if we could achieve anything with reasonable effort. A solution that might be relatively easy to do would be injecting some instrumentation into the target defs which only becomes active after.

module Crystal::Reflection
  class_property bleep = false
  
  def self.bleep(deff, location)
    if bleep
      puts "#{deff} #{location}"
    end
    bleep = false
  end
end
 
macro tell_me_where_this_ends_up(call)
  Crystal::Reflection.bleep = true
  {{ call }}
end
 
def foo(x : Int32)
  Crystal::Reflection.bleep("foo(x : Int32)", "#{__FILE__}:#{__LINE__ - 1}") # injected by compiler
end
 
def foo(x : String)
  Crystal::Reflection.bleep("foo(x : String)", "#{__FILE__}:#{__LINE__ - 1}") # injected by compiler
end
 
tell_me_where_this_ends_up foo(1 || "") # foo(x : Int32) /home/johannes/src/crystal-lang/crystal/.test/tell_me_where_this_ends_up.cr:34

[MistressRemilia]

Well my main concern was just that it could, potentially, be quite confusing on things that aren't trivial. I am definitely not a Ruby programmer, and only started tinkering with that language after discovering Crystal, so this behavior is honestly new to me ^_^ As Straight-Shoota said:

The main problem associated with this is that you call a method (such as String#someMethod in the example) with a signature that you expect to match a specific def. But when the code executes, you don't see the effect of that def.

Although my example used the String class in the stdlib, I wasn't intending it to be strictly interpreted as that case. A different example might be if I have my own class and a method on it updates an instance variable, but some other library overrides that method (and does not update the instance variable).

To use a more concrete example, I was considering extending the IO class in my own personal standard lib with some convenience methods. That's when I thought, "What if some other shard I use does the same thing?" And that's what got me thinking about this.

I'm used to seeing a simple style warning in other languages when a method/function is re-defined in a separate file. But, I honestly think even just a mention in the language docs that the order of require statements matter would help. Or documentation regarding the tool Straight-Shoota mentioned.

[Blacksmoke16]

A different example might be if I have my own class and a method on it updates an instance variable, but some other library overrides that method (and does not update the instance variable).

This is partially why you should namespace your library's types. E.g. MyApp::SomeClass, then the chances of someone else's code conflicting with yours is much much less likely.

[vlazar]

Somewhat related discussion #8422