Repository navigation
Bug: Accessing private statics in a class via its derived class is allowed #8624
Description
Activity
What's the motivation?
Elephant-Vessel not sure what you mean. This is incorrect behavior, yes?
I'm just curious about why this would be regarded as incorrect behavior. This works for example fine in C#, why would it be incorrect in the context of TypeScript?
looking at this again, i is not a bug. it behaves as intended. see https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md#822-accessibility
Private property members can be accessed only within their declaring class. Specifically, a private member M declared in a class C can be accessed only within the class body of C.
the private property
privateStaticproperty is accessed within the class declaring it. there are no restriction on what type it is accessed through (e.g. like the case with protected).Interesting. I disagree with both the TypeScript and C# spec in that accessing a member (static or otherwise) of an item implies that it's a publicly available property of that item. But, if you & the other spec writers believe otherwise, looks like the issue ends here.
Elephant-Vessel this behavior was flagged when enabling the
no-unused-variabletslint rule. We were a bit weirded out that it was allowed.Funny enough I actually encountered a bug caused by this behavior today.
class FooBase { private static privateStatic: string = "unset"; testPrivateStatic(): void { Foo.privateStatic = "set"; console.log(FooBase.privateStatic); } } class Foo extends FooBase { } // Logs "unset" but you'd expect it to log "set" new FooBase().testPrivateStatic();
Mohamed Hegazy (@mhegazy), I think the spec should be changed. Does this change your mind? :)
Elephant-Vessel In C# you're accessing the same member regardless of which class it's entering through. In TypeScript it's a member of the explicit class you're looking at.12 remaining items
- added this to the milestone
on Jun 9, 2016 Agree that you should not be able to access derived class private statics through their name
PRs are welcomed
That's not private static member access by a derived class because the
access is being done by the base class. I understand the terminology
confusion but it's really about lexical scope. The alias you use to refer
to a member has no bearing on whether you can access the member from where
you are executing code. (But it looks like that was already discussed.)On Wed, Aug 10, 2016 at 1:47 PM, Elephant-Vessel notifications@github.com
wrote:It does, if done like the code for this issue. Try this:
class FooBase
{
private static string privateStatic = "";
public void testBase()
{
Console.WriteLine(Foo.privateStatic); // Ok!
}
}
class Foo : FooBase
{
}
From: Joseph Musser [mailto:notifications@github.com]
Sent: Wednesday, August 10, 2016 19:21
To: Microsoft/TypeScript TypeScript@noreply.github.com
Cc: Elephant-Vessel elephant@pantani.se; Mention <
mention@noreply.github.com>
Subject: Re: [Microsoft/TypeScript] Bug: Accessing private statics in a
class via its derived class is allowed (#8624)Aside: I'm not sure why you guys are saying C# allows private static
member access by derived classes. It does not.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub <
https://github.com/Microsoft/TypeScript/issues/8624#issuecomment-238938212>
, or mute the thread <https://github.com/notifications/unsubscribe-auth/
AJdwdKx6rOIZqvcYkYUC5eTOLpv2VaqJks5qeghtgaJpZM4Ifbu3> . <
https://github.com/notifications/beacon/AJdwdMuZLTfpipWfcZ0gtrbaxcGHUg
Riks5qeghtgaJpZM4Ifbu3.gif>—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#8624 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AHqvr6OfzlSuY0_TW7wXtlp7ayoAn77Pks5qeg6tgaJpZM4Ifbu3
.Joseph Musser (@jnm2) Yes. To be specific, it's by the base class via the derived class.
Joseph Musser (@jnm2) Sorry for making noise, I did not see that you removed your comment before I replied. However, yes I agree, this behavior would be strange considering that TypeScript is copying the static members down the inheritance chain. I was not aware of that, but I think that this copying is even stranger, from a consumer point-of-view. Or who knows, maybe it's just a feature that I have yet to discover the value of...
Reacted by Joseph MusserInstead of copying, what would be wrong with compiling
Foo.staticMembertoFooBase.staticMember? That's conceptually the member everyone is intending, right? If this was C#, ReSharper would suggest that you replace the derived name with the base name anyway. Or is this not quite like C#?If you want two copies of a static member for some reason, I would assume you would want to declare each copy since this is a statically typed language.
Thinking about this some more, I kinda get excited about static inheritance since classes are first-class entities in typescript. It's not very relevant in languages like Java or C# where classes are not first-class entities, but why not in TS? It feels like it could have some neat expressive power.
However, this bugs me:
class Animal { public static Sound() { console.log("animal class") } public Color() { console.log("Unknown") } } class Dog extends Animal { } var test = typeof Animal; Animal.Sound = () => console.log("mutated class") // Mutating static metod Animal.prototype.Color = () => console.log("mutated color") // Mutating prototype method Dog.Sound(); // => "animal class" : Hmm.... Grumpy face. Animal.Sound(); // => "mutated class" new Dog().Color(); // => "mutated color" new Animal().Color(); // => "mutated color"
If static inheritance could be considered a feature, maybe it would benefit from behaving more like regular inheritance.
In the above example, it could theoretically be solved if Dog by default (not having any method overrides) would be compiled like this:
class Dog extends Animal { public static Sound(){ super.Sound(); } }
Then it would behave the same as regular inheritance.
And regarding Joshuas bug above, if it was compiled to something like this, there would be no problem:
class FooBase { static privateStatic: string = "unset"; testPrivateStatic(): void { Foo.privateStatic = "set"; console.log(FooBase.privateStatic); } } class Foo extends FooBase { static get privateStatic(){ return super.privateStatic; } static set privateStatic(value){ super.privateStatic = value; } } // Now logs set new FooBase().testPrivateStatic();
Just my 2 cents.
Static members fail to integrate well with inheritance. It makes no sense to have them heritable.
Static members are services attached to the Class definition and have nothing to do with instances of Class or inherited Class. ActionScript (ES4) works like this, and it is useful and clear. Another (and better) useful approach is Java.
With Typescript, I got errors when using statics as if there where not inherited (same names in parent and sub classes, different signatures). In fact, the compiler produce an error when I override a method with another signature than defined in parent class, whatever the method is static or instanced.
But generated Javascript works very well.
May be it is too late to change this, and some guys could depend now on actual behavior. Or may be, for another reasons I do not figure.
Whatever, Typescript is well featured and does a good job. Thanks!Reacted by Joseph MusserStatic members are services attached to the Class definition and have nothing to do with instances of Class or inherited Class
But classes are objects in TS. This is a very important distinction from languages like C# and Java, in TS you can handle classes just like any other objects. You can put classes in a set and iterate it and call their static methods:
type Component = { RegisterComponent: () => void; } class FooViewComponent { public static RegisterComponent= () => { /* ... */ } } class BarViewComponent { public static RegisterComponent= () => { /* ... */ } } function SetUpApplication() { var components:Component[] = [FooViewComponent, BarViewComponent]; components.forEach((component) => component.RegisterComponent()); }
If typescript embraced static inheritance, it could look like this:
interface Component { static RegisterComponent: () => void; } class ButtonComponent implements Component { public static RegisterComponent= () => { /* ... */ } } class SuperButtonComponent extends ButtonComponent { //... } function SetUpApplication() { var components:Component[] = [ButtonComponent , SuperButtonComponent ]; components.forEach((component) => component.RegisterComponent()); }
Of course you can solve the same behavior in other ways, but I think this would be really neat and expressive in TS once you slightly adjust the mental model of the class that you got from C#/Java and embrace more expressive possibilities.
Ryan Cavanaugh (@RyanCavanaugh)
Agree that you should not be able to access derived class private statics through their name
If we eventually get some breaking change regarding privates here, maybe we could do two birds at once and remove the general access to privates in a T from within any T, #471, giving us better encapsulation in the language?
Reacted by Joseph Musser- added a commit that references this issue
on Jan 12, 2019
TypeScript Version:
1.8.X
Code
Expected behavior:
Accessing
Foo.privateStaticshouldn't be allowed, regardless of the context it's called in.Actual behavior:
FooBaseis allowed to do this.(thanks Seth Brenith (@sethbrenith) for deducing this)