Skip to content

Commit 65a191f

Browse files
author
Andy
authored
For import completion of default import, convert module name to identifier (microsoft#19875)
* For import completion of default import, convert module name to identifier * Suggestions from code review
1 parent 90ae9ff commit 65a191f

8 files changed

Lines changed: 108 additions & 18 deletions

File tree

‎src/compiler/scanner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,7 @@ namespace ts {
294294
}
295295

296296
/* @internal */
297-
export function stringToToken(s: string): SyntaxKind {
297+
export function stringToToken(s: string): SyntaxKind | undefined {
298298
return textToToken.get(s);
299299
}
300300

‎src/compiler/types.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ namespace ts {
214214
UndefinedKeyword,
215215
FromKeyword,
216216
GlobalKeyword,
217-
OfKeyword, // LastKeyword and LastToken
217+
OfKeyword, // LastKeyword and LastToken and LastContextualKeyword
218218

219219
// Parse tree nodes
220220

@@ -431,7 +431,9 @@ namespace ts {
431431
FirstJSDocNode = JSDocTypeExpression,
432432
LastJSDocNode = JSDocPropertyTag,
433433
FirstJSDocTagNode = JSDocTag,
434-
LastJSDocTagNode = JSDocPropertyTag
434+
LastJSDocTagNode = JSDocPropertyTag,
435+
/* @internal */ FirstContextualKeyword = AbstractKeyword,
436+
/* @internal */ LastContextualKeyword = OfKeyword,
435437
}
436438

437439
export const enum NodeFlags {

‎src/compiler/utilities.ts‎

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

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

‎src/harness/fourslash.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3102,7 +3102,7 @@ Actual: ${stringify(fullActual)}`);
31023102
}
31033103
}
31043104

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

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

‎src/services/codefixes/importFixes.ts‎

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -699,9 +699,10 @@ namespace ts.codefix {
699699
const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol);
700700
if (defaultExport) {
701701
const localSymbol = getLocalSymbolForExportDefault(defaultExport);
702-
if (localSymbol && localSymbol.escapedName === symbolName && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) {
702+
if ((localSymbol && localSymbol.escapedName === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, context.compilerOptions.target) === symbolName)
703+
&& checkSymbolHasMeaning(localSymbol || defaultExport, currentTokenMeaning)) {
703704
// check if this symbol is already used
704-
const symbolId = getUniqueSymbolId(localSymbol, checker);
705+
const symbolId = getUniqueSymbolId(localSymbol || defaultExport, checker);
705706
symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, { ...context, kind: ImportKind.Default }));
706707
}
707708
}
@@ -731,4 +732,35 @@ namespace ts.codefix {
731732
}
732733
}
733734
}
735+
736+
export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string {
737+
return moduleSpecifierToValidIdentifier(removeFileExtension(getBaseFileName(moduleSymbol.name)), target);
738+
}
739+
740+
function moduleSpecifierToValidIdentifier(moduleSpecifier: string, target: ScriptTarget): string {
741+
let res = "";
742+
let lastCharWasValid = true;
743+
const firstCharCode = moduleSpecifier.charCodeAt(0);
744+
if (isIdentifierStart(firstCharCode, target)) {
745+
res += String.fromCharCode(firstCharCode);
746+
}
747+
else {
748+
lastCharWasValid = false;
749+
}
750+
for (let i = 1; i < moduleSpecifier.length; i++) {
751+
const ch = moduleSpecifier.charCodeAt(i);
752+
const isValid = isIdentifierPart(ch, target);
753+
if (isValid) {
754+
let char = String.fromCharCode(ch);
755+
if (!lastCharWasValid) {
756+
char = char.toUpperCase();
757+
}
758+
res += char;
759+
}
760+
lastCharWasValid = isValid;
761+
}
762+
// Need `|| "_"` to ensure result isn't empty.
763+
const token = stringToToken(res);
764+
return token === undefined || !isNonContextualKeyword(token) ? res || "_" : `_${res}`;
765+
}
734766
}

‎src/services/completions.ts‎

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

42-
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options);
42+
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options, compilerOptions.target);
4343
if (!completionData) {
4444
return undefined;
4545
}
@@ -136,12 +136,12 @@ namespace ts.Completions {
136136
typeChecker: TypeChecker,
137137
target: ScriptTarget,
138138
allowStringLiteral: boolean,
139-
origin: SymbolOriginInfo,
139+
origin: SymbolOriginInfo | undefined,
140140
): CompletionEntry | undefined {
141141
// Try to get a valid display name for this symbol, if we could not find one, then ignore it.
142142
// We would like to only show things that can be added after a dot, so for instance numeric properties can
143143
// not be accessed with a dot (a.1 <- invalid)
144-
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral);
144+
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral, origin);
145145
if (!displayName) {
146146
return undefined;
147147
}
@@ -381,7 +381,7 @@ namespace ts.Completions {
381381
{ name, source }: CompletionEntryIdentifier,
382382
allSourceFiles: ReadonlyArray<SourceFile>,
383383
): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | { type: "request", request: Request } | { type: "none" } {
384-
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true });
384+
const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target);
385385
if (!completionData) {
386386
return { type: "none" };
387387
}
@@ -395,12 +395,18 @@ namespace ts.Completions {
395395
// We don't need to perform character checks here because we're only comparing the
396396
// name against 'entryName' (which is known to be good), not building a new
397397
// completion entry.
398-
const symbol = find(symbols, s =>
399-
getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name
400-
&& getSourceFromOrigin(symbolToOriginInfoMap[getSymbolId(s)]) === source);
398+
const symbol = find(symbols, s => {
399+
const origin = symbolToOriginInfoMap[getSymbolId(s)];
400+
return getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral, origin) === name
401+
&& getSourceFromOrigin(origin) === source;
402+
});
401403
return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap } : { type: "none" };
402404
}
403405

406+
function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string {
407+
return origin && origin.isDefaultExport && symbol.name === "default" ? codefix.moduleSymbolToValidIdentifier(origin.moduleSymbol, target) : symbol.name;
408+
}
409+
404410
export interface CompletionEntryIdentifier {
405411
name: string;
406412
source?: string;
@@ -482,7 +488,7 @@ namespace ts.Completions {
482488
compilerOptions,
483489
sourceFile,
484490
formatContext,
485-
symbolName: symbol.name,
491+
symbolName: getSymbolName(symbol, symbolOriginInfo, compilerOptions.target),
486492
getCanonicalFileName: createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false),
487493
symbolToken: undefined,
488494
kind: isDefaultExport ? codefix.ImportKind.Default : codefix.ImportKind.Named,
@@ -523,6 +529,7 @@ namespace ts.Completions {
523529
position: number,
524530
allSourceFiles: ReadonlyArray<SourceFile>,
525531
options: GetCompletionsAtPositionOptions,
532+
target: ScriptTarget,
526533
): CompletionData | undefined {
527534
const isJavaScriptFile = isSourceFileJavaScript(sourceFile);
528535

@@ -921,7 +928,7 @@ namespace ts.Completions {
921928

922929
symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings);
923930
if (options.includeExternalModuleExports) {
924-
getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "");
931+
getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target);
925932
}
926933
filterGlobalCompletion(symbols);
927934

@@ -1003,7 +1010,7 @@ namespace ts.Completions {
10031010
}
10041011
}
10051012

1006-
function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string): void {
1013+
function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
10071014
const tokenTextLowerCase = tokenText.toLowerCase();
10081015

10091016
codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => {
@@ -1020,6 +1027,9 @@ namespace ts.Completions {
10201027
symbol = localSymbol;
10211028
name = localSymbol.name;
10221029
}
1030+
else {
1031+
name = codefix.moduleSymbolToValidIdentifier(moduleSymbol, target);
1032+
}
10231033
}
10241034

10251035
if (symbol.declarations && symbol.declarations.some(d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
@@ -1847,8 +1857,8 @@ namespace ts.Completions {
18471857
*
18481858
* @return undefined if the name is of external module
18491859
*/
1850-
function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean): string | undefined {
1851-
const name = symbol.name;
1860+
function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean, origin: SymbolOriginInfo | undefined): string | undefined {
1861+
const name = getSymbolName(symbol, origin, target);
18521862
if (!name) return undefined;
18531863

18541864
// 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)