Private Name Support in the Checker (#5)

- [x] treat private names as unique:
    - case 1: cannot say that a variable is of a class type unless the variable points to an instance of the class
        - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesUnique.ts)
    - case 2: private names in class hierarchies do not conflict
        - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoConflictWhenInheriting.ts)
- [x] `#constructor` is reserved
    - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNameConstructorReserved.ts)
    - check in `bindWorker`, where every node is visited
- [x] Accessibility modifiers can't be used with private names
    - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoAccessibilityModifiers.ts)
    - implemented in `checkAccessibilityModifiers`, using `ModifierFlags.AccessibilityModifier`
- [x] `delete #foo` not allowed
- [x] Private name accesses not allowed outside of the defining class
    - see test: https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNameNotAccessibleOutsideDefiningClass.ts
    - see [test](https://github.com/mheiber/TypeScript/tree/checker/tests/cases/conformance/classes/members/privateNames/privateNamesNoDelete.ts)
    - implemented in `checkDeleteExpression`
- [x] Do [the right thing](https://gist.github.com/mheiber/b6fc7adb426c2e1cdaceb5d7786fc630) for nested classes

mv private name tests together

more checker tests for private names

update naming and cleanup for check private names

for private name support in the checker:
- make names more consistent
- remove unnecessary checks
- add utility function to public API
- other small cleanup

Move getPropertyNameForUniqueESSymbol to utility

for consistency with other calculation of
special property names (starting with __),
move the calculation of property names for
unique es symbols to `utilities.ts`.

private name tests strict+es6

Update private name tests to use 'strict'
type checking and to target es6 instead of
default. Makes the js output easier to read
and tests more surface area with other
checker features.

error message for private names in obj literals

Disallow decorating private-named properties
because the spec is still in flux.

Signed-off-by: Max Heiber <[email protected]>

Author: mheiber

src/compiler/binder.ts

@@ -344,8 +344,15 @@ namespace ts {
                     Debug.assert(isWellKnownSymbolSyntactically(nameExpression));
                     return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
                 }
-                if (isPrivateName(node)) {
-                    return nodePosToString(node) as __String;
+                if (isPrivateName(name)) {
+                    // containingClass exists because private names only allowed inside classes
+                    const containingClass = getContainingClass(name.parent);
+                    if (!containingClass) {
+                        // we're in a case where there's a private name outside a class (invalid)
+                        return undefined;
+                    }
+                    const containingClassSymbol = containingClass.symbol;
+                    return getPropertyNameForPrivateNameDescription(containingClassSymbol, name.escapedText);
                 }
                 return isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
             }
@@ -403,6 +410,10 @@ namespace ts {
 
             const isDefaultExport = hasModifier(node, ModifierFlags.Default);
 
+            // need this before getDeclarationName
+            if (isNamedDeclaration(node)) {
+                node.name.parent = node;
+            }
             // The exported symbol for an export default function/class node is always named "default"
             const name = isDefaultExport && parent ? InternalSymbolName.Default : getDeclarationName(node);
 
@@ -455,11 +466,6 @@ namespace ts {
                         symbolTable.set(name, symbol = createSymbol(SymbolFlags.None, name));
                     }
                     else if (!(includes & SymbolFlags.Variable && symbol.flags & SymbolFlags.Assignment)) {
-                        // Assignment declarations are allowed to merge with variables, no matter what other flags they have.
-                        if (isNamedDeclaration(node)) {
-                            node.name.parent = node;
-                        }
-
                         // Report errors every position with duplicate declaration
                         // Report errors on previous encountered declarations
                         let message = symbol.flags & SymbolFlags.BlockScopedVariable
@@ -1983,6 +1989,18 @@ namespace ts {
             return Diagnostics.Identifier_expected_0_is_a_reserved_word_in_strict_mode;
         }
 
+        // The binder visits every node, so this is a good place to check for
+        // the reserved private name (there is only one)
+        function checkPrivateName(node: PrivateName) {
+            if (node.escapedText === "#constructor") {
+                // Report error only if there are no parse errors in file
+                if (!file.parseDiagnostics.length) {
+                    file.bindDiagnostics.push(createDiagnosticForNode(node,
+                        Diagnostics.constructor_is_a_reserved_word, declarationNameToString(node)));
+                }
+            }
+        }
+
         function checkStrictModeBinaryExpression(node: BinaryExpression) {
             if (inStrictMode && isLeftHandSideExpression(node.left) && isAssignmentOperator(node.operatorToken.kind)) {
                 // ECMA 262 (Annex C) The identifier eval or arguments may not appear as the LeftHandSideExpression of an
@@ -2255,6 +2273,8 @@ namespace ts {
                         node.flowNode = currentFlow;
                     }
                     return checkStrictModeIdentifier(<Identifier>node);
+                case SyntaxKind.PrivateName:
+                    return checkPrivateName(node as PrivateName);
                 case SyntaxKind.PropertyAccessExpression:
                 case SyntaxKind.ElementAccessExpression:
                     if (currentFlow && isNarrowableReference(<Expression>node)) {

src/compiler/checker.ts

@@ -5238,6 +5238,33 @@
     "Private names cannot be used as parameters": {
         "category": "Error",
         "code": 18009
+    },
+    "Accessibility modifiers cannot be used with private names.": {
+        "category": "Error",
+        "code": 18010
+    },
+    "The operand of a delete operator cannot be a private name.": {
+        "category": "Error",
+        "code": 18011
+    },
+    "'#constructor' is a reserved word.": {
+        "category": "Error",
+        "code": 18012
+    },
+    "Property '{0}' is not accessible outside class '{1}' because it has a private name.": {
+        "category": "Error",
+        "code": 18013
+    },
+    "This usage of '{0}' refers to the private member declared in its enclosing class. While type '{1}' has a private member with the same spelling, its declaration and accessibility are distinct.": {
+        "category": "Error",
+        "code": 18014
+    },
+    "Property '{0}' is missing in type '{1}'. While type '{1}' has a private member with the same spelling, its declaration and accessibility are distinct.": {
+        "category": "Error",
+        "code": 18015
+    },
+    "Private names are not allowed outside class bodies.": {
+        "category": "Error",
+        "code": 18016
     }
-
 }

src/compiler/diagnosticMessages.json

@@ -3184,6 +3184,7 @@ namespace ts {
         getDeclaredTypeOfSymbol(symbol: Symbol): Type;
         getPropertiesOfType(type: Type): Symbol[];
         getPropertyOfType(type: Type, propertyName: string): Symbol | undefined;
+        getPropertyForPrivateName(apparentType: Type, leftType: Type, right: PrivateName, errorNode: Node | undefined): Symbol | undefined;
         /* @internal */ getTypeOfPropertyOfType(type: Type, propertyName: string): Type | undefined;
         getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
         getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];

src/compiler/types.ts

@@ -1604,6 +1604,10 @@ namespace ts {
     export function nodeCanBeDecorated(node: ClassElement, parent: Node): boolean;
     export function nodeCanBeDecorated(node: Node, parent: Node, grandparent: Node): boolean;
     export function nodeCanBeDecorated(node: Node, parent?: Node, grandparent?: Node): boolean {
+        // private names cannot be used with decorators yet
+        if (isNamedDeclaration(node) && isPrivateName(node.name)) {
+            return false;
+        }
         switch (node.kind) {
             case SyntaxKind.ClassDeclaration:
                 // classes are valid targets
@@ -2880,10 +2884,18 @@ namespace ts {
         return node.kind === SyntaxKind.Identifier || node.kind === SyntaxKind.PrivateName ? node.escapedText : escapeLeadingUnderscores(node.text);
     }
 
+    export function getPropertyNameForUniqueESSymbol(symbol: Symbol): __String {
+        return `__@${getSymbolId(symbol)}@${symbol.escapedName}` as __String;
+    }
+
     export function getPropertyNameForKnownSymbolName(symbolName: string): __String {
         return "__@" + symbolName as __String;
     }
 
+    export function getPropertyNameForPrivateNameDescription(containingClassSymbol: Symbol, description: __String): __String {
+        return `__#${getSymbolId(containingClassSymbol)}@${description}` as __String;
+    }
+
     export function isKnownSymbol(symbol: Symbol): boolean {
         return startsWith(symbol.escapedName as string, "__@");
     }

src/compiler/utilities.ts

@@ -1958,6 +1958,7 @@ declare namespace ts {
         getDeclaredTypeOfSymbol(symbol: Symbol): Type;
         getPropertiesOfType(type: Type): Symbol[];
         getPropertyOfType(type: Type, propertyName: string): Symbol | undefined;
+        getPropertyForPrivateName(apparentType: Type, leftType: Type, right: PrivateName, errorNode: Node | undefined): Symbol | undefined;
         getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
         getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;

tests/baselines/reference/api/tsserverlibrary.d.ts

@@ -1958,6 +1958,7 @@ declare namespace ts {
         getDeclaredTypeOfSymbol(symbol: Symbol): Type;
         getPropertiesOfType(type: Type): Symbol[];
         getPropertyOfType(type: Type, propertyName: string): Symbol | undefined;
+        getPropertyForPrivateName(apparentType: Type, leftType: Type, right: PrivateName, errorNode: Node | undefined): Symbol | undefined;
         getIndexInfoOfType(type: Type, kind: IndexKind): IndexInfo | undefined;
         getSignaturesOfType(type: Type, kind: SignatureKind): readonly Signature[];
         getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;

tests/baselines/reference/api/typescript.d.ts

@@ -0,0 +1,34 @@
+EmitSkipped: false
+FileName : ./dist/index.js
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+var Foo = /** @class */ (function () {
+    function Foo() {
+    }
+    Foo.prototype.methodName = function (propName) { };
+    Foo.prototype.otherMethod = function () {
+        if (Math.random() > 0.5) {
+            return { x: 42 };
+        }
+        return { y: "yes" };
+    };
+    return Foo;
+}());
+exports.Foo = Foo;
+FileName : ./dist/index.d.ts.map
+{"version":3,"file":"index.d.ts","sourceRoot":"","sources":["../tests/cases/fourslash/index.ts"],"names":[],"mappings":"AAAA,qBAAa,GAAG;IACZ,MAAM,EAAE,MAAM,CAAC;IACf,UAAU,CAAC,QAAQ,EAAE,QAAQ,GAAG,IAAI;IACpC,WAAW;;;;;;;CAMd;AAED,MAAM,WAAW,QAAQ;IACrB,MAAM,EAAE,MAAM,CAAC;CAClB"}FileName : ./dist/index.d.ts
+export declare class Foo {
+    member: string;
+    methodName(propName: SomeType): void;
+    otherMethod(): {
+        x: number;
+        y?: undefined;
+    } | {
+        y: string;
+        x?: undefined;
+    };
+}
+export interface SomeType {
+    member: number;
+}
+//# sourceMappingURL=index.d.ts.map

tests/baselines/reference/declarationMapsGeneratedMapsEnableMapping.baseline

@@ -0,0 +1,35 @@
+EmitSkipped: false
+FileName : ./dist/index.js.map
+{"version":3,"file":"index.js","sourceRoot":"/tests/cases/fourslash/","sources":["index.ts"],"names":[],"mappings":";;AAAA;IAAA;IASA,CAAC;IAPG,wBAAU,GAAV,UAAW,QAAkB,IAAS,CAAC;IACvC,yBAAW,GAAX;QACI,IAAI,IAAI,CAAC,MAAM,EAAE,GAAG,GAAG,EAAE;YACrB,OAAO,EAAC,CAAC,EAAE,EAAE,EAAC,CAAC;SAClB;QACD,OAAO,EAAC,CAAC,EAAE,KAAK,EAAC,CAAC;IACtB,CAAC;IACL,UAAC;AAAD,CAAC,AATD,IASC;AATY,kBAAG"}FileName : ./dist/index.js
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+var Foo = /** @class */ (function () {
+    function Foo() {
+    }
+    Foo.prototype.methodName = function (propName) { };
+    Foo.prototype.otherMethod = function () {
+        if (Math.random() > 0.5) {
+            return { x: 42 };
+        }
+        return { y: "yes" };
+    };
+    return Foo;
+}());
+exports.Foo = Foo;
+//# sourceMappingURL=index.js.mapFileName : ./dist/index.d.ts.map
+{"version":3,"file":"index.d.ts","sourceRoot":"/tests/cases/fourslash/","sources":["index.ts"],"names":[],"mappings":"AAAA,qBAAa,GAAG;IACZ,MAAM,EAAE,MAAM,CAAC;IACf,UAAU,CAAC,QAAQ,EAAE,QAAQ,GAAG,IAAI;IACpC,WAAW;;;;;;;CAMd;AAED,MAAM,WAAW,QAAQ;IACrB,MAAM,EAAE,MAAM,CAAC;CAClB"}FileName : ./dist/index.d.ts
+export declare class Foo {
+    member: string;
+    methodName(propName: SomeType): void;
+    otherMethod(): {
+        x: number;
+        y?: undefined;
+    } | {
+        y: string;
+        x?: undefined;
+    };
+}
+export interface SomeType {
+    member: number;
+}
+//# sourceMappingURL=index.d.ts.map

tests/baselines/reference/declarationMapsGeneratedMapsEnableMapping2.baseline

@@ -0,0 +1,34 @@
+EmitSkipped: false
+FileName : ./dist/index.js
+"use strict";
+Object.defineProperty(exports, "__esModule", { value: true });
+var Foo = /** @class */ (function () {
+    function Foo() {
+    }
+    Foo.prototype.methodName = function (propName) { };
+    Foo.prototype.otherMethod = function () {
+        if (Math.random() > 0.5) {
+            return { x: 42 };
+        }
+        return { y: "yes" };
+    };
+    return Foo;
+}());
+exports.Foo = Foo;
+FileName : ./dist/index.d.ts.map
+{"version":3,"file":"index.d.ts","sourceRoot":"/tests/cases/fourslash/","sources":["index.ts"],"names":[],"mappings":"AAAA,qBAAa,GAAG;IACZ,MAAM,EAAE,MAAM,CAAC;IACf,UAAU,CAAC,QAAQ,EAAE,QAAQ,GAAG,IAAI;IACpC,WAAW;;;;;;;CAMd;AAED,MAAM,WAAW,QAAQ;IACrB,MAAM,EAAE,MAAM,CAAC;CAClB"}FileName : ./dist/index.d.ts
+export declare class Foo {
+    member: string;
+    methodName(propName: SomeType): void;
+    otherMethod(): {
+        x: number;
+        y?: undefined;
+    } | {
+        y: string;
+        x?: undefined;
+    };
+}
+export interface SomeType {
+    member: number;
+}
+//# sourceMappingURL=index.d.ts.map