Address code review

This commit is contained in:
Yui T
2015-03-16 15:20:40 -07:00
parent e573461745
commit 88933d54cc
8 changed files with 77 additions and 79 deletions
+28 -38
View File
@@ -2630,7 +2630,6 @@ module ts {
}
function emitSuper(node: Node) {
debugger;
if (languageVersion >= ScriptTarget.ES6) {
write("super");
}
@@ -2639,9 +2638,6 @@ module ts {
if (flags & NodeCheckFlags.SuperInstance) {
write("_super.prototype");
}
else if ((flags & NodeCheckFlags.SuperStatic) || (node.parent.kind === SyntaxKind.Constructor)) {
write("_super");
}
else {
write("_super");
}
@@ -4403,23 +4399,7 @@ module ts {
emitComputedPropertyName(<ComputedPropertyName>memberName);
}
else {
// For ES6 and above, we want to emit memberName by itself without prefix ".",
// For ES5 and below, we want to prefix memberName with ".". For example,
// Typescript:
// class C {
// x = 10;
// foo () {}
// }
// Javascript:
// var C = (function () {
// function C() {
// this.x = 10; // Property "x" need to be prefixed with "."
// }
// C.prototype.foo = function() {}; // Similarly property "foo" need to be prefixed with "."
// }
if (languageVersion < ScriptTarget.ES6 || memberName.parent.kind === SyntaxKind.PropertyDeclaration) {
write(".");
}
write(".");
emitNodeWithoutSourceMap(memberName);
}
}
@@ -4458,14 +4438,18 @@ module ts {
writeLine();
emitLeadingComments(member);
emitStart(member);
emitStart((<MethodDeclaration>member).name);
emitDeclarationName(node);
if (!(member.flags & NodeFlags.Static)) {
write(".prototype");
}
emitMemberAccessForPropertyName((<MethodDeclaration>member).name);
emitEnd((<MethodDeclaration>member).name);
write(" = ");
emitStart(member);
emitFunctionDeclaration(<MethodDeclaration>member);
emitEnd(member);
emitEnd(member);
write(";");
emitTrailingComments(member);
}
@@ -4475,12 +4459,14 @@ module ts {
writeLine();
emitStart(member);
write("Object.defineProperty(");
emitStart((<AccessorDeclaration>member).name);
emitDeclarationName(node);
if (!(member.flags & NodeFlags.Static)) {
write(".prototype");
}
write(", ");
emitExpressionForPropertyName((<AccessorDeclaration>member).name);
emitEnd((<AccessorDeclaration>member).name);
write(", {");
increaseIndent();
if (accessors.getAccessor) {
@@ -4531,7 +4517,7 @@ module ts {
if (member.flags & NodeFlags.Static) {
write("static ");
}
emitMemberAccessForPropertyName((<MethodDeclaration>member).name);
emit((<MethodDeclaration>member).name);
emitSignatureAndBody(<MethodDeclaration>member);
emitEnd(member);
emitTrailingComments(member);
@@ -4539,6 +4525,7 @@ module ts {
else if (member.kind === SyntaxKind.GetAccessor || member.kind === SyntaxKind.SetAccessor) {
var accessors = getAllAccessorDeclarations(node.members, <AccessorDeclaration>member);
if (member === accessors.firstAccessor) {
writeLine();
if (accessors.getAccessor) {
writeLine();
emitLeadingComments(accessors.getAccessor);
@@ -4547,7 +4534,7 @@ module ts {
write("static ");
}
write("get ");
emitMemberAccessForPropertyName((<MethodDeclaration>member).name);
emit((<MethodDeclaration>member).name);
emitSignatureAndBody(accessors.getAccessor);
emitEnd(accessors.getAccessor);
emitTrailingComments(accessors.getAccessor);
@@ -4561,7 +4548,7 @@ module ts {
write("static ");
}
write("set ");
emitMemberAccessForPropertyName((<MethodDeclaration>member).name);
emit((<MethodDeclaration>member).name);
emitSignatureAndBody(accessors.setAccessor);
emitEnd(accessors.setAccessor);
emitTrailingComments(accessors.setAccessor);;
@@ -4572,42 +4559,42 @@ module ts {
}
function emitConstructor(node: ClassDeclaration, baseTypeNode: TypeReferenceNode) {
debugger;
var saveTempCount = tempCount;
var saveTempVariables = tempVariables;
var saveTempParameters = tempParameters;
let saveTempCount = tempCount;
let saveTempVariables = tempVariables;
let saveTempParameters = tempParameters;
tempCount = 0;
tempVariables = undefined;
tempParameters = undefined;
var popFrame = enterNameScope();
let popFrame = enterNameScope();
// Check if we have property assignment inside class declaration.
// If there is property assignment, we need to emit constructor whether users define it or not
// If there is no property assignment, we can omit constructor if users do not define it
var hasPropertyAssignment = false;
let hasInstancePropertyWithInitializer = false;
// Emit the constructor overload pinned comments
forEach(node.members, member => {
if (member.kind === SyntaxKind.Constructor && !(<ConstructorDeclaration>member).body) {
emitPinnedOrTripleSlashComments(member);
}
if (member.kind === SyntaxKind.PropertyDeclaration && (<PropertyDeclaration>member).initializer) {
hasPropertyAssignment = true;
// Check if there is any non-static property assignment
if (member.kind === SyntaxKind.PropertyDeclaration && (<PropertyDeclaration>member).initializer && (member.flags & NodeFlags.Static) === 0) {
hasInstancePropertyWithInitializer = true;
}
});
var ctor = getFirstConstructorWithBody(node);
let ctor = getFirstConstructorWithBody(node);
// For target ES6 and above, if there is no user-defined constructor and there is no property assignment
// do not emit constructor in class declaration.
if (languageVersion >= ScriptTarget.ES6 && !ctor && !hasPropertyAssignment) {
if (languageVersion >= ScriptTarget.ES6 && !ctor && !hasInstancePropertyWithInitializer) {
return;
}
if (ctor) {
emitLeadingComments(ctor);
}
emitStart(<Node>ctor || node);
emitStart(ctor || node);
if (languageVersion < ScriptTarget.ES6) {
write("function ");
@@ -4639,7 +4626,7 @@ module ts {
scopeEmitStart(node, "constructor");
increaseIndent();
if (ctor) {
emitDetachedComments((<Block>ctor.body).statements);
emitDetachedComments(ctor.body.statements);
}
emitCaptureThisForNodeIfNecessary(node);
if (ctor) {
@@ -4662,10 +4649,12 @@ module ts {
emitEnd(baseTypeNode);
}
}
emitMemberAssignments(node, /*nonstatic*/0);
emitMemberAssignments(node, /*staticFlag*/0);
if (ctor) {
var statements: Node[] = (<Block>ctor.body).statements;
if (superCall) statements = statements.slice(1);
if (superCall) {
statements = statements.slice(1);
}
emitLines(statements);
}
emitTempDeclarations(/*newLine*/ true);
@@ -4696,6 +4685,7 @@ module ts {
write("default ");
}
}
write("class ");
emitDeclarationName(node);
var baseTypeNode = getClassBaseTypeNode(node);
+3 -8
View File
@@ -4518,8 +4518,8 @@ module ts {
function parseClassDeclaration(fullStart: number, modifiers: ModifiersArray): ClassDeclaration {
// In ES6 specification, All parts of a ClassDeclaration or a ClassExpression are strict mode code
let savedStrictModeContext = inStrictModeContext();
if (languageVersion >= ScriptTarget.ES6) {
var savedStrictModeContext = inStrictModeContext();
setStrictModeContext(true);
}
@@ -4545,13 +4545,8 @@ module ts {
}
var finishedNode = finishNode(node);
if (languageVersion >= ScriptTarget.ES6) {
setStrictModeContext(savedStrictModeContext);
return finishedNode;
}
else {
return finishedNode;
}
setStrictModeContext(savedStrictModeContext);
return finishedNode;
}
function parseHeritageClauses(isClassHeritageClause: boolean): NodeArray<HeritageClause> {
@@ -1,22 +0,0 @@
tests/cases/conformance/es6/computedProperties/computedPropertyNamesWithStaticProperty.ts(3,9): error TS1200: A computed property name cannot reference a static property
tests/cases/conformance/es6/computedProperties/computedPropertyNamesWithStaticProperty.ts(6,9): error TS1200: A computed property name cannot reference a static property
tests/cases/conformance/es6/computedProperties/computedPropertyNamesWithStaticProperty.ts(9,5): error TS1200: A computed property name cannot reference a static property
==== tests/cases/conformance/es6/computedProperties/computedPropertyNamesWithStaticProperty.ts (3 errors) ====
class C {
static staticProp = 10;
get [C.staticProp]() {
~~~~~~~~~~~~~~
!!! error TS1200: A computed property name cannot reference a static property
return "hello";
}
set [C.staticProp](x: string) {
~~~~~~~~~~~~~~
!!! error TS1200: A computed property name cannot reference a static property
var y = x;
}
[C.staticProp]() { }
~~~~~~~~~~~~~~
!!! error TS1200: A computed property name cannot reference a static property
}
@@ -0,0 +1,29 @@
=== tests/cases/conformance/es6/computedProperties/computedPropertyNamesWithStaticProperty.ts ===
class C {
>C : C
static staticProp = 10;
>staticProp : number
get [C.staticProp]() {
>C.staticProp : number
>C : typeof C
>staticProp : number
return "hello";
}
set [C.staticProp](x: string) {
>C.staticProp : number
>C : typeof C
>staticProp : number
>x : string
var y = x;
>y : string
>x : string
}
[C.staticProp]() { }
>C.staticProp : number
>C : typeof C
>staticProp : number
}
+1 -1
View File
@@ -1,2 +1,2 @@
//// [properties.js.map]
{"version":3,"file":"properties.js","sourceRoot":"","sources":["properties.ts"],"names":["MyClass","MyClass.constructor","MyClass.Count"],"mappings":"AACA;IAAAA;IAWAC,CAACA;IATcD,gDAAKA;aAAhBA;YAEIE,MAAMA,CAACA,EAAEA,CAACA;QACdA,CAACA;aAEDF,UAAiBA,KAAaA;YAE1BE,EAAEA;QACNA,CAACA;;;OALAF;IAMLA,cAACA;AAADA,CAACA,AAXD,IAWC"}
{"version":3,"file":"properties.js","sourceRoot":"","sources":["properties.ts"],"names":["MyClass","MyClass.constructor","MyClass.Count"],"mappings":"AACA;IAAAA;IAWAC,CAACA;IATGD,sBAAWA,0BAAKA;aAAhBA;YAEIE,MAAMA,CAACA,EAAEA,CAACA;QACdA,CAACA;aAEDF,UAAiBA,KAAaA;YAE1BE,EAAEA;QACNA,CAACA;;;OALAF;IAMLA,cAACA;AAADA,CAACA,AAXD,IAWC"}
@@ -43,11 +43,14 @@ sourceFile:properties.ts
---
>>> Object.defineProperty(MyClass.prototype, "Count", {
1->^^^^
2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 > ^^^^^^^^^^^^^^^^^^^^^^
3 > ^^^^^^^^^^^^^^^^^^^^^^^^^^
1->
2 > Count
1->Emitted(4, 5) Source(4, 16) + SourceIndex(0) name (MyClass)
2 >Emitted(4, 53) Source(4, 21) + SourceIndex(0) name (MyClass)
2 > public get
3 > Count
1->Emitted(4, 5) Source(4, 5) + SourceIndex(0) name (MyClass)
2 >Emitted(4, 27) Source(4, 16) + SourceIndex(0) name (MyClass)
3 >Emitted(4, 53) Source(4, 21) + SourceIndex(0) name (MyClass)
---
>>> get: function () {
1 >^^^^^^^^^^^^^
@@ -1,2 +1,2 @@
//// [sourceMapValidationClass.js.map]
{"version":3,"file":"sourceMapValidationClass.js","sourceRoot":"","sources":["sourceMapValidationClass.ts"],"names":["Greeter","Greeter.constructor","Greeter.greet","Greeter.fn","Greeter.greetings"],"mappings":"AAAA;IACIA,iBAAmBA,QAAgBA;QAAEC,WAAcA;aAAdA,WAAcA,CAAdA,sBAAcA,CAAdA,IAAcA;YAAdA,0BAAcA;;QAAhCA,aAAQA,GAARA,QAAQA,CAAQA;QAM3BA,OAAEA,GAAWA,EAAEA,CAACA;IALxBA,CAACA;IACDD,uBAAKA,GAALA;QACIE,MAAMA,CAACA,MAAMA,GAAGA,IAAIA,CAACA,QAAQA,GAAGA,OAAOA,CAACA;IAC5CA,CAACA;IAGOF,oBAAEA,GAAVA;QACIG,MAAMA,CAACA,IAAIA,CAACA,QAAQA,CAACA;IACzBA,CAACA;IACGH,oDAASA;aAAbA;YACII,MAAMA,CAACA,IAAIA,CAACA,QAAQA,CAACA;QACzBA,CAACA;aACDJ,UAAcA,SAAiBA;YAC3BI,IAAIA,CAACA,QAAQA,GAAGA,SAASA,CAACA;QAC9BA,CAACA;;;OAHAJ;IAILA,cAACA;AAADA,CAACA,AAjBD,IAiBC"}
{"version":3,"file":"sourceMapValidationClass.js","sourceRoot":"","sources":["sourceMapValidationClass.ts"],"names":["Greeter","Greeter.constructor","Greeter.greet","Greeter.fn","Greeter.greetings"],"mappings":"AAAA;IACIA,iBAAmBA,QAAgBA;QAAEC,WAAcA;aAAdA,WAAcA,CAAdA,sBAAcA,CAAdA,IAAcA;YAAdA,0BAAcA;;QAAhCA,aAAQA,GAARA,QAAQA,CAAQA;QAM3BA,OAAEA,GAAWA,EAAEA,CAACA;IALxBA,CAACA;IACDD,uBAAKA,GAALA;QACIE,MAAMA,CAACA,MAAMA,GAAGA,IAAIA,CAACA,QAAQA,GAAGA,OAAOA,CAACA;IAC5CA,CAACA;IAGOF,oBAAEA,GAAVA;QACIG,MAAMA,CAACA,IAAIA,CAACA,QAAQA,CAACA;IACzBA,CAACA;IACDH,sBAAIA,8BAASA;aAAbA;YACII,MAAMA,CAACA,IAAIA,CAACA,QAAQA,CAACA;QACzBA,CAACA;aACDJ,UAAcA,SAAiBA;YAC3BI,IAAIA,CAACA,QAAQA,GAAGA,SAASA,CAACA;QAC9BA,CAACA;;;OAHAJ;IAILA,cAACA;AAADA,CAACA,AAjBD,IAiBC"}
@@ -223,12 +223,15 @@ sourceFile:sourceMapValidationClass.ts
---
>>> Object.defineProperty(Greeter.prototype, "greetings", {
1->^^^^
2 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
2 > ^^^^^^^^^^^^^^^^^^^^^^
3 > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1->
> get
2 > greetings
1->Emitted(16, 5) Source(12, 9) + SourceIndex(0) name (Greeter)
2 >Emitted(16, 57) Source(12, 18) + SourceIndex(0) name (Greeter)
>
2 > get
3 > greetings
1->Emitted(16, 5) Source(12, 5) + SourceIndex(0) name (Greeter)
2 >Emitted(16, 27) Source(12, 9) + SourceIndex(0) name (Greeter)
3 >Emitted(16, 57) Source(12, 18) + SourceIndex(0) name (Greeter)
---
>>> get: function () {
1 >^^^^^^^^^^^^^