@OverrideShouldInvoke

[Tagir Valeev]

Hello!

I agree with Brian. We have such an annotation in JetBrains
annotations package named
org.jetbrains.annotations.MustBeInvokedByOverriders. There are also
javax.annotation.OverridingMethodsMustInvokeSuper and
com.google.errorprone.annotations.OverridingMethodsMustInvokeSuper.
IntelliJ IDEA static analyzer supports them and allows you to add your
own custom annotation with the same semantics. SpotBugs and ErrorProne
also have this functionality. So while there's no standard solution in
Java itself, users who want to have such a warning have a variety of
tools to choose from.

And I agree that such an API style is a bad idea.

With best regards,
Tagir Valeev.

On Thu, Feb 9, 2023 at 8:44 PM Brian Goetz <brian.goetz at oracle.com> wrote:
>
> In a previous life, I was involved in several static analysis efforts.  One of the common factors there is that annotations like @ShouldntIgnoreReturnValue or @ShouldWashYourHands is that these are always approximations, guidelines aimed at identifying _likely bugs_ rather than _invalid programs_.  Usually such annotations beget requests for more annotations to refine the behavior in increasingly rarefied corners.  @OverrideShouldInvoke seems like it would be equally subject to this; it's a good starting point for "this is what the superclass intends", but is unlikely to be precise enough to not create new problems.
>
> The language, on the other hand, should mostly be in the business of identifying _invalid programs_.  If something is important enough that the language should reject a program over it, it should probably be part of the language, not an annotation.  (An obvious exception is @Override; in hindsight, it was probably excessive exuberance to use the new shiny annotation tool for this, rather than introducing a conditional `override` keyword at the time (Java 5).)
>
> So my gut sense is that this belongs in an effort like Checkers rather than in the language itself.
>
> On 2/9/2023 11:07 AM, Archie Cobbs wrote:
>
> Just throwing this out there - I'm curious whether this idea resonates with anyone else... or alternately offends...
>
> First, a big picture observation:
>
> There is a lot of "activity" in Java centered around the superclass-subclass relationship. For example, we have the protected keyword. We have the final keyword. We have a bunch of rules around how constructors are allowed (or not allowed) to invoke super(). We have @Overrides. We have abstract methods that force behavior on subclasses.
>
> Basically, all of this stuff amounts to a somewhat ad hoc "API" that a superclass defines and a subclass uses (and in some cases, vice-versa).
>
> But this "API" is not always clearly or fully defined. For example, bugs caused by 'this' escapes result from what can be described as an ambiguity in the "API" between superclass and subclass.
>
> I guess I'm just saying that this "API" and how we define it (or fail to define it) is an area of interest to me.
>
> OK, back to earth...
>
> Here's a proposal that addresses one more little bit of this "API". I'm on the fence as to whether this would be worthwhile and am curious what others think.
>
> The problem is this: often a superclass method foo() implements some important superclass functionality, but it is not final so that it can be overridden. In these cases, the superclass often wants to be able to specify "If you override this method, then you should also invoke super.foo() at some point unless you really know what you're doing". An example of such a method is Object.finalize().
>
> There are arguments that such methods are bad style - instead, there should be a separate empty method provided for subclasses to override. So this idea would have to be weighed against that. But regardless, today there are lots of examples of such methods out there already.
>
> The rough proposal is:
>
> Add a new annotation @OverrideShouldInvoke. If a method Sub.foo() overrides a superclass method Sup.foo() that has an @OverrideShouldInvoke annotation, and nowhere in Sub.foo() does it invoke Sup.foo() (via a super.foo() expression), then an error occurs.
> Add a new property boolean withoutInvokingSuper default false to the @Overrides annotation that allows Sub.foo() to suppress this error if it "really knows what it's doing".
>
> For simplicity, we'd only count invocations of the overridden method exactly, not some overloaded variant. So for example if an override of ArrayList.add(E) invoked super.add(int, E), that wouldn't count. Or, if you wanted to get fancy, you could make this optional via something like @OverrideShouldInvoke(allowOverloads = true).
>
> Here's are some examples of where you might want to use this:
>
> public class Object {
>
>     @OverrideShouldInvoke
>     protected native Object clone() throws CloneNotSupportedException;
>
>     @OverrideShouldInvoke
>     protected void finalize() throws Throwable {
>         ...
>     }
> }
>
> public class FileOutputStream extends OutputStream {
>
>     @OverrideShouldInvoke
>     public void close() throws IOException {
>         ...
>     }
> }
>
> public abstract class AbstractExecutorService implements ExecutorService {
>
>     @OverrideShouldInvoke
>     public Future<?> submit(Runnable task) {
>         ...
>     }
> }
>
> Thoughts?
>
> -Archie
>
> --
> Archie L. Cobbs
>
>