From 48ae5ea6f87a0e22a57bea61db8f9d84669ba017 Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 7 Jul 2015 13:25:39 -0700 Subject: [PATCH 1/4] Remove check if node is a block --- src/compiler/checker.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 3ef2c6ada2a..7083feabb2c 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11446,7 +11446,7 @@ namespace ts { function checkGrammarDisallowedModifiersInBlockOrObjectLiteralExpression(node: Node) { if (node.modifiers) { - if (inBlockOrObjectLiteralExpression(node)) { + if (inObjectLiteralExpression(node)) { if (isAsyncFunctionLike(node)) { if (node.modifiers.length > 1) { return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); @@ -11459,9 +11459,9 @@ namespace ts { } } - function inBlockOrObjectLiteralExpression(node: Node) { + function inObjectLiteralExpression(node: Node) { while (node) { - if (node.kind === SyntaxKind.Block || node.kind === SyntaxKind.ObjectLiteralExpression) { + if (node.kind === SyntaxKind.ObjectLiteralExpression) { return true; } From 9344bd0a397e74246e657ec466548067de7fe59d Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 7 Jul 2015 13:25:53 -0700 Subject: [PATCH 2/4] Add tests --- ...ifierOnClassDeclarationMemberInFunction.js | 25 +++++++++++++++++++ ...OnClassDeclarationMemberInFunction.symbols | 18 +++++++++++++ ...erOnClassDeclarationMemberInFunction.types | 19 ++++++++++++++ ...difierOnClassExpressionMemberInFunction.js | 25 +++++++++++++++++++ ...rOnClassExpressionMemberInFunction.symbols | 19 ++++++++++++++ ...ierOnClassExpressionMemberInFunction.types | 22 ++++++++++++++++ ...ifierOnClassDeclarationMemberInFunction.ts | 9 +++++++ ...difierOnClassExpressionMemberInFunction.ts | 10 ++++++++ 8 files changed, 147 insertions(+) create mode 100644 tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.js create mode 100644 tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.symbols create mode 100644 tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.types create mode 100644 tests/baselines/reference/modifierOnClassExpressionMemberInFunction.js create mode 100644 tests/baselines/reference/modifierOnClassExpressionMemberInFunction.symbols create mode 100644 tests/baselines/reference/modifierOnClassExpressionMemberInFunction.types create mode 100644 tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts create mode 100644 tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts diff --git a/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.js b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.js new file mode 100644 index 00000000000..d571a4143ee --- /dev/null +++ b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.js @@ -0,0 +1,25 @@ +//// [modifierOnClassDeclarationMemberInFunction.ts] + +function f() { + class C { + public baz = 1; + static foo() { } + public bar() { } + } +} + +//// [modifierOnClassDeclarationMemberInFunction.js] +function f() { + var C = (function () { + function C() { + this.baz = 1; + } + C.foo = function () { }; + C.prototype.bar = function () { }; + return C; + })(); +} + + +//// [modifierOnClassDeclarationMemberInFunction.d.ts] +declare function f(): void; diff --git a/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.symbols b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.symbols new file mode 100644 index 00000000000..a411bee09cb --- /dev/null +++ b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.symbols @@ -0,0 +1,18 @@ +=== tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts === + +function f() { +>f : Symbol(f, Decl(modifierOnClassDeclarationMemberInFunction.ts, 0, 0)) + + class C { +>C : Symbol(C, Decl(modifierOnClassDeclarationMemberInFunction.ts, 1, 14)) + + public baz = 1; +>baz : Symbol(baz, Decl(modifierOnClassDeclarationMemberInFunction.ts, 2, 13)) + + static foo() { } +>foo : Symbol(C.foo, Decl(modifierOnClassDeclarationMemberInFunction.ts, 3, 23)) + + public bar() { } +>bar : Symbol(bar, Decl(modifierOnClassDeclarationMemberInFunction.ts, 4, 24)) + } +} diff --git a/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.types b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.types new file mode 100644 index 00000000000..dfd482a6bc8 --- /dev/null +++ b/tests/baselines/reference/modifierOnClassDeclarationMemberInFunction.types @@ -0,0 +1,19 @@ +=== tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts === + +function f() { +>f : () => void + + class C { +>C : C + + public baz = 1; +>baz : number +>1 : number + + static foo() { } +>foo : () => void + + public bar() { } +>bar : () => void + } +} diff --git a/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.js b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.js new file mode 100644 index 00000000000..e43e8dd8203 --- /dev/null +++ b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.js @@ -0,0 +1,25 @@ +//// [modifierOnClassExpressionMemberInFunction.ts] + +function g() { + var x = class C { + public prop1 = 1; + private foo() { } + static prop2 = 43; + } +} + +//// [modifierOnClassExpressionMemberInFunction.js] +function g() { + var x = (function () { + function C() { + this.prop1 = 1; + } + C.prototype.foo = function () { }; + C.prop2 = 43; + return C; + })(); +} + + +//// [modifierOnClassExpressionMemberInFunction.d.ts] +declare function g(): void; diff --git a/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.symbols b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.symbols new file mode 100644 index 00000000000..d603d96dcda --- /dev/null +++ b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.symbols @@ -0,0 +1,19 @@ +=== tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts === + +function g() { +>g : Symbol(g, Decl(modifierOnClassExpressionMemberInFunction.ts, 0, 0)) + + var x = class C { +>x : Symbol(x, Decl(modifierOnClassExpressionMemberInFunction.ts, 2, 7)) +>C : Symbol(C, Decl(modifierOnClassExpressionMemberInFunction.ts, 2, 11)) + + public prop1 = 1; +>prop1 : Symbol(C.prop1, Decl(modifierOnClassExpressionMemberInFunction.ts, 2, 21)) + + private foo() { } +>foo : Symbol(C.foo, Decl(modifierOnClassExpressionMemberInFunction.ts, 3, 25)) + + static prop2 = 43; +>prop2 : Symbol(C.prop2, Decl(modifierOnClassExpressionMemberInFunction.ts, 4, 25)) + } +} diff --git a/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.types b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.types new file mode 100644 index 00000000000..87a200ee56e --- /dev/null +++ b/tests/baselines/reference/modifierOnClassExpressionMemberInFunction.types @@ -0,0 +1,22 @@ +=== tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts === + +function g() { +>g : () => void + + var x = class C { +>x : typeof C +>class C { public prop1 = 1; private foo() { } static prop2 = 43; } : typeof C +>C : typeof C + + public prop1 = 1; +>prop1 : number +>1 : number + + private foo() { } +>foo : () => void + + static prop2 = 43; +>prop2 : number +>43 : number + } +} diff --git a/tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts b/tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts new file mode 100644 index 00000000000..f0d7c355e3b --- /dev/null +++ b/tests/cases/conformance/classes/classDeclarations/modifierOnClassDeclarationMemberInFunction.ts @@ -0,0 +1,9 @@ +// @declaration: true + +function f() { + class C { + public baz = 1; + static foo() { } + public bar() { } + } +} \ No newline at end of file diff --git a/tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts b/tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts new file mode 100644 index 00000000000..100c04a1d31 --- /dev/null +++ b/tests/cases/conformance/classes/classExpressions/modifierOnClassExpressionMemberInFunction.ts @@ -0,0 +1,10 @@ +// @declaration: true +// @declaration: true + +function g() { + var x = class C { + public prop1 = 1; + private foo() { } + static prop2 = 43; + } +} \ No newline at end of file From cdc999a6c51e819e7962f5acfd0c2bbf73f2814e Mon Sep 17 00:00:00 2001 From: Yui T Date: Tue, 7 Jul 2015 16:26:48 -0700 Subject: [PATCH 3/4] Only check if method declaration has modifier when method is declared in object literal expression --- src/compiler/checker.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7083feabb2c..f4bbc71793e 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11444,9 +11444,10 @@ namespace ts { forEach(node.declarationList.declarations, checkSourceElement); } - function checkGrammarDisallowedModifiersInBlockOrObjectLiteralExpression(node: Node) { + function checkGrammarDisallowedModifiersOnMethodInObjectLiteralExpression(node: Node) { if (node.modifiers) { - if (inObjectLiteralExpression(node)) { + if (node.parent.kind === SyntaxKind.ObjectLiteralExpression){ + // If this method declaration is a property of object-literal-expression if (isAsyncFunctionLike(node)) { if (node.modifiers.length > 1) { return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); @@ -11459,18 +11460,6 @@ namespace ts { } } - function inObjectLiteralExpression(node: Node) { - while (node) { - if (node.kind === SyntaxKind.ObjectLiteralExpression) { - return true; - } - - node = node.parent; - } - - return false; - } - function checkExpressionStatement(node: ExpressionStatement) { // Grammar checking checkGrammarStatementInAmbientContext(node); @@ -15026,7 +15015,7 @@ namespace ts { } function checkGrammarMethod(node: MethodDeclaration) { - if (checkGrammarDisallowedModifiersInBlockOrObjectLiteralExpression(node) || + if (checkGrammarDisallowedModifiersOnMethodInObjectLiteralExpression(node) || checkGrammarFunctionLikeDeclaration(node) || checkGrammarForGenerator(node)) { return true; From 8e15a42632aa68153b85632cff6678216eaab94d Mon Sep 17 00:00:00 2001 From: Yui T Date: Wed, 8 Jul 2015 13:56:27 -0700 Subject: [PATCH 4/4] Address code review --- src/compiler/checker.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f4bbc71793e..0b577cc12fd 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11444,19 +11444,17 @@ namespace ts { forEach(node.declarationList.declarations, checkSourceElement); } - function checkGrammarDisallowedModifiersOnMethodInObjectLiteralExpression(node: Node) { - if (node.modifiers) { - if (node.parent.kind === SyntaxKind.ObjectLiteralExpression){ - // If this method declaration is a property of object-literal-expression - if (isAsyncFunctionLike(node)) { - if (node.modifiers.length > 1) { - return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); - } - } - else { + function checkGrammarDisallowedModifiersOnObjectLiteralExpressionMethod(node: Node) { + // We only disallow modifier on a method declaration if it is a property of object-literal-expression + if (node.modifiers && node.parent.kind === SyntaxKind.ObjectLiteralExpression){ + if (isAsyncFunctionLike(node)) { + if (node.modifiers.length > 1) { return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); } } + else { + return grammarErrorOnFirstToken(node, Diagnostics.Modifiers_cannot_appear_here); + } } } @@ -15015,7 +15013,7 @@ namespace ts { } function checkGrammarMethod(node: MethodDeclaration) { - if (checkGrammarDisallowedModifiersOnMethodInObjectLiteralExpression(node) || + if (checkGrammarDisallowedModifiersOnObjectLiteralExpressionMethod(node) || checkGrammarFunctionLikeDeclaration(node) || checkGrammarForGenerator(node)) { return true;