Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2035db1
Convert to Doc Comment
Sep 24, 2016
b38d7ac
Annotate directorySeparator
Sep 26, 2016
d423aad
comments
Sep 26, 2016
8f883b9
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Sep 26, 2016
1f7b6e6
More comments
Sep 27, 2016
769d248
new test
Sep 27, 2016
8a479e8
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Sep 27, 2016
6dd5482
remove Comment
Sep 27, 2016
bfb2185
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Sep 28, 2016
629e126
Factored out hidden path test
Sep 28, 2016
f6ff6bd
factor and simplify rel path test
Sep 28, 2016
49e0de9
Fix tests
Sep 29, 2016
2195f17
Rename Tests
Sep 29, 2016
4b07377
Fix fragment handling
Sep 29, 2016
8573631
Rename Tests
Sep 30, 2016
cb13ae0
Pass linting
Sep 30, 2016
6553413
New Test
Sep 30, 2016
a2cbc63
Question added
Sep 30, 2016
d96d098
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Sep 30, 2016
b9e5dbc
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Sep 30, 2016
3e15394
add readDirectory to LSHost
Sep 30, 2016
3ce97af
Rename labels in tests
Oct 3, 2016
728252f
Merge branch 'master' into FixAbsoluteTripleSlashCompletions
Oct 3, 2016
179ceda
Removed comments and fixed indentation
Oct 3, 2016
0871628
Remove trailing comment
Oct 3, 2016
d737dc1
Responding To billti's comments
Oct 4, 2016
55d4e12
Fixed a comment
Oct 4, 2016
2e589f1
Actually fixed comments
Oct 4, 2016
eb362fe
Merge branch 'master' into FixTripleSlashCompletions
Oct 4, 2016
1baf496
merge master and add isGlobalCompletion flags to CompletionInfo
Oct 4, 2016
a12fe2e
Eliminate allocation of filtered completions
Oct 5, 2016
617daa9
Change fragment initialization
Oct 5, 2016
24938a7
Remove filtering functionality
Oct 5, 2016
77a2d0e
Merge branch 'master' into FixTripleSlashCompletions
Oct 5, 2016
4a497fb
update tests to reflect no completion filtering
Oct 5, 2016
af833aa
provide completions only in the correct bounds
Oct 5, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions src/compiler/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 6 additions & 2 deletions src/server/lsHost.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[] {
Expand Down
69 changes: 51 additions & 18 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,15 +323,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;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this doing the same thing as line 330?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function really should never get undefined as an argument for fragment, but I don't know of a way to assert that. So the check on line 330 is really a matter of null-checking. In particular, normalizeSlashes() on line 332 dereferences its argument without null-checking.

Regarding line 340, consider the case when fragment is "foo". Then getDirectoryPath(fragment) === "" because it removes the basename part of the path. So we need to check again.

Really, fragment no longer refers to a path fragment after the assignment on line 337. Perhaps a new variable called relativeOrAbsolutePath should be initialized on the LHS of line 337. I didn't make the change because I was trying to minimize the changes I was making to the codebase to get a fix in (with mixed success).

Should I add a new declaration?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the comment was specifically about the fact that line 330 uses "./" while line 340 uses "." + directorySeparator", though they do the same thing. Perhaps you should initialize fragment to "" on 330 rather than repeat yourself on 340.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

}

fragment = ensureTrailingDirectorySeparator(fragment);

const absolutePath = normalizeAndPreserveTrailingSlash(isRootedDiskPath(fragment) ? fragment : combinePaths(scriptPath, fragment));
const baseDirectory = getDirectoryPath(absolutePath);
const ignoreCase = !(host.useCaseSensitiveFileNames && host.useCaseSensitiveFileNames());
Expand All @@ -341,6 +354,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);
Expand Down Expand Up @@ -537,36 +556,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[] {
Expand Down Expand Up @@ -1671,9 +1698,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;
Expand Down
6 changes: 5 additions & 1 deletion src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[];
}

Expand Down
70 changes: 0 additions & 70 deletions tests/cases/fourslash/completionForStringLiteralRelativeImport1.ts

This file was deleted.

Original file line number Diff line number Diff line change
@@ -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);
}
Loading