From 813864f021c92a25e1e905bb2941e2a68d248d12 Mon Sep 17 00:00:00 2001 From: Andy Date: Thu, 21 Dec 2017 11:16:09 -0800 Subject: [PATCH] For import completion, use an existing namespace import (#20457) --- src/services/codefixes/importFixes.ts | 25 ++++++++++--------- src/services/completions.ts | 19 ++++++++------ src/services/textChanges.ts | 5 ++++ ...tionsImport_named_namespaceImportExists.ts | 5 ++-- .../importNameCodeFixDefaultExport1.ts | 13 ++++++++++ 5 files changed, 44 insertions(+), 23 deletions(-) create mode 100644 tests/cases/fourslash/importNameCodeFixDefaultExport1.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 04c1cd4b6d9..20d6ac4d5ba 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -30,7 +30,7 @@ namespace ts.codefix { } interface SymbolAndTokenContext extends SymbolContext { - symbolToken: Node | undefined; + symbolToken: Identifier | undefined; } interface ImportCodeFixContext extends SymbolAndTokenContext { @@ -169,7 +169,8 @@ namespace ts.codefix { const useCaseSensitiveFileNames = context.host.useCaseSensitiveFileNames ? context.host.useCaseSensitiveFileNames() : false; const { program } = context; const checker = program.getTypeChecker(); - const symbolToken = getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false); + // This will always be an Identifier, since the diagnostics we fix only fail on identifiers. + const symbolToken = cast(getTokenAtPosition(context.sourceFile, context.span.start, /*includeJsDocComment*/ false), isIdentifier); return { host: context.host, newLineCharacter: context.newLineCharacter, @@ -213,14 +214,17 @@ namespace ts.codefix { for (const declaration of declarations) { const namespace = getNamespaceImportName(declaration); if (namespace) { - actions.push(getCodeActionForUseExistingNamespaceImport(namespace.text, context, context.symbolToken)); + const moduleSymbol = context.checker.getAliasedSymbol(context.checker.getSymbolAtLocation(namespace)); + if (moduleSymbol && moduleSymbol.exports.has(escapeLeadingUnderscores(context.symbolName))) { + actions.push(getCodeActionForUseExistingNamespaceImport(namespace.text, context, context.symbolToken)); + } } } } return [...actions, ...getCodeActionsForAddImport(moduleSymbols, context, declarations)]; } - function getNamespaceImportName(declaration: AnyImportSyntax): Identifier { + function getNamespaceImportName(declaration: AnyImportSyntax): Identifier | undefined { if (declaration.kind === SyntaxKind.ImportDeclaration) { const namedBindings = declaration.importClause && isImportClause(declaration.importClause) && declaration.importClause.namedBindings; return namedBindings && namedBindings.kind === SyntaxKind.NamespaceImport ? namedBindings.name : undefined; @@ -683,7 +687,7 @@ namespace ts.codefix { } } - function getCodeActionForUseExistingNamespaceImport(namespacePrefix: string, context: SymbolContext, symbolToken: Node): ImportCodeAction { + function getCodeActionForUseExistingNamespaceImport(namespacePrefix: string, context: SymbolContext, symbolToken: Identifier): ImportCodeAction { const { symbolName, sourceFile } = context; /** @@ -696,13 +700,10 @@ namespace ts.codefix { * 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); + // Prefix the node instead of it replacing it, because this may be used for import completions and we don't want the text changes to overlap with the identifier being completed. + const changes = ChangeTracker.with(context, tracker => + tracker.changeIdentifierToPropertyAccess(sourceFile, namespacePrefix, symbolToken)); + return createCodeAction(Diagnostics.Change_0_to_1, [symbolName, `${namespacePrefix}.${symbolName}`], changes, "CodeChange", /*moduleSpecifier*/ undefined); } function getImportCodeActions(context: CodeFixContext): ImportCodeAction[] { diff --git a/src/services/completions.ts b/src/services/completions.ts index 51553a4a6b7..11119503d36 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -431,13 +431,13 @@ namespace ts.Completions { position: number, { name, source }: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, - ): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | { type: "request", request: Request } | { type: "none" } { + ): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap, previousToken: Node } | { type: "request", request: Request } | { type: "none" } { const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target); if (!completionData) { return { type: "none" }; } - const { symbols, location, allowStringLiteral, symbolToOriginInfoMap, request } = completionData; + const { symbols, location, allowStringLiteral, symbolToOriginInfoMap, request, previousToken } = completionData; if (request) { return { type: "request", request }; } @@ -451,7 +451,7 @@ namespace ts.Completions { return getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral, origin) === name && getSourceFromOrigin(origin) === source; }); - return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap } : { type: "none" }; + return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap, previousToken } : { type: "none" }; } function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string { @@ -496,8 +496,8 @@ namespace ts.Completions { } } case "symbol": { - const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, formatContext, getCanonicalFileName, allSourceFiles); + const { symbol, location, symbolToOriginInfoMap, previousToken } = symbolCompletion; + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, previousToken, formatContext, getCanonicalFileName, allSourceFiles); const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: sourceDisplay }; @@ -529,6 +529,7 @@ namespace ts.Completions { host: LanguageServiceHost, compilerOptions: CompilerOptions, sourceFile: SourceFile, + previousToken: Node, formatContext: formatting.FormatContext, getCanonicalFileName: GetCanonicalFileName, allSourceFiles: ReadonlyArray, @@ -554,9 +555,9 @@ namespace ts.Completions { formatContext, symbolName: getSymbolName(symbol, symbolOriginInfo, compilerOptions.target), getCanonicalFileName, - symbolToken: undefined, + symbolToken: tryCast(previousToken, isIdentifier), kind: isDefaultExport ? codefix.ImportKind.Default : codefix.ImportKind.Named, - }); + }).slice(0, 1); // Only take the first code action return { sourceDisplay, codeActions }; } @@ -597,6 +598,7 @@ namespace ts.Completions { keywordFilters: KeywordCompletionFilters; symbolToOriginInfoMap: SymbolOriginInfoMap; recommendedCompletion: Symbol | undefined; + previousToken: Node; } type Request = { kind: "JsDocTagName" } | { kind: "JsDocTag" } | { kind: "JsDocParameterName", tag: JSDocParameterTag }; @@ -709,6 +711,7 @@ namespace ts.Completions { keywordFilters: KeywordCompletionFilters.None, symbolToOriginInfoMap: undefined, recommendedCompletion: undefined, + previousToken: undefined, }; } @@ -849,7 +852,7 @@ namespace ts.Completions { log("getCompletionData: Semantic work: " + (timestamp() - semanticStart)); const recommendedCompletion = getRecommendedCompletion(previousToken, typeChecker); - return { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters, symbolToOriginInfoMap, recommendedCompletion }; + return { symbols, isGlobalCompletion, isMemberCompletion, allowStringLiteral, isNewIdentifierLocation, location, isRightOfDot: (isRightOfDot || isRightOfOpenTag), request, keywordFilters, symbolToOriginInfoMap, recommendedCompletion, previousToken }; type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag; diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 82fe3d20f6b..e73c639ba79 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -345,6 +345,11 @@ namespace ts.textChanges { return this.replaceWithSingle(sourceFile, startPosition, startPosition, newNode, this.getOptionsForInsertNodeBefore(before, blankLineBetween)); } + public changeIdentifierToPropertyAccess(sourceFile: SourceFile, prefix: string, node: Identifier): void { + const startPosition = getAdjustedStartPosition(sourceFile, node, {}, Position.Start); + this.replaceWithSingle(sourceFile, startPosition, startPosition, createPropertyAccess(createIdentifier(prefix), ""), {}); + } + private getOptionsForInsertNodeBefore(before: Node, doubleNewlines: boolean): ChangeNodeOptions { if (isStatement(before) || isClassElement(before)) { return { suffix: doubleNewlines ? this.newLineCharacter + this.newLineCharacter : this.newLineCharacter }; diff --git a/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts index 2ea27e7eb12..112cecba74a 100644 --- a/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts +++ b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts @@ -16,9 +16,8 @@ verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): vo verify.applyCodeActionFromCompletion("", { name: "foo", source: "/a", - description: `Import 'foo' from module "./a"`, + description: `Change 'foo' to 'a.foo'`, // TODO: GH#18445 newFileContent: `import * as a from "./a"; -import { foo } from "./a";\r -f;`, +a.f;`, }); diff --git a/tests/cases/fourslash/importNameCodeFixDefaultExport1.ts b/tests/cases/fourslash/importNameCodeFixDefaultExport1.ts new file mode 100644 index 00000000000..60fbff9f12c --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFixDefaultExport1.ts @@ -0,0 +1,13 @@ +/// + +// @Filename: /foo-bar.ts +////export default function fooBar(); + +// @Filename: /b.ts +////[|import * as fb from "./foo-bar"; +////foo/**/Bar|] + +goTo.file("/b.ts"); +// No suggestion to use `fb.fooBar` (which would be wrong) +verify.importFixAtPosition([`import fooBar, * as fb from "./foo-bar"; +fooBar`]);