Private Name Support in the Checker (#5)

* begin update checker for private names

- [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

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

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


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.


Signed-off-by: Max Heiber <max.heiber@gmail.com>

Author: mheiber

src/compiler/binder.ts

@@ -271,8 +271,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;
             }
@@ -330,6 +337,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);
 
@@ -382,11 +393,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
@@ -1802,6 +1808,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
@@ -2074,6 +2092,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

@@ -122,6 +122,7 @@ namespace ts {
             getDeclaredTypeOfSymbol,
             getPropertiesOfType,
             getPropertyOfType: (type, name) => getPropertyOfType(type, escapeLeadingUnderscores(name)),
+            getPropertyForPrivateName,
             getTypeOfPropertyOfType: (type, name) => getTypeOfPropertyOfType(type, escapeLeadingUnderscores(name)),
             getIndexInfoOfType,
             getSignaturesOfType,
@@ -1572,8 +1573,8 @@ namespace ts {
             }
         }
 
-        function diagnosticName(nameArg: __String | Identifier) {
-            return isString(nameArg) ? unescapeLeadingUnderscores(nameArg as __String) : declarationNameToString(nameArg as Identifier);
+        function diagnosticName(nameArg: __String | Identifier | PrivateName) {
+            return isString(nameArg) ? unescapeLeadingUnderscores(nameArg as __String) : declarationNameToString(nameArg as Identifier | PrivateName);
         }
 
         function isTypeParameterSymbolDeclaredInContainer(symbol: Symbol, container: Node) {
@@ -2679,15 +2680,16 @@ namespace ts {
             return getUnionType(arrayFrom(typeofEQFacts.keys(), getLiteralType));
         }
 
-        // A reserved member name starts with two underscores, but the third character cannot be an underscore
-        // or the @ symbol. A third underscore indicates an escaped form of an identifer that started
+        // A reserved member name starts with two underscores, but the third character cannot be an underscore,
+        // @, or #. A third underscore indicates an escaped form of an identifer that started
         // with at least two underscores. The @ character indicates that the name is denoted by a well known ES
-        // Symbol instance.
+        // Symbol instance and the # indicates that the name is a PrivateName.
         function isReservedMemberName(name: __String) {
             return (name as string).charCodeAt(0) === CharacterCodes._ &&
                 (name as string).charCodeAt(1) === CharacterCodes._ &&
                 (name as string).charCodeAt(2) !== CharacterCodes._ &&
-                (name as string).charCodeAt(2) !== CharacterCodes.at;
+                (name as string).charCodeAt(2) !== CharacterCodes.at &&
+                (name as string).charCodeAt(2) !== CharacterCodes.hash;
         }
 
         function getNamedMembers(members: SymbolTable): Symbol[] {
@@ -6250,7 +6252,7 @@ namespace ts {
          */
         function getLateBoundNameFromType(type: LiteralType | UniqueESSymbolType): __String {
             if (type.flags & TypeFlags.UniqueESSymbol) {
-                return `__@${type.symbol.escapedName}@${getSymbolId(type.symbol)}` as __String;
+                return getPropertyNameForUniqueESSymbol(type.symbol);
             }
             if (type.flags & (TypeFlags.StringLiteral | TypeFlags.NumberLiteral)) {
                 return escapeLeadingUnderscores("" + (<LiteralType>type).value);
@@ -9258,7 +9260,9 @@ namespace ts {
         }
 
         function getLiteralTypeFromPropertyName(prop: Symbol, include: TypeFlags) {
-            if (!(getDeclarationModifierFlagsFromSymbol(prop) & ModifierFlags.NonPublicAccessibilityModifier)) {
+            const hasNonPublicModifier = !!(getDeclarationModifierFlagsFromSymbol(prop) & ModifierFlags.NonPublicAccessibilityModifier);
+            const hasPrivateName = prop.valueDeclaration && isNamedDeclaration(prop.valueDeclaration) && isPrivateName(prop.valueDeclaration.name);
+            if (!hasNonPublicModifier && !hasPrivateName) {
                 let type = getLateBoundSymbol(prop).nameType;
                 if (!type && !isKnownSymbol(prop)) {
                     const name = prop.valueDeclaration && getNameOfDeclaration(prop.valueDeclaration);
@@ -12205,7 +12209,28 @@ namespace ts {
                 const unmatchedProperty = getUnmatchedProperty(source, target, requireOptionalProperties);
                 if (unmatchedProperty) {
                     if (reportErrors) {
-                        reportError(Diagnostics.Property_0_is_missing_in_type_1, symbolToString(unmatchedProperty), typeToString(source));
+                        let hasReported = false;
+                        // give specific error in case where private names have the same description
+                        if (
+                            unmatchedProperty.valueDeclaration
+                            && isNamedDeclaration(unmatchedProperty.valueDeclaration)
+                            && isPrivateName(unmatchedProperty.valueDeclaration.name)
+                            && isClassDeclaration(source.symbol.valueDeclaration)
+                        ) {
+                            const privateNameDescription = unmatchedProperty.valueDeclaration.name.escapedText;
+                            const symbolTableKey = getPropertyNameForPrivateNameDescription(source.symbol, privateNameDescription);
+                            if (symbolTableKey && !!getPropertyOfType(source, symbolTableKey)) {
+                                reportError(
+                                    Diagnostics.Property_0_is_missing_in_type_1_While_type_1_has_a_private_member_with_the_same_spelling_its_declaration_and_accessibility_are_distinct,
+                                    diagnosticName(privateNameDescription),
+                                    diagnosticName(source.symbol.valueDeclaration.name || ("(anonymous)" as __String))
+                                );
+                                hasReported = true;
+                            }
+                        }
+                        if (!hasReported) {
+                            reportError(Diagnostics.Property_0_is_missing_in_type_1, symbolToString(unmatchedProperty), typeToString(source));
+                        }
                     }
                     return Ternary.False;
                 }
@@ -18423,6 +18448,48 @@ namespace ts {
             return checkPropertyAccessExpressionOrQualifiedName(node, node.left, node.right);
         }
 
+        function getPropertyForPrivateName(apparentType: Type, leftType: Type, right: PrivateName, errorNode: Node | undefined): Symbol | undefined {
+            let classWithShadowedPrivateName;
+            let container = getContainingClass(right);
+            while (container) {
+                const symbolTableKey = getPropertyNameForPrivateNameDescription(container.symbol, right.escapedText);
+                if (symbolTableKey) {
+                    const prop = getPropertyOfType(apparentType, symbolTableKey);
+                    if (prop) {
+                        if (classWithShadowedPrivateName) {
+                            if (errorNode) {
+                                error(
+                                    errorNode,
+                                    Diagnostics.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,
+                                    diagnosticName(right),
+                                    diagnosticName(classWithShadowedPrivateName.name || ("(anonymous)" as __String))
+                                );
+                            }
+                            return undefined;
+                        }
+                        return prop;
+                    }
+                    else {
+                        classWithShadowedPrivateName = container;
+                    }
+                }
+                container = getContainingClass(container);
+            }
+            // If this isn't a case of shadowing, and the lhs has a property with the same
+            // private name description, then there is a privacy violation
+            if (leftType.symbol.members) {
+                const symbolTableKey = getPropertyNameForPrivateNameDescription(leftType.symbol, right.escapedText);
+                const prop = getPropertyOfType(apparentType, symbolTableKey);
+                if (prop) {
+                    if (errorNode) {
+                        error(right, Diagnostics.Property_0_is_not_accessible_outside_class_1_because_it_has_a_private_name, symbolToString(prop), typeToString(getDeclaringClass(prop)!));
+                    }
+                }
+            }
+            // not found
+            return undefined;
+        }
+
         function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier | PrivateName) {
             let propType: Type;
             const leftType = checkNonNullExpression(left);
@@ -18435,7 +18502,7 @@ namespace ts {
                 return apparentType;
             }
             const assignmentKind = getAssignmentTargetKind(node);
-            const prop = getPropertyOfType(apparentType, right.escapedText);
+            const prop = isPrivateName(right) ? getPropertyForPrivateName(apparentType, leftType, right, /* errorNode */ right) : getPropertyOfType(apparentType, right.escapedText);
             if (isIdentifier(left) && parentSymbol && !(prop && isConstEnumOrConstEnumOnlyModule(prop))) {
                 markAliasReferenced(parentSymbol, node);
             }
@@ -21376,6 +21443,9 @@ namespace ts {
                 error(expr, Diagnostics.The_operand_of_a_delete_operator_must_be_a_property_reference);
                 return booleanType;
             }
+            if (expr.kind === SyntaxKind.PropertyAccessExpression && isPrivateName((expr as PropertyAccessExpression).name)) {
+                error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_private_name);
+            }
             const links = getNodeLinks(expr);
             const symbol = getExportSymbolOfValueSymbolIfExported(links.resolvedSymbol);
             if (symbol && isReadonlySymbol(symbol)) {
@@ -22529,9 +22599,6 @@ namespace ts {
             checkGrammarDecoratorsAndModifiers(node);
 
             checkVariableLikeDeclaration(node);
-            if (node.name && isIdentifier(node.name) && node.name.originalKeywordKind === SyntaxKind.PrivateName) {
-                error(node, Diagnostics.Private_names_cannot_be_used_as_parameters);
-            }
             const func = getContainingFunction(node)!;
             if (hasModifier(node, ModifierFlags.ParameterPropertyModifier)) {
                 if (!(func.kind === SyntaxKind.Constructor && nodeIsPresent(func.body))) {
@@ -29210,6 +29277,9 @@ namespace ts {
             else if (node.kind === SyntaxKind.Parameter && (flags & ModifierFlags.ParameterPropertyModifier) && (<ParameterDeclaration>node).dotDotDotToken) {
                 return grammarErrorOnNode(node, Diagnostics.A_parameter_property_cannot_be_declared_using_a_rest_parameter);
             }
+            else if (isNamedDeclaration(node) && (flags & ModifierFlags.AccessibilityModifier) && node.name.kind === SyntaxKind.PrivateName) {
+                return grammarErrorOnNode(node, Diagnostics.Accessibility_modifiers_cannot_be_used_with_private_names);
+            }
             if (flags & ModifierFlags.Async) {
                 return checkGrammarAsyncModifier(node, lastAsync!);
             }
@@ -29611,6 +29681,10 @@ namespace ts {
                     return grammarErrorOnNode(prop.equalsToken!, Diagnostics.can_only_be_used_in_an_object_literal_property_inside_a_destructuring_assignment);
                 }
 
+                if (name.kind === SyntaxKind.PrivateName) {
+                    return grammarErrorOnNode(name, Diagnostics.Private_names_are_not_allowed_outside_class_bodies);
+                }
+
                 // Modifiers are never allowed on properties except for 'async' on a method declaration
                 if (prop.modifiers) {
                     for (const mod of prop.modifiers!) { // TODO: GH#19955
@@ -30035,10 +30109,6 @@ namespace ts {
                 checkESModuleMarker(node.name);
             }
 
-            if (isIdentifier(node.name) && node.name.originalKeywordKind === SyntaxKind.PrivateName) {
-                return grammarErrorOnNode(node.name, Diagnostics.Private_names_are_not_allowed_in_variable_declarations);
-            }
-
             const checkLetConstNames = (isLet(node) || isVarConst(node));
 
             // 1. LexicalDeclaration : LetOrConst BindingList ;

src/compiler/diagnosticMessages.json

@@ -4299,14 +4299,34 @@
         "category": "Error",
         "code": 18003
     },
-    "Private names are not allowed in variable declarations.": {
+    "Accessibility modifiers cannot be used with private names.": {
         "category": "Error",
         "code": 18004
     },
-    "Private names cannot be used as parameters": {
+    "The operand of a delete operator cannot be a private name.": {
         "category": "Error",
         "code": 18005
     },
+    "'#constructor' is a reserved word.": {
+        "category": "Error",
+        "code": 18006
+    },
+    "Property '{0}' is not accessible outside class '{1}' because it has a private name.": {
+        "category": "Error",
+        "code": 18007
+    },
+    "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": 18008
+    },
+    "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": 18009
+    },
+    "Private names are not allowed outside class bodies.": {
+        "category": "Error",
+        "code": 18010
+    },
 
     "File is a CommonJS module; it may be converted to an ES6 module.": {
         "category": "Suggestion",

src/compiler/types.ts

@@ -3026,6 +3026,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): ReadonlyArray<Signature>;

src/compiler/utilities.ts

@@ -2728,10 +2728,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, "__@");
     }

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

@@ -1876,6 +1876,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): ReadonlyArray<Signature>;
         getIndexTypeOfType(type: Type, kind: IndexKind): Type | undefined;