Skip to content

Commit

Permalink
Syntactically allow computed properties everywhere if the name looks …
Browse files Browse the repository at this point in the history
…like a built in Symbol
  • Loading branch information
JsonFreeman committed Feb 7, 2015
1 parent b30d8f3 commit 39952b1
Show file tree
Hide file tree
Showing 47 changed files with 357 additions and 12 deletions.
20 changes: 10 additions & 10 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7487,7 +7487,7 @@ module ts {

function checkPropertyDeclaration(node: PropertyDeclaration) {
// Grammar checking
checkGrammarModifiers(node) || checkGrammarProperty(node);
checkGrammarModifiers(node) || checkGrammarProperty(node) || checkGrammarComputedPropertyName(node.name);

checkVariableLikeDeclaration(node);
}
Expand Down Expand Up @@ -10771,8 +10771,8 @@ module ts {
}
}

function checkGrammarForDisallowedComputedProperty(node: DeclarationName, message: DiagnosticMessage) {
if (node.kind === SyntaxKind.ComputedPropertyName) {
function checkGrammarForNonSymbolComputedProperty(node: DeclarationName, message: DiagnosticMessage) {

This comment has been minimized.

Copy link
@yuit

yuit Feb 10, 2015

Contributor

I find it slightly off that we have to pass in the message for check Grammar. Why don't we move the entire if /else inside?

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Feb 10, 2015

Author Contributor

I am inclined to leave it for the following reason. We already have to discriminate between various syntactic forms outside the call, so moving it inside would not absolve the caller from having to make certain decisions. In fact, the decision that function would make internally would end up very similar to the caller's decisions.

if (node.kind === SyntaxKind.ComputedPropertyName && !isWellKnownSymbolSyntactically((<ComputedPropertyName>node).expression)) {
return grammarErrorOnNode(node, message);
}
}
Expand Down Expand Up @@ -10803,17 +10803,17 @@ module ts {
// and accessors are not allowed in ambient contexts in general,
// so this error only really matters for methods.
if (isInAmbientContext(node)) {
return checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_ambient_context_must_directly_refer_to_a_built_in_Symbol);
return checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_ambient_context_must_directly_refer_to_a_built_in_Symbol);
}
else if (!node.body) {
return checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_method_overload_must_directly_refer_to_a_built_in_Symbol);
return checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_method_overload_must_directly_refer_to_a_built_in_Symbol);
}
}
else if (node.parent.kind === SyntaxKind.InterfaceDeclaration) {
return checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_interface_must_directly_refer_to_a_built_in_Symbol);
return checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_interface_must_directly_refer_to_a_built_in_Symbol);
}
else if (node.parent.kind === SyntaxKind.TypeLiteral) {
return checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_type_literal_must_directly_refer_to_a_built_in_Symbol);
return checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_type_literal_must_directly_refer_to_a_built_in_Symbol);
}
}

Expand Down Expand Up @@ -11084,17 +11084,17 @@ module ts {
function checkGrammarProperty(node: PropertyDeclaration) {
if (node.parent.kind === SyntaxKind.ClassDeclaration) {
if (checkGrammarForInvalidQuestionMark(node, node.questionToken, Diagnostics.A_class_member_cannot_be_declared_optional) ||
checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_class_property_declaration_must_directly_refer_to_a_built_in_Symbol)) {
checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_class_property_declaration_must_directly_refer_to_a_built_in_Symbol)) {
return true;
}
}
else if (node.parent.kind === SyntaxKind.InterfaceDeclaration) {
if (checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_interface_must_directly_refer_to_a_built_in_Symbol)) {
if (checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_an_interface_must_directly_refer_to_a_built_in_Symbol)) {
return true;
}
}
else if (node.parent.kind === SyntaxKind.TypeLiteral) {
if (checkGrammarForDisallowedComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_type_literal_must_directly_refer_to_a_built_in_Symbol)) {
if (checkGrammarForNonSymbolComputedProperty(node.name, Diagnostics.A_computed_property_name_in_a_type_literal_must_directly_refer_to_a_built_in_Symbol)) {
return true;
}
}
Expand Down
14 changes: 12 additions & 2 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -835,11 +835,21 @@ module ts {
return SyntaxKind.FirstTriviaToken <= token && token <= SyntaxKind.LastTriviaToken;
}

export function isWellKnownSymbolSyntactically(node: Node): boolean {

This comment has been minimized.

Copy link
@yuit

yuit Feb 10, 2015

Contributor

Can you leave some comment here?

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Feb 10, 2015

Author Contributor
    /**
     * Checks if the expression is of the form:
     *    Symbol.name
     * where Symbol is literally the word "Symbol", and name is any identifierName
     */

This comment has been minimized.

Copy link
@JsonFreeman

JsonFreeman Feb 10, 2015

Author Contributor

Also, is the name bad? We can try to think of a different name.

return node.kind === SyntaxKind.PropertyAccessExpression && isESSymbolIdentifier((<PropertyAccessExpression>node).expression);
}

export function isESSymbolTypeNode(node: Node): boolean {
return node.kind === SyntaxKind.TypeReference &&
(<TypeReferenceNode>node).typeArguments === undefined &&
(<TypeReferenceNode>node).typeName.kind === SyntaxKind.Identifier &&
(<Identifier>(<TypeReferenceNode>node).typeName).text === "Symbol";
isESSymbolIdentifier((<TypeReferenceNode>node).typeName);
}

/**
* Includes the word "Symbol" with unicode escapes
*/
export function isESSymbolIdentifier(node: Node): boolean {
return node.kind === SyntaxKind.Identifier && (<Identifier>node).text === "Symbol";
}

export function isModifier(token: SyntaxKind): boolean {
Expand Down
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty1.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty1.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty1.ts (2 errors) ====
interface I {
[Symbol.iterator]: string;
~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty2.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty2.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty2.ts (2 errors) ====
interface I {
[Symbol.unscopables](): string;
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty3.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty3.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty3.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty3.ts (2 errors) ====
declare class C {
[Symbol.unscopables](): string;
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty4.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty4.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty4.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty4.ts (2 errors) ====
declare class C {
[Symbol.isRegExp]: string;
~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty5.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty5.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty5.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty5.ts (2 errors) ====
class C {
[Symbol.isRegExp]: string;
~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty6.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty6.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty6.ts (2 errors) ====
class C {
[Symbol.toStringTag]: string = "";
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty7.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty7.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty7.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty7.ts (2 errors) ====
class C {
[Symbol.toStringTag](): void { }
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty8.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty8.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty8.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty8.ts (2 errors) ====
var x: {
[Symbol.toPrimitive](): string
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserES5SymbolProperty9.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty9.ts(2,5): error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty9.ts(2,6): error TS2304: Cannot find name 'Symbol'.


==== tests/cases/conformance/parser/ecmascript5/Symbols/parserES5SymbolProperty9.ts (2 errors) ====
var x: {
[Symbol.toPrimitive]: string
~~~~~~~~~~~~~~~~~~~~
!!! error TS1167: Computed property names are only available when targeting ECMAScript 6 and higher.
~~~~~~
!!! error TS2304: Cannot find name 'Symbol'.
}
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty1.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty1.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty1.ts (1 errors) ====
interface I {
[Symbol.iterator]: string;
~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
6 changes: 6 additions & 0 deletions tests/baselines/reference/parserSymbolProperty1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [parserSymbolProperty1.ts]
interface I {
[Symbol.iterator]: string;
}

//// [parserSymbolProperty1.js]
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty2.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty2.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty2.ts (1 errors) ====
interface I {
[Symbol.unscopables](): string;
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
6 changes: 6 additions & 0 deletions tests/baselines/reference/parserSymbolProperty2.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [parserSymbolProperty2.ts]
interface I {
[Symbol.unscopables](): string;
}

//// [parserSymbolProperty2.js]
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty3.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty3.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty3.ts (1 errors) ====
declare class C {
[Symbol.unscopables](): string;
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
6 changes: 6 additions & 0 deletions tests/baselines/reference/parserSymbolProperty3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [parserSymbolProperty3.ts]
declare class C {
[Symbol.unscopables](): string;
}

//// [parserSymbolProperty3.js]
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty4.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty4.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty4.ts (1 errors) ====
declare class C {
[Symbol.isRegExp]: string;
~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
6 changes: 6 additions & 0 deletions tests/baselines/reference/parserSymbolProperty4.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
//// [parserSymbolProperty4.ts]
declare class C {
[Symbol.isRegExp]: string;
}

//// [parserSymbolProperty4.js]
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty5.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty5.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty5.ts (1 errors) ====
class C {
[Symbol.isRegExp]: string;
~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
11 changes: 11 additions & 0 deletions tests/baselines/reference/parserSymbolProperty5.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
//// [parserSymbolProperty5.ts]
class C {
[Symbol.isRegExp]: string;
}

//// [parserSymbolProperty5.js]
var C = (function () {
function C() {
}
return C;
})();
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty6.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty6.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty6.ts (1 errors) ====
class C {
[Symbol.toStringTag]: string = "";
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
12 changes: 12 additions & 0 deletions tests/baselines/reference/parserSymbolProperty6.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//// [parserSymbolProperty6.ts]
class C {
[Symbol.toStringTag]: string = "";
}

//// [parserSymbolProperty6.js]
var C = (function () {
function C() {
this[Symbol.toStringTag] = "";
}
return C;
})();
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty7.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty7.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty7.ts (1 errors) ====
class C {
[Symbol.toStringTag](): void { }
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
13 changes: 13 additions & 0 deletions tests/baselines/reference/parserSymbolProperty7.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
//// [parserSymbolProperty7.ts]
class C {
[Symbol.toStringTag](): void { }
}

//// [parserSymbolProperty7.js]
var C = (function () {
function C() {
}
C.prototype[Symbol.toStringTag] = function () {
};
return C;
})();
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty8.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty8.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty8.ts (1 errors) ====
var x: {
[Symbol.toPrimitive](): string
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
7 changes: 7 additions & 0 deletions tests/baselines/reference/parserSymbolProperty8.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [parserSymbolProperty8.ts]
var x: {
[Symbol.toPrimitive](): string
}

//// [parserSymbolProperty8.js]
var x;
9 changes: 9 additions & 0 deletions tests/baselines/reference/parserSymbolProperty9.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty9.ts(2,5): error TS2464: A computed property name must be of type 'string', 'number', or 'any'.


==== tests/cases/conformance/parser/ecmascript6/Symbols/parserSymbolProperty9.ts (1 errors) ====
var x: {
[Symbol.toPrimitive]: string
~~~~~~~~~~~~~~~~~~~~
!!! error TS2464: A computed property name must be of type 'string', 'number', or 'any'.
}
7 changes: 7 additions & 0 deletions tests/baselines/reference/parserSymbolProperty9.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
//// [parserSymbolProperty9.ts]
var x: {
[Symbol.toPrimitive]: string
}

//// [parserSymbolProperty9.js]
var x;
Loading

0 comments on commit 39952b1

Please sign in to comment.