From 59a6fe72a0551b74e59421ea8d23e2f0cb06e2f8 Mon Sep 17 00:00:00 2001 From: Kristiyan Kostadinov Date: Wed, 17 Aug 2022 17:06:41 +0200 Subject: [PATCH] refactor(compiler): replace most usages of getMutableClone (#47167) (#47271) Replaces (almost) all of the usages of the deprecated `getMutableClone` function from TypeScript which has started to log deprecation warnings in version 4.8 and will likely be removed in version 5.0. The one place we have left is in the default import handling of ngtsc which will be more difficult to remove. PR Close #47167 PR Close #47271 --- .../ngtsc/annotations/common/src/metadata.ts | 5 +- .../src/ngtsc/imports/src/default.ts | 1 + .../src/ngtsc/reflection/src/type_to_value.ts | 4 +- .../src/ts_cross_version_utils.ts | 75 ++++++++++++++++++- .../ngtsc/typecheck/src/type_check_block.ts | 19 +++-- .../src/ngtsc/typecheck/src/type_emitter.ts | 17 ++++- .../src/ngtsc/util/src/visitor.ts | 29 +++---- .../downlevel_decorators_transform.ts | 58 ++++++++------ .../downlevel_decorators_transform_spec.ts | 9 +-- tslint.json | 3 +- 10 files changed, 162 insertions(+), 58 deletions(-) diff --git a/packages/compiler-cli/src/ngtsc/annotations/common/src/metadata.ts b/packages/compiler-cli/src/ngtsc/annotations/common/src/metadata.ts index 8e6c073272ce0..86720e237e8f6 100644 --- a/packages/compiler-cli/src/ngtsc/annotations/common/src/metadata.ts +++ b/packages/compiler-cli/src/ngtsc/annotations/common/src/metadata.ts @@ -137,13 +137,12 @@ function decoratorToMetadata( } // Decorators have a type. const properties: ts.ObjectLiteralElementLike[] = [ - ts.factory.createPropertyAssignment('type', ts.getMutableClone(decorator.identifier)), + ts.factory.createPropertyAssignment('type', decorator.identifier), ]; // Sometimes they have arguments. if (decorator.args !== null && decorator.args.length > 0) { const args = decorator.args.map(arg => { - const expr = ts.getMutableClone(arg); - return wrapFunctionsInParens ? wrapFunctionExpressionsInParens(expr) : expr; + return wrapFunctionsInParens ? wrapFunctionExpressionsInParens(arg) : arg; }); properties.push( ts.factory.createPropertyAssignment('args', ts.factory.createArrayLiteralExpression(args))); diff --git a/packages/compiler-cli/src/ngtsc/imports/src/default.ts b/packages/compiler-cli/src/ngtsc/imports/src/default.ts index cec40bd0be761..47577239d56dc 100644 --- a/packages/compiler-cli/src/ngtsc/imports/src/default.ts +++ b/packages/compiler-cli/src/ngtsc/imports/src/default.ts @@ -138,6 +138,7 @@ export class DefaultImportTracker { // // TODO(alxhub): discuss with the TypeScript team and determine if there's a better way to // deal with this issue. + // tslint:disable-next-line: ban stmt = ts.getMutableClone(stmt); } return stmt; diff --git a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts index 723be19e46906..305a742be4577 100644 --- a/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts +++ b/packages/compiler-cli/src/ngtsc/reflection/src/type_to_value.ts @@ -267,7 +267,9 @@ function entityNameToValue(node: ts.EntityName): ts.Expression|null { const left = entityNameToValue(node.left); return left !== null ? ts.factory.createPropertyAccessExpression(left, node.right) : null; } else if (ts.isIdentifier(node)) { - return ts.getMutableClone(node); + const clone = ts.setOriginalNode(ts.factory.createIdentifier(node.text), node); + (clone as any).parent = node.parent; + return clone; } else { return null; } diff --git a/packages/compiler-cli/src/ngtsc/ts_compatibility/src/ts_cross_version_utils.ts b/packages/compiler-cli/src/ngtsc/ts_compatibility/src/ts_cross_version_utils.ts index 06c14e69a0321..46a31c49c8120 100644 --- a/packages/compiler-cli/src/ngtsc/ts_compatibility/src/ts_cross_version_utils.ts +++ b/packages/compiler-cli/src/ngtsc/ts_compatibility/src/ts_cross_version_utils.ts @@ -9,7 +9,7 @@ import ts from 'typescript'; /** Whether the current TypeScript version is after 4.8. */ -export const IS_AFTER_TS_48 = isAfterVersion(4, 8); +const IS_AFTER_TS_48 = isAfterVersion(4, 8); /** Whether the current TypeScript version is after 4.7. */ const IS_AFTER_TS_47 = isAfterVersion(4, 7); @@ -91,6 +91,24 @@ export const updateClassDeclaration: Ts48UpdateClassDeclarationFn = IS_AFTER_TS_ ts.factory.updateClassDeclaration as any)( node, ...splitModifiers(combinedModifiers), name, typeParameters, heritageClauses, members); +/** Type of `ts.factory.createClassDeclaration` in TS 4.8+. */ +type Ts48CreateClassDeclarationFn = + (modifiers: readonly ModifierLike[]|undefined, name: ts.Identifier|undefined, + typeParameters: readonly ts.TypeParameterDeclaration[]|undefined, + heritageClauses: readonly ts.HeritageClause[]|undefined, + members: readonly ts.ClassElement[]) => ts.ClassDeclaration; + +/** + * Creates a `ts.ClassDeclaration` declaration. + * + * TODO(crisbeto): this is a backwards-compatibility layer for versions of TypeScript less than 4.8. + * We should remove it once we have dropped support for the older versions. + */ +export const createClassDeclaration: Ts48CreateClassDeclarationFn = IS_AFTER_TS_48 ? + (ts.factory.createClassDeclaration as any) : + (combinedModifiers, name, typeParameters, heritageClauses, members) => + (ts.factory.createClassDeclaration as any)( + ...splitModifiers(combinedModifiers), name, typeParameters, heritageClauses, members); /** Type of `ts.factory.updateMethodDeclaration` in TS 4.8+. */ type Ts48UpdateMethodDeclarationFn = @@ -114,6 +132,26 @@ export const updateMethodDeclaration: Ts48UpdateMethodDeclarationFn = IS_AFTER_T node, ...splitModifiers(modifiers), asteriskToken, name, questionToken, typeParameters, parameters, type, body); +/** Type of `ts.factory.createMethodDeclaration` in TS 4.8+. */ +type Ts48CreateMethodDeclarationFn = + (modifiers: readonly ModifierLike[]|undefined, asteriskToken: ts.AsteriskToken|undefined, + name: ts.PropertyName, questionToken: ts.QuestionToken|undefined, + typeParameters: readonly ts.TypeParameterDeclaration[]|undefined, + parameters: readonly ts.ParameterDeclaration[], type: ts.TypeNode|undefined, + body: ts.Block|undefined) => ts.MethodDeclaration; + +/** + * Creates a `ts.MethodDeclaration` declaration. + * + * TODO(crisbeto): this is a backwards-compatibility layer for versions of TypeScript less than 4.8. + * We should remove it once we have dropped support for the older versions. + */ +export const createMethodDeclaration: Ts48CreateMethodDeclarationFn = IS_AFTER_TS_48 ? + (ts.factory.createMethodDeclaration as any) : + (modifiers, asteriskToken, name, questionToken, typeParameters, parameters, type, body) => + (ts.factory.createMethodDeclaration as any)( + ...splitModifiers(modifiers), asteriskToken, name, questionToken, typeParameters, + parameters, type, body); /** Type of `ts.factory.updatePropertyDeclaration` in TS 4.8+. */ type Ts48UpdatePropertyDeclarationFn = @@ -153,15 +191,12 @@ export const createPropertyDeclaration: Ts48CreatePropertyDeclarationFn = IS_AFT (ts.factory.createPropertyDeclaration as any)( ...splitModifiers(modifiers), name, questionOrExclamationToken, type, initializer); - - /** Type of `ts.factory.updateGetAccessorDeclaration` in TS 4.8+. */ type Ts48UpdateGetAccessorDeclarationFn = (node: ts.GetAccessorDeclaration, modifiers: readonly ModifierLike[]|undefined, name: ts.PropertyName, parameters: readonly ts.ParameterDeclaration[], type: ts.TypeNode|undefined, body: ts.Block|undefined) => ts.GetAccessorDeclaration; - /** * Updates a `ts.GetAccessorDeclaration` declaration. * @@ -174,6 +209,23 @@ export const updateGetAccessorDeclaration: Ts48UpdateGetAccessorDeclarationFn = (ts.factory.updateGetAccessorDeclaration as any)( node, ...splitModifiers(modifiers), name, parameters, type, body); +/** Type of `ts.factory.createGetAccessorDeclaration` in TS 4.8+. */ +type Ts48CreateGetAccessorDeclarationFn = + (modifiers: readonly ModifierLike[]|undefined, name: ts.PropertyName, + parameters: readonly ts.ParameterDeclaration[], type: ts.TypeNode|undefined, + body: ts.Block|undefined) => ts.GetAccessorDeclaration; + +/** + * Creates a `ts.GetAccessorDeclaration` declaration. + * + * TODO(crisbeto): this is a backwards-compatibility layer for versions of TypeScript less than 4.8. + * We should remove it once we have dropped support for the older versions. + */ +export const createGetAccessorDeclaration: Ts48CreateGetAccessorDeclarationFn = IS_AFTER_TS_48 ? + (ts.factory.createGetAccessorDeclaration as any) : + (modifiers, name, parameters, type, body) => (ts.factory.createGetAccessorDeclaration as any)( + ...splitModifiers(modifiers), name, parameters, type, body); + /** Type of `ts.factory.updateSetAccessorDeclaration` in TS 4.8+. */ type Ts48UpdateSetAccessorDeclarationFn = (node: ts.SetAccessorDeclaration, modifiers: readonly ModifierLike[]|undefined, @@ -191,7 +243,22 @@ export const updateSetAccessorDeclaration: Ts48UpdateSetAccessorDeclarationFn = (node, modifiers, name, parameters, body) => (ts.factory.updateSetAccessorDeclaration as any)( node, ...splitModifiers(modifiers), name, parameters, body); +/** Type of `ts.factory.createSetAccessorDeclaration` in TS 4.8+. */ +type Ts48CreateSetAccessorDeclarationFn = + (modifiers: readonly ModifierLike[]|undefined, name: ts.PropertyName, + parameters: readonly ts.ParameterDeclaration[], body: ts.Block|undefined) => + ts.SetAccessorDeclaration; +/** + * Creates a `ts.GetAccessorDeclaration` declaration. + * + * TODO(crisbeto): this is a backwards-compatibility layer for versions of TypeScript less than 4.8. + * We should remove it once we have dropped support for the older versions. + */ +export const createSetAccessorDeclaration: Ts48CreateSetAccessorDeclarationFn = IS_AFTER_TS_48 ? + (ts.factory.createSetAccessorDeclaration as any) : + (modifiers, name, parameters, body) => (ts.factory.createSetAccessorDeclaration as any)( + ...splitModifiers(modifiers), name, parameters, body); /** Type of `ts.factory.updateConstructorDeclaration` in TS 4.8+. */ type Ts48UpdateConstructorDeclarationFn = diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts index 30886ecd4e479..b720bbeca56af 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts @@ -550,7 +550,7 @@ class TcbReferenceOp extends TcbOp { override execute(): ts.Identifier { const id = this.tcb.allocateId(); - let initializer = + let initializer: ts.Expression = this.target instanceof TmplAstTemplate || this.target instanceof TmplAstElement ? this.scope.resolve(this.target) : this.scope.resolve(this.host, this.target); @@ -1305,7 +1305,7 @@ class Scope { */ resolve( node: TmplAstElement|TmplAstTemplate|TmplAstVariable|TmplAstReference, - directive?: TypeCheckableDirectiveMeta): ts.Expression { + directive?: TypeCheckableDirectiveMeta): ts.Identifier|ts.NonNullExpression { // Attempt to resolve the operation locally. const res = this.resolveLocal(node, directive); if (res !== null) { @@ -1317,10 +1317,19 @@ class Scope { // // In addition, returning a clone prevents the consumer of `Scope#resolve` from // attaching comments at the declaration site. + let clone: ts.Identifier|ts.NonNullExpression; - const clone = ts.getMutableClone(res); - ts.setSyntheticTrailingComments(clone, []); - return clone; + if (ts.isIdentifier(res)) { + clone = ts.factory.createIdentifier(res.text); + } else if (ts.isNonNullExpression(res)) { + clone = ts.factory.createNonNullExpression(res.expression); + } else { + throw new Error(`Could not resolve ${node} to an Identifier or a NonNullExpression`); + } + + ts.setOriginalNode(clone, res); + (clone as any).parent = clone.parent; + return ts.setSyntheticTrailingComments(clone, []); } else if (this.parent !== null) { // Check with the parent. return this.parent.resolve(node, directive); diff --git a/packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts b/packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts index 17624a73f732e..bb1dc1ab9cb32 100644 --- a/packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts +++ b/packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts @@ -121,7 +121,22 @@ export class TypeEmitter { // issue the literal is cloned and explicitly marked as synthesized by setting its text // range to a negative range, forcing TypeScript to determine the node's literal text from // the synthesized node's text instead of the incorrect source file. - const clone = ts.getMutableClone(node); + let clone: ts.LiteralExpression; + + if (ts.isStringLiteral(node)) { + clone = ts.factory.createStringLiteral(node.text); + } else if (ts.isNumericLiteral(node)) { + clone = ts.factory.createNumericLiteral(node.text); + } else if (ts.isBigIntLiteral(node)) { + clone = ts.factory.createBigIntLiteral(node.text); + } else if (ts.isNoSubstitutionTemplateLiteral(node)) { + clone = ts.factory.createNoSubstitutionTemplateLiteral(node.text, node.rawText); + } else if (ts.isRegularExpressionLiteral(node)) { + clone = ts.factory.createRegularExpressionLiteral(node.text); + } else { + throw new Error(`Unsupported literal kind ${ts.SyntaxKind[node.kind]}`); + } + ts.setTextRange(clone, {pos: -1, end: -1}); return clone; } else { diff --git a/packages/compiler-cli/src/ngtsc/util/src/visitor.ts b/packages/compiler-cli/src/ngtsc/util/src/visitor.ts index a4a9ef34ec98a..503d3924bd651 100644 --- a/packages/compiler-cli/src/ngtsc/util/src/visitor.ts +++ b/packages/compiler-cli/src/ngtsc/util/src/visitor.ts @@ -90,27 +90,23 @@ export abstract class Visitor { // If the visited node has a `statements` array then process them, maybe replacing the visited // node and adding additional statements. - if (hasStatements(visitedNode)) { + if (ts.isBlock(visitedNode) || ts.isSourceFile(visitedNode)) { visitedNode = this._maybeProcessStatements(visitedNode); } return visitedNode; } - private _maybeProcessStatements}>( - node: T): T { + private _maybeProcessStatements(node: T): T { // Shortcut - if every statement doesn't require nodes to be prepended or appended, // this is a no-op. if (node.statements.every(stmt => !this._before.has(stmt) && !this._after.has(stmt))) { return node; } - // There are statements to prepend, so clone the original node. - const clone = ts.getMutableClone(node); - // Build a new list of statements and patch it onto the clone. const newStatements: ts.Statement[] = []; - clone.statements.forEach(stmt => { + node.statements.forEach(stmt => { if (this._before.has(stmt)) { newStatements.push(...(this._before.get(stmt)! as ts.Statement[])); this._before.delete(stmt); @@ -121,12 +117,17 @@ export abstract class Visitor { this._after.delete(stmt); } }); - clone.statements = ts.factory.createNodeArray(newStatements, node.statements.hasTrailingComma); - return clone; - } -} -function hasStatements(node: ts.Node): node is ts.Node&{statements: ts.NodeArray} { - const block = node as {statements?: any}; - return block.statements !== undefined && Array.isArray(block.statements); + const statementsArray = + ts.factory.createNodeArray(newStatements, node.statements.hasTrailingComma); + + if (ts.isBlock(node)) { + return ts.factory.updateBlock(node, statementsArray) as T; + } else { + return ts.factory.updateSourceFile( + node, statementsArray, node.isDeclarationFile, node.referencedFiles, + node.typeReferenceDirectives, node.hasNoDefaultLib, node.libReferenceDirectives) as + T; + } + } } diff --git a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts index a67d4eb155fea..501fa85b55b8f 100644 --- a/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts +++ b/packages/compiler-cli/src/transformers/downlevel_decorators_transform/downlevel_decorators_transform.ts @@ -9,7 +9,7 @@ import ts from 'typescript'; import {Decorator, ReflectionHost} from '../../ngtsc/reflection'; -import {combineModifiers, createPropertyDeclaration, getDecorators, getModifiers, IS_AFTER_TS_48, updateClassDeclaration, updateConstructorDeclaration, updateParameterDeclaration} from '../../ngtsc/ts_compatibility'; +import {combineModifiers, createClassDeclaration, createGetAccessorDeclaration, createMethodDeclaration, createPropertyDeclaration, createSetAccessorDeclaration, getDecorators, getModifiers, ModifierLike, updateConstructorDeclaration, updateParameterDeclaration} from '../../ngtsc/ts_compatibility'; import {isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './patch_alias_reference_resolution'; @@ -375,13 +375,13 @@ export function getDownlevelDecoratorsTransform( // We can help TypeScript and avoid this non-reliable resolution by using an identifier // that is not type-only and is directly linked to the import alias declaration. if (decl.name !== undefined) { - return ts.getMutableClone(decl.name); + return ts.setOriginalNode(ts.factory.createIdentifier(decl.name.text), decl.name); } } // Clone the original entity name identifier so that it can be used to reference // its value at runtime. This is used when the identifier is resolving to a file // local declaration (otherwise it would resolve to an alias import declaration). - return ts.getMutableClone(name); + return ts.setOriginalNode(ts.factory.createIdentifier(name.text), name); } /** @@ -421,23 +421,13 @@ export function getDownlevelDecoratorsTransform( return [undefined, element, []]; } - const name = (element.name as ts.Identifier).text; - const mutable = ts.getMutableClone(element); - - if (IS_AFTER_TS_48) { - // As of TS 4.8, the decorators are part of the `modifiers` array. - (mutable as any).modifiers = decoratorsToKeep.length ? - ts.setTextRange( - ts.factory.createNodeArray( - combineModifiers(decoratorsToKeep, getModifiers(mutable))), - mutable.modifiers) : - undefined; - } else { - (mutable as any).decorators = decoratorsToKeep.length ? - ts.setTextRange(ts.factory.createNodeArray(decoratorsToKeep), mutable.decorators) : - undefined; - } - return [name, mutable, toLower]; + const modifiers = decoratorsToKeep.length ? + ts.setTextRange( + ts.factory.createNodeArray(combineModifiers(decoratorsToKeep, getModifiers(element))), + element.modifiers) : + getModifiers(element); + + return [element.name.text, cloneClassElementWithModifiers(element, modifiers), toLower]; } /** @@ -496,8 +486,6 @@ export function getDownlevelDecoratorsTransform( * - creates a propDecorators property */ function transformClassDeclaration(classDecl: ts.ClassDeclaration): ts.ClassDeclaration { - classDecl = ts.getMutableClone(classDecl); - const newMembers: ts.ClassElement[] = []; const decoratedProperties = new Map(); let classParameters: ParameterDecorationInfo[]|null = null; @@ -574,8 +562,7 @@ export function getDownlevelDecoratorsTransform( ts.factory.createNodeArray(newMembers, classDecl.members.hasTrailingComma), classDecl.members); - return updateClassDeclaration( - classDecl, + return createClassDeclaration( combineModifiers( decoratorsToKeep.size ? Array.from(decoratorsToKeep) : undefined, getModifiers(classDecl)), @@ -602,3 +589,26 @@ export function getDownlevelDecoratorsTransform( }; }; } + +function cloneClassElementWithModifiers( + node: ts.ClassElement, modifiers: readonly ModifierLike[]|undefined): ts.ClassElement { + let clone: ts.ClassElement; + + if (ts.isMethodDeclaration(node)) { + clone = createMethodDeclaration( + modifiers, node.asteriskToken, node.name, node.questionToken, node.typeParameters, + node.parameters, node.type, node.body); + } else if (ts.isPropertyDeclaration(node)) { + clone = createPropertyDeclaration( + modifiers, node.name, node.questionToken, node.type, node.initializer); + } else if (ts.isGetAccessor(node)) { + clone = + createGetAccessorDeclaration(modifiers, node.name, node.parameters, node.type, node.body); + } else if (ts.isSetAccessor(node)) { + clone = createSetAccessorDeclaration(modifiers, node.name, node.parameters, node.body); + } else { + throw new Error(`Unsupported decorated member with kind ${ts.SyntaxKind[node.kind]}`); + } + + return ts.setOriginalNode(clone, node); +} diff --git a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts index a95eaeec29be4..cbf6c168db3ec 100644 --- a/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts +++ b/packages/compiler-cli/test/downlevel_decorators_transform_spec.ts @@ -649,11 +649,10 @@ describe('downlevel decorator transform', () => { const stripAllDecoratorsTransform: ts.TransformerFactory = context => { return (sourceFile: ts.SourceFile) => { const visitNode = (node: ts.Node): ts.Node => { - if (ts.isClassDeclaration(node) || ts.isClassElement(node)) { - const cloned = ts.getMutableClone(node); - (cloned.decorators as undefined) = undefined; - (cloned.modifiers as undefined) = undefined; - return cloned; + if (ts.isClassDeclaration(node)) { + return ts.factory.createClassDeclaration( + ts.getModifiers(node), node.name, node.typeParameters, node.heritageClauses, + node.members); } return ts.visitEachChild(node, visitNode, context); }; diff --git a/tslint.json b/tslint.json index 83d7664f94d1a..3bb27f50bfcf7 100644 --- a/tslint.json +++ b/tslint.json @@ -78,7 +78,8 @@ "ban": [ true, {"name": "fdescribe", "message": "Don't keep jasmine focus methods."}, - {"name": "fit", "message": "Don't keep jasmine focus methods."} + {"name": "fit", "message": "Don't keep jasmine focus methods."}, + {"name": ["*", "getMutableClone"], "message": "Use a ts.factory.update* or ts.factory.create* method instead."} ] }, "jsRules": {