From bf42f5bd1c84b075eefa2af032d9c686ace39cc9 Mon Sep 17 00:00:00 2001 From: kingwl Date: Sun, 20 Jan 2019 12:12:19 +0800 Subject: [PATCH 1/2] avoid add missing member in declaration file --- src/services/codefixes/fixAddMissingMember.ts | 9 +++++++-- .../fourslash/addMemberInDeclarationFile.ts | 17 +++++++++++++++++ .../addMemberNotInNodeModulesDeclarationFile.ts | 13 +++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 tests/cases/fourslash/addMemberInDeclarationFile.ts create mode 100644 tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 98ddee61f8e..5272b574183 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -113,6 +113,11 @@ namespace ts.codefix { } type Info = EnumInfo | ClassOrInterfaceInfo; + function isInNodeModulesDeclarationFile(node: Node) { + const sourceFile = getSourceFileOfNode(node); + return sourceFile.isDeclarationFile && startsWith(sourceFile.resolvedPath, "node_modules/") || sourceFile.resolvedPath.indexOf("/node_modules/") !== -1; + } + function getInfo(tokenSourceFile: SourceFile, tokenPos: number, checker: TypeChecker): Info | undefined { // The identifier of the missing property. eg: // this.missing = 1; @@ -131,7 +136,7 @@ namespace ts.codefix { // Prefer to change the class instead of the interface if they are merged const classOrInterface = find(symbol.declarations, isClassLike) || find(symbol.declarations, isInterfaceDeclaration); - if (classOrInterface) { + if (classOrInterface && !isInNodeModulesDeclarationFile(classOrInterface)) { const makeStatic = ((leftExpressionType as TypeReference).target || leftExpressionType) !== checker.getDeclaredTypeOfSymbol(symbol); const declSourceFile = classOrInterface.getSourceFile(); const inJs = isSourceFileJS(declSourceFile); @@ -139,7 +144,7 @@ namespace ts.codefix { return { kind: InfoKind.ClassOrInterface, token, parentDeclaration: classOrInterface, makeStatic, declSourceFile, inJs, call }; } const enumDeclaration = find(symbol.declarations, isEnumDeclaration); - if (enumDeclaration) { + if (enumDeclaration && !isInNodeModulesDeclarationFile(enumDeclaration)) { return { kind: InfoKind.Enum, token, parentDeclaration: enumDeclaration }; } return undefined; diff --git a/tests/cases/fourslash/addMemberInDeclarationFile.ts b/tests/cases/fourslash/addMemberInDeclarationFile.ts new file mode 100644 index 00000000000..1d2e900512d --- /dev/null +++ b/tests/cases/fourslash/addMemberInDeclarationFile.ts @@ -0,0 +1,17 @@ +/// + +// @Filename: ./declarations.d.ts +//// interface Response {} + +// @Filename: foo.ts +//// import './declarations.d.ts' +//// declare const resp: Response +//// resp.test() + +goTo.file('foo.ts') + +verify.codeFixAvailable([ + { description: "Declare method 'test'" }, + { description: "Declare property 'test'" }, + { description: "Add index signature for property 'test'" } +]) diff --git a/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts b/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts new file mode 100644 index 00000000000..efdbf3214bc --- /dev/null +++ b/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts @@ -0,0 +1,13 @@ +/// + +// @Filename: /node_modules/foo/declarations.d.ts +//// interface Response {} + +// @Filename: foo.ts +//// import '/node_modules/foo/declarations.d.ts' +//// declare const resp: Response +//// resp.test() + +goTo.file('foo.ts') + +verify.not.codeFixAvailable() From 9050d0fdf832b621ccc2643b829541c4d80282b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=8E=8B=E6=96=87=E7=92=90?= Date: Wed, 27 Mar 2019 19:06:21 +0800 Subject: [PATCH 2/2] update external files api --- src/services/codefixes/fixAddMissingMember.ts | 15 +++++---------- .../addMemberNotInNodeModulesDeclarationFile.ts | 14 ++++++++++---- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/services/codefixes/fixAddMissingMember.ts b/src/services/codefixes/fixAddMissingMember.ts index 5272b574183..843d22554ed 100644 --- a/src/services/codefixes/fixAddMissingMember.ts +++ b/src/services/codefixes/fixAddMissingMember.ts @@ -12,7 +12,7 @@ namespace ts.codefix { registerCodeFix({ errorCodes, getCodeActions(context) { - const info = getInfo(context.sourceFile, context.span.start, context.program.getTypeChecker()); + const info = getInfo(context.sourceFile, context.span.start, context.program.getTypeChecker(), context.program); if (!info) return undefined; if (info.kind === InfoKind.Enum) { @@ -37,7 +37,7 @@ namespace ts.codefix { return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => { eachDiagnostic(context, errorCodes, diag => { - const info = getInfo(diag.file, diag.start, checker); + const info = getInfo(diag.file, diag.start, checker, context.program); if (!info || !addToSeen(seen, getNodeId(info.parentDeclaration) + "#" + info.token.text)) { return; } @@ -113,12 +113,7 @@ namespace ts.codefix { } type Info = EnumInfo | ClassOrInterfaceInfo; - function isInNodeModulesDeclarationFile(node: Node) { - const sourceFile = getSourceFileOfNode(node); - return sourceFile.isDeclarationFile && startsWith(sourceFile.resolvedPath, "node_modules/") || sourceFile.resolvedPath.indexOf("/node_modules/") !== -1; - } - - function getInfo(tokenSourceFile: SourceFile, tokenPos: number, checker: TypeChecker): Info | undefined { + function getInfo(tokenSourceFile: SourceFile, tokenPos: number, checker: TypeChecker, program: Program): Info | undefined { // The identifier of the missing property. eg: // this.missing = 1; // ^^^^^^^ @@ -136,7 +131,7 @@ namespace ts.codefix { // Prefer to change the class instead of the interface if they are merged const classOrInterface = find(symbol.declarations, isClassLike) || find(symbol.declarations, isInterfaceDeclaration); - if (classOrInterface && !isInNodeModulesDeclarationFile(classOrInterface)) { + if (classOrInterface && !program.isSourceFileFromExternalLibrary(classOrInterface.getSourceFile())) { const makeStatic = ((leftExpressionType as TypeReference).target || leftExpressionType) !== checker.getDeclaredTypeOfSymbol(symbol); const declSourceFile = classOrInterface.getSourceFile(); const inJs = isSourceFileJS(declSourceFile); @@ -144,7 +139,7 @@ namespace ts.codefix { return { kind: InfoKind.ClassOrInterface, token, parentDeclaration: classOrInterface, makeStatic, declSourceFile, inJs, call }; } const enumDeclaration = find(symbol.declarations, isEnumDeclaration); - if (enumDeclaration && !isInNodeModulesDeclarationFile(enumDeclaration)) { + if (enumDeclaration && !program.isSourceFileFromExternalLibrary(enumDeclaration.getSourceFile())) { return { kind: InfoKind.Enum, token, parentDeclaration: enumDeclaration }; } return undefined; diff --git a/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts b/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts index efdbf3214bc..3a97aeac6dd 100644 --- a/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts +++ b/tests/cases/fourslash/addMemberNotInNodeModulesDeclarationFile.ts @@ -1,13 +1,19 @@ /// -// @Filename: /node_modules/foo/declarations.d.ts +// @noImplicitReferences: true +// @traceResolution: true + +// @Filename: /node_modules/foo/types.d.ts //// interface Response {} -// @Filename: foo.ts -//// import '/node_modules/foo/declarations.d.ts' +// @Filename: /node_modules/foo/package.json +//// { "types": "types.d.ts" } + +// @Filename: /foo.ts +//// import { Response } from 'foo' //// declare const resp: Response //// resp.test() -goTo.file('foo.ts') +goTo.file('/foo.ts') verify.not.codeFixAvailable()