Skip to content

Commit 94d8955

Browse files
aozgaaaozgaa
authored
Merge pull request microsoft#11328 from Microsoft/FixTripleSlashCompletions
Fix triple slash completions
2 parents a67ad06 + af833aa commit 94d8955

17 files changed

Lines changed: 435 additions & 181 deletions

‎src/compiler/core.ts‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,7 +1097,9 @@ namespace ts {
10971097
return path.replace(/\\/g, "/");
10981098
}
10991099

1100-
// Returns length of path root (i.e. length of "/", "x:/", "//server/share/, file:///user/files")
1100+
/**
1101+
* Returns length of path root (i.e. length of "/", "x:/", "//server/share/, file:///user/files")
1102+
*/
11011103
export function getRootLength(path: string): number {
11021104
if (path.charCodeAt(0) === CharacterCodes.slash) {
11031105
if (path.charCodeAt(1) !== CharacterCodes.slash) return 1;
@@ -1126,9 +1128,14 @@ namespace ts {
11261128
return 0;
11271129
}
11281130

1131+
/**
1132+
* Internally, we represent paths as strings with '/' as the directory separator.
1133+
* When we make system calls (eg: LanguageServiceHost.getDirectory()),
1134+
* we expect the host to correctly handle paths in our specified format.
1135+
*/
11291136
export const directorySeparator = "/";
11301137
const directorySeparatorCharCode = CharacterCodes.slash;
1131-
function getNormalizedParts(normalizedSlashedPath: string, rootLength: number) {
1138+
function getNormalizedParts(normalizedSlashedPath: string, rootLength: number): string[] {
11321139
const parts = normalizedSlashedPath.substr(rootLength).split(directorySeparator);
11331140
const normalized: string[] = [];
11341141
for (const part of parts) {
@@ -1168,6 +1175,11 @@ namespace ts {
11681175
return path.charCodeAt(path.length - 1) === directorySeparatorCharCode;
11691176
}
11701177

1178+
/**
1179+
* Returns the path except for its basename. Eg:
1180+
*
1181+
* /path/to/file.ext -> /path/to
1182+
*/
11711183
export function getDirectoryPath(path: Path): Path;
11721184
export function getDirectoryPath(path: string): string;
11731185
export function getDirectoryPath(path: string): any {

‎src/server/lsHost.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,16 @@ namespace ts.server {
171171
return this.host.fileExists(path);
172172
}
173173

174+
readFile(fileName: string): string {
175+
return this.host.readFile(fileName);
176+
}
177+
174178
directoryExists(path: string): boolean {
175179
return this.host.directoryExists(path);
176180
}
177181

178-
readFile(fileName: string): string {
179-
return this.host.readFile(fileName);
182+
readDirectory(path: string, extensions?: string[], exclude?: string[], include?: string[]): string[] {
183+
return this.host.readDirectory(path, extensions, exclude, include);
180184
}
181185

182186
getDirectories(path: string): string[] {

‎src/services/completions.ts‎

Lines changed: 51 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -325,15 +325,28 @@ namespace ts.Completions {
325325
return result;
326326
}
327327

328+
/**
329+
* Given a path ending at a directory, gets the completions for the path, and filters for those entries containing the basename.
330+
*/
328331
function getCompletionEntriesForDirectoryFragment(fragment: string, scriptPath: string, extensions: string[], includeExtensions: boolean, span: TextSpan, exclude?: string, result: CompletionEntry[] = []): CompletionEntry[] {
329-
fragment = getDirectoryPath(fragment);
330-
if (!fragment) {
331-
fragment = "./";
332+
if (fragment === undefined) {
333+
fragment = "";
332334
}
333-
else {
334-
fragment = ensureTrailingDirectorySeparator(fragment);
335+
336+
fragment = normalizeSlashes(fragment);
337+
338+
/**
339+
* Remove the basename from the path. Note that we don't use the basename to filter completions;
340+
* the client is responsible for refining completions.
341+
*/
342+
fragment = getDirectoryPath(fragment);
343+
344+
if (fragment === "") {
345+
fragment = "." + directorySeparator;
335346
}
336347

348+
fragment = ensureTrailingDirectorySeparator(fragment);
349+
337350
const absolutePath = normalizeAndPreserveTrailingSlash(isRootedDiskPath(fragment) ? fragment : combinePaths(scriptPath, fragment));
338351
const baseDirectory = getDirectoryPath(absolutePath);
339352
const ignoreCase = !(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames());
@@ -343,6 +356,12 @@ namespace ts.Completions {
343356
const files = tryReadDirectory(host, baseDirectory, extensions, /*exclude*/undefined, /*include*/["./*"]);
344357

345358
if (files) {
359+
/**
360+
* Multiple file entries might map to the same truncated name once we remove extensions
361+
* (happens iff includeExtensions === false)so we use a set-like data structure. Eg:
362+
*
363+
* both foo.ts and foo.tsx become foo
364+
*/
346365
const foundFiles = createMap<boolean>();
347366
for (let filePath of files) {
348367
filePath = normalizePath(filePath);
@@ -539,36 +558,44 @@ namespace ts.Completions {
539558
return undefined;
540559
}
541560

561+
const completionInfo: CompletionInfo = {
562+
/**
563+
* We don't want the editor to offer any other completions, such as snippets, inside a comment.
564+
*/
565+
isGlobalCompletion: false,
566+
isMemberCompletion: false,
567+
/**
568+
* The user may type in a path that doesn't yet exist, creating a "new identifier"
569+
* with respect to the collection of identifiers the server is aware of.
570+
*/
571+
isNewIdentifierLocation: true,
572+
573+
entries: []
574+
};
575+
542576
const text = sourceFile.text.substr(range.pos, position - range.pos);
543577

544578
const match = tripleSlashDirectiveFragmentRegex.exec(text);
579+
545580
if (match) {
546581
const prefix = match[1];
547582
const kind = match[2];
548583
const toComplete = match[3];
549584

550585
const scriptPath = getDirectoryPath(sourceFile.path);
551-
let entries: CompletionEntry[];
552586
if (kind === "path") {
553587
// Give completions for a relative path
554588
const span: TextSpan = getDirectoryFragmentTextSpan(toComplete, range.pos + prefix.length);
555-
entries = getCompletionEntriesForDirectoryFragment(toComplete, scriptPath, getSupportedExtensions(compilerOptions), /*includeExtensions*/true, span, sourceFile.path);
589+
completionInfo.entries = getCompletionEntriesForDirectoryFragment(toComplete, scriptPath, getSupportedExtensions(compilerOptions), /*includeExtensions*/true, span, sourceFile.path);
556590
}
557591
else {
558592
// Give completions based on the typings available
559593
const span: TextSpan = { start: range.pos + prefix.length, length: match[0].length - prefix.length };
560-
entries = getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, span);
594+
completionInfo.entries = getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, span);
561595
}
562-
563-
return {
564-
isGlobalCompletion: false,
565-
isMemberCompletion: false,
566-
isNewIdentifierLocation: true,
567-
entries
568-
};
569596
}
570597

571-
return undefined;
598+
return completionInfo;
572599
}
573600

574601
function getCompletionEntriesFromTypings(host: LanguageServiceHost, options: CompilerOptions, scriptPath: string, span: TextSpan, result: CompletionEntry[] = []): CompletionEntry[] {
@@ -1674,9 +1701,15 @@ namespace ts.Completions {
16741701
* Matches a triple slash reference directive with an incomplete string literal for its path. Used
16751702
* to determine if the caret is currently within the string literal and capture the literal fragment
16761703
* for completions.
1677-
* For example, this matches /// <reference path="fragment
1704+
* For example, this matches
1705+
*
1706+
* /// <reference path="fragment
1707+
*
1708+
* but not
1709+
*
1710+
* /// <reference path="fragment"
16781711
*/
1679-
const tripleSlashDirectiveFragmentRegex = /^(\/\/\/\s*<reference\s+(path|types)\s*=\s*(?:'|"))([^\3]*)$/;
1712+
const tripleSlashDirectiveFragmentRegex = /^(\/\/\/\s*<reference\s+(path|types)\s*=\s*(?:'|"))([^\3"]*)$/;
16801713

16811714
interface VisibleModuleInfo {
16821715
moduleName: string;

‎src/services/types.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,11 @@ namespace ts {
505505
export interface CompletionInfo {
506506
isGlobalCompletion: boolean;
507507
isMemberCompletion: boolean;
508-
isNewIdentifierLocation: boolean; // true when the current location also allows for a new identifier
508+
509+
/**
510+
* true when the current location also allows for a new identifier
511+
*/
512+
isNewIdentifierLocation: boolean;
509513
entries: CompletionEntry[];
510514
}
511515

‎tests/cases/fourslash/completionForStringLiteralRelativeImport1.ts‎

Lines changed: 0 additions & 70 deletions
This file was deleted.

‎tests/cases/fourslash/completionForStringLiteralRelativeImportAllowJSFalse.ts‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// Should give completions for ts files only when allowJs is false.
4+
5+
// @Filename: test0.ts
6+
//// import * as foo1 from "./*import_as0*/
7+
//// import * as foo2 from ".//*import_as1*/
8+
//// import * as foo4 from "./d1//*import_as2*/
9+
10+
//// import foo6 = require("./*import_equals0*/
11+
//// import foo7 = require(".//*import_equals1*/
12+
//// import foo9 = require("./d1//*import_equals2*/
13+
14+
//// var foo11 = require("./*require0*/
15+
//// var foo12 = require(".//*require1*/
16+
//// var foo14 = require("./d1//*require2*/
17+
18+
// @Filename: d2/d3/test1.ts
19+
//// import * as foo16 from "..//*import_as3*/
20+
//// import foo17 = require("..//*import_equals3*/
21+
//// var foo18 = require("..//*require3*/
22+
23+
24+
// @Filename: f1.ts
25+
//// /*f1*/
26+
// @Filename: f2.js
27+
//// /*f2*/
28+
// @Filename: f3.d.ts
29+
//// /*f3*/
30+
// @Filename: f4.tsx
31+
//// /f4*/
32+
// @Filename: f5.js
33+
//// /*f5*/
34+
// @Filename: f6.jsx
35+
//// /*f6*/
36+
// @Filename: f7.ts
37+
//// /*f7*/
38+
// @Filename: d1/f8.ts
39+
//// /*d1f1*/
40+
// @Filename: d1/f9.ts
41+
//// /*d1f9*/
42+
// @Filename: d2/f10.ts
43+
//// /*d2f1*/
44+
// @Filename: d2/f11.ts
45+
//// /*d2f11*/
46+
47+
const kinds = ["import_as", "import_equals", "require"];
48+
49+
for (const kind of kinds) {
50+
goTo.marker(kind + "0");
51+
verify.completionListIsEmpty();
52+
53+
goTo.marker(kind + "1");
54+
verify.completionListContains("f1");
55+
verify.completionListContains("f3");
56+
verify.completionListContains("f4");
57+
verify.completionListContains("f7");
58+
verify.completionListContains("d1");
59+
verify.completionListContains("d2");
60+
verify.not.completionListItemsCountIsGreaterThan(6);
61+
62+
goTo.marker(kind + "2");
63+
verify.completionListContains("f8");
64+
verify.completionListContains("f9");
65+
verify.not.completionListItemsCountIsGreaterThan(2);
66+
67+
goTo.marker(kind + "3");
68+
verify.completionListContains("f10");
69+
verify.completionListContains("f11");
70+
verify.completionListContains("d3");
71+
verify.not.completionListItemsCountIsGreaterThan(3);
72+
}

0 commit comments

Comments
 (0)