# Introduction of an Override annotation

**URL:** <https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441>\
**Category:** Crystal Contrib\
**Created:** [November 28, 2024, 12:29am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441 "2024-11-28T00:29:43Z")\
**Posts on this page:** 12\
**Page:** 1

<div class="post-metadata">

**Author:** ![tsornson](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/tsornson/32/1651_2.png) [@tsornson](https://forum.crystal-lang.org/u/tsornson)\
**Post date:** [November 28, 2024, 12:29am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/1 "2024-11-28T00:29:43Z")

</div>

While building the [apply-types](https://forum.crystal-lang.org/t/exploring-the-compiler/7343) tool into the compiler (for fun and no profit), I’ve gotten to the point where I can safely run it on the compiler itself and have it spit out correct looking type restrictions. However, the tool doesn’t know if a given method is overriding a parent method declaration, and so what gets output no longer compiles due to abstract defs no longer being overriden correctly. I can find and detect these cases by brute force checking the ancestors for each type and method (we’ll ignore performance issues for now), but was thinking that a developer explicitly stating whether a method overrides another method or not would be really helpful for devs and tools alike.

I discovered this initial request from 9 years ago essentially introducing the exact idea I was thinking:

> <https://github.com/crystal-lang/crystal/issues/1647>
>
> this will prevent you from doing something like
> 
> \`\`\` crystal
> class Foo
> def foo…bar
> 2
> end
> end
> 
> class Bar \< Foo
> def foobaz # whoops a typo
> 3
> end
> end
> 
> Bar.new.foobar #=\> 2 - Whaat?
> \`\`\`

And it looked like an initial PR was setup by Asterite to introduce `@[Override]` and a `@[Redefine]` annotations, but was then closed due to needing to go through a “design process”:

> <https://github.com/crystal-lang/crystal/pull/6983>
>
> This PR introduces two annotations:
> \- \`@\[Redefine\]\`: when you redefine a method…, that is, you replace an existing method with another one
> \- \`@\[Override\]\`: when you define a method in a subtype that's present up in the type hierarchy
> 
> You can find examples in the specs in the diff.
> 
> The semantic is not that straightforward. Let's consider each annotation.
> 
> \## Redefine
> 
> I first named this \`Overwrite\` but I think it would be very confusing to have \`Override\` and \`Overwrite\`, and \`Redefine\` is much clearer.
> 
> This is this case:
> 
> \`\`\`crystal
> class Foo
> def foo(x)
> end
> end
> 
> class Foo
> @\[Redefine\]
> def foo(x)
> end
> end
> \`\`\`
> 
> The above example compiles fine: same method name, same arguments and same restrictions (none). But what if there are some type restrictions? We can have:
> 
> \`\`\`crystal
> \# 1. Redefines 👍 
> \# before
> def foo(x)
> \# after
> def foo(x : Int32)
> 
> \# 2. Doesn't redefine 👎 
> \# before
> def foo(x : Int32)
> \# after
> def foo(x)
> 
> \# 3. Doesn't redefine 👎 
> \# before
> def foo(x : Int32)
> \# after
> def foo(x : String)
> \`\`\`
> 
> In case 1 it redefines because the previous method allowed an \`Int32\` as an argument, and now we are redefining that behaviour. We are not redefining it for all of the types, but if it happens for at least one of them then the annotation applies.
> 
> (maybe later we can introduce a flag on the attribute to allow partial redefines like this or not, but I don't know if it's necessary)
> 
> In case 2 it doesn't redefine because if we pass an \`Int32\`, the compiler will still choose the previous definition.
> 
> In case 3 the type restrictions are different so it doesn't redefine.
> 
> \## Override
> 
> This is this case:
> 
> \`\`\`crystal
> class Foo
> def foo(x)
> end
> end
> 
> class Bar \< Foo
> @\[Override\]
> def foo(x)
> end
> end
> \`\`\`
> 
> The above example compiles fine: same method name, same arguments and same restrictions (none). But what if there are some type restrictions? We can have:
> 
> \`\`\`crystal
> \# 1. Overrides 👍 
> \# ancestor
> def foo(x)
> \# self
> def foo(x : Int32)
> 
> \# 2. Overrides 👍 
> \# ancestor
> def foo(x : Int32)
> \# self
> def foo(x)
> 
> \# 3. Doesn't override 👎 
> \# ancestor
> def foo(x : Int32)
> \# self
> def foo(x : String)
> \`\`\`
> 
> In case 1 it overrides: there's a stricter type restriction so if we invoke \`Bar.new.foo(1)\` it will call the method in \`Bar\` and not in \`Foo\`.
> 
> In case 2 it also overrides, unlike the \`Redefine\` case, because it's in a subtype: if we pass an \`Int32\` to \`Bar#foo\` then it will consider that method (there's no multiple dispatch here).
> 
> In case 3 the restrictions are different so it doesn't overwrite.
> 
> \## Caveats
> 
> Now for the caveats: I don't consider this implementation complete, but just because the whole language and all of its semantic implementation (or even definition) is incomplete. For example, right now we can redefine a method with a different argument name:
> 
> \`\`\`crystal
> class Foo
> def foo(x)
> end
> end
> 
> class Foo
> # This actually redefines the above method
> def foo(y)
> end
> end
> \`\`\`
> 
> But if we invoked the first method like \`foo(x: 1)\` then now it won't compile... so in short, we are not taking names into account when redefining methods or when considering overloads. But this is a discussion to be held somewhere else.
> 
> Then, return types are also not taken into account when overriding/redefining, so here it's fine to do that and change the return type, it will compile just fine. But that's also a discussion to be held somewhere else.
> 
> In the worst scenario, if you try to use these annotations and there's a bug or limitation in the compiler preventing you to use them, you just don't use them until these bugs/limitations are fixed. Nobody is forcing you to use them (but they are useful to be more explicit about what you are trying to do).
> 
> \## How it relates to docs
> 
> This PR doesn't modify the way docs are generates. I'd like to tackle that in a separate PR. Basically, if you redefine or override a method and don't provide additional doc comments, the doc generator should use the other method's docs. For this we'll have to store the "other" method somewhere, but doing so will make the compiler use more memory, so I'd like to find a better way to do that, maybe store these relationship outside of the AST nodes, so that the memory is only used when needed (I'm thinking a \`Hash\` could be enough). Also, we'll have to repeat the logic of finding the redefined/overridden method, so I'd like to refactor that too in a next PR.
> 
> \---
> 
> Fixes #1647

I’m curious if others would still find this type of annotation / behavior (as a purely optional protection, similar to type restrictions in general) beneficial to the language, and if so, what would the starting point for a design process be today? Start an RFC?

---

<div class="post-metadata">

**Author:** ![nobodywasishere](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/nobodywasishere/32/1983_2.png) [@nobodywasishere](https://forum.crystal-lang.org/u/nobodywasishere)\
**Post date:** [November 28, 2024, 1:50am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/2 "2024-11-28T01:50:30Z")

</div>

I personally prefer more verboseness for long-term code / library code than less, so being able to enforce that subclass methods that override use a specific annotation (and otherwise produce a lint warning or an error) is a bonus. Others will probably disagree though (which is fine), the language is already complex enough as-is. It _may_ be difficult to enforce something like this now.

An RFC would be the proper channel to go through for creating something like this, though with my recent RFC I started with a forum post (just like this) to garner ideas and feedback before putting an RFC together.

---

<div class="post-metadata">

**Author:** ![tsornson](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/tsornson/32/1651_2.png) [@tsornson](https://forum.crystal-lang.org/u/tsornson)\
**Post date:** [November 28, 2024, 4:35am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/3 "2024-11-28T04:35:59Z")

</div>

Same regarding more verboseness for long-term projects. At some point a future me comes through the code and has completely forgotten what it’s purpose is, so having some more explicitly defined parts are really helpful for that. I also don’t want to lose the magic of crystal, so for this `@[Override]` annotation I’d treat it in the same vein as type restrictions in general: optional, but if specified, can help protect your code against mistakes.

To make it explicit, here are my own preferences to what the behavior would be:

- If no `@[Override]` annotation is present, then everything behaves the same as today (no change, overriden methods are inferred based on method name and argument types).
- If an `@[Override]` annotation is present, then whatever `def` it hangs on, _must_ override at least one ancestral method. If it does not, this gets treated as a compiler error.

In the initial thread, the main use case was for typos in the method name, but the use case I’m running into is changing the type restrictions to something more strict but still different from the parent def now no longer matches either.

If anyone has any other thoughts on this annotation’s behavior, or agree or disagrees with my own thoughts, please chime in!

---

<div class="post-metadata">

**Author:** ![Blacksmoke16](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/blacksmoke16/32/1241_2.png) [@Blacksmoke16](https://forum.crystal-lang.org/u/Blacksmoke16)\
**Post date:** [November 30, 2024, 5:03pm UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/5 "2024-11-30T17:03:45Z")

</div>

Somewhat related: [Documentation - TypeScript 4.3](https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html#override-and-the---noimplicitoverride-flag).

TypeScript has a feature related to this via the `override` keyword (versus annotation), and a compiler flag that makes it an error to override any method from a superclass unless you explicitly use an `override` keyword.

---

<div class="post-metadata">

**Author:** ![ysbaddaden](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/ysbaddaden/32/2252_2.png) [@ysbaddaden](https://forum.crystal-lang.org/u/ysbaddaden)\
**Post date:** [December 1, 2024, 2:08pm UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/6 "2024-12-01T14:08:34Z")

</div>

> [@tsornson](#):
>
> I’m curious if others would still find this type of annotation / behavior (as a purely optional protection, similar to type restrictions in general) beneficial to the language.

IMO this is a _linting_ concern, it doesn’t have much, if anything, to do with the compiler. Save for the annotation type(s), this can be implemented as an [Ameba](https://github.com/crystal-ameba/ameba) rule. Maybe it wouldn’t even need annotations: typos can be detected with a levenshtein distance, same name but different signature, etc.

You like it? You enable the rule, you don’t like it, you disable the rule. This only applies to _your_ projects and libraries, and your personal preference won’t leak to users of your library: they can enable/disable the rule or not use a linter: it’s _their_ choice.

---

<div class="post-metadata">

**Author:** ![tsornson](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/tsornson/32/1651_2.png) [@tsornson](https://forum.crystal-lang.org/u/tsornson)\
**Post date:** [December 2, 2024, 5:00am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/7 "2024-12-02T05:00:15Z")

</div>

> [@ysbaddaden](#):
>
> IMO this is a _linting_ concern, it doesn’t have much, if anything, to do with the compiler

This would be a static analysis concern, linting would be too superficial to catch it. Since the compiler owns the semantic processing, and the exclusive decision making in which method overrides another method (as determined on def name matching and inferred argument type matching heuristics), this at the very least requires the full semantic processing. Since the compiler is, like, %60 semantic processing (I’m making this number up, but it’s a non-trivial fraction), I’d say it makes sense that it has something to do with the compiler.

While typos have been the main use case, the other problems I want to address are:

- As a new developer entering a code base, I see a method I’m not familiar with not having any type restrictions on it, and I add them incorrectly. Since the original method is still around, the compiler sees nothing wrong, compiles, and now I have a runtime bug using the incorrect parent implementation. (This is where the apply-types tool exposed me to the problem)
- As an author of a popular base library, I made an update to what I think is an internal method, either changing the name or the arguments. I do not realize that other libraries / applications have overridden it for some particular reason or another, and when they update their shards, they now have a runtime bug.
- (for completeness) As dev quickly iterating on things, I try to override the parent method `foo` but accidentally type in `fooo` instead. No issues are raised up and I now have a runtime bug.

All runtime bugs are conceivably caught by tests, but that’s always additional work and thoughtfulness, and not a guarantee anyways. Interestingly, the opposite direction isn’t a problem - I can mark a method as `abstract` and _force_ all subclasses to implement it or else it’s a compiler error. This, too, came up during the apply-types testing, but the compiler caught these errors for a different reason.

> [@ysbaddaden](#):
>
> You like it? You enable the rule, you don’t like it, you disable the rule. This only applies to _your_ projects and libraries, and your personal preference won’t leak to users of your library: they can enable/disable the rule or not use a linter: it’s _their_ choice.

Unless I’m missing something, the proposal still respects this - libraries using this annotation would only have the rule applied to their code, at the site of the annotation 😕 while technically other libs / applications compiling with these annotated lib methods would trigger the same check, it should presumably always pass, since the overridden method should be available. Or I missed the point of this paragraph 😅

Edit: grammar.

---

<div class="post-metadata">

**Author:** ![Xen](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/xen/32/2288_2.png) [@Xen](https://forum.crystal-lang.org/u/Xen)\
**Post date:** [December 6, 2024, 5:05pm UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/8 "2024-12-06T17:05:36Z")

</div>

> [@tsornson](#):
>
> I’m curious if others would still find this type of annotation / behavior (as a purely optional protection, similar to type restrictions in general) beneficial to the language, and if so, what would the starting point for a design process be today? Start an RFC?

PHP has recently introduced such an annotation, and I hated it on sight.

I do get the arguments about protecting against typos and superclasses getting a new method that matches one in your subclass, but if we decide that we want that, why not make it part of the language (I’ll confess to not reading the linked issues)?

I can see some value in making redefinition explicit, but make it short and sweet, `redef` as it’s done all the time. And contrary to type restrictions where you can be sorta “I’ll figure out the exact pitch of quack later”, you really shouldn’t be in doubt whether you’re overriding or not.

---

<div class="post-metadata">

**Author:** ![tsornson](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/tsornson/32/1651_2.png) [@tsornson](https://forum.crystal-lang.org/u/tsornson)\
**Post date:** [December 6, 2024, 11:20pm UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/9 "2024-12-06T23:20:05Z")

</div>

`redef` was brought up several times in the linked issue and seemed like a popular option. The only opposition I saw was from [this post](https://github.com/crystal-lang/crystal/issues/1647#issuecomment-302413611), which points out we already have syntax for special types of `def`s via `abstract def` and `private def`, and so proposed `override def` to remain consistent with those.

I made this post proposing this as an annotation since that fit into the already existing language syntax and represents a smaller change (and smaller risk to the language), but I also like the idea of really making it part of the language too.

---

<div class="post-metadata">

**Author:** ![Xen](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/xen/32/2288_2.png) [@Xen](https://forum.crystal-lang.org/u/Xen)\
**Post date:** [December 7, 2024, 12:26am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/10 "2024-12-07T00:26:52Z")

</div>

> [@tsornson](#):
>
> `override def`

Well, yeah… But you can’t really redefine a private method, can you? Doesn’t it just stay private in the superclass, and you get your own private version? And redeffing an abstract method is kinda the point. I’m not convinced, private and abstract are kinda properties of the following def, while redefinition is a bit more than just a property, in my mind.

But it needs a bit of thinking. If you reopen a class and redefine a method that way, do you use redef? You’re redefining, but not in an OO way.

---

<div class="post-metadata">

**Author:** ![Blacksmoke16](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/blacksmoke16/32/1241_2.png) [@Blacksmoke16](https://forum.crystal-lang.org/u/Blacksmoke16)\
**Post date:** [December 7, 2024, 12:34am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/11 "2024-12-07T00:34:20Z")

</div>

> [@Xen](#):
>
> Well, yeah… But you can’t really redefine a private method, can you? Doesn’t it just stay private in the superclass, and you get your own private version?

`private` doesn’t work like it does in PHP. Crystal’s `private` is more akin to PHP’s `protected` modifier, in that child classes can access private methods defined in the parent, but you can’t use them outside of the class itself. So because of this it would be possible for a child to re-define a private method defined on the parent and if the parent used that private method, the child’s implementation would be used instead.

> [@Xen](#):
>
> If you reopen a class and redefine a method that way, do you use redef? You’re redefining, but not in an OO way.

Related: [New keyword to re-open classes · Issue #8422 · crystal-lang/crystal · GitHub](https://github.com/crystal-lang/crystal/issues/8422)

---

<div class="post-metadata">

**Author:** ![Xen](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/xen/32/2288_2.png) [@Xen](https://forum.crystal-lang.org/u/Xen)\
**Post date:** [December 8, 2024, 12:07am UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/12 "2024-12-08T00:07:14Z")

</div>

> [@Blacksmoke16](#):
>
> `private` doesn’t work like it does in PHP

Oh right, I was confusing it with… Something I can’t remember.

---

<div class="post-metadata">

**Author:** ![straight-shoota](https://yyz2.discourse-cdn.com/flex036/user_avatar/forum.crystal-lang.org/straight-shoota/32/36_2.png) [@straight-shoota](https://forum.crystal-lang.org/u/straight-shoota)\
**Post date:** [January 15, 2025, 4:28pm UTC](https://forum.crystal-lang.org/t/introduction-of-an-override-annotation/7441/13 "2025-01-15T16:28:43Z")

</div>

Reactions on the original issue have been overwhelmingly positive, including many Core Team members’. So there is broad support.

It just hasn’t been implemented yet. There is the PR from Ary which serves as a proof of concept. It shows how the concept can work, but it’s missing details of the exact semantics. These we need to figure out before we can merge it.

Maybe working with the PoC implementation and testing it on existing code bases could help finding out relevant use cases and refining semantics.  
I’m sure it’ll need some work to sync the old patch with master, but it doesn’t seem very complex.  
That research could inform an RFC to specify all the details.
