Skip to content

Commit

Permalink
Change class field implementation to use initializer methods
Browse files Browse the repository at this point in the history 
Fixes #311

Rather than moving the assignments to the constructor or after the class body,
we now wrap them in methods that assign to either the instance or class (both
via `this` assignments). The generated code in the constructor or after the
class just calls those methods.

This should make line numbers always line up and should make it possible to set
debugger breakpoints in bound callback methods. It's slightly less correct, but
hopefully that won't come up in practice.
  • Loading branch information
@alangpierce
alangpierce committed Sep 29, 2018
1 parent c7c2e41 commit c53ee84
Show file tree
Hide file tree
Showing 6 changed files with 173 additions and 78 deletions.
14 changes: 10 additions & 4 deletions src/TokenProcessor.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ export default class TokenProcessor {
return this.resultCode.length;
}

getCodeInsertedSinceIndex(initialResultCodeIndex: number): string {
return this.resultCode.slice(initialResultCodeIndex);
}

reset(): void {
this.resultCode = "";
this.tokenIndex = 0;
Expand Down Expand Up @@ -185,6 +181,16 @@ export default class TokenProcessor {
this.tokenIndex++;
}

copyTokenWithPrefix(prefix: string): void {
this.resultCode += this.previousWhitespaceAndComments();
this.resultCode += prefix;
this.resultCode += this.code.slice(
this.tokens[this.tokenIndex].start,
this.tokens[this.tokenIndex].end,
);
this.tokenIndex++;
}

appendCode(code: string): void {
this.resultCode += code;
}
Expand Down
39 changes: 32 additions & 7 deletions src/transformers/RootTransformer.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ export default class RootTransformer {
}

processClass(): void {
const classInfo = getClassInfo(this, this.tokens);
const classInfo = getClassInfo(this, this.tokens, this.nameManager);

const needsCommaExpression =
classInfo.headerInfo.isExpression && classInfo.staticInitializerSuffixes.length > 0;
Expand Down Expand Up @@ -190,8 +190,15 @@ export default class RootTransformer {
* when some JS implementations support class fields, this should be made optional.
*/
processClassBody(classInfo: ClassInfo): void {
const {headerInfo, constructorInsertPos, initializerStatements, fieldRanges} = classInfo;
const {
headerInfo,
constructorInsertPos,
initializerStatements,
fields,
rangesToRemove,
} = classInfo;
let fieldIndex = 0;
let rangeToRemoveIndex = 0;
const classContextId = this.tokens.currentToken().contextId;
if (classContextId == null) {
throw new Error("Expected non-null context ID on class.");
Expand All @@ -211,15 +218,33 @@ export default class RootTransformer {
}

while (!this.tokens.matchesContextIdAndLabel(tt.braceR, classContextId)) {
if (
fieldIndex < fieldRanges.length &&
this.tokens.currentIndex() === fieldRanges[fieldIndex].start
if (fieldIndex < fields.length && this.tokens.currentIndex() === fields[fieldIndex].start) {
let needsCloseBrace = false;
if (this.tokens.matches1(tt.bracketL)) {
this.tokens.copyTokenWithPrefix(`${fields[fieldIndex].initializerName}() {this`);
} else if (this.tokens.matches1(tt.string) || this.tokens.matches1(tt.num)) {
this.tokens.copyTokenWithPrefix(`${fields[fieldIndex].initializerName}() {this[`);
needsCloseBrace = true;
} else {
this.tokens.copyTokenWithPrefix(`${fields[fieldIndex].initializerName}() {this.`);
}
while (this.tokens.currentIndex() < fields[fieldIndex].end) {
if (needsCloseBrace && this.tokens.currentIndex() === fields[fieldIndex].equalsIndex) {
this.tokens.appendCode("]");
}
this.processToken();
}
this.tokens.appendCode("}");
fieldIndex++;
} else if (
rangeToRemoveIndex < rangesToRemove.length &&
this.tokens.currentIndex() === rangesToRemove[rangeToRemoveIndex].start
) {
this.tokens.removeInitialToken();
while (this.tokens.currentIndex() < fieldRanges[fieldIndex].end) {
while (this.tokens.currentIndex() < rangesToRemove[rangeToRemoveIndex].end) {
this.tokens.removeToken();
}
fieldIndex++;
rangeToRemoveIndex++;
} else if (this.tokens.currentIndex() === constructorInsertPos) {
this.tokens.copyToken();
if (initializerStatements.length > 0) {
Expand Down
70 changes: 44 additions & 26 deletions src/util/getClassInfo.ts
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import NameManager from "../NameManager";
import {ContextualKeyword, Token} from "../parser/tokenizer";
import {TokenType as tt} from "../parser/tokenizer/types";
import TokenProcessor from "../TokenProcessor";
Expand All @@ -9,6 +10,20 @@ export interface ClassHeaderInfo {
hasSuperclass: boolean;
}

export interface TokenRange {
start: number;
end: number;
}

export interface FieldInfo extends TokenRange {
equalsIndex: number;
initializerName: string;
}

/**
* Information about a class returned to inform the implementation of class fields and constructor
* initializers.
*/
export interface ClassInfo {
headerInfo: ClassHeaderInfo;
// Array of non-semicolon-delimited code strings to go in the constructor, after super if
Expand All @@ -20,7 +35,8 @@ export interface ClassInfo {
// Token index after which we should insert initializer statements (either the start of the
// constructor, or after the super call), or null if there was no constructor.
constructorInsertPos: number | null;
fieldRanges: Array<{start: number; end: number}>;
fields: Array<FieldInfo>;
rangesToRemove: Array<TokenRange>;
}

/**
Expand All @@ -30,6 +46,7 @@ export interface ClassInfo {
export default function getClassInfo(
rootTransformer: RootTransformer,
tokens: TokenProcessor,
nameManager: NameManager,
): ClassInfo {
const snapshot = tokens.snapshot();

Expand All @@ -39,7 +56,8 @@ export default function getClassInfo(
const classInitializers: Array<string> = [];
const staticInitializerSuffixes: Array<string> = [];
let constructorInsertPos = null;
const fieldRanges = [];
const fields: Array<FieldInfo> = [];
const rangesToRemove: Array<TokenRange> = [];

const classContextId = tokens.currentToken().contextId;
if (classContextId == null) {
Expand All @@ -51,7 +69,7 @@ export default function getClassInfo(
if (tokens.matchesContextual(ContextualKeyword._constructor)) {
({constructorInitializers, constructorInsertPos} = processConstructor(tokens));
} else if (tokens.matches1(tt.semi)) {
fieldRanges.push({start: tokens.currentIndex(), end: tokens.currentIndex() + 1});
rangesToRemove.push({start: tokens.currentIndex(), end: tokens.currentIndex() + 1});
tokens.nextToken();
} else if (tokens.currentToken().isType) {
tokens.nextToken();
Expand All @@ -69,7 +87,8 @@ export default function getClassInfo(
({constructorInitializers, constructorInsertPos} = processConstructor(tokens));
continue;
}
const nameCode = getNameCode(tokens);
const nameStartIndex = tokens.currentIndex();
skipFieldName(tokens);
if (tokens.matches1(tt.lessThan) || tokens.matches1(tt.parenL)) {
// This is a method, so just skip to the next method/field. To do that, we seek forward to
// the next start of a class name (either an open bracket or an identifier, or the closing
Expand All @@ -87,27 +106,35 @@ export default function getClassInfo(
tokens.nextToken();
}
if (tokens.matches1(tt.eq)) {
const equalsIndex = tokens.currentIndex();
// This is an initializer, so we need to wrap in an initializer method.
const valueEnd = tokens.currentToken().rhsEndIndex;
if (valueEnd == null) {
throw new Error("Expected rhsEndIndex on class field assignment.");
}
tokens.nextToken();
const resultCodeStart = tokens.getResultCodeIndex();
// We can't just take this code directly; we need to transform it as well, so delegate to
// the root transformer, which has the same backing token stream. This will append to the
// code, but the snapshot restore later will restore that.
while (tokens.currentIndex() < valueEnd) {
rootTransformer.processToken();
}
// Note that this can adjust line numbers in the case of multiline expressions.
const expressionCode = tokens.getCodeInsertedSinceIndex(resultCodeStart);
let initializerName;
if (isStatic) {
staticInitializerSuffixes.push(`${nameCode} =${expressionCode}`);
initializerName = nameManager.claimFreeName("__initStaticField");
staticInitializerSuffixes.push(`.${initializerName}()`);
} else {
classInitializers.push(`this${nameCode} =${expressionCode}`);
initializerName = nameManager.claimFreeName("__initField");
classInitializers.push(`this.${initializerName}()`);
}
// Fields start at the name, so `static x = 1;` has a field range of `x = 1;`.
fields.push({
initializerName,
equalsIndex,
start: nameStartIndex,
end: tokens.currentIndex(),
});
} else {
// This is just a declaration, so doesn't need to produce any code in the output.
rangesToRemove.push({start: statementStartIndex, end: tokens.currentIndex()});
}
fieldRanges.push({start: statementStartIndex, end: tokens.currentIndex()});
}
}

Expand All @@ -117,7 +144,8 @@ export default function getClassInfo(
initializerStatements: [...constructorInitializers, ...classInitializers],
staticInitializerSuffixes,
constructorInsertPos,
fieldRanges,
fields,
rangesToRemove,
};
}

Expand Down Expand Up @@ -222,11 +250,9 @@ function isAccessModifier(token: Token): boolean {

/**
* The next token or set of tokens is either an identifier or an expression in square brackets, for
* a method or field name. Get the code that would follow `this` to access this value. Note that a
* more correct implementation would precompute computed field and method names, but that's harder,
* and TypeScript doesn't do it, so we won't either.
* a method or field name.
*/
function getNameCode(tokens: TokenProcessor): string {
function skipFieldName(tokens: TokenProcessor): void {
if (tokens.matches1(tt.bracketL)) {
const startToken = tokens.currentToken();
const classContextId = startToken.contextId;
Expand All @@ -236,16 +262,8 @@ function getNameCode(tokens: TokenProcessor): string {
while (!tokens.matchesContextIdAndLabel(tt.bracketR, classContextId)) {
tokens.nextToken();
}
const endToken = tokens.currentToken();
tokens.nextToken();
return tokens.code.slice(startToken.start, endToken.end);
} else {
const nameToken = tokens.currentToken();
tokens.nextToken();
if (nameToken.type === tt.string || nameToken.type === tt.num) {
return `[${tokens.code.slice(nameToken.start, nameToken.end)}]`;
} else {
return `.${tokens.identifierNameForToken(nameToken)}`;
}
}
}
78 changes: 62 additions & 16 deletions test/sucrase-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,8 @@ describe("sucrase", () => {
`,
`"use strict";
class A {
} A.x = 3;
static __initStaticField() {this.x = 3}
} A.__initStaticField();
`,
{transforms: ["jsx", "imports", "typescript"]},
);
Expand All @@ -300,8 +300,8 @@ describe("sucrase", () => {
`,
`"use strict"; var _class;
const A = (_class = class {
}, _class.x = 3, _class)
static __initStaticField() {this.x = 3}
}, _class.__initStaticField(), _class)
`,
{transforms: ["jsx", "imports", "typescript"]},
);
Expand All @@ -316,8 +316,8 @@ describe("sucrase", () => {
`,
`"use strict";${ESMODULE_PREFIX}
class C {
} C.x = 3; exports.default = C;
static __initStaticField() {this.x = 3}
} C.__initStaticField(); exports.default = C;
`,
{transforms: ["jsx", "imports", "typescript"]},
);
Expand All @@ -336,10 +336,10 @@ describe("sucrase", () => {
`"use strict";${IMPORT_DEFAULT_PREFIX}
var _A = require('A'); var _A2 = _interopRequireDefault(_A);
var _B = require('B'); var _B2 = _interopRequireDefault(_B);
class C {constructor() { this.a = _A2.default; }
} C.b = _B2.default;
class C {constructor() { this.__initField(); }
__initField() {this.a = _A2.default}
static __initStaticField() {this.b = _B2.default}
} C.__initStaticField();
`,
{transforms: ["jsx", "imports", "typescript"]},
);
Expand Down Expand Up @@ -518,11 +518,11 @@ describe("sucrase", () => {
`,
`"use strict";
class A {
static __initStaticField() {this.b = {}}
c () {
const d = 1;
}
} A.b = {};
} A.__initStaticField();
`,
{transforms: ["imports"]},
);
Expand All @@ -540,10 +540,10 @@ describe("sucrase", () => {
export default function() {}
`,
`"use strict";${ESMODULE_PREFIX}
class Observer {constructor() { this.update = (v) => {};this.complete = () => {};this.error = (err) => {}; }
class Observer {constructor() { this.__initField();this.__initField2();this.__initField3(); }
__initField() {this.update = (v) => {}}
__initField2() {this.complete = () => {}}
__initField3() {this.error = (err) => {}}
} exports.Observer = Observer;
exports. default = function() {}
Expand Down Expand Up @@ -583,4 +583,50 @@ describe("sucrase", () => {
{transforms: ["imports", "typescript"]},
);
});

it("handles static class fields with non-identifier names", () => {
assertResult(
`
class C {
static [f] = 3;
static 5 = 'Hello';
static "test" = "value";
}
`,
`"use strict";
class C {
static __initStaticField() {this[f] = 3}
static __initStaticField2() {this[5] = 'Hello'}
static __initStaticField3() {this["test"] = "value"}
} C.__initStaticField(); C.__initStaticField2(); C.__initStaticField3();
`,
{transforms: ["imports", "typescript"]},
);
});

it("preserves line numbers for multiline fields", () => {
assertResult(
`
class C {
f() {
}
g = () => {
console.log(1);
console.log(2);
}
}
`,
`"use strict";
class C {constructor() { this.__initField(); }
f() {
}
__initField() {this.g = () => {
console.log(1);
console.log(2);
}}
}
`,
{transforms: ["imports", "typescript"]},
);
});
});
6 changes: 3 additions & 3 deletions test/types-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@ describe("type transforms", () => {
}
`,
`"use strict";
class A {constructor() { this.x = 2;this.y = {}; }
class A {constructor() { this.__initField();this.__initField2(); }
__initField() {this.x = 2}
__initField2() {this.y = {}}
}
`,
);
Expand Down
Loading

0 comments on commit c53ee84

Please sign in to comment.