Skip to content

Commit

Permalink
refactor(compiler): replace most usages of getMutableClone
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
crisbeto committed Aug 17, 2022
1 parent 0ca5eb3 commit 04302bd
Show file tree
Hide file tree
Showing 10 changed files with 160 additions and 56 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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)));
Expand Down
1 change: 1 addition & 0 deletions packages/compiler-cli/src/ngtsc/imports/src/default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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 =
Expand Down Expand Up @@ -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.
*
Expand All @@ -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,
Expand All @@ -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 =
Expand Down
19 changes: 14 additions & 5 deletions packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
Expand Down
17 changes: 16 additions & 1 deletion packages/compiler-cli/src/ngtsc/typecheck/src/type_emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 ${node.kind}`);
}

ts.setTextRange(clone, {pos: -1, end: -1});
return clone;
} else {
Expand Down
29 changes: 15 additions & 14 deletions packages/compiler-cli/src/ngtsc/util/src/visitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<T extends ts.Node&{statements: ts.NodeArray<ts.Statement>}>(
node: T): T {
private _maybeProcessStatements<T extends ts.Block|ts.SourceFile>(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);
Expand All @@ -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<ts.Statement>} {
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;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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, IS_AFTER_TS_48, ModifierLike, updateClassDeclaration, updateConstructorDeclaration, updateParameterDeclaration} from '../../ngtsc/ts_compatibility';

import {isAliasImportDeclaration, loadIsReferencedAliasDeclarationPatch} from './patch_alias_reference_resolution';

Expand Down Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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, setClassElementModifiers(element, modifiers), toLower];
}

/**
Expand Down Expand Up @@ -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<string, ts.Decorator[]>();
let classParameters: ParameterDecorationInfo[]|null = null;
Expand Down Expand Up @@ -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)),
Expand All @@ -602,3 +589,26 @@ export function getDownlevelDecoratorsTransform(
};
};
}

function setClassElementModifiers(
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 ${node.kind}`);
}

return ts.setOriginalNode(clone, node);
}
Original file line number Diff line number Diff line change
Expand Up @@ -650,10 +650,9 @@ describe('downlevel decorator transform', () => {
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;
(node.modifiers as undefined) = undefined;
(node.decorators as undefined) = undefined;
return node;
}
return ts.visitEachChild(node, visitNode, context);
};
Expand Down
3 changes: 2 additions & 1 deletion tslint.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down

0 comments on commit 04302bd

Please sign in to comment.