Bug: Accessing private statics in a class via its derived class is allowed

[JoshuaKGoldberg]

TypeScript Version:

1.8.X

Code

class FooBase {
    private static privateStatic: string = "";

    testBase(): void {
        console.log(FooBase.privateStatic);

        // Should error, but doesn't
        console.log(Foo.privateStatic);
   }
}

class Foo extends FooBase {
    test() {
        // Should error, and does
        console.log(Foo.privateStatic);
    }
}

Expected behavior:
Accessing Foo.privateStatic shouldn't be allowed, regardless of the context it's called in.

Actual behavior:
FooBase is allowed to do this.

(thanks @sethbrenith for deducing this)

[Elephant-Vessel]

What's the motivation?

[JoshuaKGoldberg]

@Elephant-Vessel not sure what you mean. This is incorrect behavior, yes?

[Elephant-Vessel]

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?

[mhegazy]

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 privateStatic property 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).

[JoshuaKGoldberg]

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-variable tslint rule. We were a bit weirded out that it was allowed.

[JoshuaKGoldberg]

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();

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

[mhegazy]

in dead. reopening.

[Elephant-Vessel]

Yeah, that behavior don't make much sense. I think it's quite strange that derived classes clone static fields from the base class, regardless whether one should be able to access Base.StaticProp from Derived.StaticProp or not. Is that by design or is that the central bug here?

[mhegazy]

the copying is by design. witnessing an invalid state is what this issue tracks.

[Elephant-Vessel]

Is there anywhere one could read about that design desision? Just out of interest.

[mhegazy]

https://github.com/Microsoft/TypeScript/blob/master/doc/spec.md

[Elephant-Vessel]

Well alright, the specs are nice but not particularly useful when one wants to understand why the spec looks like it does in the first place. But whatever then.

Edit: If anyone else is interested, I found this and this, the later with more similar issues referenced.

[RyanCavanaugh]

Agree that you should not be able to access derived class private statics through their name

[mhegazy]

PRs are welcomed

[jnm2]

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
.

[JoshuaKGoldberg]

@jnm2 Yes. To be specific, it's by the base class via the derived class.

[Elephant-Vessel]

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

[jnm2]

Instead of copying, what would be wrong with compiling Foo.staticMember to FooBase.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.

[Elephant-Vessel]

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();

[daladaysoft]

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!

[Elephant-Vessel]

@daladaysoft

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

[Elephant-Vessel]

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