From 7fe811e6b220cb3b0daa6c931ec7fe73e478efcb Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 10 Dec 2015 16:02:46 -0800 Subject: [PATCH 1/3] Defer checks of accessor bodies in object literals --- src/compiler/checker.ts | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 50a3aee9955..c3dc5935775 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11297,6 +11297,8 @@ namespace ts { // Grammar checking accessors checkGrammarFunctionLikeDeclaration(node) || checkGrammarAccessor(node) || checkGrammarComputedPropertyName(node.name); + checkDecorators(node); + checkSignatureDeclaration(node); if (node.kind === SyntaxKind.GetAccessor) { if (!isInAmbientContext(node) && nodeIsPresent(node.body) && (node.flags & NodeFlags.HasImplicitReturn)) { if (node.flags & NodeFlags.HasExplicitReturn) { @@ -11309,7 +11311,12 @@ namespace ts { } } } - + // Do not use hasDynamicName here, because that returns false for well known symbols. + // We want to perform checkComputedPropertyName for all computed properties, including + // well known symbols. + if (node.name.kind === SyntaxKind.ComputedPropertyName) { + checkComputedPropertyName(node.name); + } if (!hasDynamicName(node)) { // TypeScript 1.0 spec (April 2014): 8.4.3 // Accessors for the same member name must specify the same accessibility. @@ -11333,8 +11340,16 @@ namespace ts { } getTypeOfAccessors(getSymbolOfNode(node)); } + if (node.parent.kind !== SyntaxKind.ObjectLiteralExpression) { + checkSourceElement(node.body); + } + } - checkFunctionLikeDeclaration(node); + function checkObjectLiteralAccessorBody(node: AccessorDeclaration) { + if (node.body) { + checkSourceElement(node.body); + checkFunctionAndClassExpressionBodies(node.body); + } } function checkMissingDeclaration(node: Node) { @@ -14379,11 +14394,16 @@ namespace ts { } break; case SyntaxKind.Constructor: - case SyntaxKind.GetAccessor: - case SyntaxKind.SetAccessor: case SyntaxKind.FunctionDeclaration: forEach((node).parameters, checkFunctionAndClassExpressionBodies); break; + case SyntaxKind.GetAccessor: + case SyntaxKind.SetAccessor: + forEach((node).parameters, checkFunctionAndClassExpressionBodies); + if (node.parent.kind === SyntaxKind.ObjectLiteralExpression) { + checkObjectLiteralAccessorBody(node); + } + break; case SyntaxKind.WithStatement: checkFunctionAndClassExpressionBodies((node).expression); break; From 2e5a4ea9837c9509e709b7fc55614995f443a3a6 Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Thu, 10 Dec 2015 16:19:02 -0800 Subject: [PATCH 2/3] Adding regression test --- .../circularObjectLiteralAccessors.js | 29 ++++++++++++++ .../circularObjectLiteralAccessors.symbols | 34 +++++++++++++++++ .../circularObjectLiteralAccessors.types | 38 +++++++++++++++++++ .../circularObjectLiteralAccessors.ts | 15 ++++++++ 4 files changed, 116 insertions(+) create mode 100644 tests/baselines/reference/circularObjectLiteralAccessors.js create mode 100644 tests/baselines/reference/circularObjectLiteralAccessors.symbols create mode 100644 tests/baselines/reference/circularObjectLiteralAccessors.types create mode 100644 tests/cases/compiler/circularObjectLiteralAccessors.ts diff --git a/tests/baselines/reference/circularObjectLiteralAccessors.js b/tests/baselines/reference/circularObjectLiteralAccessors.js new file mode 100644 index 00000000000..6d99a8f25fc --- /dev/null +++ b/tests/baselines/reference/circularObjectLiteralAccessors.js @@ -0,0 +1,29 @@ +//// [circularObjectLiteralAccessors.ts] + +// Repro from #6000 + +const a = { + b: { + get foo(): string { + return a.foo; + }, + set foo(value: string) { + a.foo = value; + } + }, + foo: '' +}; + +//// [circularObjectLiteralAccessors.js] +// Repro from #6000 +var a = { + b: { + get foo() { + return a.foo; + }, + set foo(value) { + a.foo = value; + } + }, + foo: '' +}; diff --git a/tests/baselines/reference/circularObjectLiteralAccessors.symbols b/tests/baselines/reference/circularObjectLiteralAccessors.symbols new file mode 100644 index 00000000000..21e613c97fe --- /dev/null +++ b/tests/baselines/reference/circularObjectLiteralAccessors.symbols @@ -0,0 +1,34 @@ +=== tests/cases/compiler/circularObjectLiteralAccessors.ts === + +// Repro from #6000 + +const a = { +>a : Symbol(a, Decl(circularObjectLiteralAccessors.ts, 3, 5)) + + b: { +>b : Symbol(b, Decl(circularObjectLiteralAccessors.ts, 3, 11)) + + get foo(): string { +>foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 4, 8), Decl(circularObjectLiteralAccessors.ts, 7, 10)) + + return a.foo; +>a.foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 11, 6)) +>a : Symbol(a, Decl(circularObjectLiteralAccessors.ts, 3, 5)) +>foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 11, 6)) + + }, + set foo(value: string) { +>foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 4, 8), Decl(circularObjectLiteralAccessors.ts, 7, 10)) +>value : Symbol(value, Decl(circularObjectLiteralAccessors.ts, 8, 16)) + + a.foo = value; +>a.foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 11, 6)) +>a : Symbol(a, Decl(circularObjectLiteralAccessors.ts, 3, 5)) +>foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 11, 6)) +>value : Symbol(value, Decl(circularObjectLiteralAccessors.ts, 8, 16)) + } + }, + foo: '' +>foo : Symbol(foo, Decl(circularObjectLiteralAccessors.ts, 11, 6)) + +}; diff --git a/tests/baselines/reference/circularObjectLiteralAccessors.types b/tests/baselines/reference/circularObjectLiteralAccessors.types new file mode 100644 index 00000000000..1f01432db5a --- /dev/null +++ b/tests/baselines/reference/circularObjectLiteralAccessors.types @@ -0,0 +1,38 @@ +=== tests/cases/compiler/circularObjectLiteralAccessors.ts === + +// Repro from #6000 + +const a = { +>a : { b: { foo: string; }; foo: string; } +>{ b: { get foo(): string { return a.foo; }, set foo(value: string) { a.foo = value; } }, foo: ''} : { b: { foo: string; }; foo: string; } + + b: { +>b : { foo: string; } +>{ get foo(): string { return a.foo; }, set foo(value: string) { a.foo = value; } } : { foo: string; } + + get foo(): string { +>foo : string + + return a.foo; +>a.foo : string +>a : { b: { foo: string; }; foo: string; } +>foo : string + + }, + set foo(value: string) { +>foo : string +>value : string + + a.foo = value; +>a.foo = value : string +>a.foo : string +>a : { b: { foo: string; }; foo: string; } +>foo : string +>value : string + } + }, + foo: '' +>foo : string +>'' : string + +}; diff --git a/tests/cases/compiler/circularObjectLiteralAccessors.ts b/tests/cases/compiler/circularObjectLiteralAccessors.ts new file mode 100644 index 00000000000..3546ee75e6f --- /dev/null +++ b/tests/cases/compiler/circularObjectLiteralAccessors.ts @@ -0,0 +1,15 @@ +// @target: es5 + +// Repro from #6000 + +const a = { + b: { + get foo(): string { + return a.foo; + }, + set foo(value: string) { + a.foo = value; + } + }, + foo: '' +}; \ No newline at end of file From 18e23a1b90d78afe709ea51dfe04ab649d92bebc Mon Sep 17 00:00:00 2001 From: Anders Hejlsberg Date: Fri, 11 Dec 2015 10:52:04 -0800 Subject: [PATCH 3/3] Addressing CR feedback --- src/compiler/checker.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index c3dc5935775..214748e6944 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -11170,7 +11170,7 @@ namespace ts { checkGrammarMethod(node) || checkGrammarComputedPropertyName(node.name); // Grammar checking for modifiers is done inside the function checkGrammarFunctionLikeDeclaration - checkFunctionLikeDeclaration(node); + checkFunctionOrMethodDeclaration(node); // Abstract methods cannot have an implementation. // Extra checks are to avoid reporting multiple errors relating to the "abstractness" of the node. @@ -12197,7 +12197,7 @@ namespace ts { function checkFunctionDeclaration(node: FunctionDeclaration): void { if (produceDiagnostics) { - checkFunctionLikeDeclaration(node) || checkGrammarForGenerator(node); + checkFunctionOrMethodDeclaration(node) || checkGrammarForGenerator(node); checkCollisionWithCapturedSuperVariable(node, node.name); checkCollisionWithCapturedThisVariable(node, node.name); @@ -12205,7 +12205,7 @@ namespace ts { } } - function checkFunctionLikeDeclaration(node: FunctionLikeDeclaration): void { + function checkFunctionOrMethodDeclaration(node: FunctionDeclaration | MethodDeclaration): void { checkDecorators(node); checkSignatureDeclaration(node); const isAsync = isAsyncFunctionLike(node); @@ -12252,7 +12252,7 @@ namespace ts { } checkSourceElement(node.body); - if (!isAccessor(node.kind) && !node.asteriskToken) { + if (!node.asteriskToken) { const returnOrPromisedType = node.type && (isAsync ? checkAsyncFunctionReturnType(node) : getTypeFromTypeNode(node.type)); checkAllCodePathsInNonVoidFunctionReturnOrThrow(node, returnOrPromisedType); }