Merge pull request #11328 from Microsoft/FixTripleSlashCompletions

Fix triple slash completions

Author: aozgaa

src/compiler/core.ts

@@ -1097,7 +1097,9 @@ namespace ts {
         return path.replace(/\\/g, "/");
     }
 
-    // Returns length of path root (i.e. length of "/", "x:/", "//server/share/, file:///user/files")
+    /**
+     * Returns length of path root (i.e. length of "/", "x:/", "//server/share/, file:///user/files")
+    */
     export function getRootLength(path: string): number {
         if (path.charCodeAt(0) === CharacterCodes.slash) {
             if (path.charCodeAt(1) !== CharacterCodes.slash) return 1;
@@ -1126,9 +1128,14 @@ namespace ts {
         return 0;
     }
 
+    /**
+     * Internally, we represent paths as strings with '/' as the directory separator.
+     * When we make system calls (eg: LanguageServiceHost.getDirectory()),
+     * we expect the host to correctly handle paths in our specified format.
+     */
     export const directorySeparator = "/";
     const directorySeparatorCharCode = CharacterCodes.slash;
-    function getNormalizedParts(normalizedSlashedPath: string, rootLength: number) {
+    function getNormalizedParts(normalizedSlashedPath: string, rootLength: number): string[] {
         const parts = normalizedSlashedPath.substr(rootLength).split(directorySeparator);
         const normalized: string[] = [];
         for (const part of parts) {
@@ -1168,6 +1175,11 @@ namespace ts {
         return path.charCodeAt(path.length - 1) === directorySeparatorCharCode;
     }
 
+    /**
+     * Returns the path except for its basename. Eg:
+     * 
+     * /path/to/file.ext -> /path/to
+     */
     export function getDirectoryPath(path: Path): Path;
     export function getDirectoryPath(path: string): string;
     export function getDirectoryPath(path: string): any {

src/server/lsHost.ts

@@ -171,12 +171,16 @@ namespace ts.server {
             return this.host.fileExists(path);
         }
 
+        readFile(fileName: string): string {
+            return this.host.readFile(fileName);
+        }
+
         directoryExists(path: string): boolean {
             return this.host.directoryExists(path);
         }
 
-        readFile(fileName: string): string {
-            return this.host.readFile(fileName);
+        readDirectory(path: string, extensions?: string[], exclude?: string[], include?: string[]): string[] {
+            return this.host.readDirectory(path, extensions, exclude, include);
         }
 
         getDirectories(path: string): string[] {

src/services/completions.ts

@@ -325,15 +325,28 @@ namespace ts.Completions {
             return result;
         }
 
+        /**
+         * Given a path ending at a directory, gets the completions for the path, and filters for those entries containing the basename.
+         */
         function getCompletionEntriesForDirectoryFragment(fragment: string, scriptPath: string, extensions: string[], includeExtensions: boolean, span: TextSpan, exclude?: string, result: CompletionEntry[] = []): CompletionEntry[] {
-            fragment = getDirectoryPath(fragment);
-            if (!fragment) {
-                fragment = "./";
+            if (fragment === undefined) {
+                fragment = "";
             }
-            else {
-                fragment = ensureTrailingDirectorySeparator(fragment);
+
+            fragment = normalizeSlashes(fragment);
+
+            /**
+             * Remove the basename from the path. Note that we don't use the basename to filter completions;
+             * the client is responsible for refining completions.
+             */
+            fragment = getDirectoryPath(fragment);
+
+            if (fragment === "") {
+                fragment = "." + directorySeparator;
             }
 
+            fragment = ensureTrailingDirectorySeparator(fragment);
+
             const absolutePath = normalizeAndPreserveTrailingSlash(isRootedDiskPath(fragment) ? fragment : combinePaths(scriptPath, fragment));
             const baseDirectory = getDirectoryPath(absolutePath);
             const ignoreCase = !(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames());
@@ -343,6 +356,12 @@ namespace ts.Completions {
                 const files = tryReadDirectory(host, baseDirectory, extensions, /*exclude*/undefined, /*include*/["./*"]);
 
                 if (files) {
+                    /**
+                     * Multiple file entries might map to the same truncated name once we remove extensions
+                     * (happens iff includeExtensions === false)so we use a set-like data structure. Eg:
+                     * 
+                     * both foo.ts and foo.tsx become foo
+                     */
                     const foundFiles = createMap<boolean>();
                     for (let filePath of files) {
                         filePath = normalizePath(filePath);
@@ -539,36 +558,44 @@ namespace ts.Completions {
                 return undefined;
             }
 
+            const completionInfo: CompletionInfo = {
+                /**
+                 * We don't want the editor to offer any other completions, such as snippets, inside a comment.
+                 */
+                isGlobalCompletion: false,
+                isMemberCompletion: false,
+                /**
+                 * The user may type in a path that doesn't yet exist, creating a "new identifier"
+                 * with respect to the collection of identifiers the server is aware of.
+                 */
+                isNewIdentifierLocation: true,
+
+                entries: []
+            };
+
             const text = sourceFile.text.substr(range.pos, position - range.pos);
 
             const match = tripleSlashDirectiveFragmentRegex.exec(text);
+
             if (match) {
                 const prefix = match[1];
                 const kind = match[2];
                 const toComplete = match[3];
 
                 const scriptPath = getDirectoryPath(sourceFile.path);
-                let entries: CompletionEntry[];
                 if (kind === "path") {
                     // Give completions for a relative path
                     const span: TextSpan = getDirectoryFragmentTextSpan(toComplete, range.pos + prefix.length);
-                    entries = getCompletionEntriesForDirectoryFragment(toComplete, scriptPath, getSupportedExtensions(compilerOptions), /*includeExtensions*/true, span, sourceFile.path);
+                    completionInfo.entries = getCompletionEntriesForDirectoryFragment(toComplete, scriptPath, getSupportedExtensions(compilerOptions), /*includeExtensions*/true, span, sourceFile.path);
                 }
                 else {
                     // Give completions based on the typings available
                     const span: TextSpan = { start: range.pos + prefix.length, length: match[0].length - prefix.length };
-                    entries = getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, span);
+                    completionInfo.entries = getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, span);
                 }
-
-                return {
-                    isGlobalCompletion: false,
-                    isMemberCompletion: false,
-                    isNewIdentifierLocation: true,
-                    entries
-                };
             }
 
-            return undefined;
+            return completionInfo;
         }
 
         function getCompletionEntriesFromTypings(host: LanguageServiceHost, options: CompilerOptions, scriptPath: string, span: TextSpan, result: CompletionEntry[] = []): CompletionEntry[] {
@@ -1674,9 +1701,15 @@ namespace ts.Completions {
      * Matches a triple slash reference directive with an incomplete string literal for its path. Used
      * to determine if the caret is currently within the string literal and capture the literal fragment
      * for completions.
-     * For example, this matches /// <reference path="fragment
+     * For example, this matches
+     * 
+     * /// <reference path="fragment
+     * 
+     * but not
+     *  
+     * /// <reference path="fragment"
      */
-    const tripleSlashDirectiveFragmentRegex = /^(\/\/\/\s*<reference\s+(path|types)\s*=\s*(?:'|"))([^\3]*)$/;
+    const tripleSlashDirectiveFragmentRegex = /^(\/\/\/\s*<reference\s+(path|types)\s*=\s*(?:'|"))([^\3"]*)$/;
 
     interface VisibleModuleInfo {
         moduleName: string;

src/services/types.ts

@@ -505,7 +505,11 @@ namespace ts {
     export interface CompletionInfo {
         isGlobalCompletion: boolean;
         isMemberCompletion: boolean;
-        isNewIdentifierLocation: boolean;  // true when the current location also allows for a new identifier
+
+        /**
+         * true when the current location also allows for a new identifier
+         */
+        isNewIdentifierLocation: boolean;
         entries: CompletionEntry[];
     }
 

tests/cases/fourslash/completionForStringLiteralRelativeImport1.ts

@@ -0,0 +1,72 @@
+/// <reference path='fourslash.ts' />
+
+// Should give completions for ts files only when allowJs is false.
+
+// @Filename: test0.ts
+//// import * as foo1 from "./*import_as0*/
+//// import * as foo2 from ".//*import_as1*/
+//// import * as foo4 from "./d1//*import_as2*/
+
+//// import foo6 = require("./*import_equals0*/
+//// import foo7 = require(".//*import_equals1*/
+//// import foo9 = require("./d1//*import_equals2*/
+
+//// var foo11 = require("./*require0*/
+//// var foo12 = require(".//*require1*/
+//// var foo14 = require("./d1//*require2*/
+
+// @Filename: d2/d3/test1.ts
+//// import * as foo16 from "..//*import_as3*/
+//// import foo17 = require("..//*import_equals3*/
+//// var foo18 = require("..//*require3*/
+
+
+// @Filename: f1.ts
+//// /*f1*/
+// @Filename: f2.js
+//// /*f2*/
+// @Filename: f3.d.ts
+//// /*f3*/
+// @Filename: f4.tsx
+//// /f4*/
+// @Filename: f5.js
+//// /*f5*/
+// @Filename: f6.jsx
+//// /*f6*/
+// @Filename: f7.ts
+//// /*f7*/
+// @Filename: d1/f8.ts
+//// /*d1f1*/
+// @Filename: d1/f9.ts
+//// /*d1f9*/
+// @Filename: d2/f10.ts
+//// /*d2f1*/
+// @Filename: d2/f11.ts
+//// /*d2f11*/
+
+const kinds = ["import_as", "import_equals", "require"];
+
+for (const kind of kinds) {
+    goTo.marker(kind + "0");
+    verify.completionListIsEmpty();
+
+    goTo.marker(kind + "1");
+    verify.completionListContains("f1");
+    verify.completionListContains("f3");
+    verify.completionListContains("f4");
+    verify.completionListContains("f7");
+    verify.completionListContains("d1");
+    verify.completionListContains("d2");
+    verify.not.completionListItemsCountIsGreaterThan(6);
+
+    goTo.marker(kind + "2");
+    verify.completionListContains("f8");
+    verify.completionListContains("f9");
+    verify.not.completionListItemsCountIsGreaterThan(2);
+
+    goTo.marker(kind + "3");
+    verify.completionListContains("f10");
+    verify.completionListContains("f11");
+    verify.completionListContains("d3");
+    verify.not.completionListItemsCountIsGreaterThan(3);
+}