@OverrideShouldInvoke

[Archie Cobbs]

Thanks for the comments & pointers.

I agree this idea seems like it better belongs in some kind of "companion"
tool and am glad to hear the idea already exists out there.

-Archie

On Thu, Feb 9, 2023 at 2:15 PM Kevin Bourrillion <kevinb at google.com> wrote:

> 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
>


-- 
Archie L. Cobbs
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/amber-dev/attachments/20230209/3a02686c/attachment.htm>