Warning about/deprecating sub-classing a class?

[Simon Ochsenreither]

This whole inquiry is a pre-mature suggestion resulting from my work on providing a better scala.math.BigInt class (which in turn is a tiny part of Erik's work to provide better implementation of numeric functionality in a future release of Scala).

It is not even a SIP yet, but realizing that 2.10 is not far away, I'm rushing to bring up what I see as an issue so that you can think about the possible impact.

(Of course, if the Scala team decides that the improvement is not worth the trouble, it doesn't make much sense to pursue it any further.)

Btw, sorry for bringing this up so late in the release cycle.


Question:

Is there a way to deprecate the sub-classing of a class?

Some use cases would be:

• because a class will be made final in an upcoming release or
  
• protected members might be moved around or
• there are constraints which require that the class is non-final?
  

Basically something like @deprecated, except that it only emits a warning when users subclass the annotated class (and not mereley use it).


My use case:

Currently scala.math.BigInt is non-final, like java.math.BigInteger. I don't think there is a good reason to allow subclassing a number and existing subclasses (how many are there, in the wild?) would probably even benefit from using composition instead of inheritance.

Nonetheless, I think that it is necessary to warn about such a potentially incompatible change before it happens to avoid nasty surprises when a future version of scala.math.BigInt breaks user's subclasses.

Proposed warning message: "Class $YourClass inherits from `scala.math.BigInt`. `scala.math.BigInt` will be made final in 2.11. Use composition instead of inheritance. If you think you have a valid reason to inherit from `scala.math.BigInt`, please add a comment to SI-XYZ stating your requirements."


Why do I want to do this?

The background is that I want to propose a replacement of scala.math.BigInt for 2.11, which will be written in Scala, will be faster and more memory efficient than the current implementation (which basically just wraps java.math.BigInteger).

Additionally to the speed and space benefits it will ease porting to other runtimes by reducing our dependencies from the JDK.

I'd very much prefer to make that upcoming class final to allow to evolve it more freely in future releases (like making it a value class, although there are a quite a few show-stoppers preventing that currently) without breaking user code.


To do that, I would like to provide the following options for consideration:

1 A) Warn about subclasses of scala.math.BigInt in 2.10.
  B) Replace scala.math.BigInt with the final improved implementation in 2.11.

2 A) Don't warn about subclasses of scala.math.BigInt in 2.10.
  B) Replace scala.math.BigInt with the final improved implementation in 2.11.

3 A) Don't warn about subclasses of scala.math.BigInt in 2.10.
  B) Replace scala.math.BigInt with the non-final improved implementation in 2.11 and warn about subclasses.
  C) Make scala.math.BigInt final in 2.12.

4 A) Don't warn about subclasses of scala.math.BigInt in 2.10.
  B) Replace scala.math.BigInt with the non-final improved implementation in 2.11 and never make it final.

Personally, I would prefer 1) because imho it is the cleanest solution.

The 2.10 time frame could be used to assess how many users will be impacted by that change. If there would be too many users impacted I would probably back down from making it final, regardless of the chosen approach.


What do you think?

Thanks!

Simon

[Jason Zaugg]

On Mon, Jul 30, 2012 at 11:50 PM, Simon Ochsenreither
<simon.och...@googlemail.com> wrote:
> Question:
>
> Is there a way to deprecate the sub-classing of a class?
>
> Some use cases would be:
>
> because a class will be made final in an upcoming release or
> protected members might be moved around or
> there are constraints which require that the class is non-final?
>
>
> Basically something like @deprecated, except that it only emits a warning
> when users subclass the annotated class (and not mereley use it).

That sounds wonderful. There are a *lot* of classes in the standard
library that ought to be final, including around 200 case classes such
as TupleN.

Today I made 900 classes final in my work codebase to take advantage
of better static checks on impossible pattern matches; three bugs were
uncovered in the process.

-jason

[Nils Kilden-Pedersen]

On Mon, Jul 30, 2012 at 4:50 PM, Simon Ochsenreither <simon.och...@googlemail.com> wrote:

Basically something like @deprecated, except that it only emits a warning when users subclass the annotated class (and not mereley use it).

Since any subclass must call the super constructor, one could presumably replace the primary constructor, and deprecate the old one. Sure, it involves potentially creating a constructor that doesn't have the desired signature, but since the class will be made final one release later, one could change it back at that time.

[Paul Phillips]

On Mon, Jul 30, 2012 at 3:03 PM, Jason Zaugg <jza...@gmail.com> wrote:

That sounds wonderful. There are a *lot* of classes in the standard
library that ought to be final, including around 200 case classes such
as TupleN.

@deprecateExtends? Not sure what the right word is, what I'm shooting for is something which can be used to deprecate the absence-of-finality on both classes and methods, but the latter would more naturally be @deprecateOverrides or something.

[Paul Phillips]

On Mon, Jul 30, 2012 at 3:09 PM, Nils Kilden-Pedersen <nil...@gmail.com> wrote:

Since any subclass must call the super constructor, one could presumably replace the primary constructor, and deprecate the old one. Sure, it involves potentially creating a constructor that doesn't have the desired signature, but since the class will be made final one release later, one could change it back at that time.

This doesn't do anything to prevent people from subclassing through the new primary constructor.

[Nils Kilden-Pedersen]

It does if it's private.

[Nils Kilden-Pedersen]

And then I realized that you can't subclass other than the primary constructor. 

[Jason Zaugg]

On Tue, Jul 31, 2012 at 12:10 AM, Paul Phillips <pa...@improving.org> wrote:
> @deprecateExtends? Not sure what the right word is, what I'm shooting for is
> something which can be used to deprecate the absence-of-finality on both
> classes and methods, but the latter would more naturally be
> @deprecateOverrides or something.

@finalCountdown class Foo

:)

-jason

[Sciss]

@sterile
@endOfEvolution

[Nils Kilden-Pedersen]

This is all an attempt to "be nice" presumably. But I don't think it's written anywhere that one gets one version's worth of warning. That certainly hasn't been the case in the past. While nice is good, there's an obvious cost here. 

The logic is, that someone who extended those classes can upgrade without doing anything (which they then won't) until next upgrade, where they'll be forced to change. But is that really worth the hassle? Isn't it just postponing the hurt? 

[Simon Ochsenreither]

Thanks for all your input!

Stay tuned, my patch almost there.

The preliminary name for now is @deprecatedInheritance, but I'm open to suggestions.

It certainly depends on how general the semantics are supposed to be:

"I will make this class final" << "You shouldn't inherit from this class" << "Inherit from this class at your own risk"

[Simon Ochsenreither]

I created https://issues.scala-lang.org/browse/SI-6162 to track progress.

[Paul Phillips]

On Mon, Jul 30, 2012 at 3:49 PM, Simon Ochsenreither <simon.och...@googlemail.com> wrote:

The preliminary name for now is @deprecatedInheritance, but I'm open to suggestions.

I think this name fails to describe method overriding.

It certainly depends on how general the semantics are supposed to be:

Why? I think it should mean "Inheriting from this class (or overriding this method) is deprecated." What it means beyond that is not our concern - it's the province of that project's deprecation policy.

[Simon Ochsenreither]

Why? I think it should mean "Inheriting from this class (or overriding this method) is deprecated." What it means beyond that is not our concern - it's the province of that project's deprecation policy.

Right, the annotation provides a String for the actual reason, so that should work.

[Simon Ochsenreither]

Would love to hear your feedback: https://github.com/scala/scala/pull/1026

[Josh Suereth]

What's wrong with:  @thisShouldBeFinal, or even @`This should have been final.  D'oh!!!!`

[Matthew Pocock]

Shooting everything with an elephant gun full of finals may not play very nicely with aspect oriented programming libraries from the Java world, which (usually? always?) require the things they instrument to be non-final. There are a bunch of cases where things can not possibly be over-ridden according to the scala rules (e.g. methods inherited into a trait that is sealed). However, I don't have a feel for how frequently aspects are applied to this kind of code. Perhaps it is a non-issue.

Matthew

--

Dr Matthew Pocock

Integrative Bioinformatics Group, School of Computing Science, Newcastle University

mailto: turingate...@gmail.com

gchat: turingate...@gmail.com

msn: matthew...@yahoo.co.uk

irc.freenode.net: drdozer

skype: matthew.pocock

tel: (0191) 2566550

mob: +447535664143

[Alex Cruise]

I think this is a really worthwhile feature, I hope the terminological discussion doesn't distract too much.  Maybe put the name to a vote?

-0xe1a

[Simon Ochsenreither]

After thinking a bit more about the options, the only additional idea I came up with would be something (which I think is a bit overengineered):

@deprecated(msg: String, version: String, policy: DeprecationPolicy)

enum DeprecationPolicy {
    USAGE, INHERITANCE, OVERRIDING, ...
}

Apart from that I think @deprecatedInheritance/@deprecatedOverriding are the best names. I would even volunteer to target annotations¹ for 2.11, so that @deprecatedInheritance can only be placed on classes and @deprecatedOverriding on members.

Regardless of what we do, I would try to get a minimal version done before missing “2.10's train”, so that the second part of my proposal can be discussed.

¹ http://docs.oracle.com/javase/7/docs/api/java/lang/annotation/Target.html

[Josh Suereth]

Making 2.10 is going to be tough, has to be soon if at all.

What about:  @(deprecated @usage), @(deprecated @inheritance) @(deprecated override)   with @depreated -> @(deprecated @usage) by default?

- Josh

[Alex Cruise]

On Tue, Jul 31, 2012 at 8:51 AM, Josh Suereth <joshua....@gmail.com> wrote:
> Making 2.10 is going to be tough, has to be soon if at all.
>
> What about: @(deprecated @usage), @(deprecated @inheritance) @(deprecated
> override) with @depreated -> @(deprecated @usage) by default?

I LIKE IT! Can we nail down a concise description of each one?

-0xe1a

[Simon Ochsenreither]

Making 2.10 is going to be tough, has to be soon if at all.

Well, that's the point why I brought it up in the first place :-)

What about:  @(deprecated @usage), @(deprecated @inheritance) @(deprecated override)   with @depreated -> @(deprecated @usage) by default?

I think that's completely over-engineered like my last suggestion :-) Keep in mind that it won't be done this way, there will be still 2 additional parameters (and I have no idea on where to place them with that syntax)...

I'm currently adding the functionality for deprecate override.

[Simon Ochsenreither]

The reason why I'm proposing @deprecatedInheritance/@deprecatedOverriding is that we already have @deprecatedName and I wanted to keep it consistent.

[Josh Suereth]

Right, well I wish you had brought it up before the *last* milestone release was cut that involves *all* features for both the library and the compiler.   If you want this in, it has to be done before tuesday of next week, most likely unless other critical bugs survive longer or the community build has failures.

In any case, I'd like to entertain the possibility of this feature, but it has to be a clear win-win thing with little chance of issues to make 2.10.x.   Guaranteed this kind of thing can make master, but that's not the point, is it?

SO, if you thought you had a tight deadline, this is your warning.  You're past the deadline already, so sooner is better.   Let's get consensus too.

- Josh

[Simon Ochsenreither]

Hi Josh,

Right, well I wish you had brought it up before the *last* milestone release was cut that involves *all* features for both the library and the compiler.

Sorry for that. I'm spending 99.9% of my time on my thesis currently (which is due in 4 weeks). I didn't saw it earlier than a few days ago when I read my BigInt code during a backup and realized that my class was final, Scala's wasn't and I would need to speak up *now* to avoid being stuck with a non-final class for the next few years to come.
 

If you want this in, it has to be done before tuesday of next week, most likely unless other critical bugs survive longer or the community build has failures.

I'll write the patch as soon as I take/need/get a break from writing my thesis tonight.
 

In any case, I'd like to entertain the possibility of this feature, but it has to be a clear win-win thing with little chance of issues to make 2.10.x.   Guaranteed this kind of thing can make master, but that's not the point, is it?

SO, if you thought you had a tight deadline, this is your warning.  You're past the deadline already, so sooner is better.   Let's get consensus too.

Yes! Let's get this done! :-)

Bye,

Simon

[Josh Suereth]

No worries. I'm similarly swamped and was unable to get a few things I really wanted in 2.10 as well (*cough* OSGi, *cough*) even with help from others.   Let's do what we can, but at least the improvements will *come*.  Don't give up if you miss 2.10.

- Josh

[Simon Ochsenreither]

I updated the pull request. :-) Both deprecating inheritance and deprecating overriding is supported now.

I'm similarly swamped and was unable to get a few things I really wanted in 2.10 as well (*cough* OSGi, *cough*) even with help from others.

Me too. A shame that Option#contains didn't make it. It was such a nice and small addition. :-/

Bye,

Simon