Nested private and protected class members

[AbubakerB]

Fixes #7058

Allows classes to access private and protected member of parent (enclosed) classes.

[msftclas]

Hi @AbubakerB, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

[mhegazy]

i think this should be a loop walking up and repeate the check for each enclosing class.

I would consider adding a function like forEachEnclosingClass and wrapp the exisitng consitions to the call to of that function:

e.g.: if (!enclosingClass || ! forEachEnclosingClass(node, enclosingClass=> hasBaseType(enclosingClass, declaringClass))) {

where forEachEnclosingClass is defined as:

    function forEachEnclosingClass<T>(node:Node, callback: (node:Node) => T):T {
        let result: T;

        while (true) {
            node = getContainingClass(node);
            if (!node) break;
            if (result = callback(node)) break;
        }

        return result;
    }

and isNodeWithinClass can be defined as:

    function isNodeWithinClass(node: Node, classDeclaration: ClassLikeDeclaration) {
        return !!forEachEnclosingClass(node, n => n === classDeclaration);
    }

[AbubakerB]

But in the case of

class Base {
    protected x: string;
    method() {
        var b: Base;
    }
}

class Derived1 extends Base {
    method1() {
        var b: Base;
        var d1: Derived1;

        b.x;            // Currently an Error
        d1.x;           // Currently OK
    }
}

b.x won't be an error anymore, right? Now that the
declared class = Base
Enclosed class = Derived1
Derived1 has base Base - so b.x won't be an error anymore.

Or did i misunderstand you?

[mhegazy]

that should not change. it should be 1. accessed in a protected class, and 2. through an instance of the protected class. the change is only about the first condition, not the second.

[AbubakerB]

Ah, yes, I see, my bad.

One last comment. Should I rewrite isNodeWithinClass to be Type oriented instead of ClassLikeDeclaration oriented?

hasBaseType() takes InterfaceType, and most other places that call isNodeWithinClass gets the Type's ClassLikeDeclaration (which could be skipped if it was Type oriented).
If we make
isNodeWithinClass(node: Node, classDeclaration: ClassLikeDeclaration) into isNodeWithinType(node: Node, type: Type) (or maybe a better name),
and convert the getContainingClass to a Type within forEachEnclosingClass<T>()

Or should we keep it as-is and convert the ClassLikeDeclaration to a Type within the callback of forEachEnclosingClass<T>()?

[mhegazy]

keeping it as a ClassLikeDeclarations seems appropriate.

[AbubakerB]

Sorry been quite busy lately.
Done :)

[AbubakerB]

@mhegazy are there anymore comments on this?

[mhegazy]

ooh, sorry for the delay. this fell off my radar. i can update it and merge it manually.

[mhegazy]

Thanks again for the PR, and sorry for the delay.

[AbubakerB]

No problem. I thought it was a bit stale so I closed it... my bad.

And thanks! ⛄