Skip to content

Commit 92ade59

Browse files
author
Andy
authored
For import completion of default import, convert module name to identifier (#19875) (#19883)
* For import completion of default import, convert module name to identifier * Suggestions from code review
1 parent c56a822 commit 92ade59

File tree

8 files changed

+108
-18
lines changed

8 files changed

+108
-18
lines changed

‎src/compiler/scanner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ namespace ts {
291291
}
292292

293293
/* @internal */
294-
export function stringToToken(s: string): SyntaxKind {
294+
export function stringToToken(s: string): SyntaxKind | undefined {
295295
return textToToken.get(s);
296296
}
297297

‎src/compiler/types.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,7 +201,7 @@ namespace ts {
201201
UndefinedKeyword,
202202
FromKeyword,
203203
GlobalKeyword,
204-
OfKeyword, // LastKeyword and LastToken
204+
OfKeyword, // LastKeyword and LastToken and LastContextualKeyword
205205

206206
// Parse tree nodes
207207

@@ -415,7 +415,9 @@ namespace ts {
415415
FirstJSDocNode = JSDocTypeExpression,
416416
LastJSDocNode = JSDocTypeLiteral,
417417
FirstJSDocTagNode = JSDocTag,
418-
LastJSDocTagNode = JSDocTypeLiteral
418+
LastJSDocTagNode = JSDocTypeLiteral,
419+
/* @internal */ FirstContextualKeyword = AbstractKeyword,
420+
/* @internal */ LastContextualKeyword = OfKeyword,
419421
}
420422

421423
export const enum NodeFlags {

‎src/compiler/utilities.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1903,6 +1903,14 @@ namespace ts {
19031903
return SyntaxKind.FirstKeyword <= token && token <= SyntaxKind.LastKeyword;
19041904
}
19051905

1906+
export function isContextualKeyword(token: SyntaxKind): boolean {
1907+
return SyntaxKind.FirstContextualKeyword <= token && token <= SyntaxKind.LastContextualKeyword;
1908+
}
1909+
1910+
export function isNonContextualKeyword(token: SyntaxKind): boolean {
1911+
return isKeyword(token) && !isContextualKeyword(token);
1912+
}
1913+
19061914
export function isTrivia(token: SyntaxKind) {
19071915
return SyntaxKind.FirstTriviaToken <= token && token <= SyntaxKind.LastTriviaToken;
19081916
}

‎src/harness/fourslash.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3021,7 +3021,7 @@ Actual: ${stringify(fullActual)}`);
30213021
}
30223022
}
30233023

3024-
const itemsString = items.map(item => stringify({ name: item.name, kind: item.kind })).join(",\n");
3024+
const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
30253025

30263026
this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
30273027
}

‎src/services/codefixes/importFixes.ts

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -666,9 +666,10 @@ namespace ts.codefix {
666666
const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol);
667667
if (defaultExport) {
668668
const localSymbol = getLocalSymbolForExportDefault(defaultExport);
669-
if (localSymbol && localSymbol.escapedName === symbolName && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) {
669+
if ((localSymbol && localSymbol.escapedName === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, context.compilerOptions.target) === symbolName)
670+
&& checkSymbolHasMeaning(localSymbol || defaultExport, currentTokenMeaning)) {
670671
// check if this symbol is already used
671-
const symbolId = getUniqueSymbolId(localSymbol, checker);
672+
const symbolId = getUniqueSymbolId(localSymbol || defaultExport, checker);
672673
symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, { ...context, kind: ImportKind.Default }));
673674
}
674675
}
@@ -698,4 +699,35 @@ namespace ts.codefix {
698699
}
699700
}
700701
}
702+
703+
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string {
704+
return moduleSpecifierToValidIdentifier(removeFileExtension(getBaseFileName(moduleSymbol.name)), target);
705+
}
706+
707+
function moduleSpecifierToValidIdentifier(moduleSpecifier: string, target: ScriptTarget): string {
708+
let res = "";
709+
let lastCharWasValid = true;
710+
const firstCharCode = moduleSpecifier.charCodeAt(0);
711+
if (isIdentifierStart(firstCharCode, target)) {
712+
res += String.fromCharCode(firstCharCode);
713+
}
714+
else {
715+
lastCharWasValid = false;
716+
}
717+
for (let i = 1; i < moduleSpecifier.length; i++) {
718+
const ch = moduleSpecifier.charCodeAt(i);
719+
const isValid = isIdentifierPart(ch, target);
720+
if (isValid) {
721+
let char = String.fromCharCode(ch);
722+
if (!lastCharWasValid) {
723+
char = char.toUpperCase();
724+
}
725+
res += char;
726+
}
727+
lastCharWasValid = isValid;
728+
}
729+
// Need `|| "_"` to ensure result isn't empty.
730+
const token = stringToToken(res);
731+
return token === undefined || !isNonContextualKeyword(token) ? res || "_" : `_${res}`;
732+
}
701733
}

‎src/services/completions.ts

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ namespace ts.Completions {
3838
return getStringLiteralCompletionEntries(sourceFile, position, typeChecker, compilerOptions, host, log);
3939
}
4040

41-
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options);
41+
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options, compilerOptions.target);
4242
if (!completionData) {
4343
return undefined;
4444
}
@@ -135,12 +135,12 @@ namespace ts.Completions {
135135
typeChecker: TypeChecker,
136136
target: ScriptTarget,
137137
allowStringLiteral: boolean,
138-
origin: SymbolOriginInfo,
138+
origin: SymbolOriginInfo | undefined,
139139
): CompletionEntry | undefined {
140140
// Try to get a valid display name for this symbol, if we could not find one, then ignore it.
141141
// We would like to only show things that can be added after a dot, so for instance numeric properties can
142142
// not be accessed with a dot (a.1 <- invalid)
143-
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral);
143+
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral, origin);
144144
if (!displayName) {
145145
return undefined;
146146
}
@@ -363,7 +363,7 @@ namespace ts.Completions {
363363
{ name, source }: CompletionEntryIdentifier,
364364
allSourceFiles: ReadonlyArray<SourceFile>,
365365
): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | { type: "request", request: Request } | { type: "none" } {
366-
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true });
366+
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target);
367367
if (!completionData) {
368368
return { type: "none" };
369369
}
@@ -377,12 +377,18 @@ namespace ts.Completions {
377377
// We don't need to perform character checks here because we're only comparing the
378378
// name against 'entryName' (which is known to be good), not building a new
379379
// completion entry.
380-
const symbol = find(symbols, s =>
381-
getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name
382-
&& getSourceFromOrigin(symbolToOriginInfoMap[getSymbolId(s)]) === source);
380+
const symbol = find(symbols, s => {
381+
const origin = symbolToOriginInfoMap[getSymbolId(s)];
382+
return getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral, origin) === name
383+
&& getSourceFromOrigin(origin) === source;
384+
});
383385
return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap } : { type: "none" };
384386
}
385387

388+
function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string {
389+
return origin && origin.isDefaultExport && symbol.name === "default" ? codefix.moduleSymbolToValidIdentifier(origin.moduleSymbol, target) : symbol.name;
390+
}
391+
386392
export interface CompletionEntryIdentifier {
387393
name: string;
388394
source?: string;
@@ -464,7 +470,7 @@ namespace ts.Completions {
464470
compilerOptions,
465471
sourceFile,
466472
rulesProvider,
467-
symbolName: symbol.name,
473+
symbolName: getSymbolName(symbol, symbolOriginInfo, compilerOptions.target),
468474
getCanonicalFileName: createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false),
469475
symbolToken: undefined,
470476
kind: isDefaultExport ? codefix.ImportKind.Default : codefix.ImportKind.Named,
@@ -505,6 +511,7 @@ namespace ts.Completions {
505511
position: number,
506512
allSourceFiles: ReadonlyArray<SourceFile>,
507513
options: GetCompletionsAtPositionOptions,
514+
target: ScriptTarget,
508515
): CompletionData | undefined {
509516
const isJavaScriptFile = isSourceFileJavaScript(sourceFile);
510517

@@ -903,7 +910,7 @@ namespace ts.Completions {
903910

904911
symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings);
905912
if (options.includeExternalModuleExports) {
906-
getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "");
913+
getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target);
907914
}
908915
filterGlobalCompletion(symbols);
909916

@@ -985,7 +992,7 @@ namespace ts.Completions {
985992
}
986993
}
987994

988-
function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string): void {
995+
function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
989996
const tokenTextLowerCase = tokenText.toLowerCase();
990997

991998
codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => {
@@ -1002,6 +1009,9 @@ namespace ts.Completions {
10021009
symbol = localSymbol;
10031010
name = localSymbol.name;
10041011
}
1012+
else {
1013+
name = codefix.moduleSymbolToValidIdentifier(moduleSymbol, target);
1014+
}
10051015
}
10061016

10071017
if (symbol.declarations && symbol.declarations.some(d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
@@ -1829,8 +1839,8 @@ namespace ts.Completions {
18291839
*
18301840
* @return undefined if the name is of external module
18311841
*/
1832-
function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean): string | undefined {
1833-
const name = symbol.name;
1842+
function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean, origin: SymbolOriginInfo | undefined): string | undefined {
1843+
const name = getSymbolName(symbol, origin, target);
18341844
if (!name) return undefined;
18351845

18361846
// First check of the displayName is not external module; if it is an external module, it is not valid entry

‎tests/cases/fourslash/completionsImport_default_anonymous.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// Use `/src` to test that directory names are not included in conversion from module path to identifier.
4+
5+
// @Filename: /src/foo-bar.ts
6+
////export default 0;
7+
8+
// @Filename: /src/b.ts
9+
////def/*0*/
10+
////fooB/*1*/
11+
12+
goTo.marker("0");
13+
verify.not.completionListContains({ name: "default", source: "/src/foo-bar" }, undefined, undefined, undefined, undefined, undefined, { includeExternalModuleExports: true });
14+
15+
goTo.marker("1");
16+
verify.completionListContains({ name: "fooBar", source: "/src/foo-bar" }, "(property) default: 0", "", "property", /*spanIndex*/ undefined, /*hasAction*/ true, { includeExternalModuleExports: true });
17+
verify.applyCodeActionFromCompletion("1", {
18+
name: "fooBar",
19+
source: "/src/foo-bar",
20+
description: `Import 'fooBar' from "./foo-bar".`,
21+
// TODO: GH#18445
22+
newFileContent: `import fooBar from "./foo-bar";\r
23+
\r
24+
def
25+
fooB`,
26+
});

‎tests/cases/fourslash/importNameCodeFixDefaultExport.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /foo-bar.ts
4+
////export default 0;
5+
6+
// @Filename: /b.ts
7+
////[|foo/**/Bar|]
8+
9+
goTo.file("/b.ts");
10+
verify.importFixAtPosition([`import fooBar from "./foo-bar";
11+
12+
fooBar`]);

0 commit comments

Comments
 (0)