From d99675bb74c78dc0b6297e5f5c6ebed7d11ec7f1 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:03:53 -0700 Subject: [PATCH 01/40] checker.ts: Remove null check on symbols --- src/compiler/checker.ts | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index 7913520bfdb49..18166da3b48e6 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -6216,11 +6216,13 @@ namespace ts { function symbolsToArray(symbols: SymbolTable): Symbol[] { const result: Symbol[] = []; - symbols.forEach((symbol, id) => { - if (!isReservedMemberName(id)) { - result.push(symbol); - } - }); + if (symbols) { + symbols.forEach((symbol, id) => { + if (!isReservedMemberName(id)) { + result.push(symbol); + } + }); + } return result; } From 5bef8662490304dfeb904761e820cd5f614173e5 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:05:03 -0700 Subject: [PATCH 02/40] tsserverProjectSystem.ts: add two tests --- .../unittests/tsserverProjectSystem.ts | 101 ++++++++++++++++++ 1 file changed, 101 insertions(+) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 4a7cafa245bb9..d0641caa825fd 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3638,6 +3638,43 @@ namespace ts.projectSystem { }); }); + describe("import in completion list", () => { + it("should include exported members of all source files", () => { + const file1: FileOrFolder = { + path: "/a/b/file1.ts", + content: ` + export function Test1() { } + export function Test2() { } + ` + }; + const file2: FileOrFolder = { + path: "/a/b/file2.ts", + content: ` + import { Test2 } from "./file1"; + + t` + }; + const configFile: FileOrFolder = { + path: "/a/b/tsconfig.json", + content: "{}" + }; + + const host = createServerHost([file1, file2, configFile]); + const service = createProjectService(host); + service.openClientFile(file2.path); + + const completions1 = service.configuredProjects[0].getLanguageService().getCompletionsAtPosition(file2.path, file2.path.length); + const test1Entry = find(completions1.entries, e => e.name === "Test1"); + const test2Entry = find(completions1.entries, e => e.name === "Test2"); + + assert.isDefined(test1Entry, "should contain 'Test1'"); + assert.isDefined(test2Entry, "should contain 'Test2'"); + + assert.isTrue(test1Entry.hasAction, "should set the 'hasAction' property to true for Test1"); + assert.isUndefined(test2Entry.hasAction, "should not set the 'hasAction' property for Test2"); + }); + }); + describe("import helpers", () => { it("should not crash in tsserver", () => { const f1 = { @@ -3968,6 +4005,70 @@ namespace ts.projectSystem { }); }); + describe("completion entry with code actions", () => { + it("should work for symbols from non-imported modules", () => { + const moduleFile = { + path: "/a/b/moduleFile.ts", + content: `export const guitar = 10;` + }; + const file1 = { + path: "/a/b/file2.ts", + content: `` + }; + const globalFile = { + path: "/a/b/globalFile.ts", + content: `interface Jazz { }` + }; + const ambientModuleFile = { + path: "/a/b/ambientModuleFile.ts", + content: + `declare module "windyAndWarm" { + export const chetAtkins = "great"; + }` + }; + const defaultModuleFile = { + path: "/a/b/defaultModuleFile.ts", + content: + `export default function egyptianElla() { };` + }; + const configFile = { + path: "/a/b/tsconfig.json", + content: "{}" + }; + + const host = createServerHost([moduleFile, file1, globalFile, ambientModuleFile, defaultModuleFile, configFile]); + const session = createSession(host); + const projectService = session.getProjectService(); + projectService.openClientFile(file1.path); + + checkEntryDetail("guitar", /*hasAction*/ true, `import { guitar } from "./moduleFile";\n\n`); + checkEntryDetail("Jazz", /*hasAction*/ false); + checkEntryDetail("chetAtkins", /*hasAction*/ true, `import { chetAtkins } from "windyAndWarm";\n\n`); + checkEntryDetail("egyptianElla", /*hasAction*/ true, `import egyptianElla from "./defaultModuleFile";\n\n`); + + function checkEntryDetail(entryName: string, hasAction: boolean, insertString?: string) { + const request = makeSessionRequest( + CommandNames.CompletionDetails, + { entryNames: [entryName], file: file1.path, line: 1, offset: 0, projectFileName: configFile.path }); + const response = session.executeCommand(request).response as protocol.CompletionEntryDetails[]; + assert.isTrue(response.length === 1); + + const entryDetails = response[0]; + if (!hasAction) { + assert.isUndefined(entryDetails.codeActions); + } + else { + const action = entryDetails.codeActions[0]; + assert.isTrue(action.changes[0].fileName === file1.path); + assert.deepEqual(action.changes[0], { + fileName: file1.path, + textChanges: [{ start: { line: 1, offset: 1 }, end: { line: 1, offset: 1 }, newText: insertString }] + }); + } + } + }); + }); + describe("maxNodeModuleJsDepth for inferred projects", () => { it("should be set to 2 if the project has js root files", () => { const file1: FileOrFolder = { From 087de799d2505c86f804681c55055e89289e29f0 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:14:16 -0700 Subject: [PATCH 03/40] client.ts, completions.ts, types.ts: Add codeActions member to CompletionEntryDetails --- src/server/client.ts | 4 +++- src/services/completions.ts | 8 ++++++-- src/services/types.ts | 1 + 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/server/client.ts b/src/server/client.ts index a7e0615dce866..a234060038911 100644 --- a/src/server/client.ts +++ b/src/server/client.ts @@ -198,7 +198,9 @@ namespace ts.server { const request = this.processRequest(CommandNames.CompletionDetails, args); const response = this.processResponse(request); Debug.assert(response.body.length === 1, "Unexpected length of completion details response body."); - return response.body[0]; + + const convertedCodeActions = map(response.body[0].codeActions, codeAction => this.convertCodeActions(codeAction, fileName)); + return { ...response.body[0], codeActions: convertedCodeActions }; } getCompletionEntrySymbol(_fileName: string, _position: number, _entryName: string): Symbol { diff --git a/src/services/completions.ts b/src/services/completions.ts index b49567ef21799..dd22570ef8454 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -311,6 +311,8 @@ namespace ts.Completions { const symbol = forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false) === entryName ? s : undefined); if (symbol) { + let codeActions: CodeAction[]; + const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name: entryName, @@ -318,7 +320,8 @@ namespace ts.Completions { kind: symbolKind, displayParts, documentation, - tags + tags, + codeActions }; } } @@ -335,7 +338,8 @@ namespace ts.Completions { kindModifiers: ScriptElementKindModifier.none, displayParts: [displayPart(entryName, SymbolDisplayPartKind.keyword)], documentation: undefined, - tags: undefined + tags: undefined, + codeActions: undefined }; } diff --git a/src/services/types.ts b/src/services/types.ts index 6ff7402f3ad0c..b418dd41a4112 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -674,6 +674,7 @@ namespace ts { displayParts: SymbolDisplayPart[]; documentation: SymbolDisplayPart[]; tags: JSDocTagInfo[]; + codeActions?: CodeAction[]; } export interface OutliningSpan { From a84b5b58eee76b4a2d8d3790a42baac238103160 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:18:49 -0700 Subject: [PATCH 04/40] protocol.ts, session.ts: Add codeActions member to CompletionEntryDetails protocol --- src/server/protocol.ts | 5 +++++ src/server/session.ts | 14 +++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index b6756a7d4ff11..6a435d88001fa 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1667,6 +1667,11 @@ namespace ts.server.protocol { * JSDoc tags for the symbol. */ tags: JSDocTagInfo[]; + + /** + * The associated code actions for this entry + */ + codeActions?: CodeAction[]; } export interface CompletionsResponse extends Response { diff --git a/src/server/session.ts b/src/server/session.ts index 074ba4d6ca1b9..00b88381d6942 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1195,9 +1195,17 @@ namespace ts.server { const { file, project } = this.getFileAndProject(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const position = this.getPosition(args, scriptInfo); - - return mapDefined(args.entryNames, entryName => - project.getLanguageService().getCompletionEntryDetails(file, position, entryName)); + + return mapDefined(args.entryNames, entryName => { + const details = project.getLanguageService().getCompletionEntryDetails(file, position, entryName); + if (details) { + const mappedCodeActions = map(details.codeActions, action => this.mapCodeAction(action, scriptInfo)); + return { ...details, codeActions: mappedCodeActions }; + } + else { + return undefined; + } + }); } private getCompileOnSaveAffectedFileList(args: protocol.FileRequestArgs): protocol.CompileOnSaveAffectedFileListSingleProject[] { From 15b73d09c3e5010fef9609c849aeb3f6c746f9a2 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:26:47 -0700 Subject: [PATCH 05/40] protocol.ts, session.ts, types.ts: add hasAction to CompletionEntry --- src/server/protocol.ts | 5 +++++ src/server/session.ts | 10 ++++++++-- src/services/types.ts | 1 + 3 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 6a435d88001fa..6596b2ff036f7 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1635,6 +1635,11 @@ namespace ts.server.protocol { * this span should be used instead of the default one. */ replacementSpan?: TextSpan; + /** + * Indicating if commiting this completion entry will require additional code action to be + * made to avoid errors. The code action is normally adding an additional import statement. + */ + hasAction?: true; } /** diff --git a/src/server/session.ts b/src/server/session.ts index 00b88381d6942..ccf1817879857 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1180,9 +1180,15 @@ namespace ts.server { if (simplifiedResult) { return mapDefined(completions.entries, entry => { if (completions.isMemberCompletion || (entry.name.toLowerCase().indexOf(prefix.toLowerCase()) === 0)) { - const { name, kind, kindModifiers, sortText, replacementSpan } = entry; + const { name, kind, kindModifiers, sortText, replacementSpan, hasAction } = entry; const convertedSpan = replacementSpan ? this.decorateSpan(replacementSpan, scriptInfo) : undefined; - return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan }; + + const newEntry: protocol.CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan }; + // avoid serialization when hasAction = false + if (hasAction) { + newEntry.hasAction = true; + } + return newEntry; } }).sort((a, b) => compareStrings(a.name, b.name)); } diff --git a/src/services/types.ts b/src/services/types.ts index b418dd41a4112..12eb53018fe66 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -665,6 +665,7 @@ namespace ts { * be used in that case */ replacementSpan?: TextSpan; + hasAction?: true; } export interface CompletionEntryDetails { From 8a1a12485675dfcc5cb15eff79762a9adbcbc90a Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:33:25 -0700 Subject: [PATCH 06/40] session.ts, services.ts, types.ts: Add formattingOptions parameter to getCompletionEntryDetails --- src/server/session.ts | 5 +++-- src/services/services.ts | 3 ++- src/services/types.ts | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/server/session.ts b/src/server/session.ts index ccf1817879857..719fc21bac706 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1201,9 +1201,10 @@ namespace ts.server { const { file, project } = this.getFileAndProject(args); const scriptInfo = project.getScriptInfoForNormalizedPath(file); const position = this.getPosition(args, scriptInfo); - + const formattingOptions = project.projectService.getFormatCodeOptions(file); + return mapDefined(args.entryNames, entryName => { - const details = project.getLanguageService().getCompletionEntryDetails(file, position, entryName); + const details = project.getLanguageService().getCompletionEntryDetails(file, position, entryName, formattingOptions); if (details) { const mappedCodeActions = map(details.codeActions, action => this.mapCodeAction(action, scriptInfo)); return { ...details, codeActions: mappedCodeActions }; diff --git a/src/services/services.ts b/src/services/services.ts index 6a171ad916647..9c716b24a96e4 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1373,8 +1373,9 @@ namespace ts { return Completions.getCompletionsAtPosition(host, program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position); } - function getCompletionEntryDetails(fileName: string, position: number, entryName: string): CompletionEntryDetails { + function getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails { synchronizeHostData(); + formattingOptions; return Completions.getCompletionEntryDetails(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName); } diff --git a/src/services/types.ts b/src/services/types.ts index 12eb53018fe66..ed0c5b6ae9db1 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -228,7 +228,7 @@ namespace ts { getEncodedSemanticClassifications(fileName: string, span: TextSpan): Classifications; getCompletionsAtPosition(fileName: string, position: number): CompletionInfo; - getCompletionEntryDetails(fileName: string, position: number, entryName: string): CompletionEntryDetails; + getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails; getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol; getQuickInfoAtPosition(fileName: string, position: number): QuickInfo; From 0aa865f6cccfee2c65c5ee060e7c8d612ffd9c0a Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:34:57 -0700 Subject: [PATCH 07/40] completions.ts: define SymbolOriginInfo type --- src/services/completions.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/services/completions.ts b/src/services/completions.ts index dd22570ef8454..2d64eb48b3d08 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3,6 +3,8 @@ /* @internal */ namespace ts.Completions { export type Log = (message: string) => void; + + export type SymbolOriginInfo = { moduleSymbol: Symbol, isDefaultExport?: boolean }; const enum KeywordCompletionFilters { None, From 25831a82617a2731ba3e48ce3e782c6114801154 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:39:36 -0700 Subject: [PATCH 08/40] completions.ts, services.ts: Add allSourceFiles parameter to getCompletionsAtPosition --- src/services/completions.ts | 3 ++- src/services/services.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 2d64eb48b3d08..697d7c32e465d 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -12,7 +12,8 @@ namespace ts.Completions { ConstructorParameterKeywords, // Keywords at constructor parameter } - export function getCompletionsAtPosition(host: LanguageServiceHost, typeChecker: TypeChecker, log: Log, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number): CompletionInfo | undefined { + export function getCompletionsAtPosition(host: LanguageServiceHost, typeChecker: TypeChecker, log: Log, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, allSourceFiles: SourceFile[]): CompletionInfo | undefined { + allSourceFiles; // unused if (isInReferenceComment(sourceFile, position)) { return PathCompletions.getTripleSlashReferenceCompletion(sourceFile, position, compilerOptions, host); } diff --git a/src/services/services.ts b/src/services/services.ts index 9c716b24a96e4..eebb8929db0e6 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1370,7 +1370,7 @@ namespace ts { function getCompletionsAtPosition(fileName: string, position: number): CompletionInfo { synchronizeHostData(); - return Completions.getCompletionsAtPosition(host, program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position); + return Completions.getCompletionsAtPosition(host, program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, program.getSourceFiles()); } function getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails { From 9940c9261a4ba9638589b7aeb68f1ad8fc933694 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 17:45:28 -0700 Subject: [PATCH 09/40] completions.ts, services.ts: Plumb allSourceFiles into new function getSymbolsFromOtherSourceFileExports inside getCompletionData --- src/services/completions.ts | 22 +++++++++++++--------- src/services/services.ts | 4 ++-- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 697d7c32e465d..1021f3861f357 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -3,7 +3,7 @@ /* @internal */ namespace ts.Completions { export type Log = (message: string) => void; - + export type SymbolOriginInfo = { moduleSymbol: Symbol, isDefaultExport?: boolean }; const enum KeywordCompletionFilters { @@ -13,7 +13,6 @@ namespace ts.Completions { } export function getCompletionsAtPosition(host: LanguageServiceHost, typeChecker: TypeChecker, log: Log, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, allSourceFiles: SourceFile[]): CompletionInfo | undefined { - allSourceFiles; // unused if (isInReferenceComment(sourceFile, position)) { return PathCompletions.getTripleSlashReferenceCompletion(sourceFile, position, compilerOptions, host); } @@ -22,7 +21,7 @@ namespace ts.Completions { return getStringLiteralCompletionEntries(sourceFile, position, typeChecker, compilerOptions, host, log); } - const completionData = getCompletionData(typeChecker, log, sourceFile, position); + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); if (!completionData) { return undefined; } @@ -301,9 +300,9 @@ namespace ts.Completions { } } - export function getCompletionEntryDetails(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string): CompletionEntryDetails { + export function getCompletionEntryDetails(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string, allSourceFiles: SourceFile[]): CompletionEntryDetails { // Compute all the completion symbols again. - const completionData = getCompletionData(typeChecker, log, sourceFile, position); + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); if (completionData) { const { symbols, location } = completionData; @@ -349,9 +348,9 @@ namespace ts.Completions { return undefined; } - export function getCompletionEntrySymbol(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string): Symbol | undefined { + export function getCompletionEntrySymbol(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string, allSourceFiles: SourceFile[]): Symbol | undefined { // Compute all the completion symbols again. - const completionData = getCompletionData(typeChecker, log, sourceFile, position); + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); // Find the symbol with the matching entry name. // We don't need to perform character checks here because we're only comparing the // name against 'entryName' (which is known to be good), not building a new @@ -371,7 +370,7 @@ namespace ts.Completions { } type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; - function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number): CompletionData | undefined { + function getCompletionData(typeChecker: TypeChecker, log: (message: string) => void, sourceFile: SourceFile, position: number, allSourceFiles: SourceFile[]): CompletionData | undefined { const isJavaScriptFile = isSourceFileJavaScript(sourceFile); let request: Request | undefined; @@ -525,7 +524,6 @@ namespace ts.Completions { break; } // falls through - case SyntaxKind.JsxSelfClosingElement: case SyntaxKind.JsxElement: case SyntaxKind.JsxOpeningElement: @@ -756,6 +754,8 @@ namespace ts.Completions { const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; symbols = filterGlobalCompletion(typeChecker.getSymbolsInScope(scopeNode, symbolMeanings)); + getSymbolsFromOtherSourceFileExports(previousToken === undefined ? "" : previousToken.getText()); + return true; } @@ -834,6 +834,10 @@ namespace ts.Completions { } } + function getSymbolsFromOtherSourceFileExports(_tokenText: string) { + allSourceFiles; + } + /** * Finds the first node that "embraces" the position, so that one may * accurately aggregate locals from the closest containing scope. diff --git a/src/services/services.ts b/src/services/services.ts index eebb8929db0e6..459380a4a86f4 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1376,12 +1376,12 @@ namespace ts { function getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails { synchronizeHostData(); formattingOptions; - return Completions.getCompletionEntryDetails(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName); + return Completions.getCompletionEntryDetails(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles()); } function getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol { synchronizeHostData(); - return Completions.getCompletionEntrySymbol(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName); + return Completions.getCompletionEntrySymbol(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles()); } function getQuickInfoAtPosition(fileName: string, position: number): QuickInfo { From c838093e98b2f79623c3f5b1677ef8f54cd9e5e8 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:06:14 -0700 Subject: [PATCH 10/40] completions.ts: add symbolToOriginInfoMap parameter to getCompletionEntriesFromSymbols and to return value of getCompletionData --- src/services/completions.ts | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 1021f3861f357..099fd4717549e 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -26,7 +26,7 @@ namespace ts.Completions { return undefined; } - const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, keywordFilters } = completionData; + const { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, request, keywordFilters, symbolToOriginInfoMap } = completionData; if (sourceFile.languageVariant === LanguageVariant.JSX && location && location.parent && location.parent.kind === SyntaxKind.JsxClosingElement) { @@ -58,7 +58,7 @@ namespace ts.Completions { const entries: CompletionEntry[] = []; if (isSourceFileJavaScript(sourceFile)) { - const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log); + const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log, symbolToOriginInfoMap); getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target, entries); } else { @@ -66,7 +66,7 @@ namespace ts.Completions { return undefined; } - getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log); + getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true, typeChecker, compilerOptions.target, log, symbolToOriginInfoMap); } // TODO add filter for keyword based on type/value/namespace and also location @@ -136,7 +136,8 @@ namespace ts.Completions { }; } - function getCompletionEntriesFromSymbols(symbols: Symbol[], entries: Push, location: Node, performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, log: Log): Map { + function getCompletionEntriesFromSymbols(symbols: Symbol[], entries: Push, location: Node, performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, log: Log, symbolToOriginInfoMap?: Map): Map { + symbolToOriginInfoMap; const start = timestamp(); const uniqueNames = createMap(); if (symbols) { @@ -304,7 +305,8 @@ namespace ts.Completions { // Compute all the completion symbols again. const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); if (completionData) { - const { symbols, location } = completionData; + const { symbols, location, symbolToOriginInfoMap } = completionData; + symbolToOriginInfoMap; // Find the symbol with the matching entry name. // We don't need to perform character checks here because we're only comparing the @@ -367,6 +369,7 @@ namespace ts.Completions { isRightOfDot: boolean; request?: Request; keywordFilters: KeywordCompletionFilters; + symbolToOriginInfoMap: Map; } type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; @@ -442,7 +445,7 @@ namespace ts.Completions { } if (request) { - return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, keywordFilters: KeywordCompletionFilters.None }; + return { symbols: undefined, isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, location: undefined, isRightOfDot: false, request, keywordFilters: KeywordCompletionFilters.None, symbolToOriginInfoMap: undefined }; } if (!insideJsDocTagTypeExpression) { @@ -542,6 +545,7 @@ namespace ts.Completions { let isNewIdentifierLocation: boolean; let keywordFilters = KeywordCompletionFilters.None; let symbols: Symbol[] = []; + const symbolToOriginInfoMap = createMap(); if (isRightOfDot) { getTypeScriptMemberSymbols(); @@ -578,7 +582,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); - return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters }; + return { symbols, isGlobalCompletion, isMemberCompletion, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters, symbolToOriginInfoMap }; type JSDocTagWithTypeExpression = JSDocAugmentsTag | JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag; @@ -835,6 +839,7 @@ namespace ts.Completions { } function getSymbolsFromOtherSourceFileExports(_tokenText: string) { + symbolToOriginInfoMap; allSourceFiles; } From c5cc2f148c1d36060ca4f424a137f0e4ae3f3e5e Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:13:33 -0700 Subject: [PATCH 11/40] utilities.ts: Add getOtherModuleSymbols, getUniqueSymbolIdAsString, getUniqueSymbolId --- src/services/utilities.ts | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/services/utilities.ts b/src/services/utilities.ts index a5b035154beae..0cf315ddd9243 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1311,6 +1311,31 @@ namespace ts { return ensureScriptKind(fileName, host && host.getScriptKind && host.getScriptKind(fileName)); } + export function getOtherModuleSymbols( + sourceFiles: SourceFile[], + currentSourceFile: SourceFile, + typeChecker: TypeChecker + ) { + const results: Symbol[] = typeChecker.getAmbientModules(); + for (const otherSourceFile of sourceFiles) { + if (otherSourceFile !== currentSourceFile && isExternalOrCommonJsModule(otherSourceFile)) { + results.push(otherSourceFile.symbol); + } + } + return results; + } + + export function getUniqueSymbolIdAsString(symbol: Symbol, typeChecker: TypeChecker) { + return getUniqueSymbolId(symbol, typeChecker) + ""; + } + + export function getUniqueSymbolId(symbol: Symbol, typeChecker: TypeChecker) { + if (symbol.flags & SymbolFlags.Alias) { + return getSymbolId(typeChecker.getAliasedSymbol(symbol)); + } + return getSymbolId(symbol); + } + export function getFirstNonSpaceCharacterPosition(text: string, position: number) { while (isWhiteSpaceLike(text.charCodeAt(position))) { position += 1; From b024285c23658d28d1dcdae85587e7d2c916bda9 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:12:03 -0700 Subject: [PATCH 12/40] completions.ts: Set CompletionEntry.hasAction when symbol is found in symbolToOriginInfoMap (meaning there's an import action) --- src/services/completions.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 099fd4717549e..cc3b8964a8814 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -137,7 +137,6 @@ namespace ts.Completions { } function getCompletionEntriesFromSymbols(symbols: Symbol[], entries: Push, location: Node, performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, log: Log, symbolToOriginInfoMap?: Map): Map { - symbolToOriginInfoMap; const start = timestamp(); const uniqueNames = createMap(); if (symbols) { @@ -146,6 +145,9 @@ namespace ts.Completions { if (entry) { const id = entry.name; if (!uniqueNames.has(id)) { + if (symbolToOriginInfoMap && symbolToOriginInfoMap.has(getUniqueSymbolIdAsString(symbol, typeChecker))) { + entry.hasAction = true; + } entries.push(entry); uniqueNames.set(id, true); } From 041302fa1d45ee38f7d6b0b5b0a5b9f576326b90 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:12:11 -0700 Subject: [PATCH 13/40] completions.ts: Populate list with possible exports (implement getSymbolsFromOtherSourceFileExports) --- src/services/completions.ts | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index cc3b8964a8814..5e2137cf67128 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -840,9 +840,33 @@ namespace ts.Completions { } } - function getSymbolsFromOtherSourceFileExports(_tokenText: string) { - symbolToOriginInfoMap; - allSourceFiles; + function getSymbolsFromOtherSourceFileExports(tokenText: string) { + const tokenTextLowerCase = tokenText.toLowerCase(); + const symbolIdMap = arrayToMap(symbols, s => getUniqueSymbolIdAsString(s, typeChecker)); + + const allPotentialModules = getOtherModuleSymbols(allSourceFiles, sourceFile, typeChecker); + for (const moduleSymbol of allPotentialModules) { + // check the default export + const defaultExport = typeChecker.tryGetMemberInModuleExports("default", moduleSymbol); + if (defaultExport) { + const localSymbol = getLocalSymbolForExportDefault(defaultExport); + if (localSymbol && !symbolIdMap.has(getUniqueSymbolIdAsString(localSymbol, typeChecker)) && startsWith(localSymbol.name.toLowerCase(), tokenTextLowerCase)) { + symbols.push(localSymbol); + symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(localSymbol, typeChecker), { moduleSymbol, isDefaultExport: true }); + } + } + + // check exports with the same name + const allExportedSymbols = typeChecker.getExportsOfModule(moduleSymbol); + if (allExportedSymbols) { + for (const exportedSymbol of allExportedSymbols) { + if (exportedSymbol.name && !symbolIdMap.has(getUniqueSymbolIdAsString(exportedSymbol, typeChecker)) && startsWith(exportedSymbol.name.toLowerCase(), tokenTextLowerCase)) { + symbols.push(exportedSymbol); + symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(exportedSymbol, typeChecker), { moduleSymbol }); + } + } + } + } } /** From abe1fdb0ea4e61152155ee66e89f6b66e5a9cdce Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:17:54 -0700 Subject: [PATCH 14/40] completions.ts, services.ts: Plumb host and rulesProvider into getCompletionEntryDetails --- src/services/completions.ts | 5 ++++- src/services/services.ts | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index 5e2137cf67128..f207a0b00a948 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -303,7 +303,8 @@ namespace ts.Completions { } } - export function getCompletionEntryDetails(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string, allSourceFiles: SourceFile[]): CompletionEntryDetails { + export function getCompletionEntryDetails(typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, entryName: string, allSourceFiles: SourceFile[], host?: LanguageServiceHost, rulesProvider?: formatting.RulesProvider): CompletionEntryDetails { + // Compute all the completion symbols again. const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); if (completionData) { @@ -318,6 +319,8 @@ namespace ts.Completions { if (symbol) { let codeActions: CodeAction[]; + if (host && rulesProvider) { + } const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { diff --git a/src/services/services.ts b/src/services/services.ts index 459380a4a86f4..624bc9a560d74 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1375,8 +1375,9 @@ namespace ts { function getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails { synchronizeHostData(); - formattingOptions; - return Completions.getCompletionEntryDetails(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles()); + const ruleProvider = formattingOptions ? getRuleProvider(formattingOptions) : undefined; + return Completions.getCompletionEntryDetails( + program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles(), host, ruleProvider); } function getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol { From ae0ab477f77ce3f871707ae9caee852f5cbb54a1 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:20:06 -0700 Subject: [PATCH 15/40] completions.ts: Add TODO comment --- src/services/completions.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/services/completions.ts b/src/services/completions.ts index f207a0b00a948..bdcfd4588063f 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -844,6 +844,7 @@ namespace ts.Completions { } function getSymbolsFromOtherSourceFileExports(tokenText: string) { + // TODO: I think we can consolidate this with the stuff in importFixes.ts const tokenTextLowerCase = tokenText.toLowerCase(); const symbolIdMap = arrayToMap(symbols, s => getUniqueSymbolIdAsString(s, typeChecker)); From 95a9c01ca8b044990c27211c26d963fe674531c8 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:21:11 -0700 Subject: [PATCH 16/40] importFixes.ts: Add types ImportDeclarationMap and ImportCodeFixContext --- src/services/codefixes/importFixes.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index d6fb8de9259b2..988d20149a5aa 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -11,11 +11,27 @@ namespace ts.codefix { }); type ImportCodeActionKind = "CodeChange" | "InsertingIntoExistingImport" | "NewImport"; + type ImportDeclarationMap = (ImportDeclaration | ImportEqualsDeclaration)[][]; + interface ImportCodeAction extends CodeAction { kind: ImportCodeActionKind; moduleSpecifier?: string; } + export interface ImportCodeFixContext { + host: LanguageServiceHost; + symbolName: string; + newLineCharacter: string; + rulesProvider: formatting.RulesProvider; + sourceFile: SourceFile; + checker: TypeChecker; + compilerOptions: CompilerOptions; + getCanonicalFileName: (fileName: string) => string; + // this is a module id -> module import declaration map + cachedImportDeclarations?: ImportDeclarationMap; + symbolToken?: Node; + } + enum ModuleSpecifierComparison { Better, Equal, From 380b2994b2874b03559dc5b003b65b25d401eb8c Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:30:33 -0700 Subject: [PATCH 17/40] Move getImportDeclarations into getCodeActionForImport, immediately after the implementation --- src/services/codefixes/importFixes.ts | 66 +++++++++++++-------------- 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 988d20149a5aa..8ee63c675f18f 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -207,39 +207,6 @@ namespace ts.codefix { return symbolIdActionMap.getAllActions(); - function getImportDeclarations(moduleSymbol: Symbol) { - const moduleSymbolId = getUniqueSymbolId(moduleSymbol); - - const cached = cachedImportDeclarations[moduleSymbolId]; - if (cached) { - return cached; - } - - const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; - for (const importModuleSpecifier of sourceFile.imports) { - const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); - if (importSymbol === moduleSymbol) { - existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); - } - } - cachedImportDeclarations[moduleSymbolId] = existingDeclarations; - return existingDeclarations; - - function getImportDeclaration(moduleSpecifier: LiteralExpression) { - let node: Node = moduleSpecifier; - while (node) { - if (node.kind === SyntaxKind.ImportDeclaration) { - return node; - } - if (node.kind === SyntaxKind.ImportEqualsDeclaration) { - return node; - } - node = node.parent; - } - return undefined; - } - } - function getUniqueSymbolId(symbol: Symbol) { return getSymbolId(skipAlias(symbol, checker)); } @@ -259,6 +226,39 @@ namespace ts.codefix { return [getCodeActionForNewImport()]; } + function getImportDeclarations(moduleSymbol: Symbol) { + const moduleSymbolId = getUniqueSymbolId(moduleSymbol); + + const cached = cachedImportDeclarations[moduleSymbolId]; + if (cached) { + return cached; + } + + const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; + for (const importModuleSpecifier of sourceFile.imports) { + const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); + if (importSymbol === moduleSymbol) { + existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); + } + } + cachedImportDeclarations[moduleSymbolId] = existingDeclarations; + return existingDeclarations; + + function getImportDeclaration(moduleSpecifier: LiteralExpression) { + let node: Node = moduleSpecifier; + while (node) { + if (node.kind === SyntaxKind.ImportDeclaration) { + return node; + } + if (node.kind === SyntaxKind.ImportEqualsDeclaration) { + return node; + } + node = node.parent; + } + return undefined; + } + } + function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { const actions: ImportCodeAction[] = []; From 22c3373aee618b5ae3e4d9b3731c1eb4ebfc7a45 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:32:05 -0700 Subject: [PATCH 18/40] importFixes.ts: Move createChangeTracker into getCodeActionForImport, immediately after getImportDeclarations --- src/services/codefixes/importFixes.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 8ee63c675f18f..f4fec010e3748 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -258,6 +258,10 @@ namespace ts.codefix { return undefined; } } + + function createChangeTracker() { + return textChanges.ChangeTracker.fromCodeFixContext(context); + } function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { const actions: ImportCodeAction[] = []; @@ -665,10 +669,6 @@ namespace ts.codefix { } - function createChangeTracker() { - return textChanges.ChangeTracker.fromCodeFixContext(context); - } - function createCodeAction( description: DiagnosticMessage, diagnosticArgs: string[], From 8d5e075394d9dcb60c9290608751f57ba7a93e0d Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:36:33 -0700 Subject: [PATCH 19/40] importFixes.ts: Add convertToImportCodeFixContext function and reference it from the getCodeActions lambda --- src/services/codefixes/importFixes.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index f4fec010e3748..9e8e88a45d200 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -138,7 +138,23 @@ namespace ts.codefix { } } + function convertToImportCodeFixContext(context: CodeFixContext) { + const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; + const checker = context.program.getTypeChecker(); + const token = getTokenAtPosition(context.sourceFile, context.span.start); + return { + ...context, + checker, + compilerOptions: context.program.getCompilerOptions(), + cachedImportDeclarations: [], + getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames), + symbolName: token.getText(), + symbolToken: token + }; + } + function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { + convertToImportCodeFixContext; const sourceFile = context.sourceFile; const checker = context.program.getTypeChecker(); const allSourceFiles = context.program.getSourceFiles(); From 2875c15bbd691638d50263e32deb07d2fd4e3b42 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:54:11 -0700 Subject: [PATCH 20/40] importFixes.ts: Add context: ImportCodeFixContext parameter to getCodeActionForImport, update call sites, destructure it, use compilerOptions in getModuleSpecifierForNewImport --- src/services/codefixes/importFixes.ts | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9e8e88a45d200..9d7bc8a0a685d 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -141,7 +141,7 @@ namespace ts.codefix { function convertToImportCodeFixContext(context: CodeFixContext) { const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; const checker = context.program.getTypeChecker(); - const token = getTokenAtPosition(context.sourceFile, context.span.start); + const token = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false); return { ...context, checker, @@ -186,7 +186,7 @@ namespace ts.codefix { Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); } - return getCodeActionForImport(symbol, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); + return getCodeActionForImport(symbol, undefined, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); } const candidateModules = checker.getAmbientModules(); @@ -206,7 +206,7 @@ namespace ts.codefix { if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { // check if this symbol is already used const symbolId = getUniqueSymbolId(localSymbol); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, name, /*isNamespaceImport*/ true)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name, /*isNamespaceImport*/ true)); } } @@ -217,7 +217,7 @@ namespace ts.codefix { const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, name)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name)); } } @@ -232,7 +232,12 @@ namespace ts.codefix { return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; } - function getCodeActionForImport(moduleSymbol: Symbol, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { + function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { + const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; + getCanonicalFileName; + newLineCharacter; + host; + symbolToken; const existingDeclarations = getImportDeclarations(moduleSymbol); if (existingDeclarations.length > 0) { // With an existing import statement, there are more than one actions the user can do. @@ -453,7 +458,7 @@ namespace ts.codefix { const fileName = sourceFile.fileName; const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; const sourceDirectory = getDirectoryPath(fileName); - const options = context.program.getCompilerOptions(); + const options = compilerOptions; return tryGetModuleNameFromAmbientModule() || tryGetModuleNameFromTypeRoots() || From e7d966b2e6d831ca6aea955b713c4f8168ea9e3c Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 18:57:24 -0700 Subject: [PATCH 21/40] importFixes.ts: Remove moduleSymbol parameter from getImportDeclarations and use the ambient one --- src/services/codefixes/importFixes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9d7bc8a0a685d..871d62c2ab9fd 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -238,7 +238,7 @@ namespace ts.codefix { newLineCharacter; host; symbolToken; - const existingDeclarations = getImportDeclarations(moduleSymbol); + const existingDeclarations = getImportDeclarations(); if (existingDeclarations.length > 0) { // With an existing import statement, there are more than one actions the user can do. return getCodeActionsForExistingImport(existingDeclarations); @@ -247,7 +247,7 @@ namespace ts.codefix { return [getCodeActionForNewImport()]; } - function getImportDeclarations(moduleSymbol: Symbol) { + function getImportDeclarations() { const moduleSymbolId = getUniqueSymbolId(moduleSymbol); const cached = cachedImportDeclarations[moduleSymbolId]; From e912a76682830867c2f0dcd3e3c1cdda2223c4ce Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 9 Aug 2017 19:04:18 -0700 Subject: [PATCH 22/40] importFixes.ts: Use cachedImportDeclarations from context in getCodeActionForImport --- src/services/codefixes/importFixes.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 871d62c2ab9fd..167916ffa22b4 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -164,8 +164,6 @@ namespace ts.codefix { const name = token.getText(); const symbolIdActionMap = new ImportCodeActionMap(); - // this is a module id -> module import declaration map - const cachedImportDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[][] = []; let lastImportDeclaration: Node; const currentTokenMeaning = getMeaningFromLocation(token); @@ -238,6 +236,8 @@ namespace ts.codefix { newLineCharacter; host; symbolToken; + const cachedImportDeclarations = context.cachedImportDeclarations || []; + const existingDeclarations = getImportDeclarations(); if (existingDeclarations.length > 0) { // With an existing import statement, there are more than one actions the user can do. From de7b821a8230ab189f9a36e355324de196d948cd Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:23:55 -0700 Subject: [PATCH 23/40] importFixes.ts: Move createCodeAction out, immediately above convertToImportCodeFixContext --- src/services/codefixes/importFixes.ts | 28 +++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 167916ffa22b4..e7f565958e692 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -137,6 +137,20 @@ namespace ts.codefix { return ModuleSpecifierComparison.Equal; } } + + function createCodeAction( + description: DiagnosticMessage, + diagnosticArgs: string[], + changes: FileTextChanges[], + kind: ImportCodeActionKind, + moduleSpecifier?: string): ImportCodeAction { + return { + description: formatMessage.apply(undefined, [undefined, description].concat(diagnosticArgs)), + changes, + kind, + moduleSpecifier + }; + } function convertToImportCodeFixContext(context: CodeFixContext) { const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; @@ -689,19 +703,5 @@ namespace ts.codefix { } } - - function createCodeAction( - description: DiagnosticMessage, - diagnosticArgs: string[], - changes: FileTextChanges[], - kind: ImportCodeActionKind, - moduleSpecifier?: string): ImportCodeAction { - return { - description: formatMessage.apply(undefined, [undefined, description].concat(diagnosticArgs)), - changes, - kind, - moduleSpecifier - }; - } } } From fa33d5016b32a3b8fb86a220f69147a3c1b4a761 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:26:10 -0700 Subject: [PATCH 24/40] Move the declaration for lastImportDeclaration out of the getCodeActions lambda into getCodeActionForImport --- src/services/codefixes/importFixes.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index e7f565958e692..9f946d0195c38 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -178,8 +178,6 @@ namespace ts.codefix { const name = token.getText(); const symbolIdActionMap = new ImportCodeActionMap(); - let lastImportDeclaration: Node; - const currentTokenMeaning = getMeaningFromLocation(token); if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { const umdSymbol = checker.getSymbolAtLocation(token); @@ -245,6 +243,7 @@ namespace ts.codefix { } function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { + let lastImportDeclaration: Node; const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; getCanonicalFileName; newLineCharacter; From 13a47e24052b4d3c97fc0d3600cb8ac3d5c40a96 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:32:44 -0700 Subject: [PATCH 25/40] importFixes.ts: Use symbolToken in getCodeActionForImport --- src/services/codefixes/importFixes.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9f946d0195c38..f0a5477e931a7 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -248,7 +248,6 @@ namespace ts.codefix { getCanonicalFileName; newLineCharacter; host; - symbolToken; const cachedImportDeclarations = context.cachedImportDeclarations || []; const existingDeclarations = getImportDeclarations(); @@ -340,7 +339,7 @@ namespace ts.codefix { } } - if (namespaceImportDeclaration) { + if (symbolToken && namespaceImportDeclaration) { actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); } @@ -422,7 +421,7 @@ namespace ts.codefix { return createCodeAction( Diagnostics.Change_0_to_1, [name, `${namespacePrefix}.${name}`], - createChangeTracker().replaceNode(sourceFile, token, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), + createChangeTracker().replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), "CodeChange" ); } From 8e5febb4c61eabab56a3192e67a6c1f65040c277 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:37:56 -0700 Subject: [PATCH 26/40] importFixes.ts: Remove useCaseSensitiveFileNames altogether from getCodeActions lambda --- src/services/codefixes/importFixes.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index f0a5477e931a7..91afa12b56106 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -172,7 +172,6 @@ namespace ts.codefix { const sourceFile = context.sourceFile; const checker = context.program.getTypeChecker(); const allSourceFiles = context.program.getSourceFiles(); - const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; const token = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false); const name = token.getText(); @@ -439,7 +438,6 @@ namespace ts.codefix { } } - const getCanonicalFileName = createGetCanonicalFileName(useCaseSensitiveFileNames); const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); const changeTracker = createChangeTracker(); const importClause = isDefault From 2ea36a603c05c932efec63960ca2e4b6869199db Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:40:01 -0700 Subject: [PATCH 27/40] importFixes.ts: Remove local getUniqueSymbolId function and add checker parameter to calls to it --- src/services/codefixes/importFixes.ts | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 91afa12b56106..729cce4dad609 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -214,7 +214,7 @@ namespace ts.codefix { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { // check if this symbol is already used - const symbolId = getUniqueSymbolId(localSymbol); + const symbolId = getUniqueSymbolId(localSymbol, checker); symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name, /*isNamespaceImport*/ true)); } } @@ -225,17 +225,13 @@ namespace ts.codefix { // check exports with the same name const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { - const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName); + const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName, checker); symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name)); } } return symbolIdActionMap.getAllActions(); - function getUniqueSymbolId(symbol: Symbol) { - return getSymbolId(skipAlias(symbol, checker)); - } - function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { const declarations = symbol.getDeclarations(); return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; @@ -259,7 +255,7 @@ namespace ts.codefix { } function getImportDeclarations() { - const moduleSymbolId = getUniqueSymbolId(moduleSymbol); + const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker); const cached = cachedImportDeclarations[moduleSymbolId]; if (cached) { From b11f6e8f3e1db267d4078391d8313911bf5fc67c Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:42:00 -0700 Subject: [PATCH 28/40] importFixes.ts: Move getCodeActionForImport out into an export, immediately below convertToImportCodeFixContext --- src/services/codefixes/importFixes.ts | 896 +++++++++++++------------- 1 file changed, 448 insertions(+), 448 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 729cce4dad609..28ab699f380cb 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -167,533 +167,533 @@ namespace ts.codefix { }; } - function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { - convertToImportCodeFixContext; - const sourceFile = context.sourceFile; - const checker = context.program.getTypeChecker(); - const allSourceFiles = context.program.getSourceFiles(); + export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { + let lastImportDeclaration: Node; + const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; + getCanonicalFileName; + newLineCharacter; + host; + const cachedImportDeclarations = context.cachedImportDeclarations || []; + + const existingDeclarations = getImportDeclarations(); + if (existingDeclarations.length > 0) { + // With an existing import statement, there are more than one actions the user can do. + return getCodeActionsForExistingImport(existingDeclarations); + } + else { + return [getCodeActionForNewImport()]; + } - const token = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false); - const name = token.getText(); - const symbolIdActionMap = new ImportCodeActionMap(); + function getImportDeclarations() { + const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker); - const currentTokenMeaning = getMeaningFromLocation(token); - if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { - const umdSymbol = checker.getSymbolAtLocation(token); - let symbol: ts.Symbol; - let symbolName: string; - if (umdSymbol.flags & ts.SymbolFlags.Alias) { - symbol = checker.getAliasedSymbol(umdSymbol); - symbolName = name; + const cached = cachedImportDeclarations[moduleSymbolId]; + if (cached) { + return cached; } - else if (isJsxOpeningLikeElement(token.parent) && token.parent.tagName === token) { - // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`. - symbol = checker.getAliasedSymbol(checker.resolveNameAtLocation(token, checker.getJsxNamespace(), SymbolFlags.Value)); - symbolName = symbol.name; + + const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; + for (const importModuleSpecifier of sourceFile.imports) { + const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); + if (importSymbol === moduleSymbol) { + existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); + } } - else { - Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); + cachedImportDeclarations[moduleSymbolId] = existingDeclarations; + return existingDeclarations; + + function getImportDeclaration(moduleSpecifier: LiteralExpression) { + let node: Node = moduleSpecifier; + while (node) { + if (node.kind === SyntaxKind.ImportDeclaration) { + return node; + } + if (node.kind === SyntaxKind.ImportEqualsDeclaration) { + return node; + } + node = node.parent; + } + return undefined; } - - return getCodeActionForImport(symbol, undefined, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); } - - const candidateModules = checker.getAmbientModules(); - for (const otherSourceFile of allSourceFiles) { - if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { - candidateModules.push(otherSourceFile.symbol); - } + + function createChangeTracker() { + return textChanges.ChangeTracker.fromCodeFixContext(context); } - for (const moduleSymbol of candidateModules) { - context.cancellationToken.throwIfCancellationRequested(); - - // check the default export - const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); - if (defaultExport) { - const localSymbol = getLocalSymbolForExportDefault(defaultExport); - if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { - // check if this symbol is already used - const symbolId = getUniqueSymbolId(localSymbol, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name, /*isNamespaceImport*/ true)); + function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { + const actions: ImportCodeAction[] = []; + + // It is possible that multiple import statements with the same specifier exist in the file. + // e.g. + // + // import * as ns from "foo"; + // import { member1, member2 } from "foo"; + // + // member3/**/ <-- cusor here + // + // in this case we should provie 2 actions: + // 1. change "member3" to "ns.member3" + // 2. add "member3" to the second import statement's import list + // and it is up to the user to decide which one fits best. + let namespaceImportDeclaration: ImportDeclaration | ImportEqualsDeclaration; + let namedImportDeclaration: ImportDeclaration; + let existingModuleSpecifier: string; + for (const declaration of declarations) { + if (declaration.kind === SyntaxKind.ImportDeclaration) { + const namedBindings = declaration.importClause && declaration.importClause.namedBindings; + if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { + // case: + // import * as ns from "foo" + namespaceImportDeclaration = declaration; + } + else { + // cases: + // import default from "foo" + // import { bar } from "foo" or combination with the first one + // import "foo" + namedImportDeclaration = declaration; + } + existingModuleSpecifier = declaration.moduleSpecifier.getText(); + } + else { + // case: + // import foo = require("foo") + namespaceImportDeclaration = declaration; + existingModuleSpecifier = getModuleSpecifierFromImportEqualsDeclaration(declaration); } } - // "default" is a keyword and not a legal identifier for the import, so we don't expect it here - Debug.assert(name !== "default"); - - // check exports with the same name - const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); - if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { - const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name)); + if (symbolToken && namespaceImportDeclaration) { + actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); } - } - return symbolIdActionMap.getAllActions(); - - function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { - const declarations = symbol.getDeclarations(); - return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; - } - - function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { - let lastImportDeclaration: Node; - const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; - getCanonicalFileName; - newLineCharacter; - host; - const cachedImportDeclarations = context.cachedImportDeclarations || []; - - const existingDeclarations = getImportDeclarations(); - if (existingDeclarations.length > 0) { - // With an existing import statement, there are more than one actions the user can do. - return getCodeActionsForExistingImport(existingDeclarations); + if (!isNamespaceImport && namedImportDeclaration && namedImportDeclaration.importClause && + (namedImportDeclaration.importClause.name || namedImportDeclaration.importClause.namedBindings)) { + /** + * If the existing import declaration already has a named import list, just + * insert the identifier into that list. + */ + const fileTextChanges = getTextChangeForImportClause(namedImportDeclaration.importClause); + const moduleSpecifierWithoutQuotes = stripQuotes(namedImportDeclaration.moduleSpecifier.getText()); + actions.push(createCodeAction( + Diagnostics.Add_0_to_existing_import_declaration_from_1, + [name, moduleSpecifierWithoutQuotes], + fileTextChanges, + "InsertingIntoExistingImport", + moduleSpecifierWithoutQuotes + )); } else { - return [getCodeActionForNewImport()]; + // we need to create a new import statement, but the existing module specifier can be reused. + actions.push(getCodeActionForNewImport(existingModuleSpecifier)); } + return actions; - function getImportDeclarations() { - const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker); - - const cached = cachedImportDeclarations[moduleSymbolId]; - if (cached) { - return cached; - } - - const existingDeclarations: (ImportDeclaration | ImportEqualsDeclaration)[] = []; - for (const importModuleSpecifier of sourceFile.imports) { - const importSymbol = checker.getSymbolAtLocation(importModuleSpecifier); - if (importSymbol === moduleSymbol) { - existingDeclarations.push(getImportDeclaration(importModuleSpecifier)); - } - } - cachedImportDeclarations[moduleSymbolId] = existingDeclarations; - return existingDeclarations; - - function getImportDeclaration(moduleSpecifier: LiteralExpression) { - let node: Node = moduleSpecifier; - while (node) { - if (node.kind === SyntaxKind.ImportDeclaration) { - return node; - } - if (node.kind === SyntaxKind.ImportEqualsDeclaration) { - return node; - } - node = node.parent; - } - return undefined; + function getModuleSpecifierFromImportEqualsDeclaration(declaration: ImportEqualsDeclaration) { + if (declaration.moduleReference && declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference) { + return declaration.moduleReference.expression.getText(); } + return declaration.moduleReference.getText(); } - - function createChangeTracker() { - return textChanges.ChangeTracker.fromCodeFixContext(context); - } - - function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { - const actions: ImportCodeAction[] = []; - - // It is possible that multiple import statements with the same specifier exist in the file. - // e.g. - // - // import * as ns from "foo"; - // import { member1, member2 } from "foo"; - // - // member3/**/ <-- cusor here - // - // in this case we should provie 2 actions: - // 1. change "member3" to "ns.member3" - // 2. add "member3" to the second import statement's import list - // and it is up to the user to decide which one fits best. - let namespaceImportDeclaration: ImportDeclaration | ImportEqualsDeclaration; - let namedImportDeclaration: ImportDeclaration; - let existingModuleSpecifier: string; - for (const declaration of declarations) { - if (declaration.kind === SyntaxKind.ImportDeclaration) { - const namedBindings = declaration.importClause && declaration.importClause.namedBindings; - if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { - // case: - // import * as ns from "foo" - namespaceImportDeclaration = declaration; - } - else { - // cases: - // import default from "foo" - // import { bar } from "foo" or combination with the first one - // import "foo" - namedImportDeclaration = declaration; - } - existingModuleSpecifier = declaration.moduleSpecifier.getText(); - } - else { - // case: - // import foo = require("foo") - namespaceImportDeclaration = declaration; - existingModuleSpecifier = getModuleSpecifierFromImportEqualsDeclaration(declaration); - } - } - if (symbolToken && namespaceImportDeclaration) { - actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); + function getTextChangeForImportClause(importClause: ImportClause): FileTextChanges[] { + const importList = importClause.namedBindings; + const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name)); + // case 1: + // original text: import default from "module" + // change to: import default, { name } from "module" + // case 2: + // original text: import {} from "module" + // change to: import { name } from "module" + if (!importList || importList.elements.length === 0) { + const newImportClause = createImportClause(importClause.name, createNamedImports([newImportSpecifier])); + return createChangeTracker().replaceNode(sourceFile, importClause, newImportClause).getChanges(); } - if (!isNamespaceImport && namedImportDeclaration && namedImportDeclaration.importClause && - (namedImportDeclaration.importClause.name || namedImportDeclaration.importClause.namedBindings)) { - /** - * If the existing import declaration already has a named import list, just - * insert the identifier into that list. - */ - const fileTextChanges = getTextChangeForImportClause(namedImportDeclaration.importClause); - const moduleSpecifierWithoutQuotes = stripQuotes(namedImportDeclaration.moduleSpecifier.getText()); - actions.push(createCodeAction( - Diagnostics.Add_0_to_existing_import_declaration_from_1, - [name, moduleSpecifierWithoutQuotes], - fileTextChanges, - "InsertingIntoExistingImport", - moduleSpecifierWithoutQuotes - )); + /** + * If the import list has one import per line, preserve that. Otherwise, insert on same line as last element + * import { + * foo + * } from "./module"; + */ + return createChangeTracker().insertNodeInListAfter( + sourceFile, + importList.elements[importList.elements.length - 1], + newImportSpecifier).getChanges(); + } + + function getCodeActionForNamespaceImport(declaration: ImportDeclaration | ImportEqualsDeclaration): ImportCodeAction { + let namespacePrefix: string; + if (declaration.kind === SyntaxKind.ImportDeclaration) { + namespacePrefix = (declaration.importClause.namedBindings).name.getText(); } else { - // we need to create a new import statement, but the existing module specifier can be reused. - actions.push(getCodeActionForNewImport(existingModuleSpecifier)); - } - return actions; - - function getModuleSpecifierFromImportEqualsDeclaration(declaration: ImportEqualsDeclaration) { - if (declaration.moduleReference && declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference) { - return declaration.moduleReference.expression.getText(); - } - return declaration.moduleReference.getText(); + namespacePrefix = declaration.name.getText(); } + namespacePrefix = stripQuotes(namespacePrefix); + + /** + * Cases: + * import * as ns from "mod" + * import default, * as ns from "mod" + * import ns = require("mod") + * + * Because there is no import list, we alter the reference to include the + * namespace instead of altering the import declaration. For example, "foo" would + * become "ns.foo" + */ + return createCodeAction( + Diagnostics.Change_0_to_1, + [name, `${namespacePrefix}.${name}`], + createChangeTracker().replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), + "CodeChange" + ); + } + } - function getTextChangeForImportClause(importClause: ImportClause): FileTextChanges[] { - const importList = importClause.namedBindings; - const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name)); - // case 1: - // original text: import default from "module" - // change to: import default, { name } from "module" - // case 2: - // original text: import {} from "module" - // change to: import { name } from "module" - if (!importList || importList.elements.length === 0) { - const newImportClause = createImportClause(importClause.name, createNamedImports([newImportSpecifier])); - return createChangeTracker().replaceNode(sourceFile, importClause, newImportClause).getChanges(); + function getCodeActionForNewImport(moduleSpecifier?: string): ImportCodeAction { + if (!lastImportDeclaration) { + // insert after any existing imports + for (let i = sourceFile.statements.length - 1; i >= 0; i--) { + const statement = sourceFile.statements[i]; + if (statement.kind === SyntaxKind.ImportEqualsDeclaration || statement.kind === SyntaxKind.ImportDeclaration) { + lastImportDeclaration = statement; + break; } - - /** - * If the import list has one import per line, preserve that. Otherwise, insert on same line as last element - * import { - * foo - * } from "./module"; - */ - return createChangeTracker().insertNodeInListAfter( - sourceFile, - importList.elements[importList.elements.length - 1], - newImportSpecifier).getChanges(); } + } - function getCodeActionForNamespaceImport(declaration: ImportDeclaration | ImportEqualsDeclaration): ImportCodeAction { - let namespacePrefix: string; - if (declaration.kind === SyntaxKind.ImportDeclaration) { - namespacePrefix = (declaration.importClause.namedBindings).name.getText(); - } - else { - namespacePrefix = declaration.name.getText(); - } - namespacePrefix = stripQuotes(namespacePrefix); - - /** - * Cases: - * import * as ns from "mod" - * import default, * as ns from "mod" - * import ns = require("mod") - * - * Because there is no import list, we alter the reference to include the - * namespace instead of altering the import declaration. For example, "foo" would - * become "ns.foo" - */ - return createCodeAction( - Diagnostics.Change_0_to_1, - [name, `${namespacePrefix}.${name}`], - createChangeTracker().replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), - "CodeChange" - ); - } + const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); + const changeTracker = createChangeTracker(); + const importClause = isDefault + ? createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined) + : isNamespaceImport + ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))) + : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); + const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); + if (!lastImportDeclaration) { + changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); + } + else { + changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter }); } - function getCodeActionForNewImport(moduleSpecifier?: string): ImportCodeAction { - if (!lastImportDeclaration) { - // insert after any existing imports - for (let i = sourceFile.statements.length - 1; i >= 0; i--) { - const statement = sourceFile.statements[i]; - if (statement.kind === SyntaxKind.ImportEqualsDeclaration || statement.kind === SyntaxKind.ImportDeclaration) { - lastImportDeclaration = statement; - break; - } + // if this file doesn't have any import statements, insert an import statement and then insert a new line + // between the only import statement and user code. Otherwise just insert the statement because chances + // are there are already a new line seperating code and import statements. + return createCodeAction( + Diagnostics.Import_0_from_1, + [symbolName, `"${moduleSpecifierWithoutQuotes}"`], + changeTracker.getChanges(), + "NewImport", + moduleSpecifierWithoutQuotes + ); + + function getModuleSpecifierForNewImport() { + const fileName = sourceFile.fileName; + const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; + const sourceDirectory = getDirectoryPath(fileName); + const options = compilerOptions; + + return tryGetModuleNameFromAmbientModule() || + tryGetModuleNameFromTypeRoots() || + tryGetModuleNameAsNodeModule() || + tryGetModuleNameFromBaseUrl() || + tryGetModuleNameFromRootDirs() || + removeFileExtension(getRelativePath(moduleFileName, sourceDirectory)); + + function tryGetModuleNameFromAmbientModule(): string { + const decl = moduleSymbol.valueDeclaration; + if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) { + return decl.name.text; } } - const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); - const changeTracker = createChangeTracker(); - const importClause = isDefault - ? createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined) - : isNamespaceImport - ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))) - : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); - const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); - if (!lastImportDeclaration) { - changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); - } - else { - changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter }); - } - - // if this file doesn't have any import statements, insert an import statement and then insert a new line - // between the only import statement and user code. Otherwise just insert the statement because chances - // are there are already a new line seperating code and import statements. - return createCodeAction( - Diagnostics.Import_0_from_1, - [symbolName, `"${moduleSpecifierWithoutQuotes}"`], - changeTracker.getChanges(), - "NewImport", - moduleSpecifierWithoutQuotes - ); - - function getModuleSpecifierForNewImport() { - const fileName = sourceFile.fileName; - const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; - const sourceDirectory = getDirectoryPath(fileName); - const options = compilerOptions; - - return tryGetModuleNameFromAmbientModule() || - tryGetModuleNameFromTypeRoots() || - tryGetModuleNameAsNodeModule() || - tryGetModuleNameFromBaseUrl() || - tryGetModuleNameFromRootDirs() || - removeFileExtension(getRelativePath(moduleFileName, sourceDirectory)); - - function tryGetModuleNameFromAmbientModule(): string { - const decl = moduleSymbol.valueDeclaration; - if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) { - return decl.name.text; - } + function tryGetModuleNameFromBaseUrl() { + if (!options.baseUrl) { + return undefined; } - function tryGetModuleNameFromBaseUrl() { - if (!options.baseUrl) { - return undefined; - } - - let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl); - if (!relativeName) { - return undefined; - } + let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl); + if (!relativeName) { + return undefined; + } - const relativeNameWithIndex = removeFileExtension(relativeName); - relativeName = removeExtensionAndIndexPostFix(relativeName); + const relativeNameWithIndex = removeFileExtension(relativeName); + relativeName = removeExtensionAndIndexPostFix(relativeName); - if (options.paths) { - for (const key in options.paths) { - for (const pattern of options.paths[key]) { - const indexOfStar = pattern.indexOf("*"); - if (indexOfStar === 0 && pattern.length === 1) { - continue; - } - else if (indexOfStar !== -1) { - const prefix = pattern.substr(0, indexOfStar); - const suffix = pattern.substr(indexOfStar + 1); - if (relativeName.length >= prefix.length + suffix.length && - startsWith(relativeName, prefix) && - endsWith(relativeName, suffix)) { - const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); - return key.replace("\*", matchedStar); - } - } - else if (pattern === relativeName || pattern === relativeNameWithIndex) { - return key; + if (options.paths) { + for (const key in options.paths) { + for (const pattern of options.paths[key]) { + const indexOfStar = pattern.indexOf("*"); + if (indexOfStar === 0 && pattern.length === 1) { + continue; + } + else if (indexOfStar !== -1) { + const prefix = pattern.substr(0, indexOfStar); + const suffix = pattern.substr(indexOfStar + 1); + if (relativeName.length >= prefix.length + suffix.length && + startsWith(relativeName, prefix) && + endsWith(relativeName, suffix)) { + const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); + return key.replace("\*", matchedStar); } } + else if (pattern === relativeName || pattern === relativeNameWithIndex) { + return key; + } } } - - return relativeName; } - function tryGetModuleNameFromRootDirs() { - if (options.rootDirs) { - const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, options.rootDirs); - const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, options.rootDirs); - if (normalizedTargetPath !== undefined) { - const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath) : normalizedTargetPath; - return removeFileExtension(relativePath); - } + return relativeName; + } + + function tryGetModuleNameFromRootDirs() { + if (options.rootDirs) { + const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, options.rootDirs); + const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, options.rootDirs); + if (normalizedTargetPath !== undefined) { + const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath) : normalizedTargetPath; + return removeFileExtension(relativePath); } - return undefined; } + return undefined; + } - function tryGetModuleNameFromTypeRoots() { - const typeRoots = getEffectiveTypeRoots(options, context.host); - if (typeRoots) { - const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); - for (const typeRoot of normalizedTypeRoots) { - if (startsWith(moduleFileName, typeRoot)) { - const relativeFileName = moduleFileName.substring(typeRoot.length + 1); - return removeExtensionAndIndexPostFix(relativeFileName); - } + function tryGetModuleNameFromTypeRoots() { + const typeRoots = getEffectiveTypeRoots(options, context.host); + if (typeRoots) { + const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); + for (const typeRoot of normalizedTypeRoots) { + if (startsWith(moduleFileName, typeRoot)) { + const relativeFileName = moduleFileName.substring(typeRoot.length + 1); + return removeExtensionAndIndexPostFix(relativeFileName); } } } + } - function tryGetModuleNameAsNodeModule() { - if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { - // nothing to do here - return undefined; - } + function tryGetModuleNameAsNodeModule() { + if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { + // nothing to do here + return undefined; + } - const parts = getNodeModulePathParts(moduleFileName); + const parts = getNodeModulePathParts(moduleFileName); - if (!parts) { - return undefined; - } + if (!parts) { + return undefined; + } - // Simplify the full file path to something that can be resolved by Node. - - // If the module could be imported by a directory name, use that directory's name - let moduleSpecifier = getDirectoryOrExtensionlessFileName(moduleFileName); - // Get a path that's relative to node_modules or the importing file's path - moduleSpecifier = getNodeResolvablePath(moduleSpecifier); - // If the module was found in @types, get the actual Node package name - return getPackageNameFromAtTypesDirectory(moduleSpecifier); - - function getDirectoryOrExtensionlessFileName(path: string): string { - // If the file is the main module, it can be imported by the package name - const packageRootPath = path.substring(0, parts.packageRootIndex); - const packageJsonPath = combinePaths(packageRootPath, "package.json"); - if (context.host.fileExists(packageJsonPath)) { - const packageJsonContent = JSON.parse(context.host.readFile(packageJsonPath)); - if (packageJsonContent) { - const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; - if (mainFileRelative) { - const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); - if (mainExportFile === getCanonicalFileName(path)) { - return packageRootPath; - } + // Simplify the full file path to something that can be resolved by Node. + + // If the module could be imported by a directory name, use that directory's name + let moduleSpecifier = getDirectoryOrExtensionlessFileName(moduleFileName); + // Get a path that's relative to node_modules or the importing file's path + moduleSpecifier = getNodeResolvablePath(moduleSpecifier); + // If the module was found in @types, get the actual Node package name + return getPackageNameFromAtTypesDirectory(moduleSpecifier); + + function getDirectoryOrExtensionlessFileName(path: string): string { + // If the file is the main module, it can be imported by the package name + const packageRootPath = path.substring(0, parts.packageRootIndex); + const packageJsonPath = combinePaths(packageRootPath, "package.json"); + if (context.host.fileExists(packageJsonPath)) { + const packageJsonContent = JSON.parse(context.host.readFile(packageJsonPath)); + if (packageJsonContent) { + const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; + if (mainFileRelative) { + const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); + if (mainExportFile === getCanonicalFileName(path)) { + return packageRootPath; } } } - - // We still have a file name - remove the extension - const fullModulePathWithoutExtension = removeFileExtension(path); - - // If the file is /index, it can be imported by its directory name - if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") { - return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); - } - - return fullModulePathWithoutExtension; } - function getNodeResolvablePath(path: string): string { - const basePath = path.substring(0, parts.topLevelNodeModulesIndex); - if (sourceDirectory.indexOf(basePath) === 0) { - // if node_modules folder is in this folder or any of its parent folders, no need to keep it. - return path.substring(parts.topLevelPackageNameIndex + 1); - } - else { - return getRelativePath(path, sourceDirectory); - } + // We still have a file name - remove the extension + const fullModulePathWithoutExtension = removeFileExtension(path); + + // If the file is /index, it can be imported by its directory name + if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") { + return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); } - } - } - function getNodeModulePathParts(fullPath: string) { - // If fullPath can't be valid module file within node_modules, returns undefined. - // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js - // Returns indices: ^ ^ ^ ^ - - let topLevelNodeModulesIndex = 0; - let topLevelPackageNameIndex = 0; - let packageRootIndex = 0; - let fileNameIndex = 0; - - const enum States { - BeforeNodeModules, - NodeModules, - Scope, - PackageContent + return fullModulePathWithoutExtension; } - let partStart = 0; - let partEnd = 0; - let state = States.BeforeNodeModules; - - while (partEnd >= 0) { - partStart = partEnd; - partEnd = fullPath.indexOf("/", partStart + 1); - switch (state) { - case States.BeforeNodeModules: - if (fullPath.indexOf("/node_modules/", partStart) === partStart) { - topLevelNodeModulesIndex = partStart; - topLevelPackageNameIndex = partEnd; - state = States.NodeModules; - } - break; - case States.NodeModules: - case States.Scope: - if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { - state = States.Scope; - } - else { - packageRootIndex = partEnd; - state = States.PackageContent; - } - break; - case States.PackageContent: - if (fullPath.indexOf("/node_modules/", partStart) === partStart) { - state = States.NodeModules; - } - else { - state = States.PackageContent; - } - break; + function getNodeResolvablePath(path: string): string { + const basePath = path.substring(0, parts.topLevelNodeModulesIndex); + if (sourceDirectory.indexOf(basePath) === 0) { + // if node_modules folder is in this folder or any of its parent folders, no need to keep it. + return path.substring(parts.topLevelPackageNameIndex + 1); + } + else { + return getRelativePath(path, sourceDirectory); } } + } + } - fileNameIndex = partStart; - - return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; + function getNodeModulePathParts(fullPath: string) { + // If fullPath can't be valid module file within node_modules, returns undefined. + // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js + // Returns indices: ^ ^ ^ ^ + + let topLevelNodeModulesIndex = 0; + let topLevelPackageNameIndex = 0; + let packageRootIndex = 0; + let fileNameIndex = 0; + + const enum States { + BeforeNodeModules, + NodeModules, + Scope, + PackageContent } - function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { - for (const rootDir of rootDirs) { - const relativeName = getRelativePathIfInDirectory(path, rootDir); - if (relativeName !== undefined) { - return relativeName; - } + let partStart = 0; + let partEnd = 0; + let state = States.BeforeNodeModules; + + while (partEnd >= 0) { + partStart = partEnd; + partEnd = fullPath.indexOf("/", partStart + 1); + switch (state) { + case States.BeforeNodeModules: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + topLevelNodeModulesIndex = partStart; + topLevelPackageNameIndex = partEnd; + state = States.NodeModules; + } + break; + case States.NodeModules: + case States.Scope: + if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { + state = States.Scope; + } + else { + packageRootIndex = partEnd; + state = States.PackageContent; + } + break; + case States.PackageContent: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + state = States.NodeModules; + } + else { + state = States.PackageContent; + } + break; } - return undefined; } - function removeExtensionAndIndexPostFix(fileName: string) { - fileName = removeFileExtension(fileName); - if (endsWith(fileName, "/index")) { - fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); + fileNameIndex = partStart; + + return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; + } + + function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { + for (const rootDir of rootDirs) { + const relativeName = getRelativePathIfInDirectory(path, rootDir); + if (relativeName !== undefined) { + return relativeName; } - return fileName; } + return undefined; + } - function getRelativePathIfInDirectory(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; + function removeExtensionAndIndexPostFix(fileName: string) { + fileName = removeFileExtension(fileName); + if (endsWith(fileName, "/index")) { + fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); } + return fileName; + } + + function getRelativePathIfInDirectory(path: string, directoryPath: string) { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; + } + + function getRelativePath(path: string, directoryPath: string) { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return !pathIsRelative(relativePath) ? "./" + relativePath : relativePath; + } + } + + } + + function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { + convertToImportCodeFixContext; + const sourceFile = context.sourceFile; + const checker = context.program.getTypeChecker(); + const allSourceFiles = context.program.getSourceFiles(); + + const token = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false); + const name = token.getText(); + const symbolIdActionMap = new ImportCodeActionMap(); + + const currentTokenMeaning = getMeaningFromLocation(token); + if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { + const umdSymbol = checker.getSymbolAtLocation(token); + let symbol: ts.Symbol; + let symbolName: string; + if (umdSymbol.flags & ts.SymbolFlags.Alias) { + symbol = checker.getAliasedSymbol(umdSymbol); + symbolName = name; + } + else if (isJsxOpeningLikeElement(token.parent) && token.parent.tagName === token) { + // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`. + symbol = checker.getAliasedSymbol(checker.resolveNameAtLocation(token, checker.getJsxNamespace(), SymbolFlags.Value)); + symbolName = symbol.name; + } + else { + Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); + } + + return getCodeActionForImport(symbol, undefined, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); + } + + const candidateModules = checker.getAmbientModules(); + for (const otherSourceFile of allSourceFiles) { + if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { + candidateModules.push(otherSourceFile.symbol); + } + } + + for (const moduleSymbol of candidateModules) { + context.cancellationToken.throwIfCancellationRequested(); - function getRelativePath(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return !pathIsRelative(relativePath) ? "./" + relativePath : relativePath; + // check the default export + const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); + if (defaultExport) { + const localSymbol = getLocalSymbolForExportDefault(defaultExport); + if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { + // check if this symbol is already used + const symbolId = getUniqueSymbolId(localSymbol, checker); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name, /*isNamespaceImport*/ true)); } } + // "default" is a keyword and not a legal identifier for the import, so we don't expect it here + Debug.assert(name !== "default"); + + // check exports with the same name + const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); + if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { + const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName, checker); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name)); + } + } + + return symbolIdActionMap.getAllActions(); + + function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { + const declarations = symbol.getDeclarations(); + return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; } } } From 72dd99fec91ab079739d13399bcc32e942839809 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:44:07 -0700 Subject: [PATCH 29/40] completions.ts: In getCompletionEntryDetails, if there's symbolOriginInfo, call getCodeActionForImport --- src/services/completions.ts | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/services/completions.ts b/src/services/completions.ts index bdcfd4588063f..50d8069931572 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -309,7 +309,6 @@ namespace ts.Completions { const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); if (completionData) { const { symbols, location, symbolToOriginInfoMap } = completionData; - symbolToOriginInfoMap; // Find the symbol with the matching entry name. // We don't need to perform character checks here because we're only comparing the @@ -320,6 +319,22 @@ namespace ts.Completions { if (symbol) { let codeActions: CodeAction[]; if (host && rulesProvider) { + const symbolOriginInfo = symbolToOriginInfoMap.get(getUniqueSymbolIdAsString(symbol, typeChecker)); + if (symbolOriginInfo) { + const useCaseSensitiveFileNames = host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false; + const context: codefix.ImportCodeFixContext = { + host, + checker: typeChecker, + newLineCharacter: host.getNewLine(), + compilerOptions, + sourceFile, + rulesProvider, + symbolName: symbol.name, + getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames) + }; + + codeActions = codefix.getCodeActionForImport(/*moduleSymbol*/ symbolOriginInfo.moduleSymbol, context, /*isDefault*/ symbolOriginInfo.isDefaultExport); + } } const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); @@ -844,7 +859,6 @@ namespace ts.Completions { } function getSymbolsFromOtherSourceFileExports(tokenText: string) { - // TODO: I think we can consolidate this with the stuff in importFixes.ts const tokenTextLowerCase = tokenText.toLowerCase(); const symbolIdMap = arrayToMap(symbols, s => getUniqueSymbolIdAsString(s, typeChecker)); From e68c95193013389ee5acf9839d24fa8b25c87948 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:48:38 -0700 Subject: [PATCH 30/40] importFixes.ts: Create and use importFixContext within getCodeActions lambda --- src/services/codefixes/importFixes.ts | 17 +++++++++-------- src/services/completions.ts | 2 +- 2 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 28ab699f380cb..1b892b7a9e689 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -627,16 +627,17 @@ namespace ts.codefix { } function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { - convertToImportCodeFixContext; const sourceFile = context.sourceFile; - const checker = context.program.getTypeChecker(); const allSourceFiles = context.program.getSourceFiles(); + const importFixContext = convertToImportCodeFixContext(context); - const token = getTokenAtPosition(sourceFile, context.span.start, /*includeJsDocComment*/ false); - const name = token.getText(); + const checker = importFixContext.checker; + const token = importFixContext.symbolToken; const symbolIdActionMap = new ImportCodeActionMap(); - const currentTokenMeaning = getMeaningFromLocation(token); + + const name = importFixContext.symbolName; + if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { const umdSymbol = checker.getSymbolAtLocation(token); let symbol: ts.Symbol; @@ -654,7 +655,7 @@ namespace ts.codefix { Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); } - return getCodeActionForImport(symbol, undefined, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); + return getCodeActionForImport(symbol, importFixContext, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); } const candidateModules = checker.getAmbientModules(); @@ -674,7 +675,7 @@ namespace ts.codefix { if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { // check if this symbol is already used const symbolId = getUniqueSymbolId(localSymbol, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name, /*isNamespaceImport*/ true)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, importFixContext, name, /*isNamespaceImport*/ true)); } } @@ -685,7 +686,7 @@ namespace ts.codefix { const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, undefined, name)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, importFixContext, name)); } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 50d8069931572..47a1a10fc80ce 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -333,7 +333,7 @@ namespace ts.Completions { getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames) }; - codeActions = codefix.getCodeActionForImport(/*moduleSymbol*/ symbolOriginInfo.moduleSymbol, context, /*isDefault*/ symbolOriginInfo.isDefaultExport); + codeActions = codefix.getCodeActionForImport(/*moduleSymbol*/ symbolOriginInfo.moduleSymbol, context, context.symbolName, /*isDefault*/ symbolOriginInfo.isDefaultExport); } } From bc14bb0fe4362f9b2036bdd92ef9bf4502974405 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:52:08 -0700 Subject: [PATCH 31/40] importFixes.ts: Use local newLineCharacter instead of context.newLineCharacter in getCodeActionForImport --- src/services/codefixes/importFixes.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 1b892b7a9e689..96ae540997d9d 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -171,7 +171,6 @@ namespace ts.codefix { let lastImportDeclaration: Node; const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; getCanonicalFileName; - newLineCharacter; host; const cachedImportDeclarations = context.cachedImportDeclarations || []; @@ -373,10 +372,10 @@ namespace ts.codefix { : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, createLiteral(moduleSpecifierWithoutQuotes)); if (!lastImportDeclaration) { - changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); + changeTracker.insertNodeAt(sourceFile, sourceFile.getStart(), importDecl, { suffix: `${newLineCharacter}${newLineCharacter}` }); } else { - changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: context.newLineCharacter }); + changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter }); } // if this file doesn't have any import statements, insert an import statement and then insert a new line From d1bdc25a9a7a882a9348a3126ba14c5258518de3 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:53:33 -0700 Subject: [PATCH 32/40] importFixes.ts: Use local host instead of context.host in getCodeActionForImport --- src/services/codefixes/importFixes.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 96ae540997d9d..af95099428441 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -171,7 +171,6 @@ namespace ts.codefix { let lastImportDeclaration: Node; const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; getCanonicalFileName; - host; const cachedImportDeclarations = context.cachedImportDeclarations || []; const existingDeclarations = getImportDeclarations(); @@ -462,7 +461,7 @@ namespace ts.codefix { } function tryGetModuleNameFromTypeRoots() { - const typeRoots = getEffectiveTypeRoots(options, context.host); + const typeRoots = getEffectiveTypeRoots(options, host); if (typeRoots) { const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); for (const typeRoot of normalizedTypeRoots) { @@ -499,8 +498,8 @@ namespace ts.codefix { // If the file is the main module, it can be imported by the package name const packageRootPath = path.substring(0, parts.packageRootIndex); const packageJsonPath = combinePaths(packageRootPath, "package.json"); - if (context.host.fileExists(packageJsonPath)) { - const packageJsonContent = JSON.parse(context.host.readFile(packageJsonPath)); + if (host.fileExists(packageJsonPath)) { + const packageJsonContent = JSON.parse(host.readFile(packageJsonPath)); if (packageJsonContent) { const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; if (mainFileRelative) { From f0c983a605eea267169d6e41725cce0217666796 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Thu, 10 Aug 2017 10:54:08 -0700 Subject: [PATCH 33/40] importFixes.ts: Remove dummy getCanonicalFileName line --- src/services/codefixes/importFixes.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index af95099428441..92402da11fa28 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -170,7 +170,6 @@ namespace ts.codefix { export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { let lastImportDeclaration: Node; const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; - getCanonicalFileName; const cachedImportDeclarations = context.cachedImportDeclarations || []; const existingDeclarations = getImportDeclarations(); From a41f3dfff635edf989f140ea4d8886e32c9f3744 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 16 Aug 2017 10:49:35 -0700 Subject: [PATCH 34/40] Filter symbols after gathering exports instead of before --- .../unittests/tsserverProjectSystem.ts | 20 +++++++++---------- src/services/completions.ts | 15 ++++++++------ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index d0641caa825fd..a8e187e11c8a3 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3663,7 +3663,7 @@ namespace ts.projectSystem { const service = createProjectService(host); service.openClientFile(file2.path); - const completions1 = service.configuredProjects[0].getLanguageService().getCompletionsAtPosition(file2.path, file2.path.length); + const completions1 = service.configuredProjects[0].getLanguageService().getCompletionsAtPosition(file2.path, file2.content.length); const test1Entry = find(completions1.entries, e => e.name === "Test1"); const test2Entry = find(completions1.entries, e => e.name === "Test2"); @@ -4013,7 +4013,7 @@ namespace ts.projectSystem { }; const file1 = { path: "/a/b/file2.ts", - content: `` + content: `var x:` }; const globalFile = { path: "/a/b/globalFile.ts", @@ -4041,17 +4041,17 @@ namespace ts.projectSystem { const projectService = session.getProjectService(); projectService.openClientFile(file1.path); - checkEntryDetail("guitar", /*hasAction*/ true, `import { guitar } from "./moduleFile";\n\n`); - checkEntryDetail("Jazz", /*hasAction*/ false); - checkEntryDetail("chetAtkins", /*hasAction*/ true, `import { chetAtkins } from "windyAndWarm";\n\n`); - checkEntryDetail("egyptianElla", /*hasAction*/ true, `import egyptianElla from "./defaultModuleFile";\n\n`); + checkEntryDetail(1, "guitar", /*hasAction*/ true, `import { guitar } from "./moduleFile";\n\n`); + checkEntryDetail(1, "chetAtkins", /*hasAction*/ true, `import { chetAtkins } from "windyAndWarm";\n\n`); + checkEntryDetail(1, "egyptianElla", /*hasAction*/ true, `import egyptianElla from "./defaultModuleFile";\n\n`); + checkEntryDetail(7, "Jazz", /*hasAction*/ false); - function checkEntryDetail(entryName: string, hasAction: boolean, insertString?: string) { + function checkEntryDetail(offset: number, entryName: string, hasAction: boolean, insertString?: string) { const request = makeSessionRequest( CommandNames.CompletionDetails, - { entryNames: [entryName], file: file1.path, line: 1, offset: 0, projectFileName: configFile.path }); + { entryNames: [entryName], file: file1.path, line: 1, offset: offset, projectFileName: configFile.path }); const response = session.executeCommand(request).response as protocol.CompletionEntryDetails[]; - assert.isTrue(response.length === 1); + assert.equal(response.length, 1); const entryDetails = response[0]; if (!hasAction) { @@ -4059,7 +4059,7 @@ namespace ts.projectSystem { } else { const action = entryDetails.codeActions[0]; - assert.isTrue(action.changes[0].fileName === file1.path); + assert.equal(action.changes[0].fileName, file1.path); assert.deepEqual(action.changes[0], { fileName: file1.path, textChanges: [{ start: { line: 1, offset: 1 }, end: { line: 1, offset: 1 }, newText: insertString }] diff --git a/src/services/completions.ts b/src/services/completions.ts index 47a1a10fc80ce..a8ef84b31fbd3 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -776,9 +776,10 @@ namespace ts.Completions { } const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; - symbols = filterGlobalCompletion(typeChecker.getSymbolsInScope(scopeNode, symbolMeanings)); - getSymbolsFromOtherSourceFileExports(previousToken === undefined ? "" : previousToken.getText()); + symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings); + symbols.push(...getSymbolsFromOtherSourceFileExports(symbols, previousToken === undefined ? "" : previousToken.getText())); + symbols = filterGlobalCompletion(symbols); return true; } @@ -858,9 +859,10 @@ namespace ts.Completions { } } - function getSymbolsFromOtherSourceFileExports(tokenText: string) { + function getSymbolsFromOtherSourceFileExports(knownSymbols: Symbol[], tokenText: string): Symbol[] { + let otherSourceFileExports: Symbol[] = []; const tokenTextLowerCase = tokenText.toLowerCase(); - const symbolIdMap = arrayToMap(symbols, s => getUniqueSymbolIdAsString(s, typeChecker)); + const symbolIdMap = arrayToMap(knownSymbols, s => getUniqueSymbolIdAsString(s, typeChecker)); const allPotentialModules = getOtherModuleSymbols(allSourceFiles, sourceFile, typeChecker); for (const moduleSymbol of allPotentialModules) { @@ -869,7 +871,7 @@ namespace ts.Completions { if (defaultExport) { const localSymbol = getLocalSymbolForExportDefault(defaultExport); if (localSymbol && !symbolIdMap.has(getUniqueSymbolIdAsString(localSymbol, typeChecker)) && startsWith(localSymbol.name.toLowerCase(), tokenTextLowerCase)) { - symbols.push(localSymbol); + otherSourceFileExports.push(localSymbol); symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(localSymbol, typeChecker), { moduleSymbol, isDefaultExport: true }); } } @@ -879,12 +881,13 @@ namespace ts.Completions { if (allExportedSymbols) { for (const exportedSymbol of allExportedSymbols) { if (exportedSymbol.name && !symbolIdMap.has(getUniqueSymbolIdAsString(exportedSymbol, typeChecker)) && startsWith(exportedSymbol.name.toLowerCase(), tokenTextLowerCase)) { - symbols.push(exportedSymbol); + otherSourceFileExports.push(exportedSymbol); symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(exportedSymbol, typeChecker), { moduleSymbol }); } } } } + return otherSourceFileExports; } /** From 211e3f92bb1b702270187db6545afd3e9adb5a21 Mon Sep 17 00:00:00 2001 From: Mine Starks Date: Wed, 16 Aug 2017 16:59:07 -0700 Subject: [PATCH 35/40] Lint --- src/harness/unittests/tsserverProjectSystem.ts | 2 +- src/services/codefixes/importFixes.ts | 6 +++--- src/services/completions.ts | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 8044dc51e2956..41fbc977da62b 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -4171,7 +4171,7 @@ namespace ts.projectSystem { function checkEntryDetail(offset: number, entryName: string, hasAction: boolean, insertString?: string) { const request = makeSessionRequest( CommandNames.CompletionDetails, - { entryNames: [entryName], file: file1.path, line: 1, offset: offset, projectFileName: configFile.path }); + { entryNames: [entryName], file: file1.path, line: 1, offset, projectFileName: configFile.path }); const response = session.executeCommand(request).response as protocol.CompletionEntryDetails[]; assert.equal(response.length, 1); diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 891400201a8ab..4b2fd109df554 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -137,7 +137,7 @@ namespace ts.codefix { return ModuleSpecifierComparison.Equal; } } - + function createCodeAction( description: DiagnosticMessage, diagnosticArgs: string[], @@ -213,7 +213,7 @@ namespace ts.codefix { return undefined; } } - + function createChangeTracker() { return textChanges.ChangeTracker.fromCodeFixContext(context); } @@ -388,7 +388,7 @@ namespace ts.codefix { "NewImport", moduleSpecifierWithoutQuotes ); - + function getSourceFileImportLocation(node: SourceFile) { // For a source file, it is possible there are detached comments we should not skip const text = node.text; diff --git a/src/services/completions.ts b/src/services/completions.ts index 2fe4ae0fd255b..b0f26b8da985a 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -860,7 +860,7 @@ namespace ts.Completions { } function getSymbolsFromOtherSourceFileExports(knownSymbols: Symbol[], tokenText: string): Symbol[] { - let otherSourceFileExports: Symbol[] = []; + const otherSourceFileExports: Symbol[] = []; const tokenTextLowerCase = tokenText.toLowerCase(); const symbolIdMap = arrayToMap(knownSymbols, s => getUniqueSymbolIdAsString(s, typeChecker)); From 64e30dbc25539596bee62e0831b0d451d385d2b8 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 27 Sep 2017 11:39:20 -0700 Subject: [PATCH 36/40] Test, fix bugs, refactor --- src/compiler/checker.ts | 30 +- src/compiler/commandLineParser.ts | 4 +- src/compiler/core.ts | 30 + src/compiler/diagnosticMessages.json | 4 +- src/compiler/moduleNameResolver.ts | 6 +- src/compiler/types.ts | 1 + src/compiler/utilities.ts | 15 + src/harness/fourslash.ts | 67 +- src/harness/harnessLanguageService.ts | 4 +- .../unittests/tsserverProjectSystem.ts | 101 -- src/server/protocol.ts | 2 +- src/services/codeFixProvider.ts | 4 +- src/services/codefixes/importFixes.ts | 945 +++++++++--------- src/services/completions.ts | 144 ++- src/services/pathCompletions.ts | 2 +- src/services/refactorProvider.ts | 4 +- src/services/shims.ts | 13 +- src/services/textChanges.ts | 14 +- src/services/types.ts | 3 +- src/services/utilities.ts | 31 +- ...letionsImport_default_addToNamedImports.ts | 19 + ...ionsImport_default_addToNamespaceImport.ts | 18 + ...Import_default_alreadyExistedWithRename.ts | 20 + ...letionsImport_default_didNotExistBefore.ts | 19 + .../completionsImport_fromAmbientModule.ts | 18 + ...mpletionsImport_named_addToNamedImports.ts | 19 + ...mpletionsImport_named_didNotExistBefore.ts | 20 + ...tionsImport_named_namespaceImportExists.ts | 20 + tests/cases/fourslash/fourslash.ts | 23 +- .../importNameCodeFixOptionalImport0.ts | 2 +- 30 files changed, 872 insertions(+), 730 deletions(-) create mode 100644 tests/cases/fourslash/completionsImport_default_addToNamedImports.ts create mode 100644 tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts create mode 100644 tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts create mode 100644 tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts create mode 100644 tests/cases/fourslash/completionsImport_fromAmbientModule.ts create mode 100644 tests/cases/fourslash/completionsImport_named_addToNamedImports.ts create mode 100644 tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts create mode 100644 tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts index f315ed489efae..085ee53a0d549 100644 --- a/src/compiler/checker.ts +++ b/src/compiler/checker.ts @@ -290,6 +290,7 @@ namespace ts { const jsObjectLiteralIndexInfo = createIndexInfo(anyType, /*isReadonly*/ false); const globals = createSymbolTable(); + let ambientModulesCache: Symbol[] | undefined; /** * List of every ambient module with a "*" wildcard. * Unlike other ambient modules, these can't be stored in `globals` because symbol tables only deal with exact matches. @@ -6251,13 +6252,11 @@ namespace ts { function symbolsToArray(symbols: SymbolTable): Symbol[] { const result: Symbol[] = []; - if (symbols) { - symbols.forEach((symbol, id) => { - if (!isReservedMemberName(id)) { - result.push(symbol); - } - }); - } + symbols.forEach((symbol, id) => { + if (!isReservedMemberName(id)) { + result.push(symbol); + } + }); return result; } @@ -25229,13 +25228,16 @@ namespace ts { } function getAmbientModules(): Symbol[] { - const result: Symbol[] = []; - globals.forEach((global, sym) => { - if (ambientModuleSymbolRegex.test(unescapeLeadingUnderscores(sym))) { - result.push(global); - } - }); - return result; + if (!ambientModulesCache) { + ambientModulesCache = []; + globals.forEach((global, sym) => { + // No need to `unescapeLeadingUnderscores`, an escaped symbol is never an ambient module. + if (ambientModuleSymbolRegex.test(sym as string)) { + ambientModulesCache.push(global); + } + }); + } + return ambientModulesCache; } function checkGrammarImportCallExpression(node: ImportCall): boolean { diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts index 54e5ee1d01dbc..edb581326aa6e 100644 --- a/src/compiler/commandLineParser.ts +++ b/src/compiler/commandLineParser.ts @@ -1176,8 +1176,8 @@ namespace ts { } } - function isDoubleQuotedString(node: Node) { - return node.kind === SyntaxKind.StringLiteral && getSourceTextOfNodeFromSourceFile(sourceFile, node).charCodeAt(0) === CharacterCodes.doubleQuote; + function isDoubleQuotedString(node: Node): boolean { + return isStringLiteral(node) && isStringDoubleQuoted(node, sourceFile); } } diff --git a/src/compiler/core.ts b/src/compiler/core.ts index ffa010c655127..b54b1bdf8c159 100644 --- a/src/compiler/core.ts +++ b/src/compiler/core.ts @@ -191,6 +191,18 @@ namespace ts { } return undefined; } + + /** Like `forEach`, but suitable for use with numbers and strings (which may be falsy). */ + export function firstDefined(array: ReadonlyArray | undefined, callback: (element: T, index: number) => U | undefined): U | undefined { + for (let i = 0; i < array.length; i++) { + const result = callback(array[i], i); + if (result !== undefined) { + return result; + } + } + return undefined; + } + /** * Iterates through the parent chain of a node and performs the callback on each parent until the callback * returns a truthy value, then returns that value. @@ -257,6 +269,16 @@ namespace ts { return undefined; } + export function findLast(array: ReadonlyArray, predicate: (element: T, index: number) => boolean): T | undefined { + for (let i = array.length - 1; i >= 0; i--) { + const value = array[i]; + if (predicate(value, i)) { + return value; + } + } + return undefined; + } + /** Works like Array.prototype.findIndex, returning `-1` if no element satisfying the predicate is found. */ export function findIndex(array: ReadonlyArray, predicate: (element: T, index: number) => boolean): number { for (let i = 0; i < array.length; i++) { @@ -1128,6 +1150,14 @@ namespace ts { return result; } + export function arrayToNumericMap(array: ReadonlyArray, makeKey: (value: T) => number): T[] { + const result: T[] = []; + for (const value of array) { + result[makeKey(value)] = value; + } + return result; + } + /** * Creates a set from the elements of an array. * diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index bf8fcdc4f84b3..3307233013648 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -3633,7 +3633,7 @@ "category": "Error", "code": 90010 }, - "Import {0} from {1}.": { + "Import '{0}' from \"{1}\".": { "category": "Message", "code": 90013 }, @@ -3641,7 +3641,7 @@ "category": "Message", "code": 90014 }, - "Add {0} to existing import declaration from {1}.": { + "Add '{0}' to existing import declaration from \"{1}\".": { "category": "Message", "code": 90015 }, diff --git a/src/compiler/moduleNameResolver.ts b/src/compiler/moduleNameResolver.ts index 84256b3a1b1d9..b06b835e35806 100644 --- a/src/compiler/moduleNameResolver.ts +++ b/src/compiler/moduleNameResolver.ts @@ -124,7 +124,11 @@ namespace ts { } } - export function getEffectiveTypeRoots(options: CompilerOptions, host: { directoryExists?: (directoryName: string) => boolean, getCurrentDirectory?: () => string }): string[] | undefined { + export interface GetEffectiveTypeRootsHost { + directoryExists?(directoryName: string): boolean; + getCurrentDirectory?(): string; + } + export function getEffectiveTypeRoots(options: CompilerOptions, host: GetEffectiveTypeRootsHost): string[] | undefined { if (options.typeRoots) { return options.typeRoots; } diff --git a/src/compiler/types.ts b/src/compiler/types.ts index 7a120064a64cb..9932fdd11ef5c 100644 --- a/src/compiler/types.ts +++ b/src/compiler/types.ts @@ -1055,6 +1055,7 @@ namespace ts { export interface StringLiteral extends LiteralExpression { kind: SyntaxKind.StringLiteral; /* @internal */ textSourceNode?: Identifier | StringLiteral | NumericLiteral; // Allows a StringLiteral to get its text from another node (used by transforms). + /** Note: this is only set when synthesizing a node, not during parsing. */ /* @internal */ singleQuote?: boolean; } diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index cc7002ca0590c..a77ddc1454e94 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -517,6 +517,17 @@ namespace ts { } } + /* @internal */ + export function isAnyImportSyntax(node: Node): node is AnyImportSyntax { + switch (node.kind) { + case SyntaxKind.ImportDeclaration: + case SyntaxKind.ImportEqualsDeclaration: + return true; + default: + return false; + } + } + // 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 { @@ -1372,6 +1383,10 @@ namespace ts { return charCode === CharacterCodes.singleQuote || charCode === CharacterCodes.doubleQuote; } + export function isStringDoubleQuoted(string: StringLiteral, sourceFile: SourceFile): boolean { + return getSourceTextOfNodeFromSourceFile(sourceFile, string).charCodeAt(0) === CharacterCodes.doubleQuote; + } + /** * Returns true if the node is a variable declaration whose initializer is a function expression. * This function does not test if the node is in a JavaScript file or not. diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 13db228e8898c..8cfbe04f4e4bf 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -783,10 +783,10 @@ namespace FourSlash { }); } - public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) { + public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { const completions = this.getCompletionListAtCaret(); if (completions) { - this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex); + this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex, hasAction); } else { this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`); @@ -1128,7 +1128,7 @@ namespace FourSlash { } private getCompletionEntryDetails(entryName: string) { - return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName); + return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings); } private getReferencesAtCaret() { @@ -2290,6 +2290,29 @@ namespace FourSlash { this.applyCodeActions(this.getCodeFixActions(fileName, errorCode), index); } + public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) { + this.goToMarker(markerName); + + const actualCompletion = this.getCompletionListAtCaret().entries.find(e => e.name === options.name); + + if (!actualCompletion.hasAction) { + this.raiseError(`Completion for ${options.name} does not have an associated action.`); + } + + const details = this.getCompletionEntryDetails(options.name); + if (details.codeActions.length !== 1) { + this.raiseError(`Expected one code action, got ${details.codeActions.length}`); + } + + if (details.codeActions[0].description !== options.description) { + this.raiseError(`Expected description to be:\n${options.description}\ngot:\n${details.codeActions[0].description}`); + } + + this.applyCodeActions(details.codeActions); + + this.verifyNewContent(options); + } + public verifyRangeIs(expectedText: string, includeWhiteSpace?: boolean) { const ranges = this.getRanges(); if (ranges.length !== 1) { @@ -2361,6 +2384,10 @@ namespace FourSlash { this.applyEdits(change.fileName, change.textChanges, /*isFormattingEdit*/ false); } + this.verifyNewContent(options); + } + + private verifyNewContent(options: FourSlashInterface.NewContentOptions) { if (options.newFileContent) { assert(!options.newRangeContent); this.verifyCurrentFileContent(options.newFileContent); @@ -2934,7 +2961,15 @@ namespace FourSlash { return text.substring(startPos, endPos); } - private assertItemInCompletionList(items: ts.CompletionEntry[], name: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) { + private assertItemInCompletionList( + items: ts.CompletionEntry[], + name: string, + text: string | undefined, + documentation: string | undefined, + kind: string | undefined, + spanIndex: number | undefined, + hasAction: boolean | undefined, + ) { for (const item of items) { if (item.name === name) { if (documentation !== undefined || text !== undefined) { @@ -2957,6 +2992,8 @@ namespace FourSlash { assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + name)); } + assert.equal(item.hasAction, hasAction); + return; } } @@ -3670,12 +3707,12 @@ namespace FourSlashInterface { // Verifies the completion list contains the specified symbol. The // completion list is brought up if necessary - public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number) { + public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { if (this.negative) { this.state.verifyCompletionListDoesNotContain(symbol, text, documentation, kind, spanIndex); } else { - this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex); + this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex, hasAction); } } @@ -4000,6 +4037,10 @@ namespace FourSlashInterface { this.state.getAndApplyCodeActions(errorCode, index); } + public applyCodeActionFromCompletion(markerName: string, options: VerifyCompletionActionOptions): void { + this.state.applyCodeActionFromCompletion(markerName, options); + } + public importFixAtPosition(expectedTextArray: string[], errorCode?: number): void { this.state.verifyImportFixAtPosition(expectedTextArray, errorCode); } @@ -4397,12 +4438,20 @@ namespace FourSlashInterface { isNewIdentifierLocation?: boolean; } - export interface VerifyCodeFixOptions { - description: string; - // One of these should be defined. + export interface NewContentOptions { + // Exactly one of these should be defined. newFileContent?: string; newRangeContent?: string; + } + + export interface VerifyCodeFixOptions extends NewContentOptions { + description: string; errorCode?: number; index?: number; } + + export interface VerifyCompletionActionOptions extends NewContentOptions { + name: string; + description: string; + } } diff --git a/src/harness/harnessLanguageService.ts b/src/harness/harnessLanguageService.ts index 23bc08108c764..cab027e5bdac2 100644 --- a/src/harness/harnessLanguageService.ts +++ b/src/harness/harnessLanguageService.ts @@ -405,8 +405,8 @@ namespace Harness.LanguageService { getCompletionsAtPosition(fileName: string, position: number): ts.CompletionInfo { return unwrapJSONCallResult(this.shim.getCompletionsAtPosition(fileName, position)); } - getCompletionEntryDetails(fileName: string, position: number, entryName: string): ts.CompletionEntryDetails { - return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName)); + getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: ts.FormatCodeOptions): ts.CompletionEntryDetails { + return unwrapJSONCallResult(this.shim.getCompletionEntryDetails(fileName, position, entryName, JSON.stringify(options))); } getCompletionEntrySymbol(): ts.Symbol { throw new Error("getCompletionEntrySymbol not implemented across the shim layer."); diff --git a/src/harness/unittests/tsserverProjectSystem.ts b/src/harness/unittests/tsserverProjectSystem.ts index 3871394e44a0b..651dbae7142de 100644 --- a/src/harness/unittests/tsserverProjectSystem.ts +++ b/src/harness/unittests/tsserverProjectSystem.ts @@ -3788,43 +3788,6 @@ namespace ts.projectSystem { }); }); - describe("import in completion list", () => { - it("should include exported members of all source files", () => { - const file1: FileOrFolder = { - path: "/a/b/file1.ts", - content: ` - export function Test1() { } - export function Test2() { } - ` - }; - const file2: FileOrFolder = { - path: "/a/b/file2.ts", - content: ` - import { Test2 } from "./file1"; - - t` - }; - const configFile: FileOrFolder = { - path: "/a/b/tsconfig.json", - content: "{}" - }; - - const host = createServerHost([file1, file2, configFile]); - const service = createProjectService(host); - service.openClientFile(file2.path); - - const completions1 = service.configuredProjects[0].getLanguageService().getCompletionsAtPosition(file2.path, file2.content.length); - const test1Entry = find(completions1.entries, e => e.name === "Test1"); - const test2Entry = find(completions1.entries, e => e.name === "Test2"); - - assert.isDefined(test1Entry, "should contain 'Test1'"); - assert.isDefined(test2Entry, "should contain 'Test2'"); - - assert.isTrue(test1Entry.hasAction, "should set the 'hasAction' property to true for Test1"); - assert.isUndefined(test2Entry.hasAction, "should not set the 'hasAction' property for Test2"); - }); - }); - describe("import helpers", () => { it("should not crash in tsserver", () => { const f1 = { @@ -4164,70 +4127,6 @@ namespace ts.projectSystem { }); }); - describe("completion entry with code actions", () => { - it("should work for symbols from non-imported modules", () => { - const moduleFile = { - path: "/a/b/moduleFile.ts", - content: `export const guitar = 10;` - }; - const file1 = { - path: "/a/b/file2.ts", - content: `var x:` - }; - const globalFile = { - path: "/a/b/globalFile.ts", - content: `interface Jazz { }` - }; - const ambientModuleFile = { - path: "/a/b/ambientModuleFile.ts", - content: - `declare module "windyAndWarm" { - export const chetAtkins = "great"; - }` - }; - const defaultModuleFile = { - path: "/a/b/defaultModuleFile.ts", - content: - `export default function egyptianElla() { };` - }; - const configFile = { - path: "/a/b/tsconfig.json", - content: "{}" - }; - - const host = createServerHost([moduleFile, file1, globalFile, ambientModuleFile, defaultModuleFile, configFile]); - const session = createSession(host); - const projectService = session.getProjectService(); - projectService.openClientFile(file1.path); - - checkEntryDetail(1, "guitar", /*hasAction*/ true, `import { guitar } from "./moduleFile";\n\n`); - checkEntryDetail(1, "chetAtkins", /*hasAction*/ true, `import { chetAtkins } from "windyAndWarm";\n\n`); - checkEntryDetail(1, "egyptianElla", /*hasAction*/ true, `import egyptianElla from "./defaultModuleFile";\n\n`); - checkEntryDetail(7, "Jazz", /*hasAction*/ false); - - function checkEntryDetail(offset: number, entryName: string, hasAction: boolean, insertString?: string) { - const request = makeSessionRequest( - CommandNames.CompletionDetails, - { entryNames: [entryName], file: file1.path, line: 1, offset, projectFileName: configFile.path }); - const response = session.executeCommand(request).response as protocol.CompletionEntryDetails[]; - assert.equal(response.length, 1); - - const entryDetails = response[0]; - if (!hasAction) { - assert.isUndefined(entryDetails.codeActions); - } - else { - const action = entryDetails.codeActions[0]; - assert.equal(action.changes[0].fileName, file1.path); - assert.deepEqual(action.changes[0], { - fileName: file1.path, - textChanges: [{ start: { line: 1, offset: 1 }, end: { line: 1, offset: 1 }, newText: insertString }] - }); - } - } - }); - }); - describe("maxNodeModuleJsDepth for inferred projects", () => { it("should be set to 2 if the project has js root files", () => { const file1: FileOrFolder = { diff --git a/src/server/protocol.ts b/src/server/protocol.ts index a420d0bb2613a..aa7508265f542 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1660,7 +1660,7 @@ namespace ts.server.protocol { */ replacementSpan?: TextSpan; /** - * Indicating if commiting this completion entry will require additional code action to be + * Indicates whether commiting this completion entry will require additional code action to be * made to avoid errors. The code action is normally adding an additional import statement. */ hasAction?: true; diff --git a/src/services/codeFixProvider.ts b/src/services/codeFixProvider.ts index 13e11ed4674f3..ad9ab520dabcf 100644 --- a/src/services/codeFixProvider.ts +++ b/src/services/codeFixProvider.ts @@ -5,15 +5,13 @@ namespace ts { getCodeActions(context: CodeFixContext): CodeAction[] | undefined; } - export interface CodeFixContext { + export interface CodeFixContext extends textChanges.TextChangesContext { errorCode: number; sourceFile: SourceFile; span: TextSpan; program: Program; - newLineCharacter: string; host: LanguageServiceHost; cancellationToken: CancellationToken; - rulesProvider: formatting.RulesProvider; } export namespace codefix { diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index fed6db3e11c44..cda4d628eb939 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1,5 +1,7 @@ /* @internal */ namespace ts.codefix { + import ChangeTracker = textChanges.ChangeTracker; + registerCodeFix({ errorCodes: [ Diagnostics.Cannot_find_name_0.code, @@ -11,7 +13,7 @@ namespace ts.codefix { }); type ImportCodeActionKind = "CodeChange" | "InsertingIntoExistingImport" | "NewImport"; - // this is a module id -> module import declaration map + // Map from module Id to an array of import declarations in that module. type ImportDeclarationMap = AnyImportSyntax[][]; interface ImportCodeAction extends CodeAction { @@ -19,21 +21,28 @@ namespace ts.codefix { moduleSpecifier?: string; } - export interface ImportCodeFixContext { - host: LanguageServiceHost; - symbolName: string; - newLineCharacter: string; - rulesProvider: formatting.RulesProvider; + interface SymbolContext extends textChanges.TextChangesContext { sourceFile: SourceFile; + symbolName: string; + } + + interface SymbolAndTokenContext extends SymbolContext { + symbolToken: Node | undefined; + } + + interface ImportCodeFixContext extends SymbolAndTokenContext { + host: LanguageServiceHost; checker: TypeChecker; compilerOptions: CompilerOptions; - getCanonicalFileName: (fileName: string) => string; - // this is a module id -> module import declaration map + getCanonicalFileName(fileName: string): string; cachedImportDeclarations?: ImportDeclarationMap; - symbolToken?: Node; } - enum ModuleSpecifierComparison { + export interface ImportCodeFixOptions extends ImportCodeFixContext { + kind: ImportKind; + } + + const enum ModuleSpecifierComparison { Better, Equal, Worse @@ -144,7 +153,8 @@ namespace ts.codefix { diagnosticArgs: string[], changes: FileTextChanges[], kind: ImportCodeActionKind, - moduleSpecifier?: string): ImportCodeAction { + moduleSpecifier: string | undefined, + ): ImportCodeAction { return { description: formatMessage.apply(undefined, [undefined, description].concat(diagnosticArgs)), changes, @@ -156,65 +166,31 @@ namespace ts.codefix { function convertToImportCodeFixContext(context: CodeFixContext): ImportCodeFixContext { const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; const checker = context.program.getTypeChecker(); - const token = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false); - //TODO: invalid cast! - return { - ...context, + const symbolToken = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false); + return { + host: context.host, + newLineCharacter: context.newLineCharacter, + rulesProvider: context.rulesProvider, + sourceFile: context.sourceFile, checker, compilerOptions: context.program.getCompilerOptions(), cachedImportDeclarations: [], getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames), - symbolName: token.getText(), - symbolToken: token + symbolName: symbolToken.getText(), + symbolToken, }; } - export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixContext, symbolName: string, isDefault?: boolean, isNamespaceImport?: boolean): ImportCodeAction[] { - let lastImportDeclaration: Node; - const { symbolName: name, sourceFile, getCanonicalFileName, newLineCharacter, host, checker, symbolToken, compilerOptions } = context; - const cachedImportDeclarations = context.cachedImportDeclarations || []; - - const existingDeclarations = getImportDeclarations(); - if (existingDeclarations.length > 0) { - // With an existing import statement, there are more than one actions the user can do. - return getCodeActionsForExistingImport(existingDeclarations); - } - else { - return [getCodeActionForNewImport()]; - } - - function getImportDeclarations() { - const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker); - - const cached = cachedImportDeclarations[moduleSymbolId]; - if (cached) { - return cached; - } - - const existingDeclarations = mapDefined(sourceFile.imports, importModuleSpecifier => - checker.getSymbolAtLocation(importModuleSpecifier) === moduleSymbol ? getImportDeclaration(importModuleSpecifier) : undefined); - cachedImportDeclarations[moduleSymbolId] = existingDeclarations; - return existingDeclarations; - - function getImportDeclaration({ parent }: LiteralExpression): AnyImportSyntax { - switch (parent.kind) { - case SyntaxKind.ImportDeclaration: - return parent as ImportDeclaration; - case SyntaxKind.ExternalModuleReference: - return (parent as ExternalModuleReference).parent; - default: - return undefined; - } - } - } - - function createChangeTracker() { - return textChanges.ChangeTracker.fromContext(context); - } - - function getCodeActionsForExistingImport(declarations: (ImportDeclaration | ImportEqualsDeclaration)[]): ImportCodeAction[] { - const actions: ImportCodeAction[] = []; + export const enum ImportKind { + Named, + Default, + Namespace, + } + export function getCodeActionForImport(moduleSymbol: Symbol, context: ImportCodeFixOptions): ImportCodeAction[] { + const declarations = getImportDeclarations(moduleSymbol, context.checker, context.sourceFile, context.cachedImportDeclarations); + const actions: ImportCodeAction[] = []; + if (context.symbolToken) { // It is possible that multiple import statements with the same specifier exist in the file. // e.g. // @@ -227,506 +203,513 @@ namespace ts.codefix { // 1. change "member3" to "ns.member3" // 2. add "member3" to the second import statement's import list // and it is up to the user to decide which one fits best. - let namespaceImportDeclaration: ImportDeclaration | ImportEqualsDeclaration; - let namedImportDeclaration: ImportDeclaration; - let existingModuleSpecifier: string; for (const declaration of declarations) { - if (declaration.kind === SyntaxKind.ImportDeclaration) { - const namedBindings = declaration.importClause && declaration.importClause.namedBindings; - if (namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport) { - // case: - // import * as ns from "foo" - namespaceImportDeclaration = declaration; - } - else { - // cases: - // import default from "foo" - // import { bar } from "foo" or combination with the first one - // import "foo" - namedImportDeclaration = declaration; - } - existingModuleSpecifier = declaration.moduleSpecifier.getText(); - } - else { - // case: - // import foo = require("foo") - namespaceImportDeclaration = declaration; - existingModuleSpecifier = getModuleSpecifierFromImportEqualsDeclaration(declaration); + const namespace = getNamespaceImportName(declaration); + if (namespace) { + actions.push(getCodeActionForUseExistingNamespaceImport(namespace.text, context, context.symbolToken)); } } + } + actions.push(getCodeActionForAddImport(moduleSymbol, context, declarations)); + return actions; + } - if (symbolToken && namespaceImportDeclaration) { - actions.push(getCodeActionForNamespaceImport(namespaceImportDeclaration)); - } + function getNamespaceImportName(declaration: AnyImportSyntax): Identifier { + if (declaration.kind === SyntaxKind.ImportDeclaration) { + const namedBindings = declaration.importClause && declaration.importClause.namedBindings; + return namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport ? namedBindings.name : undefined; + } + else { + return declaration.name; + } + } - if (!isNamespaceImport && namedImportDeclaration && namedImportDeclaration.importClause && - (namedImportDeclaration.importClause.name || namedImportDeclaration.importClause.namedBindings)) { - /** - * If the existing import declaration already has a named import list, just - * insert the identifier into that list. - */ - const fileTextChanges = getTextChangeForImportClause(namedImportDeclaration.importClause); - const moduleSpecifierWithoutQuotes = stripQuotes(namedImportDeclaration.moduleSpecifier.getText()); - actions.push(createCodeAction( - Diagnostics.Add_0_to_existing_import_declaration_from_1, - [name, moduleSpecifierWithoutQuotes], - fileTextChanges, - "InsertingIntoExistingImport", - moduleSpecifierWithoutQuotes - )); + // TODO(anhans): This doesn't seem important to cache... just use an iterator instead of creating a new array? + function getImportDeclarations(moduleSymbol: Symbol, checker: TypeChecker, { imports }: SourceFile, cachedImportDeclarations: ImportDeclarationMap = []): ReadonlyArray { + const moduleSymbolId = getUniqueSymbolId(moduleSymbol, checker); + let cached = cachedImportDeclarations[moduleSymbolId]; + if (!cached) { + cached = cachedImportDeclarations[moduleSymbolId] = mapDefined(imports, importModuleSpecifier => + checker.getSymbolAtLocation(importModuleSpecifier) === moduleSymbol ? getImportDeclaration(importModuleSpecifier) : undefined); + } + return cached; + } + + function getImportDeclaration({ parent }: LiteralExpression): AnyImportSyntax | undefined { + switch (parent.kind) { + case SyntaxKind.ImportDeclaration: + return parent as ImportDeclaration; + case SyntaxKind.ExternalModuleReference: + return (parent as ExternalModuleReference).parent; + default: + Debug.assert(parent.kind === SyntaxKind.ExportDeclaration); + // Ignore these, can't add imports to them. + return undefined; + } + } + + function getCodeActionForNewImport(context: SymbolContext & { kind: ImportKind }, moduleSpecifier: string): ImportCodeAction { + const { kind, sourceFile, newLineCharacter, symbolName } = context; + const lastImportDeclaration = findLast(sourceFile.statements, isAnyImportSyntax); + + const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier); + const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClauseOfKind(kind, symbolName), createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes)); + const changes = ChangeTracker.with(context, changeTracker => { + if (lastImportDeclaration) { + changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter }); } else { - // we need to create a new import statement, but the existing module specifier can be reused. - actions.push(getCodeActionForNewImport(existingModuleSpecifier)); + changeTracker.insertNodeAt(sourceFile, getSourceFileImportLocation(sourceFile), importDecl, { suffix: `${newLineCharacter}${newLineCharacter}` }); } - return actions; + }); - function getModuleSpecifierFromImportEqualsDeclaration(declaration: ImportEqualsDeclaration) { - if (declaration.moduleReference && declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference) { - return declaration.moduleReference.expression.getText(); - } - return declaration.moduleReference.getText(); - } + // if this file doesn't have any import statements, insert an import statement and then insert a new line + // between the only import statement and user code. Otherwise just insert the statement because chances + // are there are already a new line seperating code and import statements. + return createCodeAction( + Diagnostics.Import_0_from_1, + [symbolName, moduleSpecifierWithoutQuotes], + changes, + "NewImport", + moduleSpecifierWithoutQuotes, + ); + } - function getTextChangeForImportClause(importClause: ImportClause): FileTextChanges[] { - const importList = importClause.namedBindings; - const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(name)); - // case 1: - // original text: import default from "module" - // change to: import default, { name } from "module" - // case 2: - // original text: import {} from "module" - // change to: import { name } from "module" - if (!importList || importList.elements.length === 0) { - const newImportClause = createImportClause(importClause.name, createNamedImports([newImportSpecifier])); - return createChangeTracker().replaceNode(sourceFile, importClause, newImportClause).getChanges(); - } + function createStringLiteralWithQuoteStyle(sourceFile: SourceFile, text: string): StringLiteral { + const literal = createLiteral(text); + const firstModuleSpecifier = firstOrUndefined(sourceFile.imports); + literal.singleQuote = !!firstModuleSpecifier && !isStringDoubleQuoted(firstModuleSpecifier, sourceFile); + return literal; + } - /** - * If the import list has one import per line, preserve that. Otherwise, insert on same line as last element - * import { - * foo - * } from "./module"; - */ - return createChangeTracker().insertNodeInListAfter( - sourceFile, - importList.elements[importList.elements.length - 1], - newImportSpecifier).getChanges(); - } + function createImportClauseOfKind(kind: ImportKind, symbolName: string) { + switch (kind) { + case ImportKind.Default: + return createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined); + case ImportKind.Namespace: + return createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))); + case ImportKind.Named: + return createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); + default: + Debug.assertNever(kind); + } + } - function getCodeActionForNamespaceImport(declaration: ImportDeclaration | ImportEqualsDeclaration): ImportCodeAction { - let namespacePrefix: string; - if (declaration.kind === SyntaxKind.ImportDeclaration) { - namespacePrefix = (declaration.importClause.namedBindings).name.getText(); - } - else { - namespacePrefix = declaration.name.getText(); - } - namespacePrefix = stripQuotes(namespacePrefix); - - /** - * Cases: - * import * as ns from "mod" - * import default, * as ns from "mod" - * import ns = require("mod") - * - * Because there is no import list, we alter the reference to include the - * namespace instead of altering the import declaration. For example, "foo" would - * become "ns.foo" - */ - return createCodeAction( - Diagnostics.Change_0_to_1, - [name, `${namespacePrefix}.${name}`], - createChangeTracker().replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), name)).getChanges(), - "CodeChange" - ); + function getSourceFileImportLocation(node: SourceFile): number { + // For a source file, it is possible there are detached comments we should not skip + const text = node.text; + let ranges = getLeadingCommentRanges(text, 0); + if (!ranges) return 0; + let position = 0; + // However we should still skip a pinned comment at the top + if (ranges.length && ranges[0].kind === SyntaxKind.MultiLineCommentTrivia && isPinnedComment(text, ranges[0])) { + position = ranges[0].end + 1; + ranges = ranges.slice(1); + } + // As well as any triple slash references + for (const range of ranges) { + if (range.kind === SyntaxKind.SingleLineCommentTrivia && isRecognizedTripleSlashComment(node.text, range.pos, range.end)) { + position = range.end + 1; + continue; } + break; } + return position; + } - function getCodeActionForNewImport(moduleSpecifier?: string): ImportCodeAction { - if (!lastImportDeclaration) { - // insert after any existing imports - for (let i = sourceFile.statements.length - 1; i >= 0; i--) { - const statement = sourceFile.statements[i]; - if (statement.kind === SyntaxKind.ImportEqualsDeclaration || statement.kind === SyntaxKind.ImportDeclaration) { - lastImportDeclaration = statement; - break; - } - } - } + function getModuleSpecifierForNewImport(sourceFile: SourceFile, moduleSymbol: Symbol, options: CompilerOptions, getCanonicalFileName: (file: string) => string, host: LanguageServiceHost): string | undefined { + const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; + const sourceDirectory = getDirectoryPath(sourceFile.fileName); - const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier || getModuleSpecifierForNewImport()); - const changeTracker = createChangeTracker(); - const importClause = isDefault - ? createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined) - : isNamespaceImport - ? createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))) - : createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); - const moduleSpecifierLiteral = createLiteral(moduleSpecifierWithoutQuotes); - moduleSpecifierLiteral.singleQuote = getSingleQuoteStyleFromExistingImports(); - const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, importClause, moduleSpecifierLiteral); - if (!lastImportDeclaration) { - changeTracker.insertNodeAt(sourceFile, getSourceFileImportLocation(sourceFile), importDecl, { suffix: `${context.newLineCharacter}${context.newLineCharacter}` }); - } - else { - changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter }); - } + return tryGetModuleNameFromAmbientModule(moduleSymbol) || + tryGetModuleNameFromTypeRoots(options, host, getCanonicalFileName, moduleFileName) || + tryGetModuleNameAsNodeModule(options, moduleFileName, host, getCanonicalFileName, sourceDirectory) || + tryGetModuleNameFromBaseUrl(options, moduleFileName, getCanonicalFileName) || + options.rootDirs && tryGetModuleNameFromRootDirs(options.rootDirs, moduleFileName, sourceDirectory, getCanonicalFileName) || + removeFileExtension(getRelativePath(moduleFileName, sourceDirectory, getCanonicalFileName)); + } - // if this file doesn't have any import statements, insert an import statement and then insert a new line - // between the only import statement and user code. Otherwise just insert the statement because chances - // are there are already a new line seperating code and import statements. - return createCodeAction( - Diagnostics.Import_0_from_1, - [symbolName, `"${moduleSpecifierWithoutQuotes}"`], - changeTracker.getChanges(), - "NewImport", - moduleSpecifierWithoutQuotes - ); - - function getSourceFileImportLocation(node: SourceFile) { - // For a source file, it is possible there are detached comments we should not skip - const text = node.text; - let ranges = getLeadingCommentRanges(text, 0); - if (!ranges) return 0; - let position = 0; - // However we should still skip a pinned comment at the top - if (ranges.length && ranges[0].kind === SyntaxKind.MultiLineCommentTrivia && isPinnedComment(text, ranges[0])) { - position = ranges[0].end + 1; - ranges = ranges.slice(1); - } - // As well as any triple slash references - for (const range of ranges) { - if (range.kind === SyntaxKind.SingleLineCommentTrivia && isRecognizedTripleSlashComment(node.text, range.pos, range.end)) { - position = range.end + 1; + function tryGetModuleNameFromAmbientModule(moduleSymbol: Symbol): string | undefined { + const decl = moduleSymbol.valueDeclaration; + if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) { + return decl.name.text; + } + } + + function tryGetModuleNameFromBaseUrl(options: CompilerOptions, moduleFileName: string, getCanonicalFileName: (file: string) => string): string | undefined { + if (!options.baseUrl) { + return undefined; + } + + let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl, getCanonicalFileName); + if (!relativeName) { + return undefined; + } + + const relativeNameWithIndex = removeFileExtension(relativeName); + relativeName = removeExtensionAndIndexPostFix(relativeName); + + if (options.paths) { + for (const key in options.paths) { + for (const pattern of options.paths[key]) { + const indexOfStar = pattern.indexOf("*"); + if (indexOfStar === 0 && pattern.length === 1) { continue; } - break; - } - return position; - } - - function getSingleQuoteStyleFromExistingImports() { - const firstModuleSpecifier = forEach(sourceFile.statements, node => { - if (isImportDeclaration(node) || isExportDeclaration(node)) { - if (node.moduleSpecifier && isStringLiteral(node.moduleSpecifier)) { - return node.moduleSpecifier; + else if (indexOfStar !== -1) { + const prefix = pattern.substr(0, indexOfStar); + const suffix = pattern.substr(indexOfStar + 1); + if (relativeName.length >= prefix.length + suffix.length && + startsWith(relativeName, prefix) && + endsWith(relativeName, suffix)) { + const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); + return key.replace("\*", matchedStar); } } - else if (isImportEqualsDeclaration(node)) { - if (isExternalModuleReference(node.moduleReference) && isStringLiteral(node.moduleReference.expression)) { - return node.moduleReference.expression; - } + else if (pattern === relativeName || pattern === relativeNameWithIndex) { + return key; } - }); - if (firstModuleSpecifier) { - return sourceFile.text.charCodeAt(firstModuleSpecifier.getStart()) === CharacterCodes.singleQuote; } } + } - function getModuleSpecifierForNewImport() { - const fileName = sourceFile.fileName; - const moduleFileName = moduleSymbol.valueDeclaration.getSourceFile().fileName; - const sourceDirectory = getDirectoryPath(fileName); - const options = compilerOptions; - - return tryGetModuleNameFromAmbientModule() || - tryGetModuleNameFromTypeRoots() || - tryGetModuleNameAsNodeModule() || - tryGetModuleNameFromBaseUrl() || - tryGetModuleNameFromRootDirs() || - removeFileExtension(getRelativePath(moduleFileName, sourceDirectory)); - - function tryGetModuleNameFromAmbientModule(): string { - const decl = moduleSymbol.valueDeclaration; - if (isModuleDeclaration(decl) && isStringLiteral(decl.name)) { - return decl.name.text; - } - } + return relativeName; + } - function tryGetModuleNameFromBaseUrl() { - if (!options.baseUrl) { - return undefined; - } + function tryGetModuleNameFromRootDirs(rootDirs: ReadonlyArray, moduleFileName: string, sourceDirectory: string, getCanonicalFileName: (file: string) => string): string | undefined { + const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, rootDirs, getCanonicalFileName); + if (normalizedTargetPath === undefined) { + return undefined; + } - let relativeName = getRelativePathIfInDirectory(moduleFileName, options.baseUrl); - if (!relativeName) { - return undefined; - } + const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, rootDirs, getCanonicalFileName); + const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath, getCanonicalFileName) : normalizedTargetPath; + return removeFileExtension(relativePath); + } - const relativeNameWithIndex = removeFileExtension(relativeName); - relativeName = removeExtensionAndIndexPostFix(relativeName); - - if (options.paths) { - for (const key in options.paths) { - for (const pattern of options.paths[key]) { - const indexOfStar = pattern.indexOf("*"); - if (indexOfStar === 0 && pattern.length === 1) { - continue; - } - else if (indexOfStar !== -1) { - const prefix = pattern.substr(0, indexOfStar); - const suffix = pattern.substr(indexOfStar + 1); - if (relativeName.length >= prefix.length + suffix.length && - startsWith(relativeName, prefix) && - endsWith(relativeName, suffix)) { - const matchedStar = relativeName.substr(prefix.length, relativeName.length - suffix.length); - return key.replace("\*", matchedStar); - } - } - else if (pattern === relativeName || pattern === relativeNameWithIndex) { - return key; - } - } - } - } + function tryGetModuleNameFromTypeRoots( + options: CompilerOptions, + host: GetEffectiveTypeRootsHost, + getCanonicalFileName: (file: string) => string, + moduleFileName: string, + ): string | undefined { + return firstDefined(getEffectiveTypeRoots(options, host), unNormalizedTypeRoot => { + const typeRoot = toPath(unNormalizedTypeRoot, /*basePath*/ undefined, getCanonicalFileName); + if (startsWith(moduleFileName, typeRoot)) { + return removeExtensionAndIndexPostFix(moduleFileName.substring(typeRoot.length + 1)); + } + }); + } - return relativeName; - } + function tryGetModuleNameAsNodeModule( + options: CompilerOptions, + moduleFileName: string, + host: LanguageServiceHost, + getCanonicalFileName: (file: string) => string, + sourceDirectory: string, + ): string | undefined { + if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { + // nothing to do here + return undefined; + } - function tryGetModuleNameFromRootDirs() { - if (options.rootDirs) { - const normalizedTargetPath = getPathRelativeToRootDirs(moduleFileName, options.rootDirs); - const normalizedSourcePath = getPathRelativeToRootDirs(sourceDirectory, options.rootDirs); - if (normalizedTargetPath !== undefined) { - const relativePath = normalizedSourcePath !== undefined ? getRelativePath(normalizedTargetPath, normalizedSourcePath) : normalizedTargetPath; - return removeFileExtension(relativePath); - } - } - return undefined; - } + const parts = getNodeModulePathParts(moduleFileName); + + if (!parts) { + return undefined; + } - function tryGetModuleNameFromTypeRoots() { - const typeRoots = getEffectiveTypeRoots(options, host); - if (typeRoots) { - const normalizedTypeRoots = map(typeRoots, typeRoot => toPath(typeRoot, /*basePath*/ undefined, getCanonicalFileName)); - for (const typeRoot of normalizedTypeRoots) { - if (startsWith(moduleFileName, typeRoot)) { - const relativeFileName = moduleFileName.substring(typeRoot.length + 1); - return removeExtensionAndIndexPostFix(relativeFileName); - } + // Simplify the full file path to something that can be resolved by Node. + + // If the module could be imported by a directory name, use that directory's name + let moduleSpecifier = getDirectoryOrExtensionlessFileName(moduleFileName); + // Get a path that's relative to node_modules or the importing file's path + moduleSpecifier = getNodeResolvablePath(moduleSpecifier); + // If the module was found in @types, get the actual Node package name + return getPackageNameFromAtTypesDirectory(moduleSpecifier); + + function getDirectoryOrExtensionlessFileName(path: string): string { + // If the file is the main module, it can be imported by the package name + const packageRootPath = path.substring(0, parts.packageRootIndex); + const packageJsonPath = combinePaths(packageRootPath, "package.json"); + if (host.fileExists(packageJsonPath)) { + const packageJsonContent = JSON.parse(host.readFile(packageJsonPath)); + if (packageJsonContent) { + const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; + if (mainFileRelative) { + const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); + if (mainExportFile === getCanonicalFileName(path)) { + return packageRootPath; } } } + } - function tryGetModuleNameAsNodeModule() { - if (getEmitModuleResolutionKind(options) !== ModuleResolutionKind.NodeJs) { - // nothing to do here - return undefined; - } - - const parts = getNodeModulePathParts(moduleFileName); + // We still have a file name - remove the extension + const fullModulePathWithoutExtension = removeFileExtension(path); - if (!parts) { - return undefined; - } + // If the file is /index, it can be imported by its directory name + if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") { + return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); + } - // Simplify the full file path to something that can be resolved by Node. - - // If the module could be imported by a directory name, use that directory's name - let moduleSpecifier = getDirectoryOrExtensionlessFileName(moduleFileName); - // Get a path that's relative to node_modules or the importing file's path - moduleSpecifier = getNodeResolvablePath(moduleSpecifier); - // If the module was found in @types, get the actual Node package name - return getPackageNameFromAtTypesDirectory(moduleSpecifier); - - function getDirectoryOrExtensionlessFileName(path: string): string { - // If the file is the main module, it can be imported by the package name - const packageRootPath = path.substring(0, parts.packageRootIndex); - const packageJsonPath = combinePaths(packageRootPath, "package.json"); - if (host.fileExists(packageJsonPath)) { - const packageJsonContent = JSON.parse(host.readFile(packageJsonPath)); - if (packageJsonContent) { - const mainFileRelative = packageJsonContent.typings || packageJsonContent.types || packageJsonContent.main; - if (mainFileRelative) { - const mainExportFile = toPath(mainFileRelative, packageRootPath, getCanonicalFileName); - if (mainExportFile === getCanonicalFileName(path)) { - return packageRootPath; - } - } - } - } + return fullModulePathWithoutExtension; + } - // We still have a file name - remove the extension - const fullModulePathWithoutExtension = removeFileExtension(path); + function getNodeResolvablePath(path: string): string { + const basePath = path.substring(0, parts.topLevelNodeModulesIndex); + if (sourceDirectory.indexOf(basePath) === 0) { + // if node_modules folder is in this folder or any of its parent folders, no need to keep it. + return path.substring(parts.topLevelPackageNameIndex + 1); + } + else { + return getRelativePath(path, sourceDirectory, getCanonicalFileName); + } + } + } - // If the file is /index, it can be imported by its directory name - if (getCanonicalFileName(fullModulePathWithoutExtension.substring(parts.fileNameIndex)) === "/index") { - return fullModulePathWithoutExtension.substring(0, parts.fileNameIndex); - } + function getNodeModulePathParts(fullPath: string) { + // If fullPath can't be valid module file within node_modules, returns undefined. + // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js + // Returns indices: ^ ^ ^ ^ + + let topLevelNodeModulesIndex = 0; + let topLevelPackageNameIndex = 0; + let packageRootIndex = 0; + let fileNameIndex = 0; + + const enum States { + BeforeNodeModules, + NodeModules, + Scope, + PackageContent + } - return fullModulePathWithoutExtension; + let partStart = 0; + let partEnd = 0; + let state = States.BeforeNodeModules; + + while (partEnd >= 0) { + partStart = partEnd; + partEnd = fullPath.indexOf("/", partStart + 1); + switch (state) { + case States.BeforeNodeModules: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + topLevelNodeModulesIndex = partStart; + topLevelPackageNameIndex = partEnd; + state = States.NodeModules; } - - function getNodeResolvablePath(path: string): string { - const basePath = path.substring(0, parts.topLevelNodeModulesIndex); - if (sourceDirectory.indexOf(basePath) === 0) { - // if node_modules folder is in this folder or any of its parent folders, no need to keep it. - return path.substring(parts.topLevelPackageNameIndex + 1); - } - else { - return getRelativePath(path, sourceDirectory); - } + break; + case States.NodeModules: + case States.Scope: + if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { + state = States.Scope; } - } + else { + packageRootIndex = partEnd; + state = States.PackageContent; + } + break; + case States.PackageContent: + if (fullPath.indexOf("/node_modules/", partStart) === partStart) { + state = States.NodeModules; + } + else { + state = States.PackageContent; + } + break; } + } - function getNodeModulePathParts(fullPath: string) { - // If fullPath can't be valid module file within node_modules, returns undefined. - // Example of expected pattern: /base/path/node_modules/[@scope/otherpackage/@otherscope/node_modules/]package/[subdirectory/]file.js - // Returns indices: ^ ^ ^ ^ - - let topLevelNodeModulesIndex = 0; - let topLevelPackageNameIndex = 0; - let packageRootIndex = 0; - let fileNameIndex = 0; - - const enum States { - BeforeNodeModules, - NodeModules, - Scope, - PackageContent - } + fileNameIndex = partStart; - let partStart = 0; - let partEnd = 0; - let state = States.BeforeNodeModules; - - while (partEnd >= 0) { - partStart = partEnd; - partEnd = fullPath.indexOf("/", partStart + 1); - switch (state) { - case States.BeforeNodeModules: - if (fullPath.indexOf("/node_modules/", partStart) === partStart) { - topLevelNodeModulesIndex = partStart; - topLevelPackageNameIndex = partEnd; - state = States.NodeModules; - } - break; - case States.NodeModules: - case States.Scope: - if (state === States.NodeModules && fullPath.charAt(partStart + 1) === "@") { - state = States.Scope; - } - else { - packageRootIndex = partEnd; - state = States.PackageContent; - } - break; - case States.PackageContent: - if (fullPath.indexOf("/node_modules/", partStart) === partStart) { - state = States.NodeModules; - } - else { - state = States.PackageContent; - } - break; - } - } + return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; + } - fileNameIndex = partStart; + function getPathRelativeToRootDirs(path: string, rootDirs: ReadonlyArray, getCanonicalFileName: (fileName: string) => string): string | undefined { + return firstDefined(rootDirs, rootDir => getRelativePathIfInDirectory(path, rootDir, getCanonicalFileName)); + } - return state > States.NodeModules ? { topLevelNodeModulesIndex, topLevelPackageNameIndex, packageRootIndex, fileNameIndex } : undefined; - } + function removeExtensionAndIndexPostFix(fileName: string) { + fileName = removeFileExtension(fileName); + if (endsWith(fileName, "/index")) { + fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); + } + return fileName; + } - function getPathRelativeToRootDirs(path: string, rootDirs: string[]) { - for (const rootDir of rootDirs) { - const relativeName = getRelativePathIfInDirectory(path, rootDir); - if (relativeName !== undefined) { - return relativeName; - } + function getRelativePathIfInDirectory(path: string, directoryPath: string, getCanonicalFileName: (fileName: string) => string): string | undefined { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; + } + + function getRelativePath(path: string, directoryPath: string, getCanonicalFileName: (fileName: string) => string) { + const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); + return !pathIsRelative(relativePath) ? "./" + relativePath : relativePath; + } + + function getCodeActionForAddImport( + moduleSymbol: Symbol, + ctx: ImportCodeFixOptions, + declarations: ReadonlyArray): ImportCodeAction { + const fromExistingImport = firstDefined(declarations, declaration => { + if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) { + const changes = tryUpdateExistingImport(ctx, ctx.kind, declaration.importClause); + if (changes) { + const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText()); + return createCodeAction( + Diagnostics.Add_0_to_existing_import_declaration_from_1, + [ctx.symbolName, moduleSpecifierWithoutQuotes], + changes, + "InsertingIntoExistingImport", + moduleSpecifierWithoutQuotes); } - return undefined; } + }); + if (fromExistingImport) { + return fromExistingImport; + } + + const moduleSpecifier = firstDefined(declarations, moduleSpecifierFromAnyImport) + || getModuleSpecifierForNewImport(ctx.sourceFile, moduleSymbol, ctx.compilerOptions, ctx.getCanonicalFileName, ctx.host); + return getCodeActionForNewImport(ctx, moduleSpecifier); + } + + function moduleSpecifierFromAnyImport(node: AnyImportSyntax): string | undefined { + const expression = node.kind === SyntaxKind.ImportDeclaration + ? node.moduleSpecifier + : node.moduleReference.kind === SyntaxKind.ExternalModuleReference + ? node.moduleReference.expression + : undefined; + return expression && isStringLiteral(expression) ? expression.text : undefined; + } - function removeExtensionAndIndexPostFix(fileName: string) { - fileName = removeFileExtension(fileName); - if (endsWith(fileName, "/index")) { - fileName = fileName.substr(0, fileName.length - 6/* "/index".length */); + function tryUpdateExistingImport(context: SymbolContext, kind: ImportKind, importClause: ImportClause): FileTextChanges[] | undefined { + const { symbolName, sourceFile } = context; + const { name, namedBindings } = importClause; + switch (kind) { + case ImportKind.Default: + return name ? undefined : ChangeTracker.with(context, t => + t.replaceNode(sourceFile, importClause, createImportClause(createIdentifier(symbolName), namedBindings))); + + case ImportKind.Named: { + const newImportSpecifier = createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName)); + if (namedBindings && namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length !== 0) { + // There are already named imports; add another. + return ChangeTracker.with(context, t => t.insertNodeInListAfter( + sourceFile, + namedBindings.elements[namedBindings.elements.length - 1], + newImportSpecifier)); } - return fileName; + if (!namedBindings || namedBindings.kind === SyntaxKind.NamedImports && namedBindings.elements.length === 0) { + return ChangeTracker.with(context, t => + t.replaceNode(sourceFile, importClause, createImportClause(name, createNamedImports([newImportSpecifier])))); + } + return undefined; } - function getRelativePathIfInDirectory(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return isRootedDiskPath(relativePath) || startsWith(relativePath, "..") ? undefined : relativePath; - } + case ImportKind.Namespace: + return namedBindings ? undefined : ChangeTracker.with(context, t => + t.replaceNode(sourceFile, importClause, createImportClause(name, createNamespaceImport(createIdentifier(symbolName))))); - function getRelativePath(path: string, directoryPath: string) { - const relativePath = getRelativePathToDirectoryOrUrl(directoryPath, path, directoryPath, getCanonicalFileName, /*isAbsolutePathAnUrl*/ false); - return !pathIsRelative(relativePath) ? "./" + relativePath : relativePath; - } + default: + Debug.assertNever(kind); } } + function getCodeActionForUseExistingNamespaceImport(namespacePrefix: string, context: SymbolContext, symbolToken: Node): ImportCodeAction { + const { symbolName, sourceFile } = context; + + /** + * Cases: + * import * as ns from "mod" + * import default, * as ns from "mod" + * import ns = require("mod") + * + * Because there is no import list, we alter the reference to include the + * namespace instead of altering the import declaration. For example, "foo" would + * become "ns.foo" + */ + return createCodeAction( + Diagnostics.Change_0_to_1, + [symbolName, `${namespacePrefix}.${symbolName}`], + ChangeTracker.with(context, tracker => + tracker.replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), symbolName))), + "CodeChange", + /*moduleSpecifier*/ undefined); + } + function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { - const sourceFile = context.sourceFile; - const allSourceFiles = context.program.getSourceFiles(); const importFixContext = convertToImportCodeFixContext(context); + return context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code + ? getActionsForUMDImport(importFixContext) + : getActionsForNonUMDImport(importFixContext, context.program.getSourceFiles(), context.cancellationToken); + } - const checker = importFixContext.checker; - const token = importFixContext.symbolToken; - const symbolIdActionMap = new ImportCodeActionMap(); - const currentTokenMeaning = getMeaningFromLocation(token); - - const name = importFixContext.symbolName; - - if (context.errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code) { - const umdSymbol = checker.getSymbolAtLocation(token); - let symbol: ts.Symbol; - let symbolName: string; - if (umdSymbol.flags & ts.SymbolFlags.Alias) { - symbol = checker.getAliasedSymbol(umdSymbol); - symbolName = name; - } - else if (isJsxOpeningLikeElement(token.parent) && token.parent.tagName === token) { - // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`. - symbol = checker.getAliasedSymbol(checker.resolveName(checker.getJsxNamespace(), token.parent.tagName, SymbolFlags.Value)); - symbolName = symbol.name; - } - else { - Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); - } - - return getCodeActionForImport(symbol, importFixContext, symbolName, /*isDefault*/ false, /*isNamespaceImport*/ true); + function getActionsForUMDImport(context: ImportCodeFixContext): ImportCodeAction[] { + const { checker, symbolToken } = context; + const umdSymbol = checker.getSymbolAtLocation(symbolToken); + let symbol: ts.Symbol; + let symbolName: string; + if (umdSymbol.flags & ts.SymbolFlags.Alias) { + symbol = checker.getAliasedSymbol(umdSymbol); + symbolName = context.symbolName; } - - const candidateModules = checker.getAmbientModules(); - for (const otherSourceFile of allSourceFiles) { - if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { - candidateModules.push(otherSourceFile.symbol); - } + else if (isJsxOpeningLikeElement(symbolToken.parent) && symbolToken.parent.tagName === symbolToken) { + // The error wasn't for the symbolAtLocation, it was for the JSX tag itself, which needs access to e.g. `React`. + symbol = checker.getAliasedSymbol(checker.resolveName(checker.getJsxNamespace(), symbolToken.parent.tagName, SymbolFlags.Value)); + symbolName = symbol.name; } + else { + Debug.fail("Either the symbol or the JSX namespace should be a UMD global if we got here"); + } + + return getCodeActionForImport(symbol, { ...context, symbolName, kind: ImportKind.Namespace }); + } - for (const moduleSymbol of candidateModules) { - context.cancellationToken.throwIfCancellationRequested(); + function getActionsForNonUMDImport(context: ImportCodeFixContext, allSourceFiles: ReadonlyArray, cancellationToken: CancellationToken): ImportCodeAction[] { + const { sourceFile, checker, symbolName, symbolToken } = context; + // "default" is a keyword and not a legal identifier for the import, so we don't expect it here + Debug.assert(symbolName !== "default"); + const symbolIdActionMap = new ImportCodeActionMap(); + const currentTokenMeaning = getMeaningFromLocation(symbolToken); + eachOtherExternalModule(checker, allSourceFiles, sourceFile, moduleSymbol => { + cancellationToken.throwIfCancellationRequested(); // check the default export const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); if (defaultExport) { const localSymbol = getLocalSymbolForExportDefault(defaultExport); - if (localSymbol && localSymbol.escapedName === name && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { + if (localSymbol && localSymbol.escapedName === symbolName && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) { // check if this symbol is already used const symbolId = getUniqueSymbolId(localSymbol, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, importFixContext, name, /*isNamespaceImport*/ true)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, { ...context, kind: ImportKind.Default })); } } - // "default" is a keyword and not a legal identifier for the import, so we don't expect it here - Debug.assert(name !== "default"); - // check exports with the same name - const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(name, moduleSymbol); + const exportSymbolWithIdenticalName = checker.tryGetMemberInModuleExportsAndProperties(symbolName, moduleSymbol); if (exportSymbolWithIdenticalName && checkSymbolHasMeaning(exportSymbolWithIdenticalName, currentTokenMeaning)) { const symbolId = getUniqueSymbolId(exportSymbolWithIdenticalName, checker); - symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, importFixContext, name)); + symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, { ...context, kind: ImportKind.Named })); } - } + }); return symbolIdActionMap.getAllActions(); + } - function checkSymbolHasMeaning(symbol: Symbol, meaning: SemanticMeaning) { - const declarations = symbol.getDeclarations(); - return declarations ? some(symbol.declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)) : false; + function checkSymbolHasMeaning({ declarations }: Symbol, meaning: SemanticMeaning): boolean { + return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)); + } + + export function eachOtherExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, sourceFile: SourceFile, cb: (module: Symbol) => void) { + for (const ambient of checker.getAmbientModules()) { + cb(ambient); + } + for (const otherSourceFile of allSourceFiles) { + if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { + cb(otherSourceFile.symbol); + } } } } diff --git a/src/services/completions.ts b/src/services/completions.ts index f9e94c7f1acf8..e0cb781f00f04 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -4,7 +4,15 @@ namespace ts.Completions { export type Log = (message: string) => void; - export type SymbolOriginInfo = { moduleSymbol: Symbol, isDefaultExport?: boolean }; + interface SymbolOriginInfo { + moduleSymbol: Symbol; + isDefaultExport: boolean; + } + /** + * Map from symbol id -> SymbolOriginInfo. + * Only populated for symbols that come from other modules. + */ + type SymbolOriginInfoMap = SymbolOriginInfo[]; const enum KeywordCompletionFilters { None, @@ -145,7 +153,7 @@ namespace ts.Completions { } function getCompletionEntriesFromSymbols( - symbols: Symbol[], + symbols: ReadonlyArray, entries: Push, location: Node, performCharacterChecks: boolean, @@ -153,7 +161,7 @@ namespace ts.Completions { target: ScriptTarget, log: Log, allowStringLiteral: boolean, - symbolToOriginInfoMap?: Map, + symbolToOriginInfoMap?: SymbolOriginInfoMap, ): Map { const start = timestamp(); const uniqueNames = createMap(); @@ -163,7 +171,7 @@ namespace ts.Completions { if (entry) { const id = entry.name; if (!uniqueNames.has(id)) { - if (symbolToOriginInfoMap && symbolToOriginInfoMap.has(getUniqueSymbolIdAsString(symbol, typeChecker))) { + if (symbolToOriginInfoMap && symbolToOriginInfoMap[getUniqueSymbolId(symbol, typeChecker)]) { entry.hasAction = true; } entries.push(entry); @@ -327,10 +335,10 @@ namespace ts.Completions { compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, - entryName: string, + name: string, allSourceFiles: ReadonlyArray, - host?: LanguageServiceHost, - rulesProvider?: formatting.RulesProvider, + host: LanguageServiceHost, + rulesProvider: formatting.RulesProvider, ): CompletionEntryDetails { // Compute all the completion symbols again. @@ -342,62 +350,57 @@ namespace ts.Completions { // We don't need to perform character checks here because we're only comparing the // name against 'entryName' (which is known to be good), not building a new // completion entry. - const symbol = forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === entryName ? s : undefined); + const symbol = find(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name); if (symbol) { - let codeActions: CodeAction[]; - if (host && rulesProvider) { - const symbolOriginInfo = symbolToOriginInfoMap.get(getUniqueSymbolIdAsString(symbol, typeChecker)); - if (symbolOriginInfo) { - const useCaseSensitiveFileNames = host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false; - const context: codefix.ImportCodeFixContext = { - host, - checker: typeChecker, - newLineCharacter: host.getNewLine(), - compilerOptions, - sourceFile, - rulesProvider, - symbolName: symbol.name, - getCanonicalFileName: createGetCanonicalFileName(useCaseSensitiveFileNames) - }; - - codeActions = codefix.getCodeActionForImport(/*moduleSymbol*/ symbolOriginInfo.moduleSymbol, context, context.symbolName, /*isDefault*/ symbolOriginInfo.isDefaultExport); - } - } - + const codeActions = getCompletionEntryCodeActions(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, rulesProvider); + const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); - return { - name: entryName, - kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), - kind: symbolKind, - displayParts, - documentation, - tags, - codeActions - }; + return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions }; } } // Didn't find a symbol with this name. See if we can find a keyword instead. const keywordCompletion = forEach( getKeywordCompletions(KeywordCompletionFilters.None), - c => c.name === entryName + c => c.name === name ); if (keywordCompletion) { return { - name: entryName, + name, kind: ScriptElementKind.keyword, kindModifiers: ScriptElementKindModifier.none, - displayParts: [displayPart(entryName, SymbolDisplayPartKind.keyword)], + displayParts: [displayPart(name, SymbolDisplayPartKind.keyword)], documentation: undefined, tags: undefined, - codeActions: undefined + codeActions: undefined, }; } return undefined; } + function getCompletionEntryCodeActions(symbolToOriginInfoMap: SymbolOriginInfoMap, symbol: Symbol, checker: TypeChecker, host: LanguageServiceHost, compilerOptions: CompilerOptions, sourceFile: SourceFile, rulesProvider: formatting.RulesProvider): CodeAction[] | undefined { + const symbolOriginInfo = symbolToOriginInfoMap[getUniqueSymbolId(symbol, checker)]; + if (!symbolOriginInfo) { + return undefined; + } + + const { moduleSymbol, isDefaultExport } = symbolOriginInfo; + return codefix.getCodeActionForImport(moduleSymbol, { + host, + checker, + newLineCharacter: host.getNewLine(), + compilerOptions, + sourceFile, + rulesProvider, + symbolName: symbol.name, + getCanonicalFileName: createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false), + symbolToken: undefined, + kind: isDefaultExport ? codefix.ImportKind.Default : codefix.ImportKind.Named, + }); + } + export function getCompletionEntrySymbol( typeChecker: TypeChecker, log: (message: string) => void, @@ -417,7 +420,7 @@ namespace ts.Completions { // We don't need to perform character checks here because we're only comparing the // name against 'entryName' (which is known to be good), not building a new // completion entry. - return forEach(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === entryName ? s : undefined); + return find(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === entryName); } interface CompletionData { @@ -430,7 +433,7 @@ namespace ts.Completions { isRightOfDot: boolean; request?: Request; keywordFilters: KeywordCompletionFilters; - symbolToOriginInfoMap: Map; + symbolToOriginInfoMap: SymbolOriginInfoMap; } type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; @@ -605,6 +608,7 @@ namespace ts.Completions { break; } // falls through + case SyntaxKind.JsxSelfClosingElement: case SyntaxKind.JsxElement: case SyntaxKind.JsxOpeningElement: @@ -624,7 +628,7 @@ namespace ts.Completions { let isNewIdentifierLocation: boolean; let keywordFilters = KeywordCompletionFilters.None; let symbols: Symbol[] = []; - const symbolToOriginInfoMap = createMap(); + const symbolToOriginInfoMap: SymbolOriginInfoMap = []; if (isRightOfDot) { getTypeScriptMemberSymbols(); @@ -837,14 +841,14 @@ namespace ts.Completions { const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings); - symbols.push(...getSymbolsFromOtherSourceFileExports(symbols, previousToken === undefined ? "" : previousToken.getText())); - symbols = filterGlobalCompletion(symbols); + getSymbolsFromOtherSourceFileExports(symbols, previousToken === undefined ? "" : previousToken.getText()); + filterGlobalCompletion(symbols); return true; } - function filterGlobalCompletion(symbols: Symbol[]) { - return filter(symbols, symbol => { + function filterGlobalCompletion(symbols: Symbol[]): void { + filterMutate(symbols, symbol => { if (!isSourceFile(location)) { // export = /**/ here we want to get all meanings, so any symbol is ok if (isExportAssignment(location.parent)) { @@ -918,35 +922,29 @@ namespace ts.Completions { } } - function getSymbolsFromOtherSourceFileExports(knownSymbols: Symbol[], tokenText: string): Symbol[] { - const otherSourceFileExports: Symbol[] = []; + function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string): void { const tokenTextLowerCase = tokenText.toLowerCase(); - const symbolIdMap = arrayToMap(knownSymbols, s => getUniqueSymbolIdAsString(s, typeChecker)); - - const allPotentialModules = getOtherModuleSymbols(allSourceFiles, sourceFile, typeChecker); - for (const moduleSymbol of allPotentialModules) { - // check the default export - const defaultExport = typeChecker.tryGetMemberInModuleExports("default", moduleSymbol); - if (defaultExport) { - const localSymbol = getLocalSymbolForExportDefault(defaultExport); - if (localSymbol && !symbolIdMap.has(getUniqueSymbolIdAsString(localSymbol, typeChecker)) && startsWith(localSymbol.name.toLowerCase(), tokenTextLowerCase)) { - otherSourceFileExports.push(localSymbol); - symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(localSymbol, typeChecker), { moduleSymbol, isDefaultExport: true }); + const symbolIdMap = arrayToNumericMap(symbols, s => getUniqueSymbolId(s, typeChecker)); + + codefix.eachOtherExternalModule(typeChecker, allSourceFiles, sourceFile, moduleSymbol => { + for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { + let { name } = symbol; + const isDefaultExport = name === "default"; + if (isDefaultExport) { + const localSymbol = getLocalSymbolForExportDefault(symbol); + if (localSymbol) { + symbol = localSymbol; + name = localSymbol.name; + } } - } - // check exports with the same name - const allExportedSymbols = typeChecker.getExportsOfModule(moduleSymbol); - if (allExportedSymbols) { - for (const exportedSymbol of allExportedSymbols) { - if (exportedSymbol.name && !symbolIdMap.has(getUniqueSymbolIdAsString(exportedSymbol, typeChecker)) && startsWith(exportedSymbol.name.toLowerCase(), tokenTextLowerCase)) { - otherSourceFileExports.push(exportedSymbol); - symbolToOriginInfoMap.set(getUniqueSymbolIdAsString(exportedSymbol, typeChecker), { moduleSymbol }); - } + const id = getUniqueSymbolId(symbol, typeChecker); + if (!symbolIdMap[id] && startsWith(name.toLowerCase(), tokenTextLowerCase)) { + symbols.push(symbol); + symbolToOriginInfoMap[id] = { moduleSymbol, isDefaultExport }; } } - } - return otherSourceFileExports; + }); } /** @@ -1739,7 +1737,7 @@ namespace ts.Completions { // First check of the displayName is not external module; if it is an external module, it is not valid entry if (symbol.flags & SymbolFlags.Namespace) { const firstCharCode = name.charCodeAt(0); - if (firstCharCode === CharacterCodes.singleQuote || firstCharCode === CharacterCodes.doubleQuote) { + if (isSingleOrDoubleQuote(firstCharCode)) { // If the symbol is external module, don't show it in the completion list // (i.e declare module "http" { const x; } | // <= request completion here, "http" should not be there) return undefined; diff --git a/src/services/pathCompletions.ts b/src/services/pathCompletions.ts index c41ed798b1d27..429160aedd271 100644 --- a/src/services/pathCompletions.ts +++ b/src/services/pathCompletions.ts @@ -339,7 +339,7 @@ namespace ts.Completions.PathCompletions { } } else if (host.getDirectories) { - let typeRoots: string[]; + let typeRoots: ReadonlyArray; try { // Wrap in try catch because getEffectiveTypeRoots touches the filesystem typeRoots = getEffectiveTypeRoots(options, host); diff --git a/src/services/refactorProvider.ts b/src/services/refactorProvider.ts index c8dc1cf360a16..923123b44b7cb 100644 --- a/src/services/refactorProvider.ts +++ b/src/services/refactorProvider.ts @@ -14,13 +14,11 @@ namespace ts { getAvailableActions(context: RefactorContext): ApplicableRefactorInfo[] | undefined; } - export interface RefactorContext { + export interface RefactorContext extends textChanges.TextChangesContext { file: SourceFile; startPosition: number; endPosition?: number; program: Program; - newLineCharacter: string; - rulesProvider?: formatting.RulesProvider; cancellationToken?: CancellationToken; } diff --git a/src/services/shims.ts b/src/services/shims.ts index e3103364f7346..70a82763bceaa 100644 --- a/src/services/shims.ts +++ b/src/services/shims.ts @@ -141,7 +141,7 @@ namespace ts { getEncodedSemanticClassifications(fileName: string, start: number, length: number): string; getCompletionsAtPosition(fileName: string, position: number): string; - getCompletionEntryDetails(fileName: string, position: number, entryName: string): string; + getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: string/*Services.FormatCodeOptions*/): string; getQuickInfoAtPosition(fileName: string, position: number): string; @@ -601,7 +601,7 @@ namespace ts { this.logger = this.host; } - public forwardJSONCall(actionDescription: string, action: () => any): string { + public forwardJSONCall(actionDescription: string, action: () => {}): string { return forwardJSONCall(this.logger, actionDescription, action, this.logPerformance); } @@ -893,10 +893,13 @@ namespace ts { } /** Get a string based representation of a completion list entry details */ - public getCompletionEntryDetails(fileName: string, position: number, entryName: string) { + public getCompletionEntryDetails(fileName: string, position: number, entryName: string, options: string/*Services.FormatCodeOptions*/) { return this.forwardJSONCall( `getCompletionEntryDetails('${fileName}', ${position}, '${entryName}')`, - () => this.languageService.getCompletionEntryDetails(fileName, position, entryName) + () => { + const localOptions: ts.FormatCodeOptions = JSON.parse(options); + return this.languageService.getCompletionEntryDetails(fileName, position, entryName, localOptions); + } ); } @@ -1030,7 +1033,7 @@ namespace ts { super(factory); } - private forwardJSONCall(actionDescription: string, action: () => any): any { + private forwardJSONCall(actionDescription: string, action: () => {}): any { return forwardJSONCall(this.logger, actionDescription, action, this.logPerformance); } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index ecd22a44f1782..63cfa64ef07b9 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -186,15 +186,25 @@ namespace ts.textChanges { return s; } + export interface TextChangesContext { + newLineCharacter: string; + rulesProvider: formatting.RulesProvider; + } + export class ChangeTracker { private changes: Change[] = []; private readonly newLineCharacter: string; - //todo: don't include ImportCodeFixContext - public static fromContext(context: RefactorContext | CodeFixContext | ts.codefix.ImportCodeFixContext) { + public static fromContext(context: TextChangesContext): ChangeTracker { return new ChangeTracker(context.newLineCharacter === "\n" ? NewLineKind.LineFeed : NewLineKind.CarriageReturnLineFeed, context.rulesProvider); } + public static with(context: TextChangesContext, cb: (tracker: ChangeTracker) => void): FileTextChanges[] { + const tracker = ChangeTracker.fromContext(context); + cb(tracker); + return tracker.getChanges(); + } + constructor( private readonly newLine: NewLineKind, private readonly rulesProvider: formatting.RulesProvider, diff --git a/src/services/types.ts b/src/services/types.ts index be68584ec0897..9c8b2753d8910 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -228,7 +228,8 @@ namespace ts { getEncodedSemanticClassifications(fileName: string, span: TextSpan): Classifications; getCompletionsAtPosition(fileName: string, position: number): CompletionInfo; - getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails; + // "options" is optional only for backwards-compatibility + getCompletionEntryDetails(fileName: string, position: number, entryName: string, options?: FormatCodeOptions | FormatCodeSettings): CompletionEntryDetails; getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol; getQuickInfoAtPosition(fileName: string, position: number): QuickInfo; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 72994da4efb18..145fdb2fcfa5f 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1298,9 +1298,7 @@ namespace ts { */ export function stripQuotes(name: string) { const length = name.length; - if (length >= 2 && - name.charCodeAt(0) === name.charCodeAt(length - 1) && - (name.charCodeAt(0) === CharacterCodes.doubleQuote || name.charCodeAt(0) === CharacterCodes.singleQuote)) { + if (length >= 2 && name.charCodeAt(0) === name.charCodeAt(length - 1) && isSingleOrDoubleQuote(name.charCodeAt(0))) { return name.substring(1, length - 1); } return name; @@ -1313,33 +1311,12 @@ namespace ts { export function getScriptKind(fileName: string, host?: LanguageServiceHost): ScriptKind { // First check to see if the script kind was specified by the host. Chances are the host - // may override the default script kind for the file extensison. + // may override the default script kind for the file extension. return ensureScriptKind(fileName, host && host.getScriptKind && host.getScriptKind(fileName)); } - export function getOtherModuleSymbols( - sourceFiles: ReadonlyArray, - currentSourceFile: SourceFile, - typeChecker: TypeChecker - ) { - const results: Symbol[] = typeChecker.getAmbientModules(); - for (const otherSourceFile of sourceFiles) { - if (otherSourceFile !== currentSourceFile && isExternalOrCommonJsModule(otherSourceFile)) { - results.push(otherSourceFile.symbol); - } - } - return results; - } - - export function getUniqueSymbolIdAsString(symbol: Symbol, typeChecker: TypeChecker) { - return getUniqueSymbolId(symbol, typeChecker) + ""; - } - - export function getUniqueSymbolId(symbol: Symbol, typeChecker: TypeChecker) { - if (symbol.flags & SymbolFlags.Alias) { - return getSymbolId(typeChecker.getAliasedSymbol(symbol)); - } - return getSymbolId(symbol); + export function getUniqueSymbolId(symbol: Symbol, checker: TypeChecker) { + return getSymbolId(skipAlias(symbol, checker)); } export function getFirstNonSpaceCharacterPosition(text: string, position: number) { diff --git a/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts b/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts new file mode 100644 index 0000000000000..5662b57fdf678 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts @@ -0,0 +1,19 @@ +/// + +// @Filename: /a.ts +////export default function foo() {} +////export const x = 0; + +// @Filename: /b.ts +////import { x } from "./a"; +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Add 'foo' to existing import declaration from "./a".`, + newFileContent: `import foo, { x } from "./a"; +f;`, +}); diff --git a/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts b/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts new file mode 100644 index 0000000000000..b442ed21549e1 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts @@ -0,0 +1,18 @@ +/// + +// @Filename: /a.ts +////export default function foo() {} + +// @Filename: /b.ts +////import * as a from "./a"; +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Add 'foo' to existing import declaration from "./a".`, + newFileContent: `import foo, * as a from "./a"; +f;`, +}); diff --git a/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts b/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts new file mode 100644 index 0000000000000..3ee4decb0f8b7 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts @@ -0,0 +1,20 @@ +/// + +// @Filename: /a.ts +////export default function foo() {} + +// @Filename: /b.ts +////import f_o_o from "./a"; +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Import 'foo' from "./a".`, + // TODO: GH#18445 + newFileContent: `import f_o_o from "./a"; +import foo from "./a";\r +f;`, +}); diff --git a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts new file mode 100644 index 0000000000000..6dbd437d36470 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts @@ -0,0 +1,19 @@ +/// + +// @Filename: /a.ts +////export default function foo() {} + +// @Filename: /b.ts +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Import 'foo' from "./a".`, + // TODO: GH#18445 + newFileContent: `import foo from "./a";\r +\r +f;`, +}); diff --git a/tests/cases/fourslash/completionsImport_fromAmbientModule.ts b/tests/cases/fourslash/completionsImport_fromAmbientModule.ts new file mode 100644 index 0000000000000..b45ad9824865a --- /dev/null +++ b/tests/cases/fourslash/completionsImport_fromAmbientModule.ts @@ -0,0 +1,18 @@ +/// + +// @Filename: /a.ts +////declare module "m" { +//// export const x: number; +////} + +// @Filename: /b.ts +/////**/ + +verify.applyCodeActionFromCompletion("", { + name: "x", + description: `Import 'x' from "m".`, + // TODO: GH#18445 + newFileContent: `import { x } from "m";\r +\r +`, +}); diff --git a/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts new file mode 100644 index 0000000000000..23119ad491f79 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts @@ -0,0 +1,19 @@ +/// + +// @Filename: /a.ts +////export function foo() {} +////export const x = 0; + +// @Filename: /b.ts +////import { x } from "./a"; +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Add 'foo' to existing import declaration from "./a".`, + newFileContent: `import { x, foo } from "./a"; +f;`, +}); diff --git a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts new file mode 100644 index 0000000000000..95c68c2a05e88 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts @@ -0,0 +1,20 @@ +/// + +// @Filename: /a.ts +////export function Test1() {} +////export function Test2() {} + +// @Filename: /b.ts +////import { Test2 } from "./a"; +////t/**/ + +goTo.marker(""); +verify.completionListContains("Test1", "function Test1(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains("Test2", "import Test2", "", "alias", /*spanIndex*/ undefined, /*hasAction*/ undefined); + +verify.applyCodeActionFromCompletion("", { + name: "Test1", + description: `Add 'Test1' to existing import declaration from "./a".`, + newFileContent: `import { Test2, Test1 } from "./a"; +t`, +}); diff --git a/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts new file mode 100644 index 0000000000000..da00907e60fb7 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts @@ -0,0 +1,20 @@ +/// + +// @Filename: /a.ts +////export function foo() {} + +// @Filename: /b.ts +////import * as a from "./a"; +////f/**/; + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + description: `Import 'foo' from "./a".`, + // TODO: GH#18445 + newFileContent: `import * as a from "./a"; +import { foo } from "./a";\r +f;`, +}); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index f4d47abc9c9c4..e1d9607de8aaf 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -140,7 +140,14 @@ declare namespace FourSlashInterface { allowedConstructorParameterKeywords: string[]; constructor(negative?: boolean); completionListCount(expectedCount: number): void; - completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number): void; + completionListContains( + symbol: string, + text?: string, + documentation?: string, + kind?: string, + spanIndex?: number, + hasAction?: boolean, + ): void; completionListItemsCountIsGreaterThan(count: number): void; completionListIsEmpty(): void; completionListContainsClassElementKeywords(): void; @@ -173,6 +180,20 @@ declare namespace FourSlashInterface { assertHasRanges(ranges: Range[]): void; caretAtMarker(markerName?: string): void; completionsAt(markerName: string, completions: string[], options?: { isNewIdentifierLocation?: boolean }): void; + completionsAndDetailsAt( + markerName: string, + completions: { + excludes?: ReadonlyArray, + //TODO: better type + entries: ReadonlyArray<{ entry: any, details: any }>, + }, + ): void; //TODO: better type + applyCodeActionFromCompletion(markerName: string, options: { + name: string, + description: string, + newFileContent?: string, + newRangeContent?: string, + }); indentationIs(numberOfSpaces: number): void; indentationAtPositionIs(fileName: string, position: number, numberOfSpaces: number, indentStyle?: ts.IndentStyle, baseIndentSize?: number): void; textAtCaretIs(text: string): void; diff --git a/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts b/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts index 34cebad941544..218ec4fabb724 100644 --- a/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts +++ b/tests/cases/fourslash/importNameCodeFixOptionalImport0.ts @@ -8,7 +8,7 @@ //// export function foo() {}; // @Filename: a/foo.ts -//// export { foo } from "./foo/bar"; +//// export { foo } from "./foo/bar"; verify.importFixAtPosition([ `import * as ns from "./foo"; From c844297ca3c4666fb0aaa58b40bf4c921aab2fe7 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Tue, 10 Oct 2017 14:32:32 -0700 Subject: [PATCH 37/40] Suggestions from code review --- src/server/protocol.ts | 4 ++-- src/server/session.ts | 11 +++-------- src/services/codefixes/importFixes.ts | 14 +++++++++----- src/services/completions.ts | 6 +++++- 4 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/server/protocol.ts b/src/server/protocol.ts index f3ba68948f202..7b9e9fe80969a 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1659,8 +1659,8 @@ namespace ts.server.protocol { */ replacementSpan?: TextSpan; /** - * Indicates whether commiting this completion entry will require additional code action to be - * made to avoid errors. The code action is normally adding an additional import statement. + * Indicates whether commiting this completion entry will require additional code actions to be + * made to avoid errors. The CompletionEntryDetails will have these actions. */ hasAction?: true; } diff --git a/src/server/session.ts b/src/server/session.ts index 249acd8f10419..6849c9a86c7b5 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1184,17 +1184,12 @@ namespace ts.server { const completions = project.getLanguageService().getCompletionsAtPosition(file, position); if (simplifiedResult) { - return mapDefined(completions && completions.entries, entry => { + return mapDefined(completions && completions.entries, entry => { if (completions.isMemberCompletion || (entry.name.toLowerCase().indexOf(prefix.toLowerCase()) === 0)) { const { name, kind, kindModifiers, sortText, replacementSpan, hasAction } = entry; const convertedSpan = replacementSpan ? this.decorateSpan(replacementSpan, scriptInfo) : undefined; - - const newEntry: protocol.CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan }; - // avoid serialization when hasAction = false - if (hasAction) { - newEntry.hasAction = true; - } - return newEntry; + // Use `hasAction || undefined` to avoid serializing `false`. + return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined }; } }).sort((a, b) => compareStrings(a.name, b.name)); } diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 95ceb87043fea..3e4e0f9e82246 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -648,7 +648,11 @@ namespace ts.codefix { const symbolIdActionMap = new ImportCodeActionMap(); const currentTokenMeaning = getMeaningFromLocation(symbolToken); - eachOtherExternalModule(checker, allSourceFiles, sourceFile, moduleSymbol => { + forEachExternalModule(checker, allSourceFiles, moduleSymbol => { + if (moduleSymbol === sourceFile.symbol) { + return; + } + cancellationToken.throwIfCancellationRequested(); // check the default export const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol); @@ -676,13 +680,13 @@ namespace ts.codefix { return some(declarations, decl => !!(getMeaningFromDeclaration(decl) & meaning)); } - export function eachOtherExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, sourceFile: SourceFile, cb: (module: Symbol) => void) { + export function forEachExternalModule(checker: TypeChecker, allSourceFiles: ReadonlyArray, cb: (module: Symbol) => void) { for (const ambient of checker.getAmbientModules()) { cb(ambient); } - for (const otherSourceFile of allSourceFiles) { - if (otherSourceFile !== sourceFile && isExternalOrCommonJsModule(otherSourceFile)) { - cb(otherSourceFile.symbol); + for (const sourceFile of allSourceFiles) { + if (isExternalOrCommonJsModule(sourceFile)) { + cb(sourceFile.symbol); } } } diff --git a/src/services/completions.ts b/src/services/completions.ts index 4584da7765e37..71fd41f20d79c 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -925,7 +925,11 @@ namespace ts.Completions { const tokenTextLowerCase = tokenText.toLowerCase(); const symbolIdMap = arrayToNumericMap(symbols, s => getUniqueSymbolId(s, typeChecker)); - codefix.eachOtherExternalModule(typeChecker, allSourceFiles, sourceFile, moduleSymbol => { + codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => { + if (moduleSymbol === sourceFile.symbol) { + return; + } + for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) { let { name } = symbol; const isDefaultExport = name === "default"; From 5da1595ef2b606cdf204887164701a4b0cb86dc0 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Wed, 11 Oct 2017 12:02:20 -0700 Subject: [PATCH 38/40] Update api baseline --- tests/baselines/reference/api/tsserverlibrary.d.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index 3239895057564..2ec9233d94a2b 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -6040,8 +6040,8 @@ declare namespace ts.server.protocol { */ replacementSpan?: TextSpan; /** - * Indicates whether commiting this completion entry will require additional code action to be - * made to avoid errors. The code action is normally adding an additional import statement. + * Indicates whether commiting this completion entry will require additional code actions to be + * made to avoid errors. The CompletionEntryDetails will have these actions. */ hasAction?: true; } From a5b846f37b88b8ae98ea18698c81e8d2d36db4ed Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Fri, 13 Oct 2017 15:04:15 -0700 Subject: [PATCH 39/40] Fix bug if previousToken is not an Identifier --- src/services/completions.ts | 2 +- .../completionsImport_previousTokenIsSemicolon.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 2e3f6abfae0f1..331b591bab0c4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -840,7 +840,7 @@ namespace ts.Completions { const symbolMeanings = SymbolFlags.Type | SymbolFlags.Value | SymbolFlags.Namespace | SymbolFlags.Alias; symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings); - getSymbolsFromOtherSourceFileExports(symbols, previousToken === undefined ? "" : previousToken.getText()); + getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : ""); filterGlobalCompletion(symbols); return true; diff --git a/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts b/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts new file mode 100644 index 0000000000000..904de35354ac5 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts @@ -0,0 +1,11 @@ +/// + +// @Filename: /a.ts +////export function foo() {} + +// @Filename: /b.ts +////import * as a from 'a'; +/////**/ + +goTo.marker(""); +verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); From 4c607e73d5b8cf167e954d4b20bef040f2855e02 Mon Sep 17 00:00:00 2001 From: Andy Hanson Date: Mon, 16 Oct 2017 14:09:56 -0700 Subject: [PATCH 40/40] Replace `startsWith` with `stringContainsCharactersInOrder` --- src/services/completions.ts | 26 ++++++++++++++++++- .../fourslash/completionsImport_matching.ts | 22 ++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 tests/cases/fourslash/completionsImport_matching.ts diff --git a/src/services/completions.ts b/src/services/completions.ts index 331b591bab0c4..7afa85ce26df4 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -942,7 +942,7 @@ namespace ts.Completions { } const id = getUniqueSymbolId(symbol, typeChecker); - if (!symbolIdMap[id] && startsWith(name.toLowerCase(), tokenTextLowerCase)) { + if (!symbolIdMap[id] && stringContainsCharactersInOrder(name.toLowerCase(), tokenTextLowerCase)) { symbols.push(symbol); symbolToOriginInfoMap[id] = { moduleSymbol, isDefaultExport }; } @@ -950,6 +950,30 @@ namespace ts.Completions { }); } + /** + * True if you could remove some characters in `a` to get `b`. + * E.g., true for "abcdef" and "bdf". + * But not true for "abcdef" and "dbf". + */ + function stringContainsCharactersInOrder(str: string, characters: string): boolean { + if (characters.length === 0) { + return true; + } + + let characterIndex = 0; + for (let strIndex = 0; strIndex < str.length; strIndex++) { + if (str.charCodeAt(strIndex) === characters.charCodeAt(characterIndex)) { + characterIndex++; + if (characterIndex === characters.length) { + return true; + } + } + } + + // Did not find all characters + return false; + } + /** * Finds the first node that "embraces" the position, so that one may * accurately aggregate locals from the closest containing scope. diff --git a/tests/cases/fourslash/completionsImport_matching.ts b/tests/cases/fourslash/completionsImport_matching.ts new file mode 100644 index 0000000000000..3470d59bff20b --- /dev/null +++ b/tests/cases/fourslash/completionsImport_matching.ts @@ -0,0 +1,22 @@ +/// + +// @Filename: /a.ts +// Not included: +////export function abcde() {} +////export function dbf() {} +// Included: +////export function bdf() {} +////export function abcdef() {} +////export function BDF() {} + +// @Filename: /b.ts +////bdf/**/ + +goTo.marker(""); + +verify.not.completionListContains("abcde"); +verify.not.completionListContains("dbf"); + +verify.completionListContains("bdf", "function bdf(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains("abcdef", "function abcdef(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains("BDF", "function BDF(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true);