Skip to content

Parse Private Names #4

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.

Sign up for GitHub

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 1 commit into from
Aug 23, 2018
Merged

Parse Private Names #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
13 changes: 8 additions & 5 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,9 @@ namespace ts {
Debug.assert(isWellKnownSymbolSyntactically(nameExpression));
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
}
if (isPrivateName(node)) {
return nodePosToString(node) as __String;
}
Copy link
Author

Choose a reason for hiding this comment

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

ignore this: it's a mistake and to be removed in #5. Actually has no effect, since there is no way the node can be a private name here anyway.

return isPropertyNameLiteral(name) ? getEscapedTextOfIdentifierOrLiteral(name) : undefined;
}
switch (node.kind) {
Expand Down Expand Up @@ -1394,7 +1397,7 @@ namespace ts {
}
if (node.expression.kind === SyntaxKind.PropertyAccessExpression) {
const propertyAccess = <PropertyAccessExpression>node.expression;
if (isNarrowableOperand(propertyAccess.expression) && isPushOrUnshiftIdentifier(propertyAccess.name)) {
if (isIdentifier(propertyAccess.name) && isNarrowableOperand(propertyAccess.expression) && isPushOrUnshiftIdentifier(propertyAccess.name)) {
currentFlow = createFlowArrayMutation(currentFlow, node);
}
}
Expand Down Expand Up @@ -2359,7 +2362,7 @@ namespace ts {
return;
}
const lhs = node.left as PropertyAccessEntityNameExpression;
const symbol = forEachIdentifierInEntityName(lhs.expression, /*parent*/ undefined, (id, symbol) => {
const symbol = forEachIdentifierOrPrivateNameInEntityName(lhs.expression, /*parent*/ undefined, (id, symbol) => {
if (symbol) {
addDeclarationToSymbol(symbol, id, SymbolFlags.Module | SymbolFlags.JSContainer);
}
Expand Down Expand Up @@ -2518,7 +2521,7 @@ namespace ts {
// make symbols or add declarations for intermediate containers
const flags = SymbolFlags.Module | SymbolFlags.JSContainer;
const excludeFlags = SymbolFlags.ValueModuleExcludes & ~SymbolFlags.JSContainer;
namespaceSymbol = forEachIdentifierInEntityName(propertyAccess.expression, namespaceSymbol, (id, symbol, parent) => {
namespaceSymbol = forEachIdentifierOrPrivateNameInEntityName(propertyAccess.expression, namespaceSymbol, (id, symbol, parent) => {
if (symbol) {
addDeclarationToSymbol(symbol, id, flags);
return symbol;
Expand Down Expand Up @@ -2590,15 +2593,15 @@ namespace ts {
}
}

function forEachIdentifierInEntityName(e: EntityNameExpression, parent: Symbol | undefined, action: (e: Identifier, symbol: Symbol | undefined, parent: Symbol | undefined) => Symbol | undefined): Symbol | undefined {
function forEachIdentifierOrPrivateNameInEntityName(e: EntityNameExpression, parent: Symbol | undefined, action: (e: Identifier | PrivateName, symbol: Symbol | undefined, parent: Symbol | undefined) => Symbol | undefined): Symbol | undefined {
if (isExportsOrModuleExportsOrAlias(file, e)) {
return file.symbol;
}
else if (isIdentifier(e)) {
return action(e, lookupSymbolForPropertyAccess(e), parent);
}
else {
const s = forEachIdentifierInEntityName(e.expression, parent, action);
const s = forEachIdentifierOrPrivateNameInEntityName(e.expression, parent, action);
if (!s || !s.exports) return Debug.fail();
return action(e.name, s.exports.get(e.name.escapedText), s);
}
Expand Down
31 changes: 20 additions & 11 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14346,8 +14346,10 @@ namespace ts {
const root = getReferenceRoot(node);
const parent = root.parent;
const isLengthPushOrUnshift = parent.kind === SyntaxKind.PropertyAccessExpression && (
(<PropertyAccessExpression>parent).name.escapedText === "length" ||
parent.parent.kind === SyntaxKind.CallExpression && isPushOrUnshiftIdentifier((<PropertyAccessExpression>parent).name));
(<PropertyAccessExpression>parent).name.escapedText === "length" || (
parent.parent.kind === SyntaxKind.CallExpression
&& isIdentifier((parent as PropertyAccessExpression).name)
&& isPushOrUnshiftIdentifier((parent as PropertyAccessExpression).name as Identifier)));
const isElementAssignment = parent.kind === SyntaxKind.ElementAccessExpression &&
(<ElementAccessExpression>parent).expression === root &&
parent.parent.kind === SyntaxKind.BinaryExpression &&
Expand Down Expand Up @@ -17972,7 +17974,7 @@ namespace ts {
return checkPropertyAccessExpressionOrQualifiedName(node, node.left, node.right);
}

function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier) {
function checkPropertyAccessExpressionOrQualifiedName(node: PropertyAccessExpression | QualifiedName, left: Expression | QualifiedName, right: Identifier | PrivateName) {
let propType: Type;
const leftType = checkNonNullExpression(left);
const parentSymbol = getNodeLinks(left).resolvedSymbol;
Expand All @@ -17990,7 +17992,7 @@ namespace ts {
}
if (!prop) {
const indexInfo = getIndexInfoOfType(apparentType, IndexKind.String);
if (!(indexInfo && indexInfo.type)) {
if (!(indexInfo && indexInfo.type) || isPrivateName(right)) {
if (isJSLiteralType(leftType)) {
return anyType;
}
Expand Down Expand Up @@ -18048,7 +18050,7 @@ namespace ts {
return assignmentKind ? getBaseTypeOfLiteralType(flowType) : flowType;
}

function checkPropertyNotUsedBeforeDeclaration(prop: Symbol, node: PropertyAccessExpression | QualifiedName, right: Identifier): void {
function checkPropertyNotUsedBeforeDeclaration(prop: Symbol, node: PropertyAccessExpression | QualifiedName, right: Identifier | PrivateName): void {
const { valueDeclaration } = prop;
if (!valueDeclaration) {
return;
Expand Down Expand Up @@ -18118,7 +18120,7 @@ namespace ts {
return getIntersectionType(x);
}

function reportNonexistentProperty(propNode: Identifier, containingType: Type) {
function reportNonexistentProperty(propNode: Identifier | PrivateName, containingType: Type) {
let errorInfo: DiagnosticMessageChain | undefined;
let relatedInfo: Diagnostic | undefined;
if (containingType.flags & TypeFlags.Union && !(containingType.flags & TypeFlags.Primitive)) {
Expand Down Expand Up @@ -18161,11 +18163,11 @@ namespace ts {
return prop !== undefined && prop.valueDeclaration && hasModifier(prop.valueDeclaration, ModifierFlags.Static);
}

function getSuggestedSymbolForNonexistentProperty(name: Identifier | string, containingType: Type): Symbol | undefined {
function getSuggestedSymbolForNonexistentProperty(name: Identifier | PrivateName | string, containingType: Type): Symbol | undefined {
return getSpellingSuggestionForName(isString(name) ? name : idText(name), getPropertiesOfType(containingType), SymbolFlags.Value);
}

function getSuggestionForNonexistentProperty(name: Identifier | string, containingType: Type): string | undefined {
function getSuggestionForNonexistentProperty(name: Identifier | PrivateName | string, containingType: Type): string | undefined {
const suggestion = getSuggestedSymbolForNonexistentProperty(name, containingType);
return suggestion && symbolName(suggestion);
}
Expand Down Expand Up @@ -21876,6 +21878,9 @@ 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);
Copy link
Author

Choose a reason for hiding this comment

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

we plan to undo this change is in #5

}
const func = getContainingFunction(node)!;
if (hasModifier(node, ModifierFlags.ParameterPropertyModifier)) {
if (!(func.kind === SyntaxKind.Constructor && nodeIsPresent(func.body))) {
Expand Down Expand Up @@ -23523,9 +23528,9 @@ namespace ts {
}
}

function getIdentifierFromEntityNameExpression(node: Identifier | PropertyAccessExpression): Identifier;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | undefined;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | undefined {
function getIdentifierFromEntityNameExpression(node: Identifier | PropertyAccessExpression): Identifier | PrivateName;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | PrivateName | undefined;
function getIdentifierFromEntityNameExpression(node: Expression): Identifier | PrivateName | undefined {
switch (node.kind) {
case SyntaxKind.Identifier:
return node as Identifier;
Expand Down Expand Up @@ -29288,6 +29293,10 @@ 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);
}
Copy link
Author

Choose a reason for hiding this comment

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

we plan to undo this change in #5


const checkLetConstNames = (isLet(node) || isVarConst(node));

// 1. LexicalDeclaration : LetOrConst BindingList ;
Expand Down
8 changes: 8 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4166,6 +4166,14 @@
"category": "Error",
"code": 18003
},
"Private names are not allowed in variable declarations.": {
"category": "Error",
"code": 18004
},
"Private names cannot be used as parameters": {
"category": "Error",
"code": 18005
},

"File is a CommonJS module; it may be converted to an ES6 module.": {
"category": "Suggestion",
Expand Down
16 changes: 15 additions & 1 deletion src/compiler/emitter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,10 @@ namespace ts {
case SyntaxKind.Identifier:
return emitIdentifier(<Identifier>node);

// PrivateNames
case SyntaxKind.PrivateName:
return emitPrivateName(node as PrivateName);

// Parse tree nodes

// Names
Expand Down Expand Up @@ -878,6 +882,10 @@ namespace ts {
case SyntaxKind.Identifier:
return emitIdentifier(<Identifier>node);

// Private Names
case SyntaxKind.PrivateName:
return emitPrivateName(node as PrivateName);

// Reserved words
case SyntaxKind.FalseKeyword:
case SyntaxKind.NullKeyword:
Expand Down Expand Up @@ -1067,6 +1075,12 @@ namespace ts {
emitList(node, node.typeArguments, ListFormat.TypeParameters); // Call emitList directly since it could be an array of TypeParameterDeclarations _or_ type arguments
}

function emitPrivateName(node: PrivateName) {
const writeText = node.symbol ? writeSymbol : write;
writeText(getTextOfNode(node, /*includeTrivia*/ false), node.symbol);
emitList(node, /*typeArguments*/ undefined, ListFormat.TypeParameters); // Call emitList directly since it could be an array of TypeParameterDeclarations _or_ type arguments
}

//
// Names
//
Expand Down Expand Up @@ -3302,7 +3316,7 @@ namespace ts {
function getLiteralTextOfNode(node: LiteralLikeNode): string {
if (node.kind === SyntaxKind.StringLiteral && (<StringLiteral>node).textSourceNode) {
const textSourceNode = (<StringLiteral>node).textSourceNode!;
if (isIdentifier(textSourceNode)) {
if (isIdentifierOrPrivateName(textSourceNode)) {
return getEmitFlags(node) & EmitFlags.NoAsciiEscaping ?
`"${escapeString(getTextOfNode(textSourceNode))}"` :
`"${escapeNonAsciiString(getTextOfNode(textSourceNode))}"`;
Expand Down
22 changes: 14 additions & 8 deletions src/compiler/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ namespace ts {
return node;
}

function createLiteralFromNode(sourceNode: PropertyNameLiteral): StringLiteral {
function createLiteralFromNode(sourceNode: Exclude<PropertyNameLiteral, PrivateName>): StringLiteral {
const node = createStringLiteral(getTextOfIdentifierOrLiteral(sourceNode));
node.textSourceNode = sourceNode;
return node;
Expand Down Expand Up @@ -995,15 +995,15 @@ namespace ts {
: node;
}

export function createPropertyAccess(expression: Expression, name: string | Identifier | undefined) {
export function createPropertyAccess(expression: Expression, name: string | Identifier | PrivateName | undefined) {
const node = <PropertyAccessExpression>createSynthesizedNode(SyntaxKind.PropertyAccessExpression);
node.expression = parenthesizeForAccess(expression);
node.name = asName(name)!; // TODO: GH#18217
setEmitFlags(node, EmitFlags.NoIndentation);
return node;
}

export function updatePropertyAccess(node: PropertyAccessExpression, expression: Expression, name: Identifier) {
export function updatePropertyAccess(node: PropertyAccessExpression, expression: Expression, name: Identifier | PrivateName) {
// Because we are updating existed propertyAccess we want to inherit its emitFlags
// instead of using the default from createPropertyAccess
return node.expression !== expression
Expand Down Expand Up @@ -2714,7 +2714,7 @@ namespace ts {

// Utilities

function asName<T extends Identifier | BindingName | PropertyName | EntityName | ThisTypeNode | undefined>(name: string | T): T | Identifier {
function asName<T extends Identifier | PrivateName | BindingName | PropertyName | EntityName | ThisTypeNode | undefined>(name: string | T): T | Identifier {
return isString(name) ? createIdentifier(name) : name;
}

Expand Down Expand Up @@ -3099,7 +3099,7 @@ namespace ts {
}
else {
const expression = setTextRange(
isIdentifier(memberName)
(isIdentifier(memberName) || isPrivateName(memberName))
? createPropertyAccess(target, memberName)
: createElementAccess(target, memberName),
memberName
Expand Down Expand Up @@ -3529,7 +3529,7 @@ namespace ts {
}
}

export function createExpressionForPropertyName(memberName: PropertyName): Expression {
export function createExpressionForPropertyName(memberName: Exclude<PropertyName, PrivateName>): Expression {
if (isIdentifier(memberName)) {
return createLiteral(memberName);
}
Expand All @@ -3541,11 +3541,17 @@ namespace ts {
}
}

/**
* accessor declaration that can be converted to an expression (`name` field cannot be a `PrivateName`)
*/
type ExpressionableAccessorDeclaration = AccessorDeclaration & {name: Exclude<PropertyName, PrivateName>};
Copy link

Choose a reason for hiding this comment

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

Might be more consistent to define like this:

interface ExpressionableAccessorDeclaration extends AccessorDeclaration {
    name: Exclude<PropertyName, PrivateName>;
}

Copy link
Author

@mheiber mheiber Aug 22, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yours is easier on the eyes, for sure. But since AccessorDeclaration is a type it seems more consistent to me for ExpressionableAccessorDeclaration to be a type.


export function createExpressionForObjectLiteralElementLike(node: ObjectLiteralExpression, property: ObjectLiteralElementLike, receiver: Expression): Expression | undefined {
switch (property.kind) {
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
return createExpressionForAccessorDeclaration(node.properties, property, receiver, !!node.multiLine);
// type assertion `as ExpressionableAccessorDeclaration` is safe because PrivateNames are not allowed in object literals
return createExpressionForAccessorDeclaration(node.properties, property as ExpressionableAccessorDeclaration, receiver, !!node.multiLine);
case SyntaxKind.PropertyAssignment:
return createExpressionForPropertyAssignment(property, receiver);
case SyntaxKind.ShorthandPropertyAssignment:
Expand All @@ -3555,7 +3561,7 @@ namespace ts {
}
}

function createExpressionForAccessorDeclaration(properties: NodeArray<Declaration>, property: AccessorDeclaration, receiver: Expression, multiLine: boolean) {
function createExpressionForAccessorDeclaration(properties: NodeArray<Declaration>, property: ExpressionableAccessorDeclaration, receiver: Expression, multiLine: boolean) {
const { firstAccessor, getAccessor, setAccessor } = getAllAccessorDeclarations(properties, property);
if (property === firstAccessor) {
const properties: ObjectLiteralElementLike[] = [];
Expand Down
Loading