microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2026-02-06 02:33:53 -06:00
Code Issues Packages Projects Releases 212 Wiki Activity

Handle loose type parameters in Extract Method

Browse Source 
Known limitations:

1. If a type parameter on an inner symbol shadows a type parameter on an
outer symbol, the generated code will be incorrect.  We should either
rename one or more type parameters or forbid the extraction.

2. Type arguments are always passed explicitly, even if they would be
inferred correctly.
This commit is contained in:
Andrew Casey 2017-08-11 16:14:48 -07:00
parent 2350d46e44
commit 30802cda97
3 changed files with 175 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/compiler/types.ts
View File

@ -651,6 +651,20 @@ namespace ts {
}
export interface SignatureDeclaration extends NamedDeclaration {
kind: SyntaxKind.CallSignature
| SyntaxKind.ConstructSignature
| SyntaxKind.MethodSignature
| SyntaxKind.IndexSignature
| SyntaxKind.FunctionType
| SyntaxKind.ConstructorType
| SyntaxKind.JSDocFunctionType
| SyntaxKind.FunctionDeclaration
| SyntaxKind.MethodDeclaration
| SyntaxKind.Constructor
| SyntaxKind.GetAccessor
| SyntaxKind.SetAccessor
| SyntaxKind.FunctionExpression
| SyntaxKind.ArrowFunction;
name?: PropertyName;
typeParameters?: NodeArray<TypeParameterDeclaration>;
parameters: NodeArray<ParameterDeclaration>;
@ -1816,6 +1830,7 @@ namespace ts {
export type DeclarationWithTypeParameters = SignatureDeclaration | ClassLikeDeclaration | InterfaceDeclaration | TypeAliasDeclaration | JSDocTemplateTag;
export interface ClassLikeDeclaration extends NamedDeclaration {
kind: SyntaxKind.ClassDeclaration | SyntaxKind.ClassExpression;
name?: Identifier;
typeParameters?: NodeArray<TypeParameterDeclaration>;
heritageClauses?: NodeArray<HeritageClause>;

32
src/compiler/utilities.ts
View File

@ -477,6 +477,38 @@ namespace ts {
return false;
}
/* @internal */
export function isDeclarationWithTypeParameters(node: Node): node is DeclarationWithTypeParameters;
export function isDeclarationWithTypeParameters(node: DeclarationWithTypeParameters): node is DeclarationWithTypeParameters {
switch (node.kind) {
case SyntaxKind.CallSignature:
case SyntaxKind.ConstructSignature:
case SyntaxKind.MethodSignature:
case SyntaxKind.IndexSignature:
case SyntaxKind.FunctionType:
case SyntaxKind.ConstructorType:
case SyntaxKind.JSDocFunctionType:
case SyntaxKind.ClassDeclaration:
case SyntaxKind.ClassExpression:
case SyntaxKind.InterfaceDeclaration:
case SyntaxKind.TypeAliasDeclaration:
case SyntaxKind.JSDocTemplateTag:
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.MethodDeclaration:
case SyntaxKind.Constructor:
case SyntaxKind.GetAccessor:
case SyntaxKind.SetAccessor:
case SyntaxKind.FunctionExpression:
case SyntaxKind.ArrowFunction:
return true;
default:
staticAssertNever(node);
return false;
}
}
function staticAssertNever(_: never): void {}
// Gets the nearest enclosing block scope container that has the provided node
// as a descendant, that is not the provided node.
export function getEnclosingBlockScopeContainer(node: Node): Node {

135
src/services/refactors/extractMethod.ts
View File

@ -610,7 +610,7 @@ namespace ts.refactor.extractMethod {
export function extractFunctionInScope(
node: Statement | Expression | Block,
scope: Scope,
{ usages: usagesInScope, substitutions }: ScopeUsages,
{ usages: usagesInScope, typeParameterUsages, substitutions }: ScopeUsages,
range: TargetRange,
context: RefactorContext): ExtractResultForScope {
@ -652,7 +652,18 @@ namespace ts.refactor.extractMethod {
callArguments.push(createIdentifier(name));
});
// Provide explicit return types for contexutally-typed functions
const typeParametersAndDeclarations = arrayFrom(typeParameterUsages.values()).map(type => ({ type, declaration: getFirstDeclaration(type) }));
const sortedTypeParametersAndDeclarations = typeParametersAndDeclarations.sort(compareTypesByDeclarationOrder);
const typeParameters: ReadonlyArray<TypeParameterDeclaration> = sortedTypeParametersAndDeclarations.map(t => t.declaration as TypeParameterDeclaration);
// Strictly speaking, we should check whether each name actually binds to the appropriate type
// parameter. In cases of shadowing, they may not.
const callTypeArguments: ReadonlyArray<TypeNode> | undefined = typeParameters.length > 0
? typeParameters.map(decl => createTypeReferenceNode(decl.name, /*typeArguments*/ undefined))
: undefined;
// Provide explicit return types for contextually-typed functions
// to avoid problems when there are literal types present
if (isExpression(node) && !isJS) {
const contextualType = checker.getContextualType(node);
@ -677,7 +688,7 @@ namespace ts.refactor.extractMethod {
range.facts & RangeFacts.IsGenerator ? createToken(SyntaxKind.AsteriskToken) : undefined,
functionName,
/*questionToken*/ undefined,
/*typeParameters*/[],
typeParameters,
parameters,
returnType,
body
@ -689,7 +700,7 @@ namespace ts.refactor.extractMethod {
range.facts & RangeFacts.IsAsyncFunction ? [createToken(SyntaxKind.AsyncKeyword)] : undefined,
range.facts & RangeFacts.IsGenerator ? createToken(SyntaxKind.AsteriskToken) : undefined,
functionName,
/*typeParameters*/[],
typeParameters,
parameters,
returnType,
body
@ -704,7 +715,7 @@ namespace ts.refactor.extractMethod {
// replace range with function call
let call: Expression = createCall(
isClassLike(scope) ? createPropertyAccess(range.facts & RangeFacts.InStaticRegion ? createIdentifier(scope.name.getText()) : createThis(), functionReference) : functionReference,
/*typeArguments*/ undefined,
callTypeArguments, // Note that no attempt is made to take advantage of type argument inference
callArguments);
if (range.facts & RangeFacts.IsGenerator) {
call = createYield(createToken(SyntaxKind.AsteriskToken), call);
@ -774,6 +785,51 @@ namespace ts.refactor.extractMethod {
changes: changeTracker.getChanges()
};
function getFirstDeclaration(type: Type): Declaration | undefined {
let firstDeclaration = undefined;
const symbol = type.symbol;
if (symbol && symbol.declarations) {
for (const declaration of symbol.declarations) {
if (firstDeclaration === undefined || declaration.pos < firstDeclaration.pos) {
firstDeclaration = declaration;
}
}
}
return firstDeclaration;
}
function compareTypesByDeclarationOrder(
{type: type1, declaration: declaration1}: {type: Type, declaration?: Declaration},
{type: type2, declaration: declaration2}: {type: Type, declaration?: Declaration}) {
if (declaration1) {
if (declaration2) {
const positionDiff = declaration1.pos - declaration2.pos;
if (positionDiff !== 0) {
return positionDiff;
}
}
else {
return 1; // Sort undeclared type parameters to the front.
}
}
else if (declaration2) {
return -1; // Sort undeclared type parameters to the front.
}
const name1 = type1.symbol ? type1.symbol.getName() : "";
const name2 = type2.symbol ? type2.symbol.getName() : "";
const nameDiff = compareStrings(name1, name2);
if (nameDiff !== 0) {
return nameDiff;
}
// IDs are guaranteed to be unique, so this ensures a total ordering.
return type1.id - type2.id;
}
function getPropertyAssignmentsForWrites(writes: UsageEntry[]) {
return writes.map(w => createShorthandPropertyAssignment(w.symbol.name));
}
@ -867,6 +923,7 @@ namespace ts.refactor.extractMethod {
export interface ScopeUsages {
usages: Map<UsageEntry>;
typeParameterUsages: Map<TypeParameter>; // Key is type ID
substitutions: Map<Node>;
}
@ -877,6 +934,7 @@ namespace ts.refactor.extractMethod {
sourceFile: SourceFile,
checker: TypeChecker) {
const allTypeParameterUsages = createMap<TypeParameter>(); // Key is type ID
const usagesPerScope: ScopeUsages[] = [];
const substitutionsPerScope: Map<Node>[] = [];
const errorsPerScope: Diagnostic[][] = [];
@ -884,7 +942,7 @@ namespace ts.refactor.extractMethod {
// initialize results
for (const _ of scopes) {
usagesPerScope.push({ usages: createMap<UsageEntry>(), substitutions: createMap<Expression>() });
usagesPerScope.push({ usages: createMap<UsageEntry>(), typeParameterUsages: createMap<TypeParameter>(), substitutions: createMap<Expression>() });
substitutionsPerScope.push(createMap<Expression>());
errorsPerScope.push([]);
}
@ -892,8 +950,42 @@ namespace ts.refactor.extractMethod {
const target = isReadonlyArray(targetRange.range) ? createBlock(<Statement[]>targetRange.range) : targetRange.range;
const containingLexicalScopeOfExtraction = isBlockScope(scopes[0], scopes[0].parent) ? scopes[0] : getEnclosingBlockScopeContainer(scopes[0]);
const unmodifiedNode = isReadonlyArray(targetRange.range) ? targetRange.range[0] : targetRange.range;
const inGenericContext = isInGenericContext(unmodifiedNode);
collectUsages(target);
if (allTypeParameterUsages.size > 0) {
const seenTypeParameterUsages = createMap<TypeParameter>(); // Key is type ID
let i = 0;
for (let curr: Node = unmodifiedNode; curr !== undefined && i < scopes.length; curr = curr.parent) {
if (curr === scopes[i]) {
// Copy current contents of seenTypeParameterUsages into scope.
seenTypeParameterUsages.forEach((typeParameter, id) => {
usagesPerScope[i].typeParameterUsages.set(id, typeParameter);
});
i++;
}
// Note that we add the current node's type parameters *after* updating the corresponding scope.
if (isDeclarationWithTypeParameters(curr) && curr.typeParameters) {
for (const typeParameterDecl of curr.typeParameters) {
const typeParameter = checker.getTypeAtLocation(typeParameterDecl) as TypeParameter;
if (allTypeParameterUsages.has(typeParameter.id.toString())) {
seenTypeParameterUsages.set(typeParameter.id.toString(), typeParameter);
}
}
}
}
// If we didn't get through all the scopes, then there were some that weren't in our
// parent chain (impossible at time of writing). A conservative solution would be to
// copy allTypeParameterUsages into all remaining scopes.
Debug.assert(i === scopes.length);
}
for (let i = 0; i < scopes.length; i++) {
let hasWrite = false;
let readonlyClassPropertyWrite: Declaration | undefined = undefined;
@ -912,7 +1004,7 @@ namespace ts.refactor.extractMethod {
errorsPerScope[i].push(createDiagnosticForNode(targetRange.range, Messages.CannotCombineWritesAndReturns));
}
else if (readonlyClassPropertyWrite && i > 0) {
errorsPerScope[i].push(createDiagnosticForNode(readonlyClassPropertyWrite, Messages.CannotCombineWritesAndReturns));
errorsPerScope[i].push(createDiagnosticForNode(readonlyClassPropertyWrite, Messages.CannotExtractReadonlyPropertyInitializerOutsideConstructor));
}
}
@ -924,7 +1016,36 @@ namespace ts.refactor.extractMethod {
return { target, usagesPerScope, errorsPerScope };
function hasTypeParameters(node: Node) {
return isDeclarationWithTypeParameters(node) &&
node.typeParameters !== undefined &&
node.typeParameters.length > 0;
}
function isInGenericContext(node: Node) {
for (; node; node = node.parent) {
if (hasTypeParameters(node)) {
return true;
}
}
return false;
}
function collectUsages(node: Node, valueUsage = Usage.Read) {
if (inGenericContext) {
const type = checker.getTypeAtLocation(node);
const symbolWalker = checker.getSymbolWalker();
const {visitedTypes} = symbolWalker.walkType(type);
for (const visitedType of visitedTypes) {
if (visitedType.flags & TypeFlags.TypeParameter) {
allTypeParameterUsages.set(visitedType.id.toString(), visitedType as TypeParameter);
}
}
}
if (isDeclaration(node) && node.symbol) {
visibleDeclarationsInExtractedRange.push(node.symbol);
}