Accidentally overriding methods?

Help & Support
1

It is very nice that it is easy to add my own methods to structs and classes and whatnot, but what protects me from accidentally replacing an existing method?

I know I can check if there is already such a method and then decide if I want to replace it with mine,
but what if a new version of the original object (struct, class etc.) gets a new method in an upgrade that I also used? Now I am overriding it.

I know I can write tests and I do, but I know a lot of people who don’t.

I tried the following and saw that neither Crystal not ameba warns me. Is there some other tool or some flag that would warn me that I am overriding an existing method?

struct Int
  def odd?
    return true
  end
end

puts 23.odd?
puts 24.odd?

ps. I don’t expect it to tell me that my code doesn’t make sense at all :slight_smile:

2

Related discussion: https://github.com/crystal-lang/crystal/issues/1647 It mostly focuses on the opposite approach, making sure that a method overrides an ancestor’s implementation.

Avoiding involuntary overrides is more difficult. To make any sense, it would require to annotate any methods which explicitly overrides an ancestor’s implementation. That’s a lot and IMO way to noise to be useful.

3

With great power comes great responsibility. The solution here (IMO) is just don’t do it, or know that if you do do it, it comes with risks like this. I’m not sure how the compiler could tell the difference between a proper and invalid redefinition.

1 Like
4

but what if a new version of the original object (struct, class etc.) gets a new method in an upgrade that I also used?

If you have specs, or you are using the app periodically to try things out, you’ll quickly find out because a spec will fail or the app will behave in a wrong way.

There’s also a chance that the method you overwrote has a different return type, and that will lead to a compilation error.

1 Like
5

I am quite familiar with both Perl and Python. Both allow you to add or replace methods of objects albeit with very different syntax. Both languages have tools that will warn you when you replace an existing method and in both languages you can flag the specific cases when you want it to stop warning.

1 Like
6

Maybe?

# NOTE: macros are inherited, also from included modules, but not macro hooks.

macro _warn_overrides
  macro method_added(method)
    _check_override({{@type}},\{{method.name}})
  end
end
macro _check_override(type,method)
  {% type = type.resolve %}
  {% method = method.name.stringify %}
  {% type_and_ancestors = [type] + type.ancestors %}
  {% for t in type_and_ancestors %}
    {% for m in t.methods %}
      {% if m.name == method %}
        {% puts "WARNING: #{type.name} overrides #{method} from #{t.name}" %}
      {% end %}
    {% end %}
  {% end %}
end

# ---

struct Int _warn_overrides # can also put `_warn_overrides` on its own line of course but this way it stands our more 
  def odd?
    return true
  end
end

puts 23.odd?
puts 24.odd?

# spits "WARNING: Int overrides "odd?" from Int" at compile time -- better wording welcome

The bright side, it’s opt-in. Only classes with the _warn_overrides will be affected.
The down side, it’s somewhat annoying having to put that macro everywhere it may be needed.
Later I’ll try all_subclasses instead of ancestors, perhaps it makes more sense.

3 Likes
7

Yes, a per-type opt-in might be something that could work for this. It could be supported in the compiler. I’d definitely use annotations for this, though:

@[WarnOverrides]
struct Int
  def odd?
    return true
  end
end
7 Likes
8

Would it be possible to create a module annotation that warns for every type and module-scope method within?

1 Like
9

Indeed, annotations are the right tool for this kind of… annotations, that is. Will try that too.

My main question is, is it better “check this specific class for overrides of ancestors” vs “protect this specific class from overrides by subclasses”?

It could be supported in the compiler, but somehow I feel it’s better to do whatever possible without touching the compiler itself.
If it can be done in “user code”, it can be private, or a shard, or end up in standard library if it’s worth, with less effort and impact on existing code. Then it could be reimplemented more efficiently in the compiler if it really matters and the construct effectively becomes “part of the language”.

edit: I didn’t think about it at first (do I love to jump in and hack away?), but maybe this is a task for Ameba?

10

Yes, sure. If it can be done outside the compiler (especially experimenting), that’s great.

2 Likes