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

@@ -278,8 +278,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;
             }
@@ -337,6 +344,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);
 
@@ -389,11 +400,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
@@ -1858,6 +1864,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
@@ -2130,6 +2148,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

@@ -132,6 +132,7 @@ namespace ts {
             getDeclaredTypeOfSymbol,
             getPropertiesOfType,
             getPropertyOfType: (type, name) => getPropertyOfType(type, escapeLeadingUnderscores(name)),
+            getPropertyForPrivateName,
             getTypeOfPropertyOfType: (type, name) => getTypeOfPropertyOfType(type, escapeLeadingUnderscores(name)),
             getIndexInfoOfType,
             getSignaturesOfType,
@@ -1656,8 +1657,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) {
@@ -2839,15 +2840,16 @@ namespace ts {
             return type;
         }
 
-        // 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[] {
@@ -6554,7 +6556,7 @@ namespace ts {
          */
         function getPropertyNameFromType(type: StringLiteralType | NumberLiteralType | UniqueESSymbolType): __String {
             if (type.flags & TypeFlags.UniqueESSymbol) {
-                return (<UniqueESSymbolType>type).escapedName;
+                return getPropertyNameForUniqueESSymbol(type.symbol);
             }
             if (type.flags & (TypeFlags.StringLiteral | TypeFlags.NumberLiteral)) {
                 return escapeLeadingUnderscores("" + (<StringLiteralType | NumberLiteralType>type).value);
@@ -9801,6 +9803,9 @@ namespace ts {
         }
 
         function getLiteralTypeFromPropertyName(name: PropertyName) {
+            if (isPrivateName(name)) {
+                return neverType;
+            }
             return isIdentifier(name) ? getLiteralType(unescapeLeadingUnderscores(name.escapedText)) :
                 getRegularTypeOfLiteralType(isComputedPropertyName(name) ? checkComputedPropertyName(name) : checkExpression(name));
         }
@@ -13091,23 +13096,44 @@ namespace ts {
                 const unmatchedProperty = getUnmatchedProperty(source, target, requireOptionalProperties, /*matchDiscriminantProperties*/ false);
                 if (unmatchedProperty) {
                     if (reportErrors) {
-                        const props = arrayFrom(getUnmatchedProperties(source, target, requireOptionalProperties, /*matchDiscriminantProperties*/ false));
-                        if (!headMessage || (headMessage.code !== Diagnostics.Class_0_incorrectly_implements_interface_1.code &&
-                            headMessage.code !== Diagnostics.Class_0_incorrectly_implements_class_1_Did_you_mean_to_extend_1_and_inherit_its_members_as_a_subclass.code)) {
-                            suppressNextError = true; // Retain top-level error for interface implementing issues, otherwise omit it
-                        }
-                        if (props.length === 1) {
-                            const propName = symbolToString(unmatchedProperty);
-                            reportError(Diagnostics.Property_0_is_missing_in_type_1_but_required_in_type_2, propName, typeToString(source), typeToString(target));
-                            if (length(unmatchedProperty.declarations)) {
-                                associateRelatedInfo(createDiagnosticForNode(unmatchedProperty.declarations[0], Diagnostics._0_is_declared_here, propName));
+                        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;
                             }
                         }
-                        else if (props.length > 5) { // arbitrary cutoff for too-long list form
-                            reportError(Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2_and_3_more, typeToString(source), typeToString(target), map(props.slice(0, 4), p => symbolToString(p)).join(", "), props.length - 4);
-                        }
-                        else {
-                            reportError(Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2, typeToString(source), typeToString(target), map(props, p => symbolToString(p)).join(", "));
+                        if (!hasReported) {
+                            const props = arrayFrom(getUnmatchedProperties(source, target, requireOptionalProperties, /*matchDiscriminantProperties*/ false));
+                            if (!headMessage || (headMessage.code !== Diagnostics.Class_0_incorrectly_implements_interface_1.code &&
+                                headMessage.code !== Diagnostics.Class_0_incorrectly_implements_class_1_Did_you_mean_to_extend_1_and_inherit_its_members_as_a_subclass.code)) {
+                                suppressNextError = true; // Retain top-level error for interface implementing issues, otherwise omit it
+                            }
+                            if (props.length === 1) {
+                                const propName = symbolToString(unmatchedProperty);
+                                reportError(Diagnostics.Property_0_is_missing_in_type_1_but_required_in_type_2, propName, typeToString(source), typeToString(target));
+                                if (length(unmatchedProperty.declarations)) {
+                                    associateRelatedInfo(createDiagnosticForNode(unmatchedProperty.declarations[0], Diagnostics._0_is_declared_here, propName));
+                                }
+                            }
+                            else if (props.length > 5) { // arbitrary cutoff for too-long list form
+                                reportError(Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2_and_3_more, typeToString(source), typeToString(target), map(props.slice(0, 4), p => symbolToString(p)).join(", "), props.length - 4);
+                            }
+                            else {
+                                reportError(Diagnostics.Type_0_is_missing_the_following_properties_from_type_1_Colon_2, typeToString(source), typeToString(target), map(props, p => symbolToString(p)).join(", "));
+                            }
                         }
                     }
                     return Ternary.False;
@@ -19593,6 +19619,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);
@@ -19605,7 +19673,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);
             }
@@ -22625,6 +22693,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)) {
@@ -23936,9 +24007,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))) {
@@ -30695,6 +30763,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!);
             }
@@ -31092,6 +31163,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
@@ -31533,10 +31608,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 ;