From f8ed021f1e97da3974aaeba27d89800e68f8708b Mon Sep 17 00:00:00 2001 From: Ron Buckton Date: Fri, 12 Feb 2016 17:46:38 -0800 Subject: [PATCH] PR feedback and cleanup --- src/compiler/core.ts | 13 ++- src/compiler/factory.ts | 187 +++++++++++++++++++++++++----------- src/compiler/transformer.ts | 15 ++- 3 files changed, 150 insertions(+), 65 deletions(-) diff --git a/src/compiler/core.ts b/src/compiler/core.ts index 5f68777a63..55903cb34d 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -169,8 +169,19 @@ namespace ts { export function concatenate(array1: T[], array2: T[]): T[] { if (!array2 || !array2.length) return array1; if (!array1 || !array1.length) return array2; + return [...array1, ...array2]; + } - return array1.concat(array2); + export function append(array: T[], value: T): T[] { + if (value === undefined) return array; + if (!array || !array.length) return [value]; + return [...array, value]; + } + + export function prepend(array: T[], value: T): T[] { + if (value === undefined) return array; + if (!array || !array.length) return [value]; + return [value, ...array]; } export function deduplicate(array: T[]): T[] { diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts index 992c46fdcf..8b81d7d0d2 100644 --- a/src/compiler/factory.ts +++ b/src/compiler/factory.ts @@ -233,14 +233,14 @@ namespace ts { const node = createNode(SyntaxKind.PropertyAccessExpression); node.expression = parenthesizeForAccess(expression); node.dotToken = createSynthesizedNode(SyntaxKind.DotToken); - node.name = coerceIdentifier(name); + node.name = typeof name === "string" ? createIdentifier(name) : name; return node; } - export function createElementAccess(expression: Expression, index: string | number | Expression) { + export function createElementAccess(expression: Expression, index: number | Expression) { const node = createNode(SyntaxKind.ElementAccessExpression); node.expression = parenthesizeForAccess(expression); - node.argumentExpression = coerceExpression(index); + node.argumentExpression = typeof index === "number" ? createLiteral(index) : index; return node; } @@ -256,9 +256,9 @@ namespace ts { export function createBinary(left: Expression, operator: SyntaxKind, right: Expression, location?: TextRange) { const node = createNode(SyntaxKind.BinaryExpression, location); - node.left = parenthesizeForBinary(left, operator, BinaryOperand.Left); + node.left = parenthesizeBinaryOperand(operator, left, /*isLeftSideOfBinary*/ true); node.operatorToken = createSynthesizedNode(operator); - node.right = parenthesizeForBinary(right, operator, BinaryOperand.Right); + node.right = parenthesizeBinaryOperand(operator, right, /*isLeftSideOfBinary*/ false); return node; } @@ -282,7 +282,11 @@ namespace ts { } export function createArraySlice(array: Expression, start?: number | Expression) { - const argumentsList: Expression[] = start !== undefined ? [coerceExpression(start)] : []; + const argumentsList: Expression[] = []; + if (start !== undefined) { + argumentsList.push(typeof start === "number" ? createLiteral(start) : start); + } + return createCall(createPropertyAccess(array, "slice"), argumentsList); } @@ -296,94 +300,165 @@ namespace ts { return reduceLeft(expressions, createComma); } - function coerceIdentifier(value: string | Identifier) { - if (typeof value === "string") { - return createIdentifier(value); - } - else { - return value; - } - } - - function coerceExpression(value: string | number | boolean | Expression): Expression { - if (typeof value === "string" || typeof value === "number" || typeof value === "boolean") { - return createLiteral(value); - } - else { - return value; - } - } - - const enum BinaryOperand { - Left, - Right - } - - function parenthesizeForBinary(operand: Expression, operator: SyntaxKind, side: BinaryOperand) { + /** + * Wraps the operand to a BinaryExpression in parentheses if they are needed to preserve the intended + * order of operations. + * + * @param binaryOperator The operator for the BinaryExpression. + * @param operand The operand for the BinaryExpression. + * @param isLeftSideOfBinary A value indicating whether the operand is the left side of the + * BinaryExpression. + */ + function parenthesizeBinaryOperand(binaryOperator: SyntaxKind, operand: Expression, isLeftSideOfBinary: boolean) { // When diagnosing whether the expression needs parentheses, the decision should be based // on the innermost expression in a chain of nested type assertions. - while (operand.kind === SyntaxKind.TypeAssertionExpression || operand.kind === SyntaxKind.AsExpression) { - operand = (operand).expression; - } + operand = skipAssertions(operand); // If the resulting expression is already parenthesized, we do not need to do any further processing. if (operand.kind === SyntaxKind.ParenthesizedExpression) { return operand; } - return needsParenthesesForBinary(operand, operator, side) + return binaryOperandNeedsParentheses(binaryOperator, operand, isLeftSideOfBinary) ? parenthesizeExpression(operand) : operand; } - function needsParenthesesForBinary(operand: Expression, operator: SyntaxKind, side: BinaryOperand) { + /** + * Determines whether the operand to a BinaryExpression needs to be parenthesized. + * + * @param binaryOperator The operator for the BinaryExpression. + * @param operand The operand for the BinaryExpression. + * @param isLeftSideOfBinary A value indicating whether the operand is the left side of the + * BinaryExpression. + */ + function binaryOperandNeedsParentheses(binaryOperator: SyntaxKind, operand: Expression, isLeftSideOfBinary: boolean) { + // If the operand has lower precedence, then it needs to be parenthesized to preserve the + // intent of the expression. For example, if the operand is `a + b` and the operator is + // `*`, then we need to parenthesize the operand to preserve the intended order of + // operations: `(a + b) * x`. + // + // If the operand has higher precedence, then it does not need to be parenthesized. For + // example, if the operand is `a * b` and the operator is `+`, then we do not need to + // parenthesize to preserve the intended order of operations: `a * b + x`. + // + // If the operand has the same precedence, then we need to check the associativity of + // the operator based on whether this is the left or right operand of the expression. + // + // For example, if `a / d` is on the right of operator `*`, we need to parenthesize + // to preserve the intended order of operations: `x * (a / d)` + // + // If `a ** d` is on the left of operator `**`, we need to parenthesize to preserve + // the intended order of operations: `(a ** b) ** c` + const binaryOperatorPrecedence = getOperatorPrecedence(SyntaxKind.BinaryExpression, binaryOperator); const operandPrecedence = getExpressionPrecedence(operand); - const operatorPrecedence = getOperatorPrecedence(SyntaxKind.BinaryExpression, operator); - switch (compareValues(operandPrecedence, operatorPrecedence)) { + switch (compareValues(operandPrecedence, binaryOperatorPrecedence)) { case Comparison.LessThan: return true; - case Comparison.EqualTo: - return isRightAssociativeOperandOnLeftHandSide(operand, side) - || isModuloOperandOnRightHandSide(operand, operator, side); + case Comparison.GreaterThan: return false; + + case Comparison.EqualTo: + if (isLeftSideOfBinary) { + // No need to parenthesize the left operand when the binary operator is + // left associative: + // (a*b)/x -> a*b/x + // (a**b)/x -> a**b/x + + // Parentheses are needed for the left operand when the binary operator is + // right associative: + // (a/b)**x -> (a/b)**x + // (a**b)**x -> (a**b)**x + const binaryOperatorAssociativity = getOperatorAssociativity(SyntaxKind.BinaryExpression, binaryOperator); + return binaryOperatorAssociativity === Associativity.Right; + } + else { + // No need to parenthesize the right operand when the binary operator and + // operand are the same and one of the following: + // x*(a*b) => x*a*b + // x|(a|b) => x|a|b + // x&(a&b) => x&a&b + // x^(a^b) => x^a^b + if (isBinaryExpression(operand) + && operand.operatorToken.kind === binaryOperator + && isMathAssociativeOperator(binaryOperator)) { + return false; + } + + // No need to parenthesize the right operand when the operand is right + // associative: + // x/(a**b) -> x/a**b + // x**(a**b) -> x**a**b + + // Parentheses are needed for the right operand when the operand is left + // associative: + // x/(a*b) -> x/(a*b) + // x**(a/b) -> x**(a/b) + const operandAssociativity = getExpressionAssociativity(operand); + return operandAssociativity === Associativity.Left; + } } } - function isRightAssociativeOperandOnLeftHandSide(operand: Expression, side: BinaryOperand) { - return side === BinaryOperand.Left - && getExpressionAssociativity(operand) === Associativity.Right; - } - - function isModuloOperandOnRightHandSide(operand: Expression, operator: SyntaxKind, side: BinaryOperand) { - return side === BinaryOperand.Right - && operator !== SyntaxKind.PercentToken - && operand.kind === SyntaxKind.BinaryExpression - && (operand).operatorToken.kind === SyntaxKind.PercentToken; + /** + * Determines whether a binary operator is mathematically associative. + * + * @param binaryOperator The binary operator. + */ + function isMathAssociativeOperator(binaryOperator: SyntaxKind) { + // The following operators are associative in JavaScript: + // (a*b)*c -> a*(b*c) -> a*b*c + // (a|b)|c -> a|(b|c) -> a|b|c + // (a&b)&c -> a&(b&c) -> a&b&c + // (a^b)^c -> a^(b^c) -> a^b^c + // + // While addition is associative in mathematics, JavaScript's `+` is not + // guaranteed to be associative as it is overloaded with string concatenation. + return binaryOperator === SyntaxKind.AsteriskToken + || binaryOperator === SyntaxKind.BarToken + || binaryOperator === SyntaxKind.AmpersandToken + || binaryOperator === SyntaxKind.CaretToken; } + /** + * Wraps an expression in parentheses if it is needed in order to use the expression for + * property or element access. + * + * @param expr The expression node. + */ function parenthesizeForAccess(expr: Expression): LeftHandSideExpression { // When diagnosing whether the expression needs parentheses, the decision should be based // on the innermost expression in a chain of nested type assertions. - while (expr.kind === SyntaxKind.TypeAssertionExpression || expr.kind === SyntaxKind.AsExpression) { - expr = (expr).expression; - } + expr = skipAssertions(expr); // isLeftHandSideExpression is almost the correct criterion for when it is not necessary // to parenthesize the expression before a dot. The known exceptions are: // // NewExpression: // new C.x -> not the same as (new C).x - // NumberLiteral + // NumericLiteral // 1.x -> not the same as (1).x // if (isLeftHandSideExpression(expr) && expr.kind !== SyntaxKind.NewExpression && expr.kind !== SyntaxKind.NumericLiteral) { - - return expr; + return expr; } return parenthesizeExpression(expr); } + + /** + * Skips past any TypeAssertionExpression or AsExpression nodes to their inner expression. + * + * @param node The expression node. + */ + function skipAssertions(node: Expression) { + while (node.kind === SyntaxKind.TypeAssertionExpression || node.kind === SyntaxKind.AsExpression) { + node = (node).expression; + } + + return node; + } } \ No newline at end of file diff --git a/src/compiler/transformer.ts b/src/compiler/transformer.ts index d6d3dedd36..f29113e361 100644 --- a/src/compiler/transformer.ts +++ b/src/compiler/transformer.ts @@ -162,6 +162,8 @@ namespace ts { return generateNameForImportOrExportDeclaration(node); case SyntaxKind.FunctionDeclaration: case SyntaxKind.ClassDeclaration: + Debug.assert((node.flags & NodeFlags.Default) !== 0, "Can only generate a name for a default export."); + return generateNameForExportDefault(); case SyntaxKind.ExportAssignment: return generateNameForExportDefault(); case SyntaxKind.ClassExpression: @@ -227,7 +229,6 @@ namespace ts { lexicalEnvironmentVariableDeclarationsStack[lexicalEnvironmentStackOffset] = hoistedVariableDeclarations; lexicalEnvironmentFunctionDeclarationsStack[lexicalEnvironmentStackOffset] = hoistedFunctionDeclarations; lexicalEnvironmentStackOffset++; - hoistedVariableDeclarations = undefined; hoistedFunctionDeclarations = undefined; } @@ -239,15 +240,12 @@ namespace ts { function endLexicalEnvironment(): Statement[] { let statements: Statement[]; if (hoistedVariableDeclarations || hoistedFunctionDeclarations) { - statements = []; if (hoistedFunctionDeclarations) { - for (const declaration of hoistedFunctionDeclarations) { - statements.push(declaration); - } + statements = [...hoistedFunctionDeclarations]; } if (hoistedVariableDeclarations) { - statements.push( + statements = append(statements, createVariableStatement( createVariableDeclarationList(hoistedVariableDeclarations) ) @@ -325,8 +323,9 @@ namespace ts { * Makes an array from an ArrayLike. */ function arrayOf(arrayLike: ArrayLike) { - const array: T[] = []; - for (let i = 0; i < arrayLike.length; i++) { + const length = arrayLike.length; + const array: T[] = new Array(length); + for (let i = 0; i < length; i++) { array[i] = arrayLike[i]; } return array;