@OverrideShouldInvoke

Kevin Bourrillion kevinb at google.com
Thu Feb 9 20:14:59 UTC 2023


I agree and might put it like this: The minimal language rules are those
that let the platform understand deterministically *what* to do, whether
it's "safely" what the programmer actually *wants* or not. The platform
does further make various safety guarantees beyond that, requiring more
rules, but tries to keep those to just the highest-value, most-universal
ones.

Third-party static analyzers are all about saving you from as many bugs as
they can, modulo acceptable false-positive thresholds. I work
(somewhat) on Error
Prone <https://errorprone.info/bugpatterns>, which addresses this
particular issue with a check called "MissingSuperCall". But it's not
standardized, and that's where JSpecify might come in.


On Thu, Feb 9, 2023 at 11:44 AM 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
>
>
>

-- 
Kevin Bourrillion | Java Librarian | Google, Inc. | kevinb at google.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20230209/401ece6a/attachment-0001.htm>


More information about the amber-dev mailing list