Skip to content

Treat NoSubstitutionTemplateLiteral like StringLiteral in more places #26330

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
2 commits merged into from
Aug 10, 2018
Merged

Treat NoSubstitutionTemplateLiteral like StringLiteral in more places #26330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
924890a
Treat NoSubstitutionTemplateLiteral like StringLiteral in more places
Aug 9, 2018
40972ba
Move isStringOrNumericLiteral closer to its only use
Aug 9, 2018
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: 1 addition & 1 deletion src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ namespace ts {
if (name.kind === SyntaxKind.ComputedPropertyName) {
const nameExpression = name.expression;
// treat computed property names where expression is string/numeric literal as just string/numeric literal
if (isStringOrNumericLiteral(nameExpression)) {
if (isStringOrNumericLiteralLike(nameExpression)) {
return escapeLeadingUnderscores(nameExpression.text);
}

Expand Down
4 changes: 2 additions & 2 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4442,7 +4442,7 @@ namespace ts {
}

function isComputedNonLiteralName(name: PropertyName): boolean {
return name.kind === SyntaxKind.ComputedPropertyName && !isStringOrNumericLiteral(name.expression);
return name.kind === SyntaxKind.ComputedPropertyName && !isStringOrNumericLiteralLike(name.expression);
}

function getRestType(source: Type, properties: PropertyName[], symbol: Symbol | undefined): Type {
Expand Down Expand Up @@ -13715,7 +13715,7 @@ namespace ts {
case SyntaxKind.Identifier:
return idText(name);
case SyntaxKind.ComputedPropertyName:
return isStringOrNumericLiteral(name.expression) ? name.expression.text : undefined;
return isStringOrNumericLiteralLike(name.expression) ? name.expression.text : undefined;
case SyntaxKind.StringLiteral:
case SyntaxKind.NumericLiteral:
return name.text;
Expand Down
8 changes: 7 additions & 1 deletion src/compiler/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4709,7 +4709,7 @@ namespace ts {
/**
* Gets the property name of a BindingOrAssignmentElement
*/
export function getPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement) {
export function getPropertyNameOfBindingOrAssignmentElement(bindingElement: BindingOrAssignmentElement): PropertyName | undefined {
switch (bindingElement.kind) {
case SyntaxKind.BindingElement:
// `a` in `let { a: b } = ...`
Expand Down Expand Up @@ -4754,6 +4754,12 @@ namespace ts {
Debug.fail("Invalid property name for binding element.");
}

function isStringOrNumericLiteral(node: Node): node is StringLiteral | NumericLiteral {
const kind = node.kind;
return kind === SyntaxKind.StringLiteral
|| kind === SyntaxKind.NumericLiteral;
}

/**
* Gets the elements of a BindingOrAssignmentPattern
*/
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/parser.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4465,7 +4465,7 @@ namespace ts {
}
else {
const argument = allowInAnd(parseExpression);
if (isStringOrNumericLiteral(argument)) {
if (isStringOrNumericLiteralLike(argument)) {
argument.text = internIdentifier(argument.text);
}
indexedAccess.argumentExpression = argument;
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/transformers/destructuring.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ namespace ts {
const argumentExpression = ensureIdentifier(flattenContext, visitNode(propertyName.expression, flattenContext.visitor), /*reuseIdentifierExpressions*/ false, /*location*/ propertyName);
return createElementAccess(value, argumentExpression);
}
else if (isStringOrNumericLiteral(propertyName)) {
else if (isStringOrNumericLiteralLike(propertyName)) {
const argumentExpression = getSynthesizedClone(propertyName);
argumentExpression.text = argumentExpression.text;
return createElementAccess(value, argumentExpression);
Expand Down
57 changes: 28 additions & 29 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -767,9 +767,9 @@ namespace ts {
case SyntaxKind.NumericLiteral:
return escapeLeadingUnderscores(name.text);
case SyntaxKind.ComputedPropertyName:
return isStringOrNumericLiteral(name.expression) ? escapeLeadingUnderscores(name.expression.text) : undefined!; // TODO: GH#18217 Almost all uses of this assume the result to be defined!
return isStringOrNumericLiteralLike(name.expression) ? escapeLeadingUnderscores(name.expression.text) : undefined!; // TODO: GH#18217 Almost all uses of this assume the result to be defined!
default:
Debug.assertNever(name);
return Debug.assertNever(name);
}
}

Expand Down Expand Up @@ -2560,10 +2560,8 @@ namespace ts {
return false;
}

export function isStringOrNumericLiteral(node: Node): node is StringLiteral | NumericLiteral {
const kind = node.kind;
return kind === SyntaxKind.StringLiteral
|| kind === SyntaxKind.NumericLiteral;
export function isStringOrNumericLiteralLike(node: Node): node is StringLiteralLike | NumericLiteral {
Copy link
Contributor

Choose a reason for hiding this comment

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

The name implies that this includes BigInt literals once they are added to the language. I don't know if that is correct for all call sites

Copy link
Author

Choose a reason for hiding this comment

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

It looks like in most cases it would be -- { [0n]: true } is the same as { 0: true } so as long as IntLiteral#text doesn't include n this would work. @sheetalkamat is working on bigints.

Copy link
Member

Choose a reason for hiding this comment

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

@Andy-MS #25886 I think is the nearly complete implementation at this point (unless we have multiple implementations at the ready). But yeah, I guess indexing with bigint is fine - it's just odd, since in the typesystem that needs to go under the number index signature since 0n and 0 have the same string representation. Probably worth testing in #25886, at least! It's probably not a concern for this PR directly, since it'll need to be dealt with at the point they're added.

return isStringLiteralLike(node) || isNumericLiteral(node);
}

/**
Expand All @@ -2580,7 +2578,7 @@ namespace ts {

export function isDynamicName(name: DeclarationName): boolean {
return name.kind === SyntaxKind.ComputedPropertyName &&
!isStringOrNumericLiteral(name.expression) &&
!isStringOrNumericLiteralLike(name.expression) &&
!isWellKnownSymbolSyntactically(name.expression);
}

Expand All @@ -2593,24 +2591,25 @@ namespace ts {
return isPropertyAccessExpression(node) && isESSymbolIdentifier(node.expression);
}

export function getPropertyNameForPropertyNameNode(name: DeclarationName): __String | undefined {
if (name.kind === SyntaxKind.Identifier) {
return name.escapedText;
}
if (name.kind === SyntaxKind.StringLiteral || name.kind === SyntaxKind.NumericLiteral) {
return escapeLeadingUnderscores(name.text);
}
if (name.kind === SyntaxKind.ComputedPropertyName) {
const nameExpression = name.expression;
if (isWellKnownSymbolSyntactically(nameExpression)) {
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
}
else if (nameExpression.kind === SyntaxKind.StringLiteral || nameExpression.kind === SyntaxKind.NumericLiteral) {
return escapeLeadingUnderscores((<LiteralExpression>nameExpression).text);
}
export function getPropertyNameForPropertyNameNode(name: PropertyName): __String | undefined {
switch (name.kind) {
case SyntaxKind.Identifier:
return name.escapedText;
case SyntaxKind.StringLiteral:
case SyntaxKind.NumericLiteral:
return escapeLeadingUnderscores(name.text);
case SyntaxKind.ComputedPropertyName:
const nameExpression = name.expression;
if (isWellKnownSymbolSyntactically(nameExpression)) {
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
}
else if (isStringOrNumericLiteralLike(nameExpression)) {
return escapeLeadingUnderscores(nameExpression.text);
}
return undefined;
default:
return Debug.assertNever(name);
}

return undefined;
}

export type PropertyNameLiteral = Identifier | StringLiteralLike | NumericLiteral;
Expand Down Expand Up @@ -3325,17 +3324,17 @@ namespace ts {
}
}
else {
forEach(declarations, (member: Declaration) => {
if ((member.kind === SyntaxKind.GetAccessor || member.kind === SyntaxKind.SetAccessor)
forEach(declarations, member => {
if (isAccessor(member)
&& hasModifier(member, ModifierFlags.Static) === hasModifier(accessor, ModifierFlags.Static)) {
const memberName = getPropertyNameForPropertyNameNode((member as NamedDeclaration).name!);
const memberName = getPropertyNameForPropertyNameNode(member.name);
const accessorName = getPropertyNameForPropertyNameNode(accessor.name);
if (memberName === accessorName) {
if (!firstAccessor) {
firstAccessor = <AccessorDeclaration>member;
firstAccessor = member;
}
else if (!secondAccessor) {
secondAccessor = <AccessorDeclaration>member;
secondAccessor = member;
}

if (member.kind === SyntaxKind.GetAccessor && !getAccessor) {
Expand Down
2 changes: 1 addition & 1 deletion src/services/navigationBar.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ namespace ts.NavigationBar {
}

const declName = getNameOfDeclaration(<Declaration>node);
if (declName) {
if (declName && isPropertyName(declName)) {
return unescapeLeadingUnderscores(getPropertyNameForPropertyNameNode(declName)!); // TODO: GH#18217
}
switch (node.kind) {
Expand Down
2 changes: 1 addition & 1 deletion src/services/rename.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace ts.Rename {
if (isStringLiteralLike(node) && tryGetImportFromModuleSpecifier(node)) return undefined;

const kind = SymbolDisplay.getSymbolKind(typeChecker, symbol, node);
const specifierName = (isImportOrExportSpecifierName(node) || isStringOrNumericLiteral(node) && node.parent.kind === SyntaxKind.ComputedPropertyName)
const specifierName = (isImportOrExportSpecifierName(node) || isStringOrNumericLiteralLike(node) && node.parent.kind === SyntaxKind.ComputedPropertyName)
? stripQuotes(getTextOfIdentifierOrLiteral(node))
: undefined;
const displayName = specifierName || typeChecker.symbolToString(symbol);
Expand Down
4 changes: 2 additions & 2 deletions src/services/services.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2142,7 +2142,7 @@ namespace ts {
function initializeNameTable(sourceFile: SourceFile): void {
const nameTable = sourceFile.nameTable = createUnderscoreEscapedMap<number>();
sourceFile.forEachChild(function walk(node) {
if (isIdentifier(node) && node.escapedText || isStringOrNumericLiteral(node) && literalIsName(node)) {
if (isIdentifier(node) && node.escapedText || isStringOrNumericLiteralLike(node) && literalIsName(node)) {
const text = getEscapedTextOfIdentifierOrLiteral(node);
nameTable.set(text, nameTable.get(text) === undefined ? node.pos : -1);
}
Expand All @@ -2162,7 +2162,7 @@ namespace ts {
* then we want 'something' to be in the name table. Similarly, if we have
* "a['propname']" then we want to store "propname" in the name table.
*/
function literalIsName(node: StringLiteral | NumericLiteral): boolean {
function literalIsName(node: StringLiteralLike | NumericLiteral): boolean {
return isDeclarationName(node) ||
node.parent.kind === SyntaxKind.ExternalModuleReference ||
isArgumentOfElementAccessExpression(node) ||
Expand Down
2 changes: 1 addition & 1 deletion src/services/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1240,7 +1240,7 @@ namespace ts {
export function getNameFromPropertyName(name: PropertyName): string | undefined {
return name.kind === SyntaxKind.ComputedPropertyName
// treat computed property names where expression is string/numeric literal as just string/numeric literal
? isStringOrNumericLiteral(name.expression) ? name.expression.text : undefined
? isStringOrNumericLiteralLike(name.expression) ? name.expression.text : undefined
: getTextOfIdentifierOrLiteral(name);
}

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/computedPropertyNames11_ES5.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ var a: any;
>a : any

var v = {
>v : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; }
>{ get [s]() { return 0; }, set [n](v) { }, get [s + s]() { return 0; }, set [s + n](v) { }, get [+s]() { return 0; }, set [""](v) { }, get [0]() { return 0; }, set [a](v) { }, get [<any>true]() { return 0; }, set [`hello bye`](v) { }, get [`hello ${a} bye`]() { return 0; }} : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; }
>v : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; [`hello bye`]: any; }
>{ get [s]() { return 0; }, set [n](v) { }, get [s + s]() { return 0; }, set [s + n](v) { }, get [+s]() { return 0; }, set [""](v) { }, get [0]() { return 0; }, set [a](v) { }, get [<any>true]() { return 0; }, set [`hello bye`](v) { }, get [`hello ${a} bye`]() { return 0; }} : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; [`hello bye`]: any; }

get [s]() { return 0; },
>[s] : number
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/computedPropertyNames11_ES6.types
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ var a: any;
>a : any

var v = {
>v : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; }
>{ get [s]() { return 0; }, set [n](v) { }, get [s + s]() { return 0; }, set [s + n](v) { }, get [+s]() { return 0; }, set [""](v) { }, get [0]() { return 0; }, set [a](v) { }, get [<any>true]() { return 0; }, set [`hello bye`](v) { }, get [`hello ${a} bye`]() { return 0; }} : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; }
>v : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; [`hello bye`]: any; }
>{ get [s]() { return 0; }, set [n](v) { }, get [s + s]() { return 0; }, set [s + n](v) { }, get [+s]() { return 0; }, set [""](v) { }, get [0]() { return 0; }, set [a](v) { }, get [<any>true]() { return 0; }, set [`hello bye`](v) { }, get [`hello ${a} bye`]() { return 0; }} : { [x: string]: any; [x: number]: any; [""]: any; readonly [0]: number; [`hello bye`]: any; }

get [s]() { return 0; },
>[s] : number
Expand Down
5 changes: 1 addition & 4 deletions tests/baselines/reference/computedPropertyNames12_ES5.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(8,
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(9,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(12,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(13,12): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(14,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(15,12): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.


==== tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts (9 errors) ====
==== tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts (8 errors) ====
var s: string;
var n: number;
var a: any;
Expand Down Expand Up @@ -38,8 +37,6 @@ tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES5.ts(15
~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
[`hello bye`] = 0;
~~~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
static [`hello ${a} bye`] = 0
~~~~~~~~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
Expand Down
5 changes: 1 addition & 4 deletions tests/baselines/reference/computedPropertyNames12_ES6.errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(8,
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(9,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(12,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(13,12): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(14,5): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(15,12): error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.


==== tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts (9 errors) ====
==== tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts (8 errors) ====
var s: string;
var n: number;
var a: any;
Expand Down Expand Up @@ -38,8 +37,6 @@ tests/cases/conformance/es6/computedProperties/computedPropertyNames12_ES6.ts(15
~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
[`hello bye`] = 0;
~~~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
static [`hello ${a} bye`] = 0
~~~~~~~~~~~~~~~~~~
!!! error TS1166: A computed property name in a class property declaration must refer to an expression whose type is a literal type or a 'unique symbol' type.
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/computerPropertiesInES5ShouldBeTransformed.types
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
=== tests/cases/compiler/computerPropertiesInES5ShouldBeTransformed.ts ===
const b = ({ [`key`]: renamed }) => renamed;
>b : ({ [`key`]: renamed }: {}) => any
>({ [`key`]: renamed }) => renamed : ({ [`key`]: renamed }: {}) => any
>b : ({ [`key`]: renamed }: { key: any; }) => any
>({ [`key`]: renamed }) => renamed : ({ [`key`]: renamed }: { key: any; }) => any
>`key` : "key"
>renamed : any
>renamed : any
Expand Down