For import completion of default import, convert module name to identifier (microsoft#19875)

Andy · web-flow · commit 65a191fa2b0d · 2017-11-09T13:13:23.000-08:00
* For import completion of default import, convert module name to identifier

* Suggestions from code review
diff --git a/src/compiler/scanner.ts b/src/compiler/scanner.ts
@@ -294,7 +294,7 @@ namespace ts {
     }
 
     /* @internal */
-    export function stringToToken(s: string): SyntaxKind {
+    export function stringToToken(s: string): SyntaxKind | undefined {
         return textToToken.get(s);
     }
 
diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -214,7 +214,7 @@ namespace ts {
         UndefinedKeyword,
         FromKeyword,
         GlobalKeyword,
-        OfKeyword, // LastKeyword and LastToken
+        OfKeyword, // LastKeyword and LastToken and LastContextualKeyword
 
         // Parse tree nodes
 
@@ -431,7 +431,9 @@ namespace ts {
         FirstJSDocNode = JSDocTypeExpression,
         LastJSDocNode = JSDocPropertyTag,
         FirstJSDocTagNode = JSDocTag,
-        LastJSDocTagNode = JSDocPropertyTag
+        LastJSDocTagNode = JSDocPropertyTag,
+        /* @internal */ FirstContextualKeyword = AbstractKeyword,
+        /* @internal */ LastContextualKeyword = OfKeyword,
     }
 
     export const enum NodeFlags {
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1905,6 +1905,14 @@ namespace ts {
         return SyntaxKind.FirstKeyword <= token && token <= SyntaxKind.LastKeyword;
     }
 
+    export function isContextualKeyword(token: SyntaxKind): boolean {
+        return SyntaxKind.FirstContextualKeyword <= token && token <= SyntaxKind.LastContextualKeyword;
+    }
+
+    export function isNonContextualKeyword(token: SyntaxKind): boolean {
+        return isKeyword(token) && !isContextualKeyword(token);
+    }
+
     export function isTrivia(token: SyntaxKind) {
         return SyntaxKind.FirstTriviaToken <= token && token <= SyntaxKind.LastTriviaToken;
     }
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -3102,7 +3102,7 @@ Actual: ${stringify(fullActual)}`);
                 }
             }
 
-            const itemsString = items.map(item => stringify({ name: item.name, kind: item.kind })).join(",\n");
+            const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
 
             this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
         }
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -699,9 +699,10 @@ namespace ts.codefix {
             const defaultExport = checker.tryGetMemberInModuleExports("default", moduleSymbol);
             if (defaultExport) {
                 const localSymbol = getLocalSymbolForExportDefault(defaultExport);
-                if (localSymbol && localSymbol.escapedName === symbolName && checkSymbolHasMeaning(localSymbol, currentTokenMeaning)) {
+                if ((localSymbol && localSymbol.escapedName === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, context.compilerOptions.target) === symbolName)
+                    && checkSymbolHasMeaning(localSymbol || defaultExport, currentTokenMeaning)) {
                     // check if this symbol is already used
-                    const symbolId = getUniqueSymbolId(localSymbol, checker);
+                    const symbolId = getUniqueSymbolId(localSymbol || defaultExport, checker);
                     symbolIdActionMap.addActions(symbolId, getCodeActionForImport(moduleSymbol, { ...context, kind: ImportKind.Default }));
                 }
             }
@@ -731,4 +732,35 @@ namespace ts.codefix {
             }
         }
     }
+
+    export function moduleSymbolToValidIdentifier(moduleSymbol: Symbol, target: ScriptTarget): string {
+        return moduleSpecifierToValidIdentifier(removeFileExtension(getBaseFileName(moduleSymbol.name)), target);
+    }
+
+    function moduleSpecifierToValidIdentifier(moduleSpecifier: string, target: ScriptTarget): string {
+        let res = "";
+        let lastCharWasValid = true;
+        const firstCharCode = moduleSpecifier.charCodeAt(0);
+        if (isIdentifierStart(firstCharCode, target)) {
+            res += String.fromCharCode(firstCharCode);
+        }
+        else {
+            lastCharWasValid = false;
+        }
+        for (let i = 1; i < moduleSpecifier.length; i++) {
+            const ch = moduleSpecifier.charCodeAt(i);
+            const isValid = isIdentifierPart(ch, target);
+            if (isValid) {
+                let char = String.fromCharCode(ch);
+                if (!lastCharWasValid) {
+                    char = char.toUpperCase();
+                }
+                res += char;
+            }
+            lastCharWasValid = isValid;
+        }
+        // Need `|| "_"` to ensure result isn't empty.
+        const token = stringToToken(res);
+        return token === undefined || !isNonContextualKeyword(token) ? res || "_" : `_${res}`;
+    }
 }
diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -39,7 +39,7 @@ namespace ts.Completions {
             return getStringLiteralCompletionEntries(sourceFile, position, typeChecker, compilerOptions, host, log);
         }
 
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options);
+        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, options, compilerOptions.target);
         if (!completionData) {
             return undefined;
         }
@@ -136,12 +136,12 @@ namespace ts.Completions {
         typeChecker: TypeChecker,
         target: ScriptTarget,
         allowStringLiteral: boolean,
-        origin: SymbolOriginInfo,
+        origin: SymbolOriginInfo | undefined,
     ): CompletionEntry | undefined {
         // Try to get a valid display name for this symbol, if we could not find one, then ignore it.
         // We would like to only show things that can be added after a dot, so for instance numeric properties can
         // not be accessed with a dot (a.1 <- invalid)
-        const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral);
+        const displayName = getCompletionEntryDisplayNameForSymbol(symbol, target, performCharacterChecks, allowStringLiteral, origin);
         if (!displayName) {
             return undefined;
         }
@@ -381,7 +381,7 @@ namespace ts.Completions {
         { name, source }: CompletionEntryIdentifier,
         allSourceFiles: ReadonlyArray<SourceFile>,
     ): { type: "symbol", symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | { type: "request", request: Request } | { type: "none" } {
-        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true });
+        const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles, { includeExternalModuleExports: true }, compilerOptions.target);
         if (!completionData) {
             return { type: "none" };
         }
@@ -395,12 +395,18 @@ 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 = find(symbols, s =>
-            getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name
-            && getSourceFromOrigin(symbolToOriginInfoMap[getSymbolId(s)]) === source);
+        const symbol = find(symbols, s => {
+            const origin = symbolToOriginInfoMap[getSymbolId(s)];
+            return getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral, origin) === name
+                && getSourceFromOrigin(origin) === source;
+        });
         return symbol ? { type: "symbol", symbol, location, symbolToOriginInfoMap } : { type: "none" };
     }
 
+    function getSymbolName(symbol: Symbol, origin: SymbolOriginInfo | undefined, target: ScriptTarget): string {
+        return origin && origin.isDefaultExport && symbol.name === "default" ? codefix.moduleSymbolToValidIdentifier(origin.moduleSymbol, target) : symbol.name;
+    }
+
     export interface CompletionEntryIdentifier {
         name: string;
         source?: string;
@@ -482,7 +488,7 @@ namespace ts.Completions {
             compilerOptions,
             sourceFile,
             formatContext,
-            symbolName: symbol.name,
+            symbolName: getSymbolName(symbol, symbolOriginInfo, compilerOptions.target),
             getCanonicalFileName: createGetCanonicalFileName(host.useCaseSensitiveFileNames ? host.useCaseSensitiveFileNames() : false),
             symbolToken: undefined,
             kind: isDefaultExport ? codefix.ImportKind.Default : codefix.ImportKind.Named,
@@ -523,6 +529,7 @@ namespace ts.Completions {
         position: number,
         allSourceFiles: ReadonlyArray<SourceFile>,
         options: GetCompletionsAtPositionOptions,
+        target: ScriptTarget,
     ): CompletionData | undefined {
         const isJavaScriptFile = isSourceFileJavaScript(sourceFile);
 
@@ -921,7 +928,7 @@ namespace ts.Completions {
 
             symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings);
             if (options.includeExternalModuleExports) {
-                getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "");
+                getSymbolsFromOtherSourceFileExports(symbols, previousToken && isIdentifier(previousToken) ? previousToken.text : "", target);
             }
             filterGlobalCompletion(symbols);
 
@@ -1003,7 +1010,7 @@ namespace ts.Completions {
             }
         }
 
-        function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string): void {
+        function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string, target: ScriptTarget): void {
             const tokenTextLowerCase = tokenText.toLowerCase();
 
             codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => {
@@ -1020,6 +1027,9 @@ namespace ts.Completions {
                             symbol = localSymbol;
                             name = localSymbol.name;
                         }
+                        else {
+                            name = codefix.moduleSymbolToValidIdentifier(moduleSymbol, target);
+                        }
                     }
 
                     if (symbol.declarations && symbol.declarations.some(d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
@@ -1847,8 +1857,8 @@ namespace ts.Completions {
      *
      * @return undefined if the name is of external module
      */
-    function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean): string | undefined {
-        const name = symbol.name;
+    function getCompletionEntryDisplayNameForSymbol(symbol: Symbol, target: ScriptTarget, performCharacterChecks: boolean, allowStringLiteral: boolean, origin: SymbolOriginInfo | undefined): string | undefined {
+        const name = getSymbolName(symbol, origin, target);
         if (!name) return undefined;
 
         // First check of the displayName is not external module; if it is an external module, it is not valid entry
diff --git a/tests/cases/fourslash/completionsImport_default_anonymous.ts b/tests/cases/fourslash/completionsImport_default_anonymous.ts
@@ -0,0 +1,26 @@
+/// <reference path="fourslash.ts" />
+
+// Use `/src` to test that directory names are not included in conversion from module path to identifier.
+
+// @Filename: /src/foo-bar.ts
+////export default 0;
+
+// @Filename: /src/b.ts
+////def/*0*/
+////fooB/*1*/
+
+goTo.marker("0");
+verify.not.completionListContains({ name: "default", source: "/src/foo-bar" }, undefined, undefined, undefined, undefined, undefined, { includeExternalModuleExports: true });
+
+goTo.marker("1");
+verify.completionListContains({ name: "fooBar", source: "/src/foo-bar" }, "(property) default: 0", "", "property", /*spanIndex*/ undefined, /*hasAction*/ true, { includeExternalModuleExports: true });
+verify.applyCodeActionFromCompletion("1", {
+    name: "fooBar",
+    source: "/src/foo-bar",
+    description: `Import 'fooBar' from "./foo-bar".`,
+    // TODO: GH#18445
+    newFileContent: `import fooBar from "./foo-bar";\r
+\r
+def
+fooB`,
+});
diff --git a/tests/cases/fourslash/importNameCodeFixDefaultExport.ts b/tests/cases/fourslash/importNameCodeFixDefaultExport.ts
@@ -0,0 +1,12 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: /foo-bar.ts
+////export default 0;
+
+// @Filename: /b.ts
+////[|foo/**/Bar|]
+
+goTo.file("/b.ts");
+verify.importFixAtPosition([`import fooBar from "./foo-bar";
+
+fooBar`]);

Original file line number	Diff line number	Diff line change
`@@ -294,7 +294,7 @@ namespace ts {`
`294`	`294`	`}`
`295`	`295`
`296`	`296`	`/* @internal */`
`297`		`- export function stringToToken(s: string): SyntaxKind {`
	`297`	`+ export function stringToToken(s: string): SyntaxKind \| undefined {`
`298`	`298`	`return textToToken.get(s);`
`299`	`299`	`}`
`300`	`300`
Original file line number	Diff line number	Diff line change
`@@ -1905,6 +1905,14 @@ namespace ts {`
`1905`	`1905`	`return SyntaxKind.FirstKeyword <= token && token <= SyntaxKind.LastKeyword;`
`1906`	`1906`	`}`
`1907`	`1907`
	`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`	`+`
`1908`	`1916`	`export function isTrivia(token: SyntaxKind) {`
`1909`	`1917`	`return SyntaxKind.FirstTriviaToken <= token && token <= SyntaxKind.LastTriviaToken;`
`1910`	`1918`	`}`
Original file line number	Diff line number	Diff line change
@@ -3102,7 +3102,7 @@ Actual: ${stringify(fullActual)}`);
`3102`	`3102`	`}`
`3103`	`3103`	`}`
`3104`	`3104`
`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");`
`3106`	`3106`
`3107`	`3107`	this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
`3108`	`3108`	`}`