Merge pull request #18864 from amcasey/GH18857
Handle repeated substitution of the same symbol during extraction
This commit is contained in:
commit
b244cd4fb4
|
@ -84,12 +84,11 @@ namespace ts {
|
|||
let x = [#|t + 1|];
|
||||
}`);
|
||||
|
||||
// TODO (18857): handle repeated substitution
|
||||
// testExtractConstant("extractConstant_RepeatedSubstitution",
|
||||
// `namespace X {
|
||||
// export const j = 10;
|
||||
// export const y = [#|j * j|];
|
||||
// }`);
|
||||
testExtractConstant("extractConstant_RepeatedSubstitution",
|
||||
`namespace X {
|
||||
export const j = 10;
|
||||
export const y = [#|j * j|];
|
||||
}`);
|
||||
|
||||
testExtractConstant("extractConstant_VariableList_const",
|
||||
`const a = 1, b = [#|a + 1|];`);
|
||||
|
|
|
@ -355,12 +355,11 @@ function parsePrimaryExpression(): any {
|
|||
[#|function G() { }|]
|
||||
}`);
|
||||
|
||||
// TODO (18857): handle repeated substitution
|
||||
// testExtractFunction("extractFunction_RepeatedSubstitution",
|
||||
// `namespace X {
|
||||
// export const j = 10;
|
||||
// export const y = [#|j * j|];
|
||||
// }`);
|
||||
testExtractFunction("extractFunction_RepeatedSubstitution",
|
||||
`namespace X {
|
||||
export const j = 10;
|
||||
export const y = [#|j * j|];
|
||||
}`);
|
||||
});
|
||||
|
||||
function testExtractFunction(caption: string, text: string) {
|
||||
|
|
|
@ -1048,7 +1048,7 @@ namespace ts.refactor.extractSymbol {
|
|||
}
|
||||
}
|
||||
|
||||
function transformFunctionBody(body: Node, writes: ReadonlyArray<UsageEntry>, substitutions: ReadonlyMap<Node>, hasReturn: boolean): { body: Block, returnValueProperty: string } {
|
||||
function transformFunctionBody(body: Node, writes: ReadonlyArray<UsageEntry>, substitutions: ReadonlyMap<() => Node>, hasReturn: boolean): { body: Block, returnValueProperty: string } {
|
||||
if (isBlock(body) && !writes && substitutions.size === 0) {
|
||||
// already block, no writes to propagate back, no substitutions - can use node as is
|
||||
return { body: createBlock(body.statements, /*multLine*/ true), returnValueProperty: undefined };
|
||||
|
@ -1096,21 +1096,21 @@ namespace ts.refactor.extractSymbol {
|
|||
const oldIgnoreReturns = ignoreReturns;
|
||||
ignoreReturns = ignoreReturns || isFunctionLikeDeclaration(node) || isClassLike(node);
|
||||
const substitution = substitutions.get(getNodeId(node).toString());
|
||||
const result = substitution || visitEachChild(node, visitor, nullTransformationContext);
|
||||
const result = substitution ? substitution() : visitEachChild(node, visitor, nullTransformationContext);
|
||||
ignoreReturns = oldIgnoreReturns;
|
||||
return result;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function transformConstantInitializer(initializer: Expression, substitutions: ReadonlyMap<Node>): Expression {
|
||||
function transformConstantInitializer(initializer: Expression, substitutions: ReadonlyMap<() => Node>): Expression {
|
||||
return substitutions.size
|
||||
? visitor(initializer) as Expression
|
||||
: initializer;
|
||||
|
||||
function visitor(node: Node): VisitResult<Node> {
|
||||
const substitution = substitutions.get(getNodeId(node).toString());
|
||||
return substitution || visitEachChild(node, visitor, nullTransformationContext);
|
||||
return substitution ? substitution() : visitEachChild(node, visitor, nullTransformationContext);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1239,7 +1239,7 @@ namespace ts.refactor.extractSymbol {
|
|||
interface ScopeUsages {
|
||||
readonly usages: Map<UsageEntry>;
|
||||
readonly typeParameterUsages: Map<TypeParameter>; // Key is type ID
|
||||
readonly substitutions: Map<Node>;
|
||||
readonly substitutions: Map<() => Node>;
|
||||
}
|
||||
|
||||
interface ReadsAndWrites {
|
||||
|
@ -1258,7 +1258,7 @@ namespace ts.refactor.extractSymbol {
|
|||
|
||||
const allTypeParameterUsages = createMap<TypeParameter>(); // Key is type ID
|
||||
const usagesPerScope: ScopeUsages[] = [];
|
||||
const substitutionsPerScope: Map<Node>[] = [];
|
||||
const substitutionsPerScope: Map<() => Node>[] = [];
|
||||
const functionErrorsPerScope: Diagnostic[][] = [];
|
||||
const constantErrorsPerScope: Diagnostic[][] = [];
|
||||
const visibleDeclarationsInExtractedRange: Symbol[] = [];
|
||||
|
@ -1270,8 +1270,8 @@ namespace ts.refactor.extractSymbol {
|
|||
|
||||
// initialize results
|
||||
for (const scope of scopes) {
|
||||
usagesPerScope.push({ usages: createMap<UsageEntry>(), typeParameterUsages: createMap<TypeParameter>(), substitutions: createMap<Expression>() });
|
||||
substitutionsPerScope.push(createMap<Expression>());
|
||||
usagesPerScope.push({ usages: createMap<UsageEntry>(), typeParameterUsages: createMap<TypeParameter>(), substitutions: createMap<() => Expression>() });
|
||||
substitutionsPerScope.push(createMap<() => Expression>());
|
||||
|
||||
functionErrorsPerScope.push(
|
||||
isFunctionLikeDeclaration(scope) && scope.kind !== SyntaxKind.FunctionDeclaration
|
||||
|
@ -1567,18 +1567,20 @@ namespace ts.refactor.extractSymbol {
|
|||
}
|
||||
}
|
||||
|
||||
function tryReplaceWithQualifiedNameOrPropertyAccess(symbol: Symbol, scopeDecl: Node, isTypeNode: boolean): PropertyAccessExpression | EntityName {
|
||||
function tryReplaceWithQualifiedNameOrPropertyAccess(symbol: Symbol, scopeDecl: Node, isTypeNode: boolean): () => (PropertyAccessExpression | EntityName) {
|
||||
if (!symbol) {
|
||||
return undefined;
|
||||
}
|
||||
if (symbol.getDeclarations().some(d => d.parent === scopeDecl)) {
|
||||
return createIdentifier(symbol.name);
|
||||
return () => createIdentifier(symbol.name);
|
||||
}
|
||||
const prefix = tryReplaceWithQualifiedNameOrPropertyAccess(symbol.parent, scopeDecl, isTypeNode);
|
||||
if (prefix === undefined) {
|
||||
return undefined;
|
||||
}
|
||||
return isTypeNode ? createQualifiedName(<EntityName>prefix, createIdentifier(symbol.name)) : createPropertyAccess(<Expression>prefix, symbol.name);
|
||||
return isTypeNode
|
||||
? () => createQualifiedName(<EntityName>prefix(), createIdentifier(symbol.name))
|
||||
: () => createPropertyAccess(<Expression>prefix(), symbol.name);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -0,0 +1,19 @@
|
|||
// ==ORIGINAL==
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
export const y = j * j;
|
||||
}
|
||||
// ==SCOPE::Extract to constant in enclosing scope==
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
const newLocal = j * j;
|
||||
|
||||
export const y = /*RENAME*/newLocal;
|
||||
}
|
||||
// ==SCOPE::Extract to constant in global scope==
|
||||
const newLocal = X.j * X.j;
|
||||
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
export const y = /*RENAME*/newLocal;
|
||||
}
|
|
@ -0,0 +1,22 @@
|
|||
// ==ORIGINAL==
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
export const y = j * j;
|
||||
}
|
||||
// ==SCOPE::Extract to function in namespace 'X'==
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
export const y = /*RENAME*/newFunction();
|
||||
|
||||
function newFunction() {
|
||||
return j * j;
|
||||
}
|
||||
}
|
||||
// ==SCOPE::Extract to function in global scope==
|
||||
namespace X {
|
||||
export const j = 10;
|
||||
export const y = /*RENAME*/newFunction();
|
||||
}
|
||||
function newFunction() {
|
||||
return X.j * X.j;
|
||||
}
|
Loading…
Reference in a new issue