RFR: 8343540: Report preview error for inherited effectively-preview methods

[Alex Buckley]

On 11/7/2024 9:22 AM, Jan Lahoda wrote:
> If `ExistingRetrofittedClass` would override the effectively preview
> method "`test`", without marking the method as preview, I am not
> sure it is desirable for the uses of the overriding method in
> `ExistingRetrofittedClass` to report errors/warnings.
> 
> There are two reasons for that:
> - what if the `test` method existed before, and now is it is just
> being included in the superinterface? The method existed, and should
> still be generally usable, but if it would automatically get the
> preview handling, there would be no way to "un-subscribe" from the
> preview handling. But if the overriding method is by default an
> ordinary, non-preview, method, and the library developer desires the
> override to also be handled as a preview method, the solution is
> simple: use `@PreviewFeature` of the overriding method.
>
> - for each method invocation, javac would need to go through all
> methods that the invoked method overrides, to see if some of them
> are (effectively) preview methods. This would be fairly costly, but
> only very few (if any) methods are overriding a preview method. So,
> overall, it seems it would be a considerable cost in the compilation
> time, for a very small gain. (Where the gain is only avoiding the
> addition of `@PreviewFeature` on a very limited number of methods.)

You're right. If a participating class (ExistingRetrofittedClass) 
overrides an "effectively" preview method, and the participating class 
doesn't mark its *overriding* method with @PreviewFeature, then the 
"effectively" preview status of the *overridden* method should be 
invisible to clients of the participating class.

Now the maintainer of the participating class is in charge. If they have 
been declaring a method for years, they won't mark it. If they just 
started declaring a method in order to override something in a preview 
supertype, they ought to mark it -- even if their new declaration is 
simply `throw new UnsupportedOperationException("Not supported by this 
class")`.

I retract my earlier comment:

 >> Suppose ExistingRetrofittedClass does indeed override the test
 >> method. I would expect both of the examples in your original mail
 >> to give a warning/error. Both the invocation of `c.test()`, and
 >> the overriding of test in Test1, refer to an "effectively" preview
 >> method.

Alex