Skip to content

Commit 194037c

Browse files
author
Max Heiber
committed
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 Signed-off-by: Max Heiber <mheiber@bloomberg.net>
1 parent 853486f commit 194037c

File tree

73 files changed

+1521
-33
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

73 files changed

+1521
-33
lines changed

src/compiler/binder.ts

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,10 @@ namespace ts {
271271
Debug.assert(isWellKnownSymbolSyntactically(nameExpression));
272272
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
273273
}
274-
if (isPrivateName(node)) {
275-
return nodePosToString(node) as __String;
274+
if (isPrivateName(name)) {
275+
// containingClass exists because private names only allowed inside classes
276+
const containingClassSymbol = getContainingClass(name.parent)!.symbol;
277+
return getPropertyNameForPrivateNameDescription(containingClassSymbol, name.escapedText);
276278
}
277279
return isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
278280
}
@@ -330,6 +332,10 @@ namespace ts {
330332

331333
const isDefaultExport = hasModifier(node, ModifierFlags.Default);
332334

335+
// need this before getDeclarationName
336+
if (isNamedDeclaration(node)) {
337+
node.name.parent = node;
338+
}
333339
// The exported symbol for an export default function/class node is always named "default"
334340
const name = isDefaultExport && parent ? InternalSymbolName.Default : getDeclarationName(node);
335341

@@ -1802,6 +1808,18 @@ namespace ts {
18021808
return Diagnostics.Identifier_expected_0_is_a_reserved_word_in_strict_mode;
18031809
}
18041810

1811+
// The binder visits every node, so this is a good place to check for
1812+
// the reserved private name (there is only one)
1813+
function checkPrivateName(node: PrivateName) {
1814+
if (node.escapedText === "#constructor") {
1815+
// Report error only if there are no parse errors in file
1816+
if (!file.parseDiagnostics.length) {
1817+
file.bindDiagnostics.push(createDiagnosticForNode(node,
1818+
Diagnostics.constructor_is_a_reserved_word, declarationNameToString(node)));
1819+
}
1820+
}
1821+
}
1822+
18051823
function checkStrictModeBinaryExpression(node: BinaryExpression) {
18061824
if (inStrictMode && isLeftHandSideExpression(node.left) && isAssignmentOperator(node.operatorToken.kind)) {
18071825
// ECMA 262 (Annex C) The identifier eval or arguments may not appear as the LeftHandSideExpression of an
@@ -2074,6 +2092,8 @@ namespace ts {
20742092
node.flowNode = currentFlow;
20752093
}
20762094
return checkStrictModeIdentifier(<Identifier>node);
2095+
case SyntaxKind.PrivateName:
2096+
return checkPrivateName(node as PrivateName);
20772097
case SyntaxKind.PropertyAccessExpression:
20782098
case SyntaxKind.ElementAccessExpression:
20792099
if (currentFlow && isNarrowableReference(<Expression>node)) {

src/compiler/checker.ts

Lines changed: 84 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1577,8 +1577,8 @@ namespace ts {
15771577
}
15781578
}
15791579

1580-
function diagnosticName(nameArg: __String | Identifier) {
1581-
return isString(nameArg) ? unescapeLeadingUnderscores(nameArg as __String) : declarationNameToString(nameArg as Identifier);
1580+
function diagnosticName(nameArg: __String | Identifier | PrivateName) {
1581+
return isString(nameArg) ? unescapeLeadingUnderscores(nameArg as __String) : declarationNameToString(nameArg as Identifier | PrivateName);
15821582
}
15831583

15841584
function isTypeParameterSymbolDeclaredInContainer(symbol: Symbol, container: Node) {
@@ -2676,15 +2676,16 @@ namespace ts {
26762676
return getUnionType(arrayFrom(typeofEQFacts.keys(), getLiteralType));
26772677
}
26782678

2679-
// A reserved member name starts with two underscores, but the third character cannot be an underscore
2680-
// or the @ symbol. A third underscore indicates an escaped form of an identifer that started
2679+
// A reserved member name starts with two underscores, but the third character cannot be an underscore,
2680+
// @ or #. A third underscore indicates an escaped form of an identifer that started
26812681
// with at least two underscores. The @ character indicates that the name is denoted by a well known ES
2682-
// Symbol instance.
2682+
// Symbol instance and the # indicates that the name is a PrivateName.
26832683
function isReservedMemberName(name: __String) {
26842684
return (name as string).charCodeAt(0) === CharacterCodes._ &&
26852685
(name as string).charCodeAt(1) === CharacterCodes._ &&
26862686
(name as string).charCodeAt(2) !== CharacterCodes._ &&
2687-
(name as string).charCodeAt(2) !== CharacterCodes.at;
2687+
(name as string).charCodeAt(2) !== CharacterCodes.at &&
2688+
(name as string).charCodeAt(2) !== CharacterCodes.hash;
26882689
}
26892690

26902691
function getNamedMembers(members: SymbolTable): Symbol[] {
@@ -9211,7 +9212,9 @@ namespace ts {
92119212
}
92129213

92139214
function getLiteralTypeFromPropertyName(prop: Symbol, include: TypeFlags) {
9214-
if (!(getDeclarationModifierFlagsFromSymbol(prop) & ModifierFlags.NonPublicAccessibilityModifier)) {
9215+
const hasNonPublicModifier = !!(getDeclarationModifierFlagsFromSymbol(prop) & ModifierFlags.NonPublicAccessibilityModifier);
9216+
const hasPrivateName = prop.valueDeclaration && isNamedDeclaration(prop.valueDeclaration) && isPrivateName(prop.valueDeclaration.name);
9217+
if (!hasNonPublicModifier && !hasPrivateName) {
92159218
let type = getLateBoundSymbol(prop).nameType;
92169219
if (!type && !isKnownSymbol(prop)) {
92179220
const name = prop.valueDeclaration && getNameOfDeclaration(prop.valueDeclaration);
@@ -12132,7 +12135,28 @@ namespace ts {
1213212135
const unmatchedProperty = getUnmatchedProperty(source, target, requireOptionalProperties);
1213312136
if (unmatchedProperty) {
1213412137
if (reportErrors) {
12135-
reportError(Diagnostics.Property_0_is_missing_in_type_1, symbolToString(unmatchedProperty), typeToString(source));
12138+
let hasReported = false;
12139+
// give specific error in case where private names have the same description
12140+
if (
12141+
unmatchedProperty.valueDeclaration
12142+
&& isNamedDeclaration(unmatchedProperty.valueDeclaration)
12143+
&& isPrivateName(unmatchedProperty.valueDeclaration.name)
12144+
&& isClassDeclaration(source.symbol.valueDeclaration)
12145+
) {
12146+
const privateNameDescription = unmatchedProperty.valueDeclaration.name.escapedText;
12147+
const symbolTableKey = getPropertyNameForPrivateNameDescription(source.symbol, privateNameDescription);
12148+
if (symbolTableKey && !!getPropertyOfType(source, symbolTableKey)) {
12149+
reportError(
12150+
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,
12151+
diagnosticName(privateNameDescription),
12152+
diagnosticName(source.symbol.valueDeclaration.name || ("anonymous" as __String))
12153+
);
12154+
hasReported = true;
12155+
}
12156+
}
12157+
if (!hasReported) {
12158+
reportError(Diagnostics.Property_0_is_missing_in_type_1, symbolToString(unmatchedProperty), typeToString(source));
12159+
}
1213612160
}
1213712161
return Ternary.False;
1213812162
}
@@ -18621,6 +18645,46 @@ namespace ts {
1862118645
return checkPropertyAccessExpressionOrQualifiedName(node, node.left, node.right);
1862218646
}
1862318647

18648+
function getPropertyByPrivateName(apparentType: Type, leftType: Type, right: PrivateName): Symbol | undefined {
18649+
let classWithShadowedPrivateName;
18650+
let klass = getContainingClass(right);
18651+
while (klass) {
18652+
const symbolTableKey = getPropertyNameForPrivateNameDescription(klass.symbol, right.escapedText);
18653+
if (symbolTableKey) {
18654+
const prop = getPropertyOfType(apparentType, symbolTableKey);
18655+
if (prop) {
18656+
if (classWithShadowedPrivateName) {
18657+
error(
18658+
right,
18659+
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,
18660+
diagnosticName(right),
18661+
diagnosticName(classWithShadowedPrivateName.name || ("anonymous" as __String))
18662+
);
18663+
return undefined;
18664+
}
18665+
return prop;
18666+
}
18667+
else {
18668+
classWithShadowedPrivateName = klass;
18669+
}
18670+
}
18671+
klass = getContainingClass(klass);
18672+
}
18673+
// If this isn't a case of shadowing, and the lhs has a property with the same
18674+
// private name description, then there is a privacy violation
18675+
if (leftType.symbol.members) {
18676+
const symbolTableKey = getPropertyNameForPrivateNameDescription(leftType.symbol, right.escapedText);
18677+
if (symbolTableKey) {
18678+
const prop = getPropertyOfType(apparentType, symbolTableKey);
18679+
if (prop) {
18680+
error(right, Diagnostics.Property_0_is_not_accessible_outside_class_1_because_it_has_a_private_name, symbolToString(prop), typeToString(getDeclaringClass(prop)!));
18681+
}
18682+
}
18683+
}
18684+
// not found
18685+
return undefined;
18686+
}
18687+
1862418688
function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier | PrivateName) {
1862518689
let propType: Type;
1862618690
const leftType = checkNonNullExpression(left);
@@ -18633,7 +18697,7 @@ namespace ts {
1863318697
return apparentType;
1863418698
}
1863518699
const assignmentKind = getAssignmentTargetKind(node);
18636-
const prop = getPropertyOfType(apparentType, right.escapedText);
18700+
const prop = isPrivateName(right) ? getPropertyByPrivateName(apparentType, leftType, right) : getPropertyOfType(apparentType, right.escapedText);
1863718701
if (isIdentifier(left) && parentSymbol && !(prop && isConstEnumOrConstEnumOnlyModule(prop))) {
1863818702
markAliasReferenced(parentSymbol, node);
1863918703
}
@@ -21489,10 +21553,16 @@ namespace ts {
2148921553
error(expr, Diagnostics.The_operand_of_a_delete_operator_must_be_a_property_reference);
2149021554
return booleanType;
2149121555
}
21556+
if (expr.kind === SyntaxKind.PropertyAccessExpression && isPrivateName((expr as PropertyAccessExpression).name)) {
21557+
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_private_name);
21558+
21559+
}
2149221560
const links = getNodeLinks(expr);
2149321561
const symbol = getExportSymbolOfValueSymbolIfExported(links.resolvedSymbol);
21494-
if (symbol && isReadonlySymbol(symbol)) {
21495-
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_read_only_property);
21562+
if (symbol) {
21563+
if (isReadonlySymbol(symbol)) {
21564+
error(expr, Diagnostics.The_operand_of_a_delete_operator_cannot_be_a_read_only_property);
21565+
}
2149621566
}
2149721567
return booleanType;
2149821568
}
@@ -22642,9 +22712,6 @@ namespace ts {
2264222712
checkGrammarDecoratorsAndModifiers(node);
2264322713

2264422714
checkVariableLikeDeclaration(node);
22645-
if (node.name && isIdentifier(node.name) && node.name.originalKeywordKind === SyntaxKind.PrivateName) {
22646-
error(node, Diagnostics.Private_names_cannot_be_used_as_parameters);
22647-
}
2264822715
const func = getContainingFunction(node)!;
2264922716
if (hasModifier(node, ModifierFlags.ParameterPropertyModifier)) {
2265022717
if (!(func.kind === SyntaxKind.Constructor && nodeIsPresent(func.body))) {
@@ -29321,6 +29388,9 @@ namespace ts {
2932129388
else if (node.kind === SyntaxKind.Parameter && (flags & ModifierFlags.ParameterPropertyModifier) && (<ParameterDeclaration>node).dotDotDotToken) {
2932229389
return grammarErrorOnNode(node, Diagnostics.A_parameter_property_cannot_be_declared_using_a_rest_parameter);
2932329390
}
29391+
else if (isNamedDeclaration(node) && (flags & ModifierFlags.AccessibilityModifier) && node.name.kind === SyntaxKind.PrivateName) {
29392+
return grammarErrorOnNode(node, Diagnostics.Accessibility_modifiers_cannot_be_used_with_private_names);
29393+
}
2932429394
if (flags & ModifierFlags.Async) {
2932529395
return checkGrammarAsyncModifier(node, lastAsync!);
2932629396
}
@@ -30146,10 +30216,6 @@ namespace ts {
3014630216
checkESModuleMarker(node.name);
3014730217
}
3014830218

30149-
if (isIdentifier(node.name) && node.name.originalKeywordKind === SyntaxKind.PrivateName) {
30150-
return grammarErrorOnNode(node.name, Diagnostics.Private_names_are_not_allowed_in_variable_declarations);
30151-
}
30152-
3015330219
const checkLetConstNames = (isLet(node) || isVarConst(node));
3015430220

3015530221
// 1. LexicalDeclaration : LetOrConst BindingList ;

src/compiler/diagnosticMessages.json

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4262,14 +4262,30 @@
42624262
"category": "Error",
42634263
"code": 18003
42644264
},
4265-
"Private names are not allowed in variable declarations.": {
4265+
"Accessibility modifiers cannot be used with private names.": {
42664266
"category": "Error",
42674267
"code": 18004
42684268
},
4269-
"Private names cannot be used as parameters": {
4269+
"The operand of a delete operator cannot be a private name.": {
42704270
"category": "Error",
42714271
"code": 18005
42724272
},
4273+
"#constructor is a reserved word.": {
4274+
"category": "Error",
4275+
"code": 18006
4276+
},
4277+
"Property '{0}' is not accessible outside class '{1}' because it has a private name.": {
4278+
"category": "Error",
4279+
"code": 18007
4280+
},
4281+
"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.": {
4282+
"category": "Error",
4283+
"code": 18008
4284+
},
4285+
"Property '{0}' is missing in type '{1}'. While type '{1}' has a private member with the same spelling, its declaration and accessibility are distinct": {
4286+
"category": "Error",
4287+
"code": 18009
4288+
},
42734289

42744290
"File is a CommonJS module; it may be converted to an ES6 module.": {
42754291
"category": "Suggestion",

src/compiler/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2698,6 +2698,10 @@ namespace ts {
26982698
return "__@" + symbolName as __String;
26992699
}
27002700

2701+
export function getPropertyNameForPrivateNameDescription(containingClassSymbol: Symbol, description: __String): __String {
2702+
return `__#${getSymbolId(containingClassSymbol)}@${description}` as __String;
2703+
}
2704+
27012705
export function isKnownSymbol(symbol: Symbol): boolean {
27022706
return startsWith(symbol.escapedName as string, "__@");
27032707
}
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
EmitSkipped: false
2+
FileName : ./dist/index.js
3+
"use strict";
4+
Object.defineProperty(exports, "__esModule", { value: true });
5+
var Foo = /** @class */ (function () {
6+
function Foo() {
7+
}
8+
Foo.prototype.methodName = function (propName) { };
9+
Foo.prototype.otherMethod = function () {
10+
if (Math.random() > 0.5) {
11+
return { x: 42 };
12+
}
13+
return { y: "yes" };
14+
};
15+
return Foo;
16+
}());
17+
exports.Foo = Foo;
18+
FileName : ./dist/index.d.ts.map
19+
{"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
20+
export declare class Foo {
21+
member: string;
22+
methodName(propName: SomeType): void;
23+
otherMethod(): {
24+
x: number;
25+
y?: undefined;
26+
} | {
27+
y: string;
28+
x?: undefined;
29+
};
30+
}
31+
export interface SomeType {
32+
member: number;
33+
}
34+
//# sourceMappingURL=index.d.ts.map
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
EmitSkipped: false
2+
FileName : ./dist/index.js.map
3+
{"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
4+
"use strict";
5+
Object.defineProperty(exports, "__esModule", { value: true });
6+
var Foo = /** @class */ (function () {
7+
function Foo() {
8+
}
9+
Foo.prototype.methodName = function (propName) { };
10+
Foo.prototype.otherMethod = function () {
11+
if (Math.random() > 0.5) {
12+
return { x: 42 };
13+
}
14+
return { y: "yes" };
15+
};
16+
return Foo;
17+
}());
18+
exports.Foo = Foo;
19+
//# sourceMappingURL=index.js.mapFileName : ./dist/index.d.ts.map
20+
{"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
21+
export declare class Foo {
22+
member: string;
23+
methodName(propName: SomeType): void;
24+
otherMethod(): {
25+
x: number;
26+
y?: undefined;
27+
} | {
28+
y: string;
29+
x?: undefined;
30+
};
31+
}
32+
export interface SomeType {
33+
member: number;
34+
}
35+
//# sourceMappingURL=index.d.ts.map
Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
EmitSkipped: false
2+
FileName : ./dist/index.js
3+
"use strict";
4+
Object.defineProperty(exports, "__esModule", { value: true });
5+
var Foo = /** @class */ (function () {
6+
function Foo() {
7+
}
8+
Foo.prototype.methodName = function (propName) { };
9+
Foo.prototype.otherMethod = function () {
10+
if (Math.random() > 0.5) {
11+
return { x: 42 };
12+
}
13+
return { y: "yes" };
14+
};
15+
return Foo;
16+
}());
17+
exports.Foo = Foo;
18+
FileName : ./dist/index.d.ts.map
19+
{"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
20+
export declare class Foo {
21+
member: string;
22+
methodName(propName: SomeType): void;
23+
otherMethod(): {
24+
x: number;
25+
y?: undefined;
26+
} | {
27+
y: string;
28+
x?: undefined;
29+
};
30+
}
31+
export interface SomeType {
32+
member: number;
33+
}
34+
//# sourceMappingURL=index.d.ts.map

0 commit comments

Comments
 (0)