From 3c3e081f31578e0c8bb16b29c38ce331734bcc25 Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Mon, 18 Apr 2016 13:23:53 -0700 Subject: [PATCH] Fix source maps for rest params, class properties, parameter properties, and accessors. --- src/compiler/factory.ts | 104 +++++++++++------- src/compiler/printer.ts | 16 ++- src/compiler/sourcemap.ts | 7 +- src/compiler/transformers/es6.ts | 37 ++++--- src/compiler/transformers/ts.ts | 16 ++- src/compiler/types.ts | 9 +- .../reference/sourceMapValidationClass.js.map | 2 +- .../sourceMapValidationClass.sourcemap.txt | 16 +-- 8 files changed, 130 insertions(+), 77 deletions(-) diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 8721a03ad71..7935801ec2b 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -106,7 +106,7 @@ namespace ts { * Gets a clone of a node with a unique node ID. */ export function getUniqueClone(node: T): T { - const clone = getSynthesizedClone(node); + const clone = getMutableClone(node); clone.id = undefined; getNodeId(clone); return clone; @@ -926,34 +926,8 @@ namespace ts { ); } - function createPropertyDescriptor({ get, set, value, enumerable, configurable, writable }: PropertyDescriptorOptions, preferNewLine?: boolean, location?: TextRange, descriptorLocations?: PropertyDescriptorLocations) { - const properties: ObjectLiteralElement[] = []; - addPropertyAssignment(properties, "get", get, preferNewLine, descriptorLocations); - addPropertyAssignment(properties, "set", set, preferNewLine, descriptorLocations); - addPropertyAssignment(properties, "value", value, preferNewLine, descriptorLocations); - addPropertyAssignment(properties, "enumerable", enumerable, preferNewLine, descriptorLocations); - addPropertyAssignment(properties, "configurable", configurable, preferNewLine, descriptorLocations); - addPropertyAssignment(properties, "writable", writable, preferNewLine, descriptorLocations); - return createObjectLiteral(properties, location, preferNewLine); - } - - function addPropertyAssignment(properties: ObjectLiteralElement[], name: string, value: boolean | Expression, preferNewLine: boolean, descriptorLocations?: PropertyDescriptorLocations) { - if (value !== undefined) { - const property = createPropertyAssignment( - name, - typeof value === "boolean" ? createLiteral(value) : value, - descriptorLocations ? descriptorLocations[name] : undefined - ); - - if (preferNewLine) { - startOnNewLine(property); - } - - properties.push(property); - } - } - export interface PropertyDescriptorOptions { + [key: string]: boolean | Expression; get?: Expression; set?: Expression; value?: Expression; @@ -962,17 +936,23 @@ namespace ts { writable?: boolean | Expression; } - export interface PropertyDescriptorLocations { - [key: string]: TextRange; - get?: TextRange; - set?: TextRange; - value?: TextRange; - enumerable?: TextRange; - configurable?: TextRange; - writable?: TextRange; + export interface PropertyDescriptorExtendedOptions { + [key: string]: PropertyDescriptorExtendedOption; + get?: PropertyDescriptorExtendedOption; + set?: PropertyDescriptorExtendedOption; + value?: PropertyDescriptorExtendedOption; + enumerable?: PropertyDescriptorExtendedOption; + configurable?: PropertyDescriptorExtendedOption; + writable?: PropertyDescriptorExtendedOption; } - export function createObjectDefineProperty(target: Expression, memberName: Expression, descriptor: PropertyDescriptorOptions, preferNewLine?: boolean, location?: TextRange, descriptorLocations?: PropertyDescriptorLocations) { + export interface PropertyDescriptorExtendedOption { + location?: TextRange; + emitFlags?: NodeEmitFlags; + newLine?: boolean; + } + + export function createObjectDefineProperty(target: Expression, memberName: Expression, descriptor: PropertyDescriptorOptions, preferNewLine?: boolean, location?: TextRange, descriptorOptions?: PropertyDescriptorExtendedOptions, context?: TransformationContext) { return createCall( createPropertyAccess( createIdentifier("Object"), @@ -981,12 +961,60 @@ namespace ts { [ target, memberName, - createPropertyDescriptor(descriptor, preferNewLine, /*location*/ undefined, descriptorLocations) + createObjectLiteral( + createPropertyDescriptorProperties(descriptor, descriptorOptions, preferNewLine, context), + /*location*/ undefined, + /*multiLine*/ preferNewLine + ) ], location ); } + function createPropertyDescriptorProperties(descriptor: PropertyDescriptorOptions, descriptorExtendedOptions: PropertyDescriptorExtendedOptions, preferNewLine: boolean, context: TransformationContext) { + const properties: ObjectLiteralElement[] = []; + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "get", descriptor, descriptorExtendedOptions, preferNewLine, context); + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "set", descriptor, descriptorExtendedOptions, preferNewLine, context); + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "value", descriptor, descriptorExtendedOptions, preferNewLine, context); + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "enumerable", descriptor, descriptorExtendedOptions, preferNewLine, context); + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "configurable", descriptor, descriptorExtendedOptions, preferNewLine, context); + addPropertyDescriptorPropertyAssignmentIfNeeded(properties, "writable", descriptor, descriptorExtendedOptions, preferNewLine, context); + return properties; + } + + function addPropertyDescriptorPropertyAssignmentIfNeeded(properties: ObjectLiteralElement[], name: string, descriptor: PropertyDescriptorOptions, descriptorExtendedOptions: PropertyDescriptorExtendedOptions, preferNewLine: boolean, context: TransformationContext) { + const value = getProperty(descriptor, name); + if (value !== undefined) { + const options = getProperty(descriptorExtendedOptions, name); + let location: TextRange; + let emitFlags: NodeEmitFlags; + if (isDefined(options)) { + location = options.location; + emitFlags = options.emitFlags; + if (isDefined(options.newLine)) { + preferNewLine = options.newLine; + } + } + + const property = createPropertyAssignment( + name, + typeof value === "boolean" ? createLiteral(value) : value, + location + ); + + if (isDefined(emitFlags)) { + Debug.assert(isDefined(context), "TransformationContext must be supplied when emitFlags are provided."); + context.setNodeEmitFlags(property, emitFlags); + } + + if (preferNewLine) { + startOnNewLine(property); + } + + properties.push(property); + } + } + function createObjectCreate(prototype: Expression) { return createCall( createPropertyAccess(createIdentifier("Object"), "create"), diff --git a/src/compiler/printer.ts b/src/compiler/printer.ts index b5cc8206688..de8ee5a5d1c 100644 --- a/src/compiler/printer.ts +++ b/src/compiler/printer.ts @@ -1154,7 +1154,7 @@ const _super = (function (geti, seti) { emitExpression(node.left); increaseIndentIf(indentBeforeOperator, isCommaOperator ? " " : undefined); - writeTokenNode(node.operatorToken); + writeTokenText(node.operatorToken.kind); increaseIndentIf(indentAfterOperator, " "); emitExpression(node.right); decreaseIndentIf(indentBeforeOperator, indentAfterOperator); @@ -1353,7 +1353,7 @@ const _super = (function (geti, seti) { } function emitReturnStatement(node: ReturnStatement) { - write("return"); + writeToken(SyntaxKind.ReturnKeyword, node.pos, node, shouldSkipSourceMapForToken); emitExpressionWithPrefix(" ", node.expression); write(";"); } @@ -2301,14 +2301,20 @@ const _super = (function (geti, seti) { } } - function writeToken(token: SyntaxKind, tokenStartPos: number) { + function writeToken(token: SyntaxKind, tokenStartPos: number): number; + function writeToken(token: SyntaxKind, tokenStartPos: number, contextNode: Node, shouldIgnoreSourceMapForTokenCallback: (contextNode: Node) => boolean): number; + function writeToken(token: SyntaxKind, tokenStartPos: number, contextNode?: Node, shouldIgnoreSourceMapForTokenCallback?: (contextNode: Node) => boolean) { tokenStartPos = skipTrivia(currentText, tokenStartPos); - emitPos(tokenStartPos); + emitPos(tokenStartPos, contextNode, shouldIgnoreSourceMapForTokenCallback); const tokenEndPos = writeTokenText(token, tokenStartPos); - emitPos(tokenEndPos); + emitPos(tokenEndPos, contextNode, shouldIgnoreSourceMapForTokenCallback); return tokenEndPos; } + function shouldSkipSourceMapForToken(contextNode: Node) { + return (getNodeEmitFlags(contextNode) & NodeEmitFlags.NoTokenSourceMaps) !== 0; + } + function writeTokenText(token: SyntaxKind, pos?: number) { const tokenString = tokenToString(token); write(tokenString); diff --git a/src/compiler/sourcemap.ts b/src/compiler/sourcemap.ts index d292e7a7060..832a89a4c56 100644 --- a/src/compiler/sourcemap.ts +++ b/src/compiler/sourcemap.ts @@ -5,6 +5,7 @@ namespace ts { export interface SourceMapWriter { getSourceMapData(): SourceMapData; setSourceFile(sourceFile: SourceFile): void; + emitPos(pos: number, contextNode: Node, shouldIgnorePosCallback: (node: Node) => boolean): void; emitPos(pos: number): void; emitStart(node: Node, shouldIgnoreNodeCallback?: (node: Node) => boolean, shouldIgnoreChildrenCallback?: (node: Node) => boolean): void; emitStart(range: TextRange): void; @@ -271,11 +272,15 @@ namespace ts { sourceMapData.sourceMapDecodedMappings.push(lastEncodedSourceMapSpan); } - function emitPos(pos: number) { + function emitPos(pos: number, contextNode?: Node, shouldIgnorePosCallback?: (node: Node) => boolean) { if (positionIsSynthesized(pos) || disableDepth > 0) { return; } + if (shouldIgnorePosCallback && shouldIgnorePosCallback(contextNode)) { + return; + } + const sourceLinePos = getLineAndCharacterOfPosition(currentSourceFile, pos); // Convert the location to be one-based. diff --git a/src/compiler/transformers/es6.ts b/src/compiler/transformers/es6.ts index 8a001450c69..160e577b03e 100644 --- a/src/compiler/transformers/es6.ts +++ b/src/compiler/transformers/es6.ts @@ -674,7 +674,7 @@ namespace ts { const statement = createReturn(outer); statement.pos = closingBraceLocation.pos; statements.push(statement); - setNodeEmitFlags(statement, NodeEmitFlags.NoComments); + setNodeEmitFlags(statement, NodeEmitFlags.NoComments | NodeEmitFlags.NoTokenSourceMaps); addRange(statements, endLexicalEnvironment()); const block = createBlock(createNodeArray(statements, /*location*/ node.members), /*location*/ undefined, /*multiLine*/ true); @@ -1001,7 +1001,8 @@ namespace ts { name, createArrayLiteral([]) ) - ]) + ]), + /*location*/ parameter ) ); @@ -1012,12 +1013,13 @@ namespace ts { createFor( createVariableDeclarationList([ createVariableDeclaration(temp, createLiteral(restIndex)) - ]), + ], /*location*/ parameter), createLessThan( temp, - createPropertyAccess(createIdentifier("arguments"), "length") + createPropertyAccess(createIdentifier("arguments"), "length"), + /*location*/ parameter ), - createPostfixIncrement(temp), + createPostfixIncrement(temp, /*location*/ parameter), createBlock([ startOnNewLine( createStatement( @@ -1027,7 +1029,8 @@ namespace ts { createSubtract(temp, createLiteral(restIndex)) ), createElementAccess(createIdentifier("arguments"), temp) - ) + ), + /*location*/ parameter ) ) ]) @@ -1158,13 +1161,18 @@ namespace ts { * @param receiver The receiver for the member. */ function transformAccessorsToExpression(receiver: LeftHandSideExpression, { firstAccessor, getAccessor, setAccessor }: AllAccessorDeclarations): Expression { + // To align with source maps in the old emitter, the receiver and property name + // arguments are both mapped contiguously to the accessor name. + const target = getSynthesizedClone(receiver); + target.pos = firstAccessor.name.pos; + + const propertyName = createExpressionForPropertyName(visitNode(firstAccessor.name, visitor, isPropertyName)); + propertyName.end = firstAccessor.name.end; + return setNodeEmitFlags( createObjectDefineProperty( - receiver, - createExpressionForPropertyName( - visitNode(firstAccessor.name, visitor, isPropertyName), - /*location*/ firstAccessor.name - ), + target, + propertyName, /*descriptor*/ { get: getAccessor && transformFunctionLikeToExpression(getAccessor, /*location*/ getAccessor, /*name*/ undefined), set: setAccessor && transformFunctionLikeToExpression(setAccessor, /*location*/ setAccessor, /*name*/ undefined), @@ -1174,9 +1182,10 @@ namespace ts { /*preferNewLine*/ true, /*location*/ undefined, /*descriptorLocations*/ { - get: getAccessor, - set: setAccessor - } + get: { location: getAccessor, emitFlags: NodeEmitFlags.NoSourceMap }, + set: { location: setAccessor, emitFlags: NodeEmitFlags.NoSourceMap } + }, + context ), NodeEmitFlags.NoComments ); diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts index c63c8fc8a76..288f5c85092 100644 --- a/src/compiler/transformers/ts.ts +++ b/src/compiler/transformers/ts.ts @@ -898,13 +898,17 @@ namespace ts { function transformParameterWithPropertyAssignment(node: ParameterDeclaration) { Debug.assert(isIdentifier(node.name)); - const name = getSynthesizedClone(node.name); return startOnNewLine( createStatement( createAssignment( - createPropertyAccess(createThis(), name), - name - ) + createPropertyAccess( + createThis(), + getSynthesizedClone(node.name), + /*location*/ node.name + ), + getUniqueClone(node.name) + ), + /*location*/ node ) ); } @@ -994,7 +998,7 @@ namespace ts { const propertyName = visitPropertyNameOfClassElement(property); const initializer = visitNode(property.initializer, visitor, isExpression); return createAssignment( - createMemberAccessForPropertyName(receiver, propertyName), + createMemberAccessForPropertyName(receiver, propertyName, /*location*/ propertyName), initializer, location ); @@ -1754,7 +1758,7 @@ namespace ts { function getExpressionForPropertyName(member: ClassElement | EnumMember, generateNameForComputedPropertyName: boolean): Expression { const name = member.name; if (isComputedPropertyName(name)) { - return generateNameForComputedPropertyName + return generateNameForComputedPropertyName ? getGeneratedNameForNode(name) : (name).expression; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 4c99417950d..0c13b134d1e 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -2875,10 +2875,11 @@ namespace ts { CapturesThis = 1 << 9, // The function captures a lexical `this` NoSourceMap = 1 << 10, // Do not emit a source map location for this node. NoNestedSourceMaps = 1 << 11, // Do not emit source map locations for children of this node. - NoComments = 1 << 12, // Do not emit comments for this node. - ExportName = 1 << 13, // Ensure an export prefix is added for an identifier that points to an exported declaration with a local name (see SymbolFlags.ExportHasLocal). - LocalName = 1 << 14, // Ensure an export prefix is not added for an identifier that points to an exported declaration. - Indented = 1 << 15, // Adds an explicit extra indentation level for class and function bodies when printing (used to match old emitter). + NoTokenSourceMaps = 1 << 12, // Do not emit source map locations for tokens of this node. + NoComments = 1 << 13, // Do not emit comments for this node. + ExportName = 1 << 14, // Ensure an export prefix is added for an identifier that points to an exported declaration with a local name (see SymbolFlags.ExportHasLocal). + LocalName = 1 << 15, // Ensure an export prefix is not added for an identifier that points to an exported declaration. + Indented = 1 << 16, // Adds an explicit extra indentation level for class and function bodies when printing (used to match old emitter). } /** Additional context provided to `visitEachChild` */ diff --git a/tests/baselines/reference/sourceMapValidationClass.js.map b/tests/baselines/reference/sourceMapValidationClass.js.map index 6176f13bc51..8baf4772a27 100644 --- a/tests/baselines/reference/sourceMapValidationClass.js.map +++ b/tests/baselines/reference/sourceMapValidationClass.js.map @@ -1,2 +1,2 @@ //// [sourceMapValidationClass.js.map] -{"version":3,"file":"sourceMapValidationClass.js","sourceRoot":"","sources":["sourceMapValidationClass.ts"],"names":[],"mappings":"AAAA;IACI,iBAAmB,QAAgB;QAAE,WAAc;aAAd,WAAc,CAAd,sBAAc,CAAd,IAAc;YAAd,0BAAc;;QAAhC,aAAQ,GAAR,QAAQ,CAAQ;QAM3B,OAAE,GAAW,EAAE,CAAC;IALxB,CAAC;IACD,uBAAK,GAAL;QACI,MAAM,CAAC,MAAM,GAAG,IAAI,CAAC,QAAQ,GAAG,OAAO,CAAC;IAC5C,CAAC;IAGO,oBAAE,GAAV;QACI,MAAM,CAAC,IAAI,CAAC,QAAQ,CAAC;IACzB,CAAC;IACD,sBAAI,8BAAS;aAAb;YACI,MAAM,CAAC,IAAI,CAAC,QAAQ,CAAC;QACzB,CAAC;aACD,UAAc,SAAiB;YAC3B,IAAI,CAAC,QAAQ,GAAG,SAAS,CAAC;QAC9B,CAAC;;;OAHA;IAIL,cAAC;AAAD,CAAC,AAjBD,IAiBC"} \ No newline at end of file +{"version":3,"file":"sourceMapValidationClass.js","sourceRoot":"","sources":["sourceMapValidationClass.ts"],"names":[],"mappings":"AAAA;IACI,iBAAmB,QAAgB;QAAE,WAAc;aAAd,UAAc,EAAd,qBAAc,EAAd,IAAc;YAAd,0BAAc;;QAAhC,aAAQ,GAAR,QAAQ,CAAQ;QAM3B,OAAE,GAAW,EAAE,CAAC;IALxB,CAAC;IACD,uBAAK,GAAL;QACI,MAAM,CAAC,MAAM,GAAG,IAAI,CAAC,QAAQ,GAAG,OAAO,CAAC;IAC5C,CAAC;IAGO,oBAAE,GAAV;QACI,MAAM,CAAC,IAAI,CAAC,QAAQ,CAAC;IACzB,CAAC;IACD,sBAAI,8BAAS;aAAb;YACI,MAAM,CAAC,IAAI,CAAC,QAAQ,CAAC;QACzB,CAAC;aACD,UAAc,SAAiB;YAC3B,IAAI,CAAC,QAAQ,GAAG,SAAS,CAAC;QAC9B,CAAC;;;OAHA;IAIL,cAAC;AAAD,CAAC,AAjBD,IAiBC"} \ No newline at end of file diff --git a/tests/baselines/reference/sourceMapValidationClass.sourcemap.txt b/tests/baselines/reference/sourceMapValidationClass.sourcemap.txt index 4173ae0ff0d..08fdfe2bfc6 100644 --- a/tests/baselines/reference/sourceMapValidationClass.sourcemap.txt +++ b/tests/baselines/reference/sourceMapValidationClass.sourcemap.txt @@ -37,21 +37,21 @@ sourceFile:sourceMapValidationClass.ts --- >>> for (var _i = 1; _i < arguments.length; _i++) { 1->^^^^^^^^^^^^^ -2 > ^^^^^^^^^^^ -3 > ^ -4 > ^^^^^^^^^^^^^^^^^^^^^^ -5 > ^ +2 > ^^^^^^^^^^ +3 > ^^ +4 > ^^^^^^^^^^^^^^^^^^^^^ +5 > ^^ 6 > ^^^^ 1-> 2 > ...b: string[] -3 > +3 > 4 > ...b: string[] -5 > +5 > 6 > ...b: string[] 1->Emitted(4, 14) Source(2, 42) + SourceIndex(0) -2 >Emitted(4, 25) Source(2, 56) + SourceIndex(0) +2 >Emitted(4, 24) Source(2, 56) + SourceIndex(0) 3 >Emitted(4, 26) Source(2, 42) + SourceIndex(0) -4 >Emitted(4, 48) Source(2, 56) + SourceIndex(0) +4 >Emitted(4, 47) Source(2, 56) + SourceIndex(0) 5 >Emitted(4, 49) Source(2, 42) + SourceIndex(0) 6 >Emitted(4, 53) Source(2, 56) + SourceIndex(0) ---