createNodeArray mutates its input if not already a NodeArray

Andy Hanson · Andy Hanson · commit 20f1b2c3a3da · 2017-12-05T08:29:42.000-08:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -2798,7 +2798,7 @@ namespace ts {
                             entityName = nameIdentifier;
                         }
 
-                        let typeArgumentNodes: ReadonlyArray<TypeNode> | undefined;
+                        let typeArgumentNodes: TypeNode[] | undefined;
                         if (typeArguments.length > 0) {
                             const typeParameterCount = (type.target.typeParameters || emptyArray).length;
                             typeArgumentNodes = mapToTypeNodes(typeArguments.slice(i, typeParameterCount), context);
@@ -3026,7 +3026,7 @@ namespace ts {
                 function createEntityNameFromSymbolChain(chain: Symbol[], index: number): EntityName {
                     Debug.assert(chain && 0 <= index && index < chain.length);
                     const symbol = chain[index];
-                    let typeParameterNodes: ReadonlyArray<TypeNode> | undefined;
+                    let typeParameterNodes: TypeNode[] | undefined;
                     if (context.flags & NodeBuilderFlags.WriteTypeParametersInQualifiedName && index > 0) {
                         const parentSymbol = chain[index - 1];
                         let typeParameters: TypeParameter[];
diff --git a/src/compiler/factory.ts b/src/compiler/factory.ts
diff --git a/src/compiler/transformers/es2017.ts b/src/compiler/transformers/es2017.ts
@@ -259,7 +259,7 @@ namespace ts {
                 const declarations = endLexicalEnvironment();
                 if (some(declarations)) {
                     const block = convertToFunctionBody(expression);
-                    return updateBlock(block, setTextRange(createNodeArray(concatenate(block.statements, declarations)), block.statements));
+                    return updateBlock(block, concatToNodeArray(block.statements, declarations));
                 }
 
                 return expression;
@@ -274,7 +274,7 @@ namespace ts {
                 startLexicalEnvironment();
                 const visited = convertToFunctionBody(visitNode(body, visitor, isConciseBody));
                 const declarations = endLexicalEnvironment();
-                return updateBlock(visited, setTextRange(createNodeArray(concatenate(visited.statements, declarations)), visited.statements));
+                return updateBlock(visited, concatToNodeArray(visited.statements, declarations));
             }
         }
 
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -2,7 +2,8 @@
 
 /* @internal */
 namespace ts {
-    export const emptyArray: never[] = [] as never[];
+    // TODO: GH#16312 Should be a ReadonlyArray<never>
+    export const emptyArray: never[] = Object.freeze([]) as never[];
     export const emptyMap: ReadonlyMap<never> = createMap<never>();
 
     export const externalHelpersModuleNameText = "tslib";
@@ -2841,7 +2842,7 @@ namespace ts {
      * Gets the effective type parameters. If the node was parsed in a
      * JavaScript file, gets the type parameters from the `@template` tag from JSDoc.
      */
-    export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters, checkJSDoc?: boolean): ReadonlyArray<TypeParameterDeclaration> {
+    export function getEffectiveTypeParameterDeclarations(node: DeclarationWithTypeParameters, checkJSDoc?: boolean): NodeArray<TypeParameterDeclaration> {
         if (node.typeParameters) {
             return node.typeParameters;
         }
diff --git a/src/compiler/visitor.ts b/src/compiler/visitor.ts
@@ -144,10 +144,14 @@ namespace ts {
         context.startLexicalEnvironment();
         statements = visitNodes(statements, visitor, isStatement, start);
         if (ensureUseStrict && !startsWithUseStrict(statements)) {
-            statements = setTextRange(createNodeArray([createStatement(createLiteral("use strict")), ...statements]), statements);
+            statements = setTextRange(createNodeArray([createStatement(createLiteral("use strict")), ...statements]), statements); //don't think this call to setTextRange is necessary
         }
-        const declarations = context.endLexicalEnvironment();
-        return setTextRange(createNodeArray(concatenate(statements, declarations)), statements);
+        return concatToNodeArray(statements, context.endLexicalEnvironment());
+    }
+
+    /* @internal */
+    export function concatToNodeArray<T extends Node>(a: NodeArray<T>, b: ReadonlyArray<T> | undefined): NodeArray<T> {
+        return b === undefined ? a : setTextRange(createNodeArray(a.concat(b)), a);
     }
 
     /**
@@ -1448,7 +1452,7 @@ namespace ts {
         }
 
         return isNodeArray(statements)
-            ? setTextRange(createNodeArray(concatenate(statements, declarations)), statements)
+            ? setTextRange(createNodeArray(statements.concat(declarations)), statements)
             : addRange(statements, declarations);
     }
 
diff --git a/src/harness/unittests/printer.ts b/src/harness/unittests/printer.ts
@@ -143,7 +143,7 @@ namespace ts {
                     /*decorators*/ undefined,
                     /*modifiers*/ [createToken(SyntaxKind.DeclareKeyword)],
                     createIdentifier("global"),
-                    createModuleBlock(emptyArray),
+                    createModuleBlock([]),
                     NodeFlags.GlobalAugmentation),
                 createSourceFile("source.ts", "", ScriptTarget.ES2015)
             ));
@@ -154,7 +154,7 @@ namespace ts {
                     /*decorators*/ undefined,
                     /*modifiers*/ undefined,
                     createIdentifier("global"),
-                    createModuleBlock(emptyArray),
+                    createModuleBlock([]),
                     NodeFlags.GlobalAugmentation),
                 createSourceFile("source.ts", "", ScriptTarget.ES2015)
             ));
diff --git a/src/harness/unittests/textChanges.ts b/src/harness/unittests/textChanges.ts
@@ -98,7 +98,7 @@ namespace M
                         /*asteriskToken*/ undefined,
                         /*name*/ "bar",
                         /*typeParameters*/ undefined,
-                        /*parameters*/ emptyArray,
+                        /*parameters*/ [],
                         /*type*/ createKeywordTypeNode(SyntaxKind.AnyKeyword),
                         /*body */ createBlock(statements)
                 );
@@ -110,7 +110,7 @@ namespace M
                     createCall(
                         /*expression*/ newFunction.name,
                         /*typeArguments*/ undefined,
-                        /*argumentsArray*/ emptyArray
+                        /*argumentsArray*/ [],
                     ));
                 changeTracker.replaceNodeRange(sourceFile, statements[0], lastOrUndefined(statements), newStatement, { suffix: newLineCharacter });
             });
@@ -346,8 +346,8 @@ namespace M {
         function createTestSuperCall() {
             const superCall = createCall(
                 createSuper(),
-                    /*typeArguments*/ undefined,
-                    /*argumentsArray*/ emptyArray
+                /*typeArguments*/ undefined,
+                /*argumentsArray*/ [],
             );
             return createStatement(superCall);
         }
diff --git a/src/services/codefixes/fixConstructorForDerivedNeedSuperCall.ts b/src/services/codefixes/fixConstructorForDerivedNeedSuperCall.ts
@@ -11,7 +11,7 @@ namespace ts.codefix {
             }
 
             const changeTracker = textChanges.ChangeTracker.fromContext(context);
-            const superCall = createStatement(createCall(createSuper(), /*typeArguments*/ undefined, /*argumentsArray*/ emptyArray));
+            const superCall = createStatement(createCall(createSuper(), /*typeArguments*/ undefined, /*argumentsArray*/ []));
             changeTracker.insertNodeAfter(sourceFile, getOpenBrace(<ConstructorDeclaration>token.parent, sourceFile), superCall, { suffix: context.newLineCharacter });
 
             return [{
diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts
@@ -185,7 +185,7 @@ namespace ts.codefix {
         return parameters;
     }
 
-    function createMethodImplementingSignatures(signatures: ReadonlyArray<Signature>, name: PropertyName, optional: boolean, modifiers: ReadonlyArray<Modifier> | undefined): MethodDeclaration {
+    function createMethodImplementingSignatures(signatures: ReadonlyArray<Signature>, name: PropertyName, optional: boolean, modifiers: Nodes<Modifier> | undefined): MethodDeclaration {
         /** This is *a* signature with the maximal number of arguments,
          * such that if there is a "maximal" signature without rest arguments,
          * this is one of them.
@@ -230,11 +230,11 @@ namespace ts.codefix {
     }
 
     export function createStubbedMethod(
-        modifiers: ReadonlyArray<Modifier>,
+        modifiers: Nodes<Modifier>,
         name: PropertyName,
         optional: boolean,
-        typeParameters: ReadonlyArray<TypeParameterDeclaration> | undefined,
-        parameters: ReadonlyArray<ParameterDeclaration>,
+        typeParameters: Nodes<TypeParameterDeclaration> | undefined,
+        parameters: Nodes<ParameterDeclaration>,
         returnType: TypeNode | undefined) {
         return createMethod(
             /*decorators*/ undefined,
diff --git a/src/services/refactors/annotateWithTypeFromJSDoc.ts b/src/services/refactors/annotateWithTypeFromJSDoc.ts
@@ -157,7 +157,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
         switch (node.kind) {
             case SyntaxKind.JSDocAllType:
             case SyntaxKind.JSDocUnknownType:
-                return createTypeReferenceNode("any", emptyArray);
+                return createTypeReferenceNode("any", []);
             case SyntaxKind.JSDocOptionalType:
                 return transformJSDocOptionalType(node as JSDocOptionalType);
             case SyntaxKind.JSDocNonNullableType:
@@ -180,11 +180,11 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
     }
 
     function transformJSDocOptionalType(node: JSDocOptionalType) {
-        return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("undefined", emptyArray)]);
+        return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("undefined", [])]);
     }
 
     function transformJSDocNullableType(node: JSDocNullableType) {
-        return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("null", emptyArray)]);
+        return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("null", [])]);
     }
 
     function transformJSDocVariadicType(node: JSDocVariadicType) {
@@ -193,7 +193,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
 
     function transformJSDocFunctionType(node: JSDocFunctionType) {
         const parameters = node.parameters && node.parameters.map(transformJSDocType);
-        return createFunctionTypeNode(emptyArray, parameters as ParameterDeclaration[], node.type);
+        return createFunctionTypeNode([], parameters as ParameterDeclaration[], node.type);
     }
 
     function transformJSDocParameter(node: ParameterDeclaration) {
@@ -227,7 +227,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {
             }
             name = createIdentifier(text);
             if ((text === "Array" || text === "Promise") && !node.typeArguments) {
-                args = createNodeArray([createTypeReferenceNode("any", emptyArray)]);
+                args = createNodeArray([createTypeReferenceNode("any", [])]);
             }
             else {
                 args = visitNodes(node.typeArguments, transformJSDocType);
diff --git a/src/services/refactors/convertFunctionToEs6Class.ts b/src/services/refactors/convertFunctionToEs6Class.ts
@@ -263,7 +263,7 @@ namespace ts.refactor.convertFunctionToES6Class {
         }
 
         function getModifierKindFromSource(source: Node, kind: SyntaxKind) {
-            return filter(source.modifiers, modifier => modifier.kind === kind);
+            return source.modifiers && source.modifiers.filter(modifier => modifier.kind === kind);
         }
     }
 
diff --git a/src/services/refactors/extractSymbol.ts b/src/services/refactors/extractSymbol.ts
@@ -751,13 +751,13 @@ namespace ts.refactor.extractSymbol {
         const typeParametersAndDeclarations = arrayFrom(typeParameterUsages.values()).map(type => ({ type, declaration: getFirstDeclaration(type) }));
         const sortedTypeParametersAndDeclarations = typeParametersAndDeclarations.sort(compareTypesByDeclarationOrder);
 
-        const typeParameters: ReadonlyArray<TypeParameterDeclaration> | undefined = sortedTypeParametersAndDeclarations.length === 0
+        const typeParameters = sortedTypeParametersAndDeclarations.length === 0
             ? undefined
             : sortedTypeParametersAndDeclarations.map(t => t.declaration as TypeParameterDeclaration);
 
         // Strictly speaking, we should check whether each name actually binds to the appropriate type
         // parameter.  In cases of shadowing, they may not.
-        const callTypeArguments: ReadonlyArray<TypeNode> | undefined = typeParameters !== undefined
+        const callTypeArguments = typeParameters !== undefined
             ? typeParameters.map(decl => createTypeReferenceNode(decl.name, /*typeArguments*/ undefined))
             : undefined;
 
diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts
diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts

Original file line number	Diff line number	Diff line change
`@@ -259,7 +259,7 @@ namespace ts {`
`259`	`259`	`const declarations = endLexicalEnvironment();`
`260`	`260`	`if (some(declarations)) {`
`261`	`261`	`const block = convertToFunctionBody(expression);`
`262`		`- return updateBlock(block, setTextRange(createNodeArray(concatenate(block.statements, declarations)), block.statements));`
	`262`	`+ return updateBlock(block, concatToNodeArray(block.statements, declarations));`
`263`	`263`	`}`
`264`	`264`
`265`	`265`	`return expression;`
`@@ -274,7 +274,7 @@ namespace ts {`
`274`	`274`	`startLexicalEnvironment();`
`275`	`275`	`const visited = convertToFunctionBody(visitNode(body, visitor, isConciseBody));`
`276`	`276`	`const declarations = endLexicalEnvironment();`
`277`		`- return updateBlock(visited, setTextRange(createNodeArray(concatenate(visited.statements, declarations)), visited.statements));`
	`277`	`+ return updateBlock(visited, concatToNodeArray(visited.statements, declarations));`
`278`	`278`	`}`
`279`	`279`	`}`
`280`	`280`
Original file line number	Diff line number	Diff line change
`@@ -144,10 +144,14 @@ namespace ts {`
`144`	`144`	`context.startLexicalEnvironment();`
`145`	`145`	`statements = visitNodes(statements, visitor, isStatement, start);`
`146`	`146`	`if (ensureUseStrict && !startsWithUseStrict(statements)) {`
`147`		`- statements = setTextRange(createNodeArray([createStatement(createLiteral("use strict")), ...statements]), statements);`
	`147`	`+ statements = setTextRange(createNodeArray([createStatement(createLiteral("use strict")), ...statements]), statements); //don't think this call to setTextRange is necessary`
`148`	`148`	`}`
`149`		`- const declarations = context.endLexicalEnvironment();`
`150`		`- return setTextRange(createNodeArray(concatenate(statements, declarations)), statements);`
	`149`	`+ return concatToNodeArray(statements, context.endLexicalEnvironment());`
	`150`	`+ }`
	`151`	`+`
	`152`	`+ /* @internal */`
	`153`	`+ export function concatToNodeArray<T extends Node>(a: NodeArray<T>, b: ReadonlyArray<T> \| undefined): NodeArray<T> {`
	`154`	`+ return b === undefined ? a : setTextRange(createNodeArray(a.concat(b)), a);`
`151`	`155`	`}`
`152`	`156`
`153`	`157`	`/**`
`@@ -1448,7 +1452,7 @@ namespace ts {`
`1448`	`1452`	`}`
`1449`	`1453`
`1450`	`1454`	`return isNodeArray(statements)`
`1451`		`- ? setTextRange(createNodeArray(concatenate(statements, declarations)), statements)`
	`1455`	`+ ? setTextRange(createNodeArray(statements.concat(declarations)), statements)`
`1452`	`1456`	`: addRange(statements, declarations);`
`1453`	`1457`	`}`
`1454`	`1458`
Original file line number	Diff line number	Diff line change
`@@ -11,7 +11,7 @@ namespace ts.codefix {`
`11`	`11`	`}`
`12`	`12`
`13`	`13`	`const changeTracker = textChanges.ChangeTracker.fromContext(context);`
`14`		`- const superCall = createStatement(createCall(createSuper(), /typeArguments/ undefined, /argumentsArray/ emptyArray));`
	`14`	`+ const superCall = createStatement(createCall(createSuper(), /typeArguments/ undefined, /argumentsArray/ []));`
`15`	`15`	`changeTracker.insertNodeAfter(sourceFile, getOpenBrace(<ConstructorDeclaration>token.parent, sourceFile), superCall, { suffix: context.newLineCharacter });`
`16`	`16`
`17`	`17`	`return [{`
Original file line number	Diff line number	Diff line change
`@@ -157,7 +157,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {`
`157`	`157`	`switch (node.kind) {`
`158`	`158`	`case SyntaxKind.JSDocAllType:`
`159`	`159`	`case SyntaxKind.JSDocUnknownType:`
`160`		`- return createTypeReferenceNode("any", emptyArray);`
	`160`	`+ return createTypeReferenceNode("any", []);`
`161`	`161`	`case SyntaxKind.JSDocOptionalType:`
`162`	`162`	`return transformJSDocOptionalType(node as JSDocOptionalType);`
`163`	`163`	`case SyntaxKind.JSDocNonNullableType:`
`@@ -180,11 +180,11 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {`
`180`	`180`	`}`
`181`	`181`
`182`	`182`	`function transformJSDocOptionalType(node: JSDocOptionalType) {`
`183`		`- return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("undefined", emptyArray)]);`
	`183`	`+ return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("undefined", [])]);`
`184`	`184`	`}`
`185`	`185`
`186`	`186`	`function transformJSDocNullableType(node: JSDocNullableType) {`
`187`		`- return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("null", emptyArray)]);`
	`187`	`+ return createUnionTypeNode([visitNode(node.type, transformJSDocType), createTypeReferenceNode("null", [])]);`
`188`	`188`	`}`
`189`	`189`
`190`	`190`	`function transformJSDocVariadicType(node: JSDocVariadicType) {`
`@@ -193,7 +193,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {`
`193`	`193`
`194`	`194`	`function transformJSDocFunctionType(node: JSDocFunctionType) {`
`195`	`195`	`const parameters = node.parameters && node.parameters.map(transformJSDocType);`
`196`		`- return createFunctionTypeNode(emptyArray, parameters as ParameterDeclaration[], node.type);`
	`196`	`+ return createFunctionTypeNode([], parameters as ParameterDeclaration[], node.type);`
`197`	`197`	`}`
`198`	`198`
`199`	`199`	`function transformJSDocParameter(node: ParameterDeclaration) {`
`@@ -227,7 +227,7 @@ namespace ts.refactor.annotateWithTypeFromJSDoc {`
`227`	`227`	`}`
`228`	`228`	`name = createIdentifier(text);`
`229`	`229`	`if ((text === "Array" \|\| text === "Promise") && !node.typeArguments) {`
`230`		`- args = createNodeArray([createTypeReferenceNode("any", emptyArray)]);`
	`230`	`+ args = createNodeArray([createTypeReferenceNode("any", [])]);`
`231`	`231`	`}`
`232`	`232`	`else {`
`233`	`233`	`args = visitNodes(node.typeArguments, transformJSDocType);`
Original file line number	Diff line number	Diff line change
`@@ -263,7 +263,7 @@ namespace ts.refactor.convertFunctionToES6Class {`
`263`	`263`	`}`
`264`	`264`
`265`	`265`	`function getModifierKindFromSource(source: Node, kind: SyntaxKind) {`
`266`		`- return filter(source.modifiers, modifier => modifier.kind === kind);`
	`266`	`+ return source.modifiers && source.modifiers.filter(modifier => modifier.kind === kind);`
`267`	`267`	`}`
`268`	`268`	`}`
`269`	`269`