Skip to content

Commit

Permalink
ES private field check (#44648)
Browse files Browse the repository at this point in the history
* es private fields in in (#52)

add support for the 'private-fields-in-in' TC39 proposal

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [fixup] include inToken when walking forEachChild(node, cb)

* [squash] re-accept lib definition baseline changes

* [squash] reduce if/else to ternary

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] drop 'originalName' and rename parameter instead

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] extend spelling suggestion to all privateIdentifiers

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] revert the added lexical spelling suggestions logic

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] update baseline

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] inline variable as per PR suggestion

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] test targets both esnext and es2020 as per PR comment

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* switch to using a binary expression

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] PrivateIdentifier now extends PrimaryExpression

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] accept public api baseline changes

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] classPrivateFieldInHelper now has documentation

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] type-check now follows existing in-expression path

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] parser now follows existing binaryExpression path

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] correct typo in comment

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] no longer use esNext flag

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] swap 'reciever, state' helper params

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] remove change to parenthesizerRules

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] apply suggested changes to checker

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] remove need for assertion in fixSpelling

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] improve comment hint in test

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] fix comment typos

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] add flow-test for Foo | FooSub | Bar

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] add checkExternalEmitHelpers call and new test case

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] simplify and correct parser

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] move most of the added checker logic to expression level

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] always error when privateId could not be resolved

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] reword comment

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] fix codeFixSpelling test

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] do less work

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* store symbol by priateId not binaryExpression

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* moved parsePrivateIdentifier into parsePrimaryExpression

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] checkInExpressionn bails out early on silentNeverType

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] more detailed error messages

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] resolves conflict in diagnosticMessages.json

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] update baseline for importHelpersES6

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] remove redundent if and comment from parser

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] split up grammar/check/symbolLookup

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>

* [squash] reword message for existing left side of in-expression error

Signed-off-by: Ashley Claymore <acutmore@users.noreply.github.com>
  • Loading branch information
acutmore committed Sep 24, 2021
1 parent 59fb373 commit af689cc
Show file tree
Hide file tree
Showing 48 changed files with 2,398 additions and 45 deletions.
89 changes: 82 additions & 7 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23917,6 +23917,9 @@ namespace ts {
case SyntaxKind.InstanceOfKeyword:
return narrowTypeByInstanceof(type, expr, assumeTrue);
case SyntaxKind.InKeyword:
if (isPrivateIdentifier(expr.left)) {
return narrowTypeByPrivateIdentifierInInExpression(type, expr, assumeTrue);
}
const target = getReferenceCandidate(expr.right);
const leftType = getTypeOfNode(expr.left);
if (leftType.flags & TypeFlags.StringLiteral) {
Expand Down Expand Up @@ -23947,6 +23950,24 @@ namespace ts {
return type;
}

function narrowTypeByPrivateIdentifierInInExpression(type: Type, expr: BinaryExpression, assumeTrue: boolean): Type {
const target = getReferenceCandidate(expr.right);
if (!isMatchingReference(reference, target)) {
return type;
}

Debug.assertNode(expr.left, isPrivateIdentifier);
const symbol = getSymbolForPrivateIdentifierExpression(expr.left);
if (symbol === undefined) {
return type;
}
const classSymbol = symbol.parent!;
const targetType = hasStaticModifier(Debug.checkDefined(symbol.valueDeclaration, "should always have a declaration"))
? getTypeOfSymbol(classSymbol) as InterfaceType
: getDeclaredTypeOfSymbol(classSymbol);
return getNarrowedType(type, targetType, assumeTrue, isTypeDerivedFrom);
}

function narrowTypeByOptionalChainContainment(type: Type, operator: SyntaxKind, value: Expression, assumeTrue: boolean): Type {
// We are in a branch of obj?.foo === value (or any one of the other equality operators). We narrow obj as follows:
// When operator is === and type of value excludes undefined, null and undefined is removed from type of obj in true branch.
Expand Down Expand Up @@ -27790,6 +27811,40 @@ namespace ts {
}
}

function checkGrammarPrivateIdentifierExpression(privId: PrivateIdentifier): boolean {
if (!getContainingClass(privId)) {
return grammarErrorOnNode(privId, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
}
if (!isExpressionNode(privId)) {
return grammarErrorOnNode(privId, Diagnostics.Private_identifiers_are_only_allowed_in_class_bodies_and_may_only_be_used_as_part_of_a_class_member_declaration_property_access_or_on_the_left_hand_side_of_an_in_expression);
}
if (!getSymbolForPrivateIdentifierExpression(privId)) {
return grammarErrorOnNode(privId, Diagnostics.Cannot_find_name_0, idText(privId));
}
return false;
}

function checkPrivateIdentifierExpression(privId: PrivateIdentifier): Type {
checkGrammarPrivateIdentifierExpression(privId);
const symbol = getSymbolForPrivateIdentifierExpression(privId);
if (symbol) {
markPropertyAsReferenced(symbol, /* nodeForCheckWriteOnly: */ undefined, /* isThisAccess: */ false);
}
return anyType;
}

function getSymbolForPrivateIdentifierExpression(privId: PrivateIdentifier): Symbol | undefined {
if (!isExpressionNode(privId)) {
return undefined;
}

const links = getNodeLinks(privId);
if (links.resolvedSymbol === undefined) {
links.resolvedSymbol = lookupSymbolForPrivateIdentifierDeclaration(privId.escapedText, privId);
}
return links.resolvedSymbol;
}

function getPrivateIdentifierPropertyOfType(leftType: Type, lexicallyScopedIdentifier: Symbol): Symbol | undefined {
return getPropertyOfType(leftType, lexicallyScopedIdentifier.escapedName);
}
Expand Down Expand Up @@ -32110,11 +32165,29 @@ namespace ts {
if (leftType === silentNeverType || rightType === silentNeverType) {
return silentNeverType;
}
leftType = checkNonNullType(leftType, left);
if (isPrivateIdentifier(left)) {
if (languageVersion < ScriptTarget.ESNext) {
checkExternalEmitHelpers(left, ExternalEmitHelpers.ClassPrivateFieldIn);
}
// Unlike in 'checkPrivateIdentifierExpression' we now have access to the RHS type
// which provides us with the opportunity to emit more detailed errors
if (!getNodeLinks(left).resolvedSymbol && getContainingClass(left)) {
const isUncheckedJS = isUncheckedJSSuggestion(left, rightType.symbol, /*excludeClasses*/ true);
reportNonexistentProperty(left, rightType, isUncheckedJS);
}
}
else {
leftType = checkNonNullType(leftType, left);
// TypeScript 1.0 spec (April 2014): 4.15.5
// Require the left operand to be of type Any, the String primitive type, or the Number primitive type.
if (!(allTypesAssignableToKind(leftType, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.ESSymbolLike) ||
isTypeAssignableToKind(leftType, TypeFlags.Index | TypeFlags.TemplateLiteral | TypeFlags.StringMapping | TypeFlags.TypeParameter))) {
error(left, Diagnostics.The_left_hand_side_of_an_in_expression_must_be_a_private_identifier_or_of_type_any_string_number_or_symbol);
}
}
rightType = checkNonNullType(rightType, right);
// TypeScript 1.0 spec (April 2014): 4.15.5
// The in operator requires the left operand to be of type Any, the String primitive type, or the Number primitive type,
// and the right operand to be
// The in operator requires the right operand to be
//
// 1. assignable to the non-primitive type,
// 2. an unconstrained type parameter,
Expand All @@ -32132,10 +32205,6 @@ namespace ts {
// unless *all* instantiations would result in an error.
//
// The result is always of the Boolean primitive type.
if (!(allTypesAssignableToKind(leftType, TypeFlags.StringLike | TypeFlags.NumberLike | TypeFlags.ESSymbolLike) ||
isTypeAssignableToKind(leftType, TypeFlags.Index | TypeFlags.TemplateLiteral | TypeFlags.StringMapping | TypeFlags.TypeParameter))) {
error(left, Diagnostics.The_left_hand_side_of_an_in_expression_must_be_of_type_any_string_number_or_symbol);
}
const rightTypeConstraint = getConstraintOfType(rightType);
if (!allTypesAssignableToKind(rightType, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive) ||
rightTypeConstraint && (
Expand Down Expand Up @@ -33517,6 +33586,8 @@ namespace ts {
switch (kind) {
case SyntaxKind.Identifier:
return checkIdentifier(node as Identifier, checkMode);
case SyntaxKind.PrivateIdentifier:
return checkPrivateIdentifierExpression(node as PrivateIdentifier);
case SyntaxKind.ThisKeyword:
return checkThisExpression(node);
case SyntaxKind.SuperKeyword:
Expand Down Expand Up @@ -40296,6 +40367,9 @@ namespace ts {
}
return result;
}
else if (isPrivateIdentifier(name)) {
return getSymbolForPrivateIdentifierExpression(name);
}
else if (name.kind === SyntaxKind.PropertyAccessExpression || name.kind === SyntaxKind.QualifiedName) {
const links = getNodeLinks(name);
if (links.resolvedSymbol) {
Expand Down Expand Up @@ -41712,6 +41786,7 @@ namespace ts {
case ExternalEmitHelpers.MakeTemplateObject: return "__makeTemplateObject";
case ExternalEmitHelpers.ClassPrivateFieldGet: return "__classPrivateFieldGet";
case ExternalEmitHelpers.ClassPrivateFieldSet: return "__classPrivateFieldSet";
case ExternalEmitHelpers.ClassPrivateFieldIn: return "__classPrivateFieldIn";
case ExternalEmitHelpers.CreateBinding: return "__createBinding";
default: return Debug.fail("Unrecognized helper");
}
Expand Down
6 changes: 5 additions & 1 deletion src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -1392,6 +1392,10 @@
"category": "Message",
"code": 1450
},
"Private identifiers are only allowed in class bodies and may only be used as part of a class member declaration, property access, or on the left-hand-side of an 'in' expression": {
"category": "Error",
"code": 1451
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -1654,7 +1658,7 @@
"category": "Error",
"code": 2359
},
"The left-hand side of an 'in' expression must be of type 'any', 'string', 'number', or 'symbol'.": {
"The left-hand side of an 'in' expression must be a private identifier or of type 'any', 'string', 'number', or 'symbol'.": {
"category": "Error",
"code": 2360
},
Expand Down
2 changes: 2 additions & 0 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1684,6 +1684,8 @@ namespace ts {
// Identifiers
case SyntaxKind.Identifier:
return emitIdentifier(node as Identifier);
case SyntaxKind.PrivateIdentifier:
return emitPrivateIdentifier(node as PrivateIdentifier);

// Expressions
case SyntaxKind.ArrayLiteralExpression:
Expand Down
30 changes: 30 additions & 0 deletions src/compiler/factory/emitHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ namespace ts {
// Class Fields Helpers
createClassPrivateFieldGetHelper(receiver: Expression, state: Identifier, kind: PrivateIdentifierKind, f: Identifier | undefined): Expression;
createClassPrivateFieldSetHelper(receiver: Expression, state: Identifier, value: Expression, kind: PrivateIdentifierKind, f: Identifier | undefined): Expression;
createClassPrivateFieldInHelper(state: Identifier, receiver: Expression): Expression;
}

export function createEmitHelperFactory(context: TransformationContext): EmitHelperFactory {
Expand Down Expand Up @@ -75,6 +76,7 @@ namespace ts {
// Class Fields Helpers
createClassPrivateFieldGetHelper,
createClassPrivateFieldSetHelper,
createClassPrivateFieldInHelper
};

/**
Expand Down Expand Up @@ -395,6 +397,10 @@ namespace ts {
return factory.createCallExpression(getUnscopedHelperName("__classPrivateFieldSet"), /*typeArguments*/ undefined, args);
}

function createClassPrivateFieldInHelper(state: Identifier, receiver: Expression) {
context.requestEmitHelper(classPrivateFieldInHelper);
return factory.createCallExpression(getUnscopedHelperName("__classPrivateFieldIn"), /* typeArguments*/ undefined, [state, receiver]);
}
}

/* @internal */
Expand Down Expand Up @@ -961,6 +967,29 @@ namespace ts {
};`
};

/**
* Parameters:
* @param state — One of the following:
* - A WeakMap when the member is a private instance field.
* - A WeakSet when the member is a private instance method or accessor.
* - A function value that should be the undecorated class constructor when the member is a private static field, method, or accessor.
* @param receiver — The object being checked if it has the private member.
*
* Usage:
* This helper is used to transform `#field in expression` to
* `__classPrivateFieldIn(<weakMap/weakSet/constructor>, expression)`
*/
export const classPrivateFieldInHelper: UnscopedEmitHelper = {
name: "typescript:classPrivateFieldIn",
importName: "__classPrivateFieldIn",
scoped: false,
text: `
var __classPrivateFieldIn = (this && this.__classPrivateFieldIn) || function(state, receiver) {
if (receiver === null || (typeof receiver !== "object" && typeof receiver !== "function")) throw new TypeError("Cannot use 'in' operator on non-object");
return typeof state === "function" ? receiver === state : state.has(receiver);
};`
};

let allUnscopedEmitHelpers: ReadonlyESMap<string, UnscopedEmitHelper> | undefined;

export function getAllUnscopedEmitHelpers() {
Expand All @@ -986,6 +1015,7 @@ namespace ts {
exportStarHelper,
classPrivateFieldGetHelper,
classPrivateFieldSetHelper,
classPrivateFieldInHelper,
createBindingHelper,
setModuleDefaultHelper
], helper => helper.name));
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/factory/parenthesizerRules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -452,4 +452,4 @@ namespace ts {
parenthesizeConstituentTypesOfUnionOrIntersectionType: nodes => cast(nodes, isNodeArray),
parenthesizeTypeArguments: nodes => nodes && cast(nodes, isNodeArray),
};
}
}
2 changes: 2 additions & 0 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5617,6 +5617,8 @@ namespace ts {
break;
case SyntaxKind.TemplateHead:
return parseTemplateExpression(/* isTaggedTemplate */ false);
case SyntaxKind.PrivateIdentifier:
return parsePrivateIdentifier();
}

return parseIdentifier(Diagnostics.Expression_expected);
Expand Down
34 changes: 33 additions & 1 deletion src/compiler/transformers/classFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,15 +266,44 @@ namespace ts {

/**
* If we visit a private name, this means it is an undeclared private name.
* Replace it with an empty identifier to indicate a problem with the code.
* Replace it with an empty identifier to indicate a problem with the code,
* unless we are in a statement position - otherwise this will not trigger
* a SyntaxError.
*/
function visitPrivateIdentifier(node: PrivateIdentifier) {
if (!shouldTransformPrivateElementsOrClassStaticBlocks) {
return node;
}
if (isStatement(node.parent)) {
return node;
}
return setOriginalNode(factory.createIdentifier(""), node);
}

/**
* Visits `#id in expr`
*/
function visitPrivateIdentifierInInExpression(node: BinaryExpression) {
if (!shouldTransformPrivateElementsOrClassStaticBlocks) {
return node;
}
const privId = node.left;
Debug.assertNode(privId, isPrivateIdentifier);
Debug.assert(node.operatorToken.kind === SyntaxKind.InKeyword);
const info = accessPrivateIdentifier(privId);
if (info) {
const receiver = visitNode(node.right, visitor, isExpression);

return setOriginalNode(
context.getEmitHelperFactory().createClassPrivateFieldInHelper(info.brandCheckIdentifier, receiver),
node
);
}

// Private name has not been declared. Subsequent transformers will handle this error
return visitEachChild(node, visitor, context);
}

/**
* Visits the members of a class that has fields.
*
Expand Down Expand Up @@ -827,6 +856,9 @@ namespace ts {
}
}
}
if (node.operatorToken.kind === SyntaxKind.InKeyword && isPrivateIdentifier(node.left)) {
return visitPrivateIdentifierInInExpression(node);
}
return visitEachChild(node, visitor, context);
}

Expand Down
6 changes: 4 additions & 2 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,8 @@ namespace ts {
readonly expression: Expression;
}

export interface PrivateIdentifier extends Node {
// Typed as a PrimaryExpression due to its presence in BinaryExpressions (#field in expr)
export interface PrivateIdentifier extends PrimaryExpression {
readonly kind: SyntaxKind.PrivateIdentifier;
// escaping not strictly necessary
// avoids gotchas in transforms and utils
Expand Down Expand Up @@ -6854,7 +6855,8 @@ namespace ts {
MakeTemplateObject = 1 << 18, // __makeTemplateObject (used for constructing template string array objects)
ClassPrivateFieldGet = 1 << 19, // __classPrivateFieldGet (used by the class private field transformation)
ClassPrivateFieldSet = 1 << 20, // __classPrivateFieldSet (used by the class private field transformation)
CreateBinding = 1 << 21, // __createBinding (use by the module transform for (re)exports and namespace imports)
ClassPrivateFieldIn = 1 << 21, // __classPrivateFieldIn (used by the class private field transformation)
CreateBinding = 1 << 22, // __createBinding (use by the module transform for (re)exports and namespace imports)
FirstEmitHelper = Extends,
LastEmitHelper = CreateBinding,

Expand Down
3 changes: 3 additions & 0 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1949,6 +1949,8 @@ namespace ts {
node = node.parent;
}
return node.parent.kind === SyntaxKind.TypeQuery || isJSDocLinkLike(node.parent) || isJSDocNameReference(node.parent) || isJSDocMemberName(node.parent) || isJSXTagName(node);
case SyntaxKind.PrivateIdentifier:
return isBinaryExpression(node.parent) && node.parent.left === node && node.parent.operatorToken.kind === SyntaxKind.InKeyword;
case SyntaxKind.Identifier:
if (node.parent.kind === SyntaxKind.TypeQuery || isJSDocLinkLike(node.parent) || isJSDocNameReference(node.parent) || isJSDocMemberName(node.parent) || isJSXTagName(node)) {
return true;
Expand Down Expand Up @@ -3706,6 +3708,7 @@ namespace ts {
case SyntaxKind.ThisKeyword:
case SyntaxKind.SuperKeyword:
case SyntaxKind.Identifier:
case SyntaxKind.PrivateIdentifier:
case SyntaxKind.NullKeyword:
case SyntaxKind.TrueKeyword:
case SyntaxKind.FalseKeyword:
Expand Down
1 change: 1 addition & 0 deletions src/compiler/utilitiesPublic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1516,6 +1516,7 @@ namespace ts {
case SyntaxKind.ClassExpression:
case SyntaxKind.FunctionExpression:
case SyntaxKind.Identifier:
case SyntaxKind.PrivateIdentifier: // technically this is only an Expression if it's in a `#field in expr` BinaryExpression
case SyntaxKind.RegularExpressionLiteral:
case SyntaxKind.NumericLiteral:
case SyntaxKind.BigIntLiteral:
Expand Down
4 changes: 2 additions & 2 deletions src/services/codefixes/fixForgottenThisPropertyAccess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace ts.codefix {
const didYouMeanStaticMemberCode = Diagnostics.Cannot_find_name_0_Did_you_mean_the_static_member_1_0.code;
const errorCodes = [
Diagnostics.Cannot_find_name_0_Did_you_mean_the_instance_member_this_0.code,
Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies.code,
Diagnostics.Private_identifiers_are_only_allowed_in_class_bodies_and_may_only_be_used_as_part_of_a_class_member_declaration_property_access_or_on_the_left_hand_side_of_an_in_expression.code,
didYouMeanStaticMemberCode,
];
registerCodeFix({
Expand Down Expand Up @@ -32,7 +32,7 @@ namespace ts.codefix {

function getInfo(sourceFile: SourceFile, pos: number, diagCode: number): Info | undefined {
const node = getTokenAtPosition(sourceFile, pos);
if (isIdentifier(node)) {
if (isIdentifier(node) || isPrivateIdentifier(node)) {
return { node, className: diagCode === didYouMeanStaticMemberCode ? getContainingClass(node)!.name!.text : undefined };
}
}
Expand Down

0 comments on commit af689cc

Please sign in to comment.