From ddb95c51844c5d8aee2187712ba948e41baaf0aa 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) 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 --- .../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 | 56 ++++++++------ .../downlevel_decorators_transform_spec.ts | 9 +-- tslint.json | 3 +- 10 files changed, 161 insertions(+), 57 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 8e6c073272c..86720e237e8 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 cec40bd0be7..47577239d56 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 723be19e469..305a742be45 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 06c14e69a03..46a31c49c81 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 30886ecd4e4..b720bbeca56 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 17624a73f73..bb1dc1ab9cb 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 a4a9ef34ec9..503d3924bd6 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; + + 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; + } } } - -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); -} 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 a67d4eb155f..501fa85b55b 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); + const modifiers = decoratorsToKeep.length ? + ts.setTextRange( + ts.factory.createNodeArray(combineModifiers(decoratorsToKeep, getModifiers(element))), + element.modifiers) : + getModifiers(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]; + 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 a95eaeec29b..cbf6c168db3 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 83d7664f94d..3bb27f50bfc 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": {