Skip to content

es private fields in in #52

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
306397b
basic parsing of private-id-in-in
acutmore Feb 18, 2021
df7e97d
basic type check and narrow
acutmore Feb 22, 2021
219899f
narrow to instanceOf klass not typeof klass
acutmore Feb 23, 2021
fa3a4c7
tweaking tests
acutmore Feb 24, 2021
7df6a99
fix emit with type syntax on rhs
acutmore Feb 25, 2021
df5a86f
add test case for subclass union narrow
acutmore Feb 25, 2021
3560223
es2020 emit
acutmore Feb 25, 2021
17c95e6
fix linting errors
acutmore Feb 25, 2021
ec072dc
mark as used
acutmore Feb 26, 2021
45e1b48
improve error message when inside a class
acutmore Mar 5, 2021
9127d55
respect whitespace and comments in emit
acutmore Mar 5, 2021
4399cf5
dont parse single privateIdentifier to reenable quick fix
acutmore Mar 9, 2021
0462595
remove need for lookupClassForPrivateIdentifierDeclaration
acutmore Mar 9, 2021
23c8f98
add tests for never
acutmore Mar 9, 2021
43a7d9f
give child class different structure from parent
acutmore Mar 9, 2021
170e61b
cleanup testing for unused field
acutmore Mar 9, 2021
7e33365
use isTypeDerivedFrom
acutmore Mar 9, 2021
f2efbcf
add TODO for static private field emit
acutmore Mar 9, 2021
598215b
include in find-all-references results
acutmore Mar 15, 2021
f7ff235
emit as helper with runtime type check
acutmore Mar 15, 2021
f3d1e52
correct issues caused by rebase
acutmore Mar 29, 2021
dbd7113
update emit for static fields and methods
acutmore Mar 29, 2021
91cfecb
narrow to class for static field
acutmore Mar 29, 2021
5eead91
tidy up TODOs
acutmore Mar 29, 2021
85a367b
add quickFix
acutmore Mar 30, 2021
02f59a3
update typescript.d.ts baseline
acutmore Mar 30, 2021
8928039
post PR review improvements:
acutmore May 24, 2021
211b39e
Merge branch 'main' into es-private-fields-in-in
acutmore Jun 9, 2021
a846027
fix linting
acutmore Jun 9, 2021
d2bddf3
update .d.ts baselines
acutmore Jun 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/compiler/binder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -879,6 +879,8 @@ namespace ts {
return (expr as PrefixUnaryExpression).operator === SyntaxKind.ExclamationToken && isNarrowingExpression((expr as PrefixUnaryExpression).operand);
case SyntaxKind.TypeOfExpression:
return isNarrowingExpression((expr as TypeOfExpression).expression);
case SyntaxKind.PrivateIdentifierInInExpression:
return isNarrowingExpression((expr as PrivateIdentifierInInExpression).expression);
}
return false;
}
Expand Down
98 changes: 96 additions & 2 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23590,6 +23590,32 @@ namespace ts {
return type;
}

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

const privateId = expr.name;
const symbol = lookupSymbolForPrivateIdentifierDeclaration(privateId.escapedText, privateId);
if (symbol === undefined) {
return type;
}
const classSymbol = symbol.parent!;
const classType = getTypeOfSymbol(classSymbol) as InterfaceType;
const classDecl = symbol.valueDeclaration;
Debug.assert(classDecl, "should always have a declaration");
let targetType: Type;
if (hasStaticModifier(classDecl)) {
targetType = classType;
}
else {
const classInstanceType = getDeclaredTypeOfSymbol(classSymbol);
targetType = classInstanceType;
}
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 @@ -24022,6 +24048,8 @@ namespace ts {
return narrowType(type, (expr as ParenthesizedExpression | NonNullExpression).expression, assumeTrue);
case SyntaxKind.BinaryExpression:
return narrowTypeByBinaryExpression(type, expr as BinaryExpression, assumeTrue);
case SyntaxKind.PrivateIdentifierInInExpression:
return narrowTypeByPrivateIdentifierInInExpression(type, expr as PrivateIdentifierInInExpression, assumeTrue);
case SyntaxKind.PrefixUnaryExpression:
if ((expr as PrefixUnaryExpression).operator === SyntaxKind.ExclamationToken) {
return narrowType(type, (expr as PrefixUnaryExpression).operand, !assumeTrue);
Expand Down Expand Up @@ -27726,6 +27754,7 @@ namespace ts {
}

function getSuggestedSymbolForNonexistentProperty(name: Identifier | PrivateIdentifier | string, containingType: Type): Symbol | undefined {
const originalName = name;
let props = getPropertiesOfType(containingType);
if (typeof name !== "string") {
const parent = name.parent;
Expand All @@ -27734,7 +27763,23 @@ namespace ts {
}
name = idText(name);
}
return getSpellingSuggestionForName(name, props, SymbolFlags.Value);
const suggestion = getSpellingSuggestionForName(name, props, SymbolFlags.Value);
if (suggestion) {
return suggestion;
}
// If we have `#typo in expr` then we can still look up potential privateIdentifiers from the surrounding classes
if (typeof originalName !== "string" && isPrivateIdentifierInInExpression(originalName.parent)) {
const privateIdentifiers: Symbol[] = [];
forEachEnclosingClass(originalName, (klass: ClassLikeDeclaration) => {
forEach(klass.members, member => {
if (isPrivateIdentifierClassElementDeclaration(member)) {
privateIdentifiers.push(member.symbol);
}
});
});
return getSpellingSuggestionForName(name, privateIdentifiers, SymbolFlags.Value);
}
return undefined;
}

function getSuggestedSymbolForNonexistentJSXAttribute(name: Identifier | PrivateIdentifier | string, containingType: Type): Symbol | undefined {
Expand Down Expand Up @@ -31494,6 +31539,11 @@ namespace ts {
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);
}
checkInExpressionRHS(right, rightType);
return booleanType;
}

function checkInExpressionRHS(right: Expression, rightType: Type) {
const rightTypeConstraint = getConstraintOfType(rightType);
if (!allTypesAssignableToKind(rightType, TypeFlags.NonPrimitive | TypeFlags.InstantiableNonPrimitive) ||
rightTypeConstraint && (
Expand All @@ -31503,7 +31553,6 @@ namespace ts {
) {
error(right, Diagnostics.The_right_hand_side_of_an_in_expression_must_not_be_a_primitive);
}
return booleanType;
}

function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type, rightIsThis?: boolean): Type {
Expand Down Expand Up @@ -31740,6 +31789,40 @@ namespace ts {
return (target.flags & TypeFlags.Nullable) !== 0 || isTypeComparableTo(source, target);
}

function checkPrivateIdentifierInInExpression(node: PrivateIdentifierInInExpression, checkMode?: CheckMode) {
const privateId = node.name;
const exp = node.expression;
let rightType = checkExpression(exp, checkMode);

const lexicallyScopedSymbol = lookupSymbolForPrivateIdentifierDeclaration(privateId.escapedText, privateId);
if (lexicallyScopedSymbol === undefined) {
if (!getContainingClass(node)) {
error(privateId, Diagnostics.Private_identifiers_are_not_allowed_outside_class_bodies);
}
else {
const suggestion = getSuggestedSymbolForNonexistentProperty(privateId, rightType);
if (suggestion) {
const suggestedName = symbolName(suggestion);
error(privateId, Diagnostics.Cannot_find_name_0_Did_you_mean_1, diagnosticName(privateId), suggestedName);
}
else {
error(privateId, Diagnostics.Cannot_find_name_0, diagnosticName(privateId));
}
}
return anyType;
}

markPropertyAsReferenced(lexicallyScopedSymbol, /* nodeForCheckWriteOnly: */ undefined, /* isThisAccess: */ false);
getNodeLinks(node).resolvedSymbol = lexicallyScopedSymbol;

if (rightType === silentNeverType) {
return silentNeverType;
}
rightType = checkNonNullType(rightType, exp);
checkInExpressionRHS(exp, rightType);
return booleanType;
}

function createCheckBinaryExpression() {
interface WorkArea {
readonly checkMode: CheckMode | undefined;
Expand Down Expand Up @@ -32921,6 +33004,8 @@ namespace ts {
return checkPostfixUnaryExpression(node as PostfixUnaryExpression);
case SyntaxKind.BinaryExpression:
return checkBinaryExpression(node as BinaryExpression, checkMode);
case SyntaxKind.PrivateIdentifierInInExpression:
return checkPrivateIdentifierInInExpression(node as PrivateIdentifierInInExpression, checkMode);
case SyntaxKind.ConditionalExpression:
return checkConditionalExpression(node as ConditionalExpression, checkMode);
case SyntaxKind.SpreadElement:
Expand Down Expand Up @@ -39416,6 +39501,15 @@ namespace ts {
return resolveEntityName(name as Identifier, /*meaning*/ SymbolFlags.FunctionScopedVariable);
}

if (isPrivateIdentifier(name) && isPrivateIdentifierInInExpression(name.parent)) {
const links = getNodeLinks(name.parent);
if (links.resolvedSymbol) {
return links.resolvedSymbol;
}
checkPrivateIdentifierInInExpression(name.parent);
return links.resolvedSymbol;
}

return undefined;
}

Expand Down
20 changes: 20 additions & 0 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1716,6 +1716,8 @@ namespace ts {
return emitPostfixUnaryExpression(node as PostfixUnaryExpression);
case SyntaxKind.BinaryExpression:
return emitBinaryExpression(node as BinaryExpression);
case SyntaxKind.PrivateIdentifierInInExpression:
return emitPrivateIdentifierInInExpression(node as PrivateIdentifierInInExpression);
case SyntaxKind.ConditionalExpression:
return emitConditionalExpression(node as ConditionalExpression);
case SyntaxKind.TemplateExpression:
Expand Down Expand Up @@ -2690,6 +2692,24 @@ namespace ts {
}
}

function emitPrivateIdentifierInInExpression(node: PrivateIdentifierInInExpression) {
const linesBeforeIn = getLinesBetweenNodes(node, node.name, node.inToken);
const linesAfterIn = getLinesBetweenNodes(node, node.inToken, node.expression);

emitLeadingCommentsOfPosition(node.name.pos);
emitPrivateIdentifier(node.name);
emitTrailingCommentsOfPosition(node.name.end);

writeLinesAndIndent(linesBeforeIn, /*writeSpaceIfNotIndenting*/ true);
emitLeadingCommentsOfPosition(node.inToken.pos);
writeTokenNode(node.inToken, writeKeyword);
emitTrailingCommentsOfPosition(node.inToken.end, /*prefixSpace*/ true);
writeLinesAndIndent(linesAfterIn, /*writeSpaceIfNotIndenting*/ true);

emit(node.expression);
decreaseIndentIf(linesBeforeIn, linesAfterIn);
}

function emitConditionalExpression(node: ConditionalExpression) {
const linesBeforeQuestion = getLinesBetweenNodes(node, node.condition, node.questionToken);
const linesAfterQuestion = getLinesBetweenNodes(node, node.questionToken, node.whenTrue);
Expand Down
18 changes: 18 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(receiver: Expression, state: Identifier): Expression;
}

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

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

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

/* @internal */
Expand Down Expand Up @@ -954,6 +960,17 @@ namespace ts {
};`
};

export const classPrivateFieldInHelper: UnscopedEmitHelper = {
name: "typescript:classPrivateFieldIn",
importName: "__classPrivateFieldIn",
scoped: false,
text: `
var __classPrivateFieldIn = (this && this.__classPrivateFieldIn) || function(receiver, state) {
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 @@ -979,6 +996,7 @@ namespace ts {
exportStarHelper,
classPrivateFieldGetHelper,
classPrivateFieldSetHelper,
classPrivateFieldInHelper,
createBindingHelper,
setModuleDefaultHelper
], helper => helper.name));
Expand Down
30 changes: 30 additions & 0 deletions src/compiler/factory/nodeFactory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ namespace ts {
createMergeDeclarationMarker,
createSyntheticReferenceExpression,
updateSyntheticReferenceExpression,
createPrivateIdentifierInInExpression,
updatePrivateIdentifierInInExpression,
cloneNode,

// Lazily load factory methods for common operator factories and utilities
Expand Down Expand Up @@ -3066,6 +3068,34 @@ namespace ts {
: node;
}

// @api
function createPrivateIdentifierInInExpression(name: PrivateIdentifier, inToken: Token<SyntaxKind.InKeyword>, expression: Expression) {
const node = createBaseExpression<PrivateIdentifierInInExpression>(SyntaxKind.PrivateIdentifierInInExpression);
node.name = name;
node.inToken = inToken;
node.expression = expression;
node.transformFlags |=
propagateChildFlags(node.name) |
propagateChildFlags(node.inToken) |
propagateChildFlags(node.expression) |
TransformFlags.ContainsESNext;
return node;
}

// @api
function updatePrivateIdentifierInInExpression(
node: PrivateIdentifierInInExpression,
name: PrivateIdentifier,
inToken: Token<SyntaxKind.InKeyword>,
expression: Expression
): PrivateIdentifierInInExpression {
return node.name !== name
|| node.inToken !== inToken
|| node.expression !== expression
? update(createPrivateIdentifierInInExpression(name, inToken, expression), node)
: node;
}

//
// Misc
//
Expand Down
9 changes: 9 additions & 0 deletions src/compiler/factory/nodeTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,11 @@ namespace ts {
return node.kind === SyntaxKind.EqualsGreaterThanToken;
}

/*@internal*/
export function isInKeyword(node: Node): node is InKeyword {
return node.kind === SyntaxKind.InKeyword;
}

// Identifiers

export function isIdentifier(node: Node): node is Identifier {
Expand Down Expand Up @@ -444,6 +449,10 @@ namespace ts {
return node.kind === SyntaxKind.CommaListExpression;
}

export function isPrivateIdentifierInInExpression(node: Node): node is PrivateIdentifierInInExpression {
return node.kind === SyntaxKind.PrivateIdentifierInInExpression;
}

// Misc

export function isTemplateSpan(node: Node): node is TemplateSpan {
Expand Down
33 changes: 32 additions & 1 deletion src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,9 @@ namespace ts {
return visitNodes(cbNode, cbNodes, node.decorators);
case SyntaxKind.CommaListExpression:
return visitNodes(cbNode, cbNodes, (node as CommaListExpression).elements);
case SyntaxKind.PrivateIdentifierInInExpression:
return visitNode(cbNode, (node as PrivateIdentifierInInExpression).name) ||
visitNode(cbNode, (node as PrivateIdentifierInInExpression).expression);

case SyntaxKind.JsxElement:
return visitNode(cbNode, (node as JsxElement).openingElement) ||
Expand Down Expand Up @@ -4446,9 +4449,32 @@ namespace ts {
);
}

function parsePrivateIdentifierInInExpression(pos: number): Expression {
// PrivateIdentifierInInExpression[in]:
// [+in] PrivateIdentifier in RelationalExpression[?in]

Debug.assert(token() === SyntaxKind.PrivateIdentifier, "parsePrivateIdentifierInInExpression should only have been called if we had a privateIdentifier");
Debug.assert(inDisallowInContext() === false, "parsePrivateIdentifierInInExpression should only have been called if 'in' is allowed");
const id = parsePrivateIdentifier();
if (token() !== SyntaxKind.InKeyword) {
return createMissingNode(SyntaxKind.InKeyword, /*reportAtCurrentPosition*/ true, Diagnostics._0_expected, tokenToString(SyntaxKind.InKeyword));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should think of common parsing mistakes like if(#prop.a in foo). This currently issues the error 'in' expected.. I would argue an error along the lines of: You might have meant this.#props.a would be better,

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking into this there are existing errors and quickFixes for 'bare' privateFields but my branch was preventing them from running as I was consuming the field in the parser. I've changed the parser to lookahead and only match when the in keyword is present.

}

const inToken = parseTokenNode<Token<SyntaxKind.InKeyword>>();
const exp = parseBinaryExpressionOrHigher(OperatorPrecedence.Relational);
return finishNode(factory.createPrivateIdentifierInInExpression(id, inToken, exp), pos);
}

function parseBinaryExpressionOrHigher(precedence: OperatorPrecedence): Expression {
// parse a BinaryExpression the LHS is either:
// 1) a PrivateIdentifierInInExpression when 'in' flag allowed and lookahead matches 'PrivateIdentifier in'
// 2) a UnaryExpression

const pos = getNodePos();
const leftOperand = parseUnaryExpressionOrHigher();
const tryPrivateIdentifierInIn = token() === SyntaxKind.PrivateIdentifier && !inDisallowInContext() && lookAhead(nextTokenIsInKeyword);
const leftOperand = tryPrivateIdentifierInIn
? parsePrivateIdentifierInInExpression(pos)
: parseUnaryExpressionOrHigher();
return parseBinaryExpressionRest(precedence, leftOperand, pos);
}

Expand Down Expand Up @@ -5977,6 +6003,11 @@ namespace ts {
return (tokenIsIdentifierOrKeyword(token()) || token() === SyntaxKind.NumericLiteral || token() === SyntaxKind.BigIntLiteral || token() === SyntaxKind.StringLiteral) && !scanner.hasPrecedingLineBreak();
}

function nextTokenIsInKeyword() {
nextToken();
return token() === SyntaxKind.InKeyword;
}

function isDeclaration(): boolean {
while (true) {
switch (token()) {
Expand Down
Loading