Skip to content

Commit

Permalink
refactor(compiler): replace most usages of getMutableClone (#47167) (#…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
crisbeto authored and alxhub committed Aug 26, 2022
1 parent fed626d commit 59a6fe7
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 58 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 @@ -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);
Expand Down 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 ${ts.SyntaxKind[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, ModifierLike, 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, cloneClassElementWithModifiers(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 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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -649,11 +649,10 @@ describe('downlevel decorator transform', () => {
const stripAllDecoratorsTransform: ts.TransformerFactory<ts.SourceFile> = 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);
};
Expand Down

0 comments on commit 59a6fe7

Please sign in to comment.