From 3d2bf6a75f87e08bb432ee03b70008bf914d4b7a Mon Sep 17 00:00:00 2001 From: Wesley Wigham Date: Mon, 14 Jan 2019 13:56:27 -0800 Subject: [PATCH] Fix implement interface quickfix import types (#29410) * Pass module specifier resolution host thru types constructed by implements quickfixes * Add regression test * Fix scope node for generated methods, fix lints --- src/compiler/types.ts | 2 + src/services/codefixes/fixAddMissingMember.ts | 2 +- ...sDoesntImplementInheritedAbstractMember.ts | 11 +++-- .../fixClassIncorrectlyImplementsInterface.ts | 14 +++--- src/services/codefixes/helpers.ts | 48 +++++++++++++++---- ...ClassImplementInterface_typeInOtherFile.ts | 4 +- .../codeFixUndeclaredAcrossFiles3.ts | 2 +- ...erfaceUnreachableTypeUsesRelativeImport.ts | 26 ++++++++++ 8 files changed, 83 insertions(+), 26 deletions(-) create mode 100644 tests/cases/fourslash/quickfixImplementInterfaceUnreachableTypeUsesRelativeImport.ts diff --git a/src/compiler/types.ts b/src/compiler/types.ts index e46a3d70403..125c3297863 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -3037,8 +3037,10 @@ namespace ts { /* @internal */ typeToTypeNode(type: Type, enclosingDeclaration?: Node, flags?: NodeBuilderFlags, tracker?: SymbolTracker): TypeNode | undefined; // tslint:disable-line unified-signatures /** Note that the resulting nodes cannot be checked. */ signatureToSignatureDeclaration(signature: Signature, kind: SyntaxKind, enclosingDeclaration?: Node, flags?: NodeBuilderFlags): SignatureDeclaration & {typeArguments?: NodeArray} | undefined; + /* @internal */ signatureToSignatureDeclaration(signature: Signature, kind: SyntaxKind, enclosingDeclaration?: Node, flags?: NodeBuilderFlags, tracker?: SymbolTracker): SignatureDeclaration & {typeArguments?: NodeArray} | undefined; // tslint:disable-line unified-signatures /** Note that the resulting nodes cannot be checked. */ indexInfoToIndexSignatureDeclaration(indexInfo: IndexInfo, kind: IndexKind, enclosingDeclaration?: Node, flags?: NodeBuilderFlags): IndexSignatureDeclaration | undefined; + /* @internal */ indexInfoToIndexSignatureDeclaration(indexInfo: IndexInfo, kind: IndexKind, enclosingDeclaration?: Node, flags?: NodeBuilderFlags, tracker?: SymbolTracker): IndexSignatureDeclaration | undefined; // tslint:disable-line unified-signatures /** Note that the resulting nodes cannot be checked. */ symbolToEntityName(symbol: Symbol, meaning: SymbolFlags, enclosingDeclaration?: Node, flags?: NodeBuilderFlags): EntityName | undefined; /** Note that the resulting nodes cannot be checked. */ diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index bbad3e03824..98ddee61f8e 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -275,7 +275,7 @@ namespace ts.codefix { inJs: boolean, preferences: UserPreferences, ): void { - const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, preferences, !isInterfaceDeclaration(typeDecl)); + const methodDeclaration = createMethodFromCallExpression(context, callExpression, token.text, inJs, makeStatic, preferences, typeDecl); const containingMethodDeclaration = getAncestor(callExpression, SyntaxKind.MethodDeclaration); if (containingMethodDeclaration && containingMethodDeclaration.parent === typeDecl) { diff --git a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts index 9d9972c03e8..d18bc50639a 100644 --- a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts +++ b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts @@ -8,9 +8,9 @@ namespace ts.codefix { registerCodeFix({ errorCodes, getCodeActions(context) { - const { program, sourceFile, span } = context; + const { sourceFile, span } = context; const changes = textChanges.ChangeTracker.with(context, t => - addMissingMembers(getClass(sourceFile, span.start), sourceFile, program.getTypeChecker(), t, context.preferences)); + addMissingMembers(getClass(sourceFile, span.start), sourceFile, context, t, context.preferences)); return changes.length === 0 ? undefined : [createCodeFixAction(fixId, changes, Diagnostics.Implement_inherited_abstract_class, fixId, Diagnostics.Implement_all_inherited_abstract_classes)]; }, fixIds: [fixId], @@ -19,7 +19,7 @@ namespace ts.codefix { return codeFixAll(context, errorCodes, (changes, diag) => { const classDeclaration = getClass(diag.file, diag.start); if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) { - addMissingMembers(classDeclaration, context.sourceFile, context.program.getTypeChecker(), changes, context.preferences); + addMissingMembers(classDeclaration, context.sourceFile, context, changes, context.preferences); } }); }, @@ -32,15 +32,16 @@ namespace ts.codefix { return cast(token.parent, isClassLike); } - function addMissingMembers(classDeclaration: ClassLikeDeclaration, sourceFile: SourceFile, checker: TypeChecker, changeTracker: textChanges.ChangeTracker, preferences: UserPreferences): void { + function addMissingMembers(classDeclaration: ClassLikeDeclaration, sourceFile: SourceFile, context: TypeConstructionContext, changeTracker: textChanges.ChangeTracker, preferences: UserPreferences): void { const extendsNode = getEffectiveBaseTypeNode(classDeclaration)!; + const checker = context.program.getTypeChecker(); const instantiatedExtendsType = checker.getTypeAtLocation(extendsNode); // Note that this is ultimately derived from a map indexed by symbol names, // so duplicates cannot occur. const abstractAndNonPrivateExtendsSymbols = checker.getPropertiesOfType(instantiatedExtendsType).filter(symbolPointsToNonPrivateAndAbstractMember); - createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, checker, preferences, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member)); + createMissingMemberNodes(classDeclaration, abstractAndNonPrivateExtendsSymbols, context, preferences, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member)); } function symbolPointsToNonPrivateAndAbstractMember(symbol: Symbol): boolean { diff --git a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts index 4401625000a..59385f2e2e9 100644 --- a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts +++ b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts @@ -6,11 +6,10 @@ namespace ts.codefix { registerCodeFix({ errorCodes, getCodeActions(context) { - const { program, sourceFile, span } = context; + const { sourceFile, span } = context; const classDeclaration = getClass(sourceFile, span.start); - const checker = program.getTypeChecker(); return mapDefined(getClassImplementsHeritageClauseElements(classDeclaration), implementedTypeNode => { - const changes = textChanges.ChangeTracker.with(context, t => addMissingDeclarations(checker, implementedTypeNode, sourceFile, classDeclaration, t, context.preferences)); + const changes = textChanges.ChangeTracker.with(context, t => addMissingDeclarations(context, implementedTypeNode, sourceFile, classDeclaration, t, context.preferences)); return changes.length === 0 ? undefined : createCodeFixAction(fixId, changes, [Diagnostics.Implement_interface_0, implementedTypeNode.getText(sourceFile)], fixId, Diagnostics.Implement_all_unimplemented_interfaces); }); }, @@ -21,7 +20,7 @@ namespace ts.codefix { const classDeclaration = getClass(diag.file, diag.start); if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) { for (const implementedTypeNode of getClassImplementsHeritageClauseElements(classDeclaration)!) { - addMissingDeclarations(context.program.getTypeChecker(), implementedTypeNode, diag.file, classDeclaration, changes, context.preferences); + addMissingDeclarations(context, implementedTypeNode, diag.file, classDeclaration, changes, context.preferences); } } }); @@ -37,13 +36,14 @@ namespace ts.codefix { } function addMissingDeclarations( - checker: TypeChecker, + context: TypeConstructionContext, implementedTypeNode: ExpressionWithTypeArguments, sourceFile: SourceFile, classDeclaration: ClassLikeDeclaration, changeTracker: textChanges.ChangeTracker, preferences: UserPreferences, ): void { + const checker = context.program.getTypeChecker(); const maybeHeritageClauseSymbol = getHeritageClauseSymbolTable(classDeclaration, checker); // Note that this is ultimately derived from a map indexed by symbol names, // so duplicates cannot occur. @@ -60,12 +60,12 @@ namespace ts.codefix { createMissingIndexSignatureDeclaration(implementedType, IndexKind.String); } - createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, checker, preferences, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member)); + createMissingMemberNodes(classDeclaration, nonPrivateAndNotExistedInHeritageClauseMembers, context, preferences, member => changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, member)); function createMissingIndexSignatureDeclaration(type: InterfaceType, kind: IndexKind): void { const indexInfoOfKind = checker.getIndexInfoOfType(type, kind); if (indexInfoOfKind) { - changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, checker.indexInfoToIndexSignatureDeclaration(indexInfoOfKind, kind, classDeclaration)!); + changeTracker.insertNodeAtClassStart(sourceFile, classDeclaration, checker.indexInfoToIndexSignatureDeclaration(indexInfoOfKind, kind, classDeclaration, /*flags*/ undefined, getNoopSymbolTrackerWithResolver(context))!); } } } diff --git a/src/services/codefixes/helpers.ts b/src/services/codefixes/helpers.ts index 66488d5d697..4309e6ec35d 100644 --- a/src/services/codefixes/helpers.ts +++ b/src/services/codefixes/helpers.ts @@ -6,23 +6,48 @@ namespace ts.codefix { * @param possiblyMissingSymbols The collection of symbols to filter and then get insertions for. * @returns Empty string iff there are no member insertions. */ - export function createMissingMemberNodes(classDeclaration: ClassLikeDeclaration, possiblyMissingSymbols: ReadonlyArray, checker: TypeChecker, preferences: UserPreferences, out: (node: ClassElement) => void): void { + export function createMissingMemberNodes(classDeclaration: ClassLikeDeclaration, possiblyMissingSymbols: ReadonlyArray, context: TypeConstructionContext, preferences: UserPreferences, out: (node: ClassElement) => void): void { const classMembers = classDeclaration.symbol.members!; for (const symbol of possiblyMissingSymbols) { if (!classMembers.has(symbol.escapedName)) { - addNewNodeForMemberSymbol(symbol, classDeclaration, checker, preferences, out); + addNewNodeForMemberSymbol(symbol, classDeclaration, context, preferences, out); } } } + function getModuleSpecifierResolverHost(context: TypeConstructionContext): SymbolTracker["moduleResolverHost"] { + return { + directoryExists: context.host.directoryExists ? d => context.host.directoryExists!(d) : undefined, + fileExists: context.host.fileExists ? f => context.host.fileExists!(f) : undefined, + getCurrentDirectory: context.host.getCurrentDirectory ? () => context.host.getCurrentDirectory!() : undefined, + readFile: context.host.readFile ? f => context.host.readFile!(f) : undefined, + useCaseSensitiveFileNames: context.host.useCaseSensitiveFileNames ? () => context.host.useCaseSensitiveFileNames!() : undefined, + getSourceFiles: () => context.program.getSourceFiles(), + getCommonSourceDirectory: () => context.program.getCommonSourceDirectory(), + }; + } + + export function getNoopSymbolTrackerWithResolver(context: TypeConstructionContext): SymbolTracker { + return { + trackSymbol: noop, + moduleResolverHost: getModuleSpecifierResolverHost(context), + }; + } + + export interface TypeConstructionContext { + program: Program; + host: ModuleSpecifierResolutionHost; + } + /** * @returns Empty string iff there we can't figure out a representation for `symbol` in `enclosingDeclaration`. */ - function addNewNodeForMemberSymbol(symbol: Symbol, enclosingDeclaration: ClassLikeDeclaration, checker: TypeChecker, preferences: UserPreferences, out: (node: Node) => void): void { + function addNewNodeForMemberSymbol(symbol: Symbol, enclosingDeclaration: ClassLikeDeclaration, context: TypeConstructionContext, preferences: UserPreferences, out: (node: Node) => void): void { const declarations = symbol.getDeclarations(); if (!(declarations && declarations.length)) { return undefined; } + const checker = context.program.getTypeChecker(); const declaration = declarations[0]; const name = getSynthesizedDeepClone(getNameOfDeclaration(declaration), /*includeTrivia*/ false) as PropertyName; @@ -36,7 +61,7 @@ namespace ts.codefix { case SyntaxKind.SetAccessor: case SyntaxKind.PropertySignature: case SyntaxKind.PropertyDeclaration: - const typeNode = checker.typeToTypeNode(type, enclosingDeclaration); + const typeNode = checker.typeToTypeNode(type, enclosingDeclaration, /*flags*/ undefined, getNoopSymbolTrackerWithResolver(context)); out(createProperty( /*decorators*/undefined, modifiers, @@ -83,13 +108,13 @@ namespace ts.codefix { } function outputMethod(signature: Signature, modifiers: NodeArray | undefined, name: PropertyName, body?: Block): void { - const method = signatureToMethodDeclaration(checker, signature, enclosingDeclaration, modifiers, name, optional, body); + const method = signatureToMethodDeclaration(context, signature, enclosingDeclaration, modifiers, name, optional, body); if (method) out(method); } } function signatureToMethodDeclaration( - checker: TypeChecker, + context: TypeConstructionContext, signature: Signature, enclosingDeclaration: ClassLikeDeclaration, modifiers: NodeArray | undefined, @@ -97,7 +122,8 @@ namespace ts.codefix { optional: boolean, body: Block | undefined, ): MethodDeclaration | undefined { - const signatureDeclaration = checker.signatureToSignatureDeclaration(signature, SyntaxKind.MethodDeclaration, enclosingDeclaration, NodeBuilderFlags.NoTruncation | NodeBuilderFlags.SuppressAnyReturnType); + const program = context.program; + const signatureDeclaration = program.getTypeChecker().signatureToSignatureDeclaration(signature, SyntaxKind.MethodDeclaration, enclosingDeclaration, NodeBuilderFlags.NoTruncation | NodeBuilderFlags.SuppressAnyReturnType, getNoopSymbolTrackerWithResolver(context)); if (!signatureDeclaration) { return undefined; } @@ -117,18 +143,20 @@ namespace ts.codefix { inJs: boolean, makeStatic: boolean, preferences: UserPreferences, - body: boolean, + contextNode: Node, ): MethodDeclaration { + const body = !isInterfaceDeclaration(contextNode); const { typeArguments, arguments: args, parent } = call; const checker = context.program.getTypeChecker(); + const tracker = getNoopSymbolTrackerWithResolver(context); const types = map(args, arg => // Widen the type so we don't emit nonsense annotations like "function fn(x: 3) {" - checker.typeToTypeNode(checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)))); + checker.typeToTypeNode(checker.getBaseTypeOfLiteralType(checker.getTypeAtLocation(arg)), contextNode, /*flags*/ undefined, tracker)); const names = map(args, arg => isIdentifier(arg) ? arg.text : isPropertyAccessExpression(arg) ? arg.name.text : undefined); const contextualType = checker.getContextualType(call); - const returnType = inJs ? undefined : contextualType && checker.typeToTypeNode(contextualType, call) || createKeywordTypeNode(SyntaxKind.AnyKeyword); + const returnType = inJs ? undefined : contextualType && checker.typeToTypeNode(contextualType, contextNode, /*flags*/ undefined, tracker) || createKeywordTypeNode(SyntaxKind.AnyKeyword); return createMethod( /*decorators*/ undefined, /*modifiers*/ makeStatic ? [createToken(SyntaxKind.StaticKeyword)] : undefined, diff --git a/tests/cases/fourslash/codeFixClassImplementInterface_typeInOtherFile.ts b/tests/cases/fourslash/codeFixClassImplementInterface_typeInOtherFile.ts index 712bf48e059..ed5d29cd767 100644 --- a/tests/cases/fourslash/codeFixClassImplementInterface_typeInOtherFile.ts +++ b/tests/cases/fourslash/codeFixClassImplementInterface_typeInOtherFile.ts @@ -17,8 +17,8 @@ verify.codeFix({ newFileContent: `import { I } from "./I"; export class C implements I { - x: import("/I").J; - m(): import("/I").J { + x: import("./I").J; + m(): import("./I").J { throw new Error("Method not implemented."); } }`, diff --git a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts index 2b07493f923..dcf5c999d6a 100644 --- a/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts +++ b/tests/cases/fourslash/codeFixUndeclaredAcrossFiles3.ts @@ -20,7 +20,7 @@ verify.getAndApplyCodeFix(/*errorCode*/ undefined, 0); verify.rangeIs(` - m0(arg0: D): any { + m0(arg0: import("./f2").D): any { throw new Error("Method not implemented."); } `); \ No newline at end of file diff --git a/tests/cases/fourslash/quickfixImplementInterfaceUnreachableTypeUsesRelativeImport.ts b/tests/cases/fourslash/quickfixImplementInterfaceUnreachableTypeUsesRelativeImport.ts new file mode 100644 index 00000000000..ba6cd702ea0 --- /dev/null +++ b/tests/cases/fourslash/quickfixImplementInterfaceUnreachableTypeUsesRelativeImport.ts @@ -0,0 +1,26 @@ +/// + +// @Filename: class.ts +////export class Class { } +// @Filename: interface.ts +////import { Class } from './class'; +//// +////export interface Foo { +//// x: Class; +////} +// @Filename: index.ts +////import { Foo } from './interface'; +//// +////class /*1*/X implements Foo {} +goTo.marker("1"); +verify.codeFix({ + index: 0, + description: "Implement interface 'Foo'", + newFileContent: { + "/tests/cases/fourslash/index.ts": `import { Foo } from './interface'; + +class X implements Foo { + x: import("./class").Class; +}` + } +});