From 7e002aeb7b08b2ee9ca837de79dce5f8e2ba969e Mon Sep 17 00:00:00 2001 From: Andy Date: Fri, 22 Sep 2017 08:49:56 -0700 Subject: [PATCH] Avoid calling `indexOf` when checking array element types (#18619) * Avoid calling `indexOf` when checking array element types * Add 'indexOfNode' and use it in cases which may handle long lists. (#18635) * Fix bug where contextual type was not reused if undefined --- src/compiler/checker.ts | 39 +++++++++++++++++++++---------------- src/compiler/utilities.ts | 12 ++++++++++++ src/services/textChanges.ts | 4 ++-- src/services/utilities.ts | 2 +- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index e00465caa14..b135b52f6a9 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -13198,16 +13198,11 @@ namespace ts { // the type of the property with the numeric name N in T, if one exists. Otherwise, if T has a numeric index signature, // it is the type of the numeric index signature in T. Otherwise, in ES6 and higher, the contextual type is the iterated // type of T. - function getContextualTypeForElementExpression(node: Expression): Type { - const arrayLiteral = node.parent; - const type = getApparentTypeOfContextualType(arrayLiteral); - if (type) { - const index = indexOf(arrayLiteral.elements, node); - return getTypeOfPropertyOfContextualType(type, "" + index as __String) - || getIndexTypeOfContextualType(type, IndexKind.Number) - || getIteratedTypeOrElementType(type, /*errorNode*/ undefined, /*allowStringInput*/ false, /*allowAsyncIterables*/ false, /*checkAssignability*/ false); - } - return undefined; + function getContextualTypeForElementExpression(arrayContextualType: Type | undefined, index: number): Type | undefined { + return arrayContextualType && ( + getTypeOfPropertyOfContextualType(arrayContextualType, "" + index as __String) + || getIndexTypeOfContextualType(arrayContextualType, IndexKind.Number) + || getIteratedTypeOrElementType(arrayContextualType, /*errorNode*/ undefined, /*allowStringInput*/ false, /*allowAsyncIterables*/ false, /*checkAssignability*/ false)); } // In a contextually typed conditional expression, the true/false expressions are contextually typed by the same type. @@ -13321,8 +13316,11 @@ namespace ts { return getContextualTypeForObjectLiteralElement(parent); case SyntaxKind.SpreadAssignment: return getApparentTypeOfContextualType(parent.parent as ObjectLiteralExpression); - case SyntaxKind.ArrayLiteralExpression: - return getContextualTypeForElementExpression(node); + case SyntaxKind.ArrayLiteralExpression: { + const arrayLiteral = parent; + const type = getApparentTypeOfContextualType(arrayLiteral); + return getContextualTypeForElementExpression(type, indexOfNode(arrayLiteral.elements, node)); + } case SyntaxKind.ConditionalExpression: return getContextualTypeForConditionalOperand(node); case SyntaxKind.TemplateSpan: @@ -13451,12 +13449,14 @@ namespace ts { (node.kind === SyntaxKind.BinaryExpression && (node).operatorToken.kind === SyntaxKind.EqualsToken); } - function checkArrayLiteral(node: ArrayLiteralExpression, checkMode?: CheckMode): Type { + function checkArrayLiteral(node: ArrayLiteralExpression, checkMode: CheckMode | undefined): Type { const elements = node.elements; let hasSpreadElement = false; const elementTypes: Type[] = []; const inDestructuringPattern = isAssignmentTarget(node); - for (const e of elements) { + const contextualType = getApparentTypeOfContextualType(node); + for (let index = 0; index < elements.length; index++) { + const e = elements[index]; if (inDestructuringPattern && e.kind === SyntaxKind.SpreadElement) { // Given the following situation: // var c: {}; @@ -13478,7 +13478,8 @@ namespace ts { } } else { - const type = checkExpressionForMutableLocation(e, checkMode); + const elementContextualType = getContextualTypeForElementExpression(contextualType, index); + const type = checkExpressionForMutableLocation(e, checkMode, elementContextualType); elementTypes.push(type); } hasSpreadElement = hasSpreadElement || e.kind === SyntaxKind.SpreadElement; @@ -18023,9 +18024,13 @@ namespace ts { return false; } - function checkExpressionForMutableLocation(node: Expression, checkMode?: CheckMode): Type { + function checkExpressionForMutableLocation(node: Expression, checkMode: CheckMode, contextualType?: Type): Type { + if (arguments.length === 2) { + contextualType = getContextualType(node); + } const type = checkExpression(node, checkMode); - return isTypeAssertion(node) || isLiteralContextualType(getContextualType(node)) ? type : getWidenedLiteralType(type); + const shouldWiden = isTypeAssertion(node) || isLiteralContextualType(contextualType); + return shouldWiden ? type : getWidenedLiteralType(type); } function checkPropertyAssignment(node: PropertyAssignment, checkMode?: CheckMode): Type { diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index f3b40c702b6..ede4c5a45e9 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -321,6 +321,18 @@ namespace ts { return getSourceTextOfNodeFromSourceFile(getSourceFileOfNode(node), node, includeTrivia); } + /** + * Note: it is expected that the `nodeArray` and the `node` are within the same file. + * For example, searching for a `SourceFile` in a `SourceFile[]` wouldn't work. + */ + export function indexOfNode(nodeArray: ReadonlyArray, node: Node) { + return binarySearch(nodeArray, node, compareNodePos); + } + + function compareNodePos({ pos: aPos }: Node, { pos: bPos}: Node) { + return aPos < bPos ? Comparison.LessThan : bPos < aPos ? Comparison.GreaterThan : Comparison.EqualTo; + } + /** * Gets flags that control emit behavior of a node. */ diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 5a01752f38e..9e4c2ed3a60 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -226,7 +226,7 @@ namespace ts.textChanges { Debug.fail("node is not a list element"); return this; } - const index = containingList.indexOf(node); + const index = indexOfNode(containingList, node); if (index < 0) { return this; } @@ -358,7 +358,7 @@ namespace ts.textChanges { Debug.fail("node is not a list element"); return this; } - const index = containingList.indexOf(after); + const index = indexOfNode(containingList, after); if (index < 0) { return this; } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index c3a1d5d571d..3dd3ddd456e 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -597,7 +597,7 @@ namespace ts { } const children = list.getChildren(); - const listItemIndex = indexOf(children, node); + const listItemIndex = indexOfNode(children, node); return { listItemIndex,