Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support final methods #33446

Closed
cbutterfield opened this issue Sep 16, 2019 · 14 comments
Closed

Support final methods #33446

cbutterfield opened this issue Sep 16, 2019 · 14 comments
Labels
Duplicate An existing issue was already created

Comments

@cbutterfield
Copy link

cbutterfield commented Sep 16, 2019

This ticket is intended to be an open ticket referring to the many long-standing (and long ignored) requests for final methods. These requests have been made in two related, but closed tickets, namely

  1. Support final classes (non-subclassable) #8306 - Support final classes (non-subclassable)
  2. Suggestion: Final keyword for classes and methods #9264 - Suggestion: Final keyword for classes and methods

Sadly, both of these tickets have been closed, due solely to arguments by the developers against final classes. The completely distinct requests for final methods were totally ignored (even though they now comprise the bulk of the comments).

So hopefully the topic of final methods can be addressed on its own merits.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 16, 2019

Just in case tl;dr,

#8306 (comment)

Honestly, a few years ago, I would have agreed with you. But I simply don't write much OOP code anymore.

The final keyword won't see much use, even if I did still write a lot of OOP code.


In case it wasn't clear, I mean both final classes and final methods won't see much use (from me), even if I still did write a lot of OOP code.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 16, 2019

Stats for final methods are hard to find. But stats for final classes are easier.

If someone can provide the stats for final methods, please help =(


Interestingly enough, searching Github shows that sealed/final class is used a lot more than abstract class... In Java and C#. This surprises me because I thought it would be the other way around for all languages.

PHP still has a really high final-to-abstract ratio (higher than I expected) but it matches my personal experience that abstract is used more thansealed/final

C++ matches my expectations closer. But I suspect it is because it's harder to search for the final specifier in C++ (can someone perform a better search for me? My Github-Fu is bad).
Most of the C++ matches come from comments and people naming classes FinalClass

I'm surprised JavaScript got results at all for final class. Looks like it also comes from people naming classes FinalClass.

Scala is treated differently because the syntax is abstract class, sealed class, sealed abstract class

Language abstract class sealed/final class abstract:sealed/final Ratio sealed/final:abstract Ratio
Java 3,638,053 5,263,178 0.69122743 1.44670185
C# 688,912 1,925,587 0.357767268 2.79511316
PHP 7,559,313 2,589,909 2.91875622 0.34261169
C++ 228,096 24,064 9.4787234 0.105499439
JavaScript 106,635 24,725 4.31284125 0.23186571
Scala 89,978 7,067+35,088 = 42,155 2.13445617 0.468503412

What's also interesting (to me) is that JS doesn't have concepts like abstract and final, natively, but some people still seek these concepts out.

However, the numbers are tiny for JS. For both abstract and final.

But final is used way less. 1/5 as much as abstract.


https://github.com/search?q=%22abstract+class%22+in%3Afile&type=Code

Language Count "abstract class" in:file
PHP 7,559,313
Java 3,638,053
C# 688,912
HTML 550,308
C++ 228,096
Ruby 180,558
Python 157,500
JavaScript 106,635
Scala 89,978
Markdown 85,682

https://github.com/search?q=%22final+class%22+in%3Afile&type=Code

Language Count "final class" in:file
Java 5,263,178
PHP 2,589,909
HTML 600,170
Swift 87,939
Python 44,220
Text 31,763
ActionScript 27,916
JavaScript 24,725
C++ 24,064
XML 20,189

https://github.com/search?q=%22abstract+final+class%22+in%3Afile&type=Code

Language Count "abstract final class" in:file
Java 3,450
Assembly 294
SQL 268
Hack 205
PHP 147
Markdown 89
C++ 71
D 51
HTML 50
Text 44

https://github.com/search?q=%22sealed+class%22+in%3Afile&type=Code

Language Count "sealed class" in:file
C# 1,925,587
XML 89,737
C++ 69,765
YAML 28,226
Kotlin 15,801
Pascal 8,021
Scala 7,067
Markdown 6,508
HTML 6,164
JSON 4,196

https://github.com/search?q=%22abstract+sealed+class%22+in%3Afile&type=Code

Language Count "abstract sealed class" in:file
Scala 2,456
C# 207
HTML 96
Markdown 73
Kotlin 36
XML 28
Pascal 22
Text 17
Dylan 12
Lua 7

https://github.com/search?q=%22sealed+abstract+class%22+in%3Afile&type=Code

Language Count "sealed abstract class" in:file
Scala 35,088
Markdown 589
HTML 467
Text 123
Dylan 101
XML 94
F# 58
reStructuredText 49
JupyterNotebook 33
C# 26

@cbutterfield
Copy link
Author

Right, most people would not code a final method. The main use case is for authors of base classes to prevent sublasses from accidentally overriding base class methods that are critical to proper operation. Thus by definition most people will not author a final method, but they will definitely be protected with subclassing toolkit base classes from accidentally blowing the framework invariants.

So sure, I'm glad you don't have to worry about that. And maybe you never use subclasses at all. OK, So why on earth does it bother you that other people do use subclasses? Any why does it bother you that some base class authors need final to avoid accidents?

@poseidonCore
Copy link

Before this flames out tediously, can we please address the main point that Ryan raised (#8306 (comment)):

CLASSES ARE A JAVASCRIPT FEATURE, NOT A TYPESCRIPT FEATURE. If you really feel like classes are completely useless without final, that is something to take up with the TC39 committee or bring to es-discuss. If no one is willing or able to convince TC39 that final is a must-have feature, then we can consider adding it to TypeScript.

Is there any citable evidence that this is progressing in the ECMAScript spec. If not, the discussion should really be taken elsewhere.

@cbutterfield
Copy link
Author

Does that comment address final METHODS? Perhaps it does, I'm having trouble parsing it.

@AnyhowStep
Copy link
Contributor

AnyhowStep commented Sep 16, 2019

Any why does it bother you that some base class authors need final to avoid accidents?

Because it takes time away from other features I want the TS team to work on =P


Does that comment address final METHODS?

Much of what Ryan said about final class is applicable to final methods, though.


Honestly, though, I'm very interested in the stats, regarding final-vs-abstract-vs-plain methods.

@RyanCavanaugh RyanCavanaugh added the Duplicate An existing issue was already created label Sep 16, 2019
@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Sep 16, 2019

Ignored !== didn't go the way you wanted them to.

We've considered both final classes and final methods and still consider them to be something we don't need to add to the language ahead of TC39.

@cbutterfield
Copy link
Author

Two distinct questions:

  1. I had a hard time finding documented concerns with final methods by themselves. It seems final methods were conflated with final classes in the majority of your commentary. Would you mind pasting herein the specific issues with final methods (not mixed in with a discussion of final classes which are a whole different beast entirely)? Perhaps the issues are clearly stated and obvious, but I was confused by the mixture of class and methods in the same discussion.

  2. Separately I'm confused about the TC39 reference. I guess that is going after what exactly is the charter of Typescript, which I have always loosely assumed was intended to add a useful degree of compile time safety to make JS runtime behavior more robust. I did not think it was intended to only add features that were on an approved ECMA roadmap. Is that the current thinking? Has it always been that way and I never noticed?

@RyanCavanaugh
Copy link
Member

  1. I don't see any lack of duality between final classes and final methods; the latter is simply a more fine-grained control of the former and the motivations are largely the same. The general consensus on final classes was that from a type perspective, interactions between a base class and its derived classes are a complex interplay that can't be accurately described by providing a few keywords. By that measure, final of any form would be a "half measure" that doesn't really completely solve what is really an extremely complex problem.

  2. Speaking to e.g. C++ or Java, final in these languages has very clear and direct runtime performance implications: calls to final methods can be guaranteed to be non-virtual, saving on vtable space and runtime indirections. JavaScript could easily want those same outcomes for the same reasons. If you view final in this regard, it's in the same family as C's inline - an explicit instruction to the compiler to take a flexibility/performance trade-off. Again under this metric, it has no type meaning apart from restrictions implied by the requested low-level runtime behavior. So by this metric it's clearly in TC39's domain.

Now you might view final as a type instruction with sidechannel runtime effects, which is valid, but you could say the same about a lot of things.

Similar to override, it's also fairly straightforward to detect final violations at runtime and throw an error (which is not true of the vast majority of type system features). So that's an additional hurdle.

@Xample
Copy link

Xample commented Sep 24, 2019

I rarely need the final method, but this morning it would have prevented a coworker of mine to introduce a bad side effect.

export abstract class Base<T, U= any> {
    constructor(protected data: T ) {}

  // I wished this to have been final
    public getData(): T { 
        return this.data;
    }
}

export class SubClass extends Base<AnInterface>  {

  // to prevent this side effect
    public getData(): (AnInterface & AnotherInterface) {
        return { ...super.getData(), otherField: {} };
    }

  // I was wondering why no data were empty
    public isEmpty(): boolean {
        return !this.getData();
    }
}

It crashed during the runtime which is in my opinion, too late.

@cbutterfield
Copy link
Author

Here is my understanding of this topic: final methods are desirable for some toolkit designers, but pose problems for the language implementers.

Ryan - I'm NOT trying to re-open this debate (nor to get more educated at this time) just trying to frame and summarize the issue for any future readers. I do hope that someday there will be the time and energy to support final methods in JS/TS, but I understand it will not be any time soon.

@epferrari
Copy link

Workaround we use on our team, seems to do the job:

class Foo {
  protected readonly libraryMethod = () => {
    // do library things here
  };
}

class Bar extends Foo {
  protected libraryMethod = () => {...}  // compile error
  protected libraryMethod() {...} // compile error
  protected doSomethingElse() {
    this.libraryMethod(); // this is fine
  }
}

@shicks
Copy link
Contributor

shicks commented Sep 15, 2020

This workaround has an unfortunate runtime performance cost, since it instantiates a new closure on every constructor invocation. If you have a large number of final methods on an otherwise cheap class that's instantiated many times, this can be a non-starter. It can be generalized, though:

class Foo {
  readonly libraryMethod: () => void;
}

(Foo.prototype as any).libraryMethod = function() {
  // now this is cheap again
};

It's not great for readability, but it's more performant for cases where that matters.

@dgreensp
Copy link

The Dart programming language's solution, fwiw, is a @nonVirtual method annotation that is checked by an analyzer plugin. This annotation is not in core (you have to import it), but it's maintained by the Dart team. The analyzer is basically the static type-checker, distinct from the linter and the compiler(s).

As of Dart 3.0 (2023), final class is a thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

8 participants