Skip to content

Commit

Permalink
For import completion, use an existing namespace import (microsoft#20457
Browse files Browse the repository at this point in the history
)
  • Loading branch information
Andy authored Dec 21, 2017
1 parent 98b212d commit 813864f
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 23 deletions.
25 changes: 13 additions & 12 deletions src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace ts.codefix {
}

interface SymbolAndTokenContext extends SymbolContext {
symbolToken: Node | undefined;
symbolToken: Identifier | undefined;
}

interface ImportCodeFixContext extends SymbolAndTokenContext {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;

/**
Expand All @@ -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[] {
Expand Down
19 changes: 11 additions & 8 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -431,13 +431,13 @@ namespace ts.Completions {
position: number,
{ name, source }: CompletionEntryIdentifier,
allSourceFiles: ReadonlyArray<SourceFile>,
): { 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 };
}
Expand All @@ -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 {
Expand Down Expand Up @@ -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 };
Expand Down Expand Up @@ -529,6 +529,7 @@ namespace ts.Completions {
host: LanguageServiceHost,
compilerOptions: CompilerOptions,
sourceFile: SourceFile,
previousToken: Node,
formatContext: formatting.FormatContext,
getCanonicalFileName: GetCanonicalFileName,
allSourceFiles: ReadonlyArray<SourceFile>,
Expand All @@ -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 };
}

Expand Down Expand Up @@ -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 };

Expand Down Expand Up @@ -709,6 +711,7 @@ namespace ts.Completions {
keywordFilters: KeywordCompletionFilters.None,
symbolToOriginInfoMap: undefined,
recommendedCompletion: undefined,
previousToken: undefined,
};
}

Expand Down Expand Up @@ -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;

Expand Down
5 changes: 5 additions & 0 deletions src/services/textChanges.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;`,
});
13 changes: 13 additions & 0 deletions tests/cases/fourslash/importNameCodeFixDefaultExport1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
/// <reference path="fourslash.ts" />

// @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`]);

0 comments on commit 813864f

Please sign in to comment.