PR feedback

This commit is contained in:
Ron Buckton 2016-02-23 15:36:57 -08:00
parent de9866f450
commit 02b85f8174
5 changed files with 50 additions and 26 deletions

View file

@ -67,6 +67,11 @@ namespace ts {
// Source map data
let sourceMapData: SourceMapData;
// This keeps track of the number of times `disable` has been called without a
// corresponding call to `enable`. As long as this value is non-zero, mappings will not
// be recorded.
// This is primarily used to provide a better experience when debugging binding
// patterns and destructuring assignments for simple expressions.
let disableDepth: number;
return {
@ -160,12 +165,18 @@ namespace ts {
disableDepth = 0;
}
/**
* Re-enables the recording of mappings.
*/
function enable() {
if (disableDepth > 0) {
disableDepth--;
}
}
/**
* Disables the recording of mappings.
*/
function disable() {
disableDepth++;
}

View file

@ -18,22 +18,29 @@ namespace ts {
recordTempVariable: (node: Identifier) => void,
visitor?: (node: Node) => Node) {
let location: TextRange = node;
let value = node.right;
if (isEmptyObjectLiteralOrArrayLiteral(node.left)) {
return value;
return node.right;
}
let location: TextRange = node;
let value = node.right;
const expressions: Expression[] = [];
if (needsValue) {
// Temporary assignment needed to emit root should highlight whole binary expression
value = ensureIdentifier(node.right, /*reuseIdentifierExpressions*/ true, node, emitTempVariableAssignment);
// If the right-hand value of the destructuring assignment needs to be preserved (as
// is the case when the destructuring assignmen) is part of a larger expression),
// then we need to cache the right-hand value.
//
// The source map location for the assignment should point to the entire binary
// expression.
value = ensureIdentifier(node.right, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment);
}
else if (nodeIsSynthesized(node)) {
// Source map node for root.left = root.right is root
// but if root is synthetic, which could be in below case, use the target which is { a }
// for ({a} of {a: string}) {
// }
// Generally, the source map location for a destructuring assignment is the root
// expression.
//
// However, if the root expression is synthesized (as in the case
// of the initializer when transforming a ForOfStatement), then the source map
// location should point to the right-hand value of the expression.
location = node.right;
}
@ -255,21 +262,22 @@ namespace ts {
function emitArrayLiteralAssignment(target: ArrayLiteralExpression, value: Expression, location: TextRange) {
const elements = target.elements;
if (elements.length !== 1) {
const numElements = elements.length;
if (numElements !== 1) {
// For anything but a single element destructuring we need to generate a temporary
// to ensure value is evaluated exactly once.
// When doing so we want to hightlight the passed in source map node since thats the one needing this temp assignment
value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ true, location, emitTempVariableAssignment);
}
for (let i = 0; i < elements.length; i++) {
for (let i = 0; i < numElements; i++) {
const e = elements[i];
if (e.kind !== SyntaxKind.OmittedExpression) {
// Assignment for target = value.propName should highligh whole property, hence use e as source map node
if (e.kind !== SyntaxKind.SpreadElementExpression) {
emitDestructuringAssignment(e, createElementAccess(value, createLiteral(i)), e);
}
else if (i === elements.length - 1) {
else if (i === numElements - 1) {
emitDestructuringAssignment((<SpreadElementExpression>e).expression, createArraySlice(value, i), e);
}
}
@ -299,11 +307,11 @@ namespace ts {
// so in that case, we'll intentionally create that temporary.
value = ensureIdentifier(value, /*reuseIdentifierExpressions*/ numElements !== 0, target, emitTempVariableAssignment);
}
for (let i = 0; i < elements.length; i++) {
let element = elements[i];
for (let i = 0; i < numElements; i++) {
const element = elements[i];
if (name.kind === SyntaxKind.ObjectBindingPattern) {
// Rewrite element to a declaration with an initializer that fetches property
let propName = element.propertyName || <Identifier>element.name;
const propName = element.propertyName || <Identifier>element.name;
emitBindingElement(element, createDestructuringPropertyAccess(value, propName));
}
else if (element.kind !== SyntaxKind.OmittedExpression) {
@ -311,7 +319,7 @@ namespace ts {
// Rewrite element to a declaration that accesses array element at index i
emitBindingElement(element, createElementAccess(value, i));
}
else if (i === elements.length - 1) {
else if (i === numElements - 1) {
emitBindingElement(element, createArraySlice(value, i));
}
}
@ -332,16 +340,23 @@ namespace ts {
);
}
function createDestructuringPropertyAccess(object: Expression, propertyName: PropertyName): LeftHandSideExpression {
/**
* Creates either a PropertyAccessExpression or an ElementAccessExpression for the
* right-hand side of a transformed destructuring assignment.
*
* @param expression The right-hand expression that is the source of the property.
* @param propertyName The destructuring property name.
*/
function createDestructuringPropertyAccess(expression: Expression, propertyName: PropertyName): LeftHandSideExpression {
if (isComputedPropertyName(propertyName)) {
return createElementAccess(
object,
ensureIdentifier(propertyName.expression, /*reuseIdentifierExpressions*/ false, propertyName, emitTempVariableAssignment)
expression,
ensureIdentifier(propertyName.expression, /*reuseIdentifierExpressions*/ false, /*location*/ propertyName, emitTempVariableAssignment)
);
}
else if (isIdentifier(propertyName)) {
return createPropertyAccess(
object,
expression,
propertyName.text
);
}
@ -349,7 +364,7 @@ namespace ts {
// We create a synthetic copy of the identifier in order to avoid the rewriting that might
// otherwise occur when the identifier is emitted.
return createElementAccess(
object,
expression,
cloneNode(propertyName)
);
}

View file

@ -5,8 +5,6 @@
namespace ts {
// TODO(rbuckton): ES7->ES6 transformer
export function transformES7(context: TransformationContext) {
const { hoistVariableDeclaration } = context;
return transformSourceFile;
function transformSourceFile(node: SourceFile) {

View file

@ -202,7 +202,7 @@ namespace ts {
case SyntaxKind.Constructor:
// TypeScript constructors are elided. The constructor of a class will be
// reordered to the start of the member list in `transformClassDeclaration`.
// transformed as part of `transformClassDeclaration`.
return undefined;
case SyntaxKind.ClassDeclaration:
@ -1964,7 +1964,7 @@ namespace ts {
)
);
const block = createBlock(statements);
const block = createBlock(statements, /*location*/ node.body);
// Minor optimization, emit `_super` helper to capture `super` access in an arrow.
// This step isn't needed if we eventually transform this to ES5.

View file

@ -2792,7 +2792,7 @@ namespace ts {
else if (kind === SyntaxKind.ConditionalExpression) {
return isSimpleExpressionWorker((<ConditionalExpression>node).condition, depth + 1)
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenTrue, depth + 1)
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenFalse, depth + 1)
&& isSimpleExpressionWorker((<ConditionalExpression>node).whenFalse, depth + 1);
}
else if (kind === SyntaxKind.VoidExpression
|| kind === SyntaxKind.TypeOfExpression