Skip to content

Commit 000f4d5

Browse files
author
Andy Hanson
committed
For path completions, include extension as a kindModifier
1 parent fe2a33f commit 000f4d5

26 files changed

+181
-69
lines changed

src/harness/fourslash.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -886,8 +886,8 @@ namespace FourSlash {
886886
}
887887

888888
private verifyCompletionEntry(actual: ts.CompletionEntry, expected: FourSlashInterface.ExpectedCompletionEntry) {
889-
const { insertText, replacementSpan, hasAction, isRecommended, kind, text, documentation, tags, source, sourceDisplay } = typeof expected === "string"
890-
? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, text: undefined, documentation: undefined, tags: undefined, source: undefined, sourceDisplay: undefined }
889+
const { insertText, replacementSpan, hasAction, isRecommended, kind, kindModifiers, text, documentation, tags, source, sourceDisplay } = typeof expected === "string"
890+
? { insertText: undefined, replacementSpan: undefined, hasAction: undefined, isRecommended: undefined, kind: undefined, kindModifiers: undefined, text: undefined, documentation: undefined, tags: undefined, source: undefined, sourceDisplay: undefined }
891891
: expected;
892892

893893
if (actual.insertText !== insertText) {
@@ -901,7 +901,10 @@ namespace FourSlash {
901901
this.raiseError(`Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`);
902902
}
903903

904-
if (kind !== undefined) assert.equal(actual.kind, kind);
904+
if (kind !== undefined || kindModifiers !== undefined) {
905+
assert.equal(actual.kind, kind);
906+
assert.equal(actual.kindModifiers, kindModifiers || "");
907+
}
905908

906909
assert.equal(actual.hasAction, hasAction);
907910
assert.equal(actual.isRecommended, isRecommended);
@@ -4784,6 +4787,7 @@ namespace FourSlashInterface {
47844787
readonly hasAction?: boolean, // If not specified, will assert that this is false.
47854788
readonly isRecommended?: boolean; // If not specified, will assert that this is false.
47864789
readonly kind?: string, // If not specified, won't assert about this
4790+
readonly kindModifiers?: string, // Must be paired with 'kind'
47874791
readonly text: string;
47884792
readonly documentation: string;
47894793
readonly sourceDisplay?: string;

src/services/completions.ts

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,23 @@ namespace ts.Completions {
101101
function convertPathCompletions(pathCompletions: ReadonlyArray<PathCompletions.PathCompletion>): CompletionInfo {
102102
const isGlobalCompletion = false; // We don't want the editor to offer any other completions, such as snippets, inside a comment.
103103
const isNewIdentifierLocation = true; // 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.
104-
const entries = pathCompletions.map(({ name, kind, span }) => ({ name, kind, kindModifiers: ScriptElementKindModifier.none, sortText: "0", replacementSpan: span }));
104+
const entries = pathCompletions.map(({ name, kind, span, extension }): CompletionEntry =>
105+
({ name, kind, kindModifiers: kindModifiersFromExtension(extension), sortText: "0", replacementSpan: span }));
105106
return { isGlobalCompletion, isMemberCompletion: false, isNewIdentifierLocation, entries };
106107
}
108+
function kindModifiersFromExtension(extension: Extension | undefined): ScriptElementKindModifier {
109+
switch (extension) {
110+
case Extension.Dts: return ScriptElementKindModifier.dtsModifier;
111+
case Extension.Js: return ScriptElementKindModifier.jsModifier;
112+
case Extension.Json: return ScriptElementKindModifier.jsonModifier;
113+
case Extension.Jsx: return ScriptElementKindModifier.jsxModifier;
114+
case Extension.Ts: return ScriptElementKindModifier.tsModifier;
115+
case Extension.Tsx: return ScriptElementKindModifier.tsxModifier;
116+
case undefined: return ScriptElementKindModifier.none;
117+
default:
118+
return Debug.assertNever(extension);
119+
}
120+
}
107121

108122
function jsdocCompletionInfo(entries: CompletionEntry[]): CompletionInfo {
109123
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
@@ -638,7 +652,7 @@ namespace ts.Completions {
638652
switch (completion.kind) {
639653
case StringLiteralCompletionKind.Paths: {
640654
const match = find(completion.paths, p => p.name === name);
641-
return match && createCompletionDetails(name, ScriptElementKindModifier.none, match.kind, [textPart(name)]);
655+
return match && createCompletionDetails(name, kindModifiersFromExtension(match.extension), match.kind, [textPart(name)]);
642656
}
643657
case StringLiteralCompletionKind.Properties: {
644658
const match = find(completion.symbols, s => s.name === name);

src/services/pathCompletions.ts

Lines changed: 37 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,22 @@ namespace ts.Completions.PathCompletions {
33
export interface NameAndKind {
44
readonly name: string;
55
readonly kind: ScriptElementKind.scriptElement | ScriptElementKind.directory | ScriptElementKind.externalModuleName;
6+
readonly extension: Extension | undefined;
67
}
78
export interface PathCompletion extends NameAndKind {
89
readonly span: TextSpan | undefined;
910
}
1011

11-
function nameAndKind(name: string, kind: NameAndKind["kind"]): NameAndKind {
12-
return { name, kind };
12+
function nameAndKind(name: string, kind: NameAndKind["kind"], extension: Extension | undefined): NameAndKind {
13+
return { name, kind, extension };
1314
}
15+
function directoryResult(name: string): NameAndKind {
16+
return nameAndKind(name, ScriptElementKind.directory, /*extension*/ undefined);
17+
}
18+
1419
function addReplacementSpans(text: string, textStart: number, names: ReadonlyArray<NameAndKind>): ReadonlyArray<PathCompletion> {
1520
const span = getDirectoryFragmentTextSpan(text, textStart);
16-
return names.map(({ name, kind }): PathCompletion => ({ name, kind, span }));
21+
return names.map(({ name, kind, extension }): PathCompletion => ({ name, kind, extension, span }));
1722
}
1823

1924
export function getStringLiteralCompletionsFromModuleNames(sourceFile: SourceFile, node: LiteralExpression, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): ReadonlyArray<PathCompletion> {
@@ -129,22 +134,19 @@ namespace ts.Completions.PathCompletions {
129134
*
130135
* both foo.ts and foo.tsx become foo
131136
*/
132-
const foundFiles = createMap<true>();
137+
const foundFiles = createMap<Extension | undefined>(); // maps file to its extension
133138
for (let filePath of files) {
134139
filePath = normalizePath(filePath);
135140
if (exclude && comparePaths(filePath, exclude, scriptPath, ignoreCase) === Comparison.EqualTo) {
136141
continue;
137142
}
138143

139144
const foundFileName = includeExtensions || fileExtensionIs(filePath, Extension.Json) ? getBaseFileName(filePath) : removeFileExtension(getBaseFileName(filePath));
140-
141-
if (!foundFiles.has(foundFileName)) {
142-
foundFiles.set(foundFileName, true);
143-
}
145+
foundFiles.set(foundFileName, tryGetExtensionFromPath(filePath));
144146
}
145147

146-
forEachKey(foundFiles, foundFile => {
147-
result.push(nameAndKind(foundFile, ScriptElementKind.scriptElement));
148+
foundFiles.forEach((ext, foundFile) => {
149+
result.push(nameAndKind(foundFile, ScriptElementKind.scriptElement, ext));
148150
});
149151
}
150152

@@ -155,7 +157,7 @@ namespace ts.Completions.PathCompletions {
155157
for (const directory of directories) {
156158
const directoryName = getBaseFileName(normalizePath(directory));
157159
if (directoryName !== "@types") {
158-
result.push(nameAndKind(directoryName, ScriptElementKind.directory));
160+
result.push(directoryResult(directoryName));
159161
}
160162
}
161163
}
@@ -183,10 +185,10 @@ namespace ts.Completions.PathCompletions {
183185
if (!hasProperty(paths, path)) continue;
184186
const patterns = paths[path];
185187
if (patterns) {
186-
for (const { name, kind } of getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)) {
188+
for (const { name, kind, extension } of getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)) {
187189
// Path mappings may provide a duplicate way to get to something we've already added, so don't add again.
188190
if (!result.some(entry => entry.name === name)) {
189-
result.push(nameAndKind(name, kind));
191+
result.push(nameAndKind(name, kind, extension));
190192
}
191193
}
192194
}
@@ -200,7 +202,7 @@ namespace ts.Completions.PathCompletions {
200202
* Modules from node_modules (i.e. those listed in package.json)
201203
* This includes all files that are found in node_modules/moduleName/ with acceptable file extensions
202204
*/
203-
function getCompletionEntriesForNonRelativeModules(fragment: string, scriptPath: string, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): NameAndKind[] {
205+
function getCompletionEntriesForNonRelativeModules(fragment: string, scriptPath: string, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): ReadonlyArray<NameAndKind> {
204206
const { baseUrl, paths } = compilerOptions;
205207

206208
const result: NameAndKind[] = [];
@@ -217,7 +219,7 @@ namespace ts.Completions.PathCompletions {
217219

218220
const fragmentDirectory = getFragmentDirectory(fragment);
219221
for (const ambientName of getAmbientModuleCompletions(fragment, fragmentDirectory, typeChecker)) {
220-
result.push(nameAndKind(ambientName, ScriptElementKind.externalModuleName));
222+
result.push(nameAndKind(ambientName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
221223
}
222224

223225
getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, fragmentDirectory, extensionOptions, result);
@@ -230,7 +232,7 @@ namespace ts.Completions.PathCompletions {
230232
for (const moduleName of enumerateNodeModulesVisibleToScript(host, scriptPath)) {
231233
if (!result.some(entry => entry.name === moduleName)) {
232234
foundGlobal = true;
233-
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName));
235+
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
234236
}
235237
}
236238
}
@@ -265,7 +267,7 @@ namespace ts.Completions.PathCompletions {
265267
getModulesForPathsPattern(remainingFragment, baseUrl, pattern, fileExtensions, host));
266268

267269
function justPathMappingName(name: string): ReadonlyArray<NameAndKind> {
268-
return startsWith(name, fragment) ? [{ name, kind: ScriptElementKind.directory }] : emptyArray;
270+
return startsWith(name, fragment) ? [directoryResult(name)] : emptyArray;
269271
}
270272
}
271273

@@ -301,15 +303,21 @@ namespace ts.Completions.PathCompletions {
301303
// doesn't support. For now, this is safer but slower
302304
const includeGlob = normalizedSuffix ? "**/*" : "./*";
303305

304-
const matches = tryReadDirectory(host, baseDirectory, fileExtensions, /*exclude*/ undefined, [includeGlob]).map<NameAndKind>(name => ({ name, kind: ScriptElementKind.scriptElement }));
305-
const directories = tryGetDirectories(host, baseDirectory).map(d => combinePaths(baseDirectory, d)).map<NameAndKind>(name => ({ name, kind: ScriptElementKind.directory }));
306-
307-
// Trim away prefix and suffix
308-
return mapDefined<NameAndKind, NameAndKind>(concatenate(matches, directories), ({ name, kind }) => {
309-
const normalizedMatch = normalizePath(name);
310-
const inner = withoutStartAndEnd(normalizedMatch, completePrefix, normalizedSuffix);
311-
return inner !== undefined ? { name: removeLeadingDirectorySeparator(removeFileExtension(inner)), kind } : undefined;
306+
const matches = mapDefined(tryReadDirectory(host, baseDirectory, fileExtensions, /*exclude*/ undefined, [includeGlob]), match => {
307+
const extension = tryGetExtensionFromPath(match);
308+
const name = trimPrefixAndSuffix(match);
309+
return name === undefined ? undefined : nameAndKind(removeFileExtension(name), ScriptElementKind.scriptElement, extension);
310+
});
311+
const directories = mapDefined(tryGetDirectories(host, baseDirectory).map(d => combinePaths(baseDirectory, d)), dir => {
312+
const name = trimPrefixAndSuffix(dir);
313+
return name === undefined ? undefined : directoryResult(name);
312314
});
315+
return [...matches, ...directories];
316+
317+
function trimPrefixAndSuffix(path: string): string | undefined {
318+
const inner = withoutStartAndEnd(normalizePath(path), completePrefix, normalizedSuffix);
319+
return inner === undefined ? undefined : removeLeadingDirectorySeparator(inner);
320+
}
313321
}
314322

315323
function withoutStartAndEnd(s: string, start: string, end: string): string | undefined {
@@ -382,7 +390,10 @@ namespace ts.Completions.PathCompletions {
382390
if (options.types && !contains(options.types, packageName)) continue;
383391

384392
if (fragmentDirectory === undefined) {
385-
pushResult(packageName);
393+
if (!seen.has(packageName)) {
394+
result.push(nameAndKind(packageName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
395+
seen.set(packageName, true);
396+
}
386397
}
387398
else {
388399
const baseDirectory = combinePaths(directory, typeDirectoryName);
@@ -393,13 +404,6 @@ namespace ts.Completions.PathCompletions {
393404
}
394405
}
395406
}
396-
397-
function pushResult(moduleName: string) {
398-
if (!seen.has(moduleName)) {
399-
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName));
400-
seen.set(moduleName, true);
401-
}
402-
}
403407
}
404408

405409
function findPackageJsons(directory: string, host: LanguageServiceHost): string[] {

src/services/types.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1126,7 +1126,14 @@ namespace ts {
11261126
ambientModifier = "declare",
11271127
staticModifier = "static",
11281128
abstractModifier = "abstract",
1129-
optionalModifier = "optional"
1129+
optionalModifier = "optional",
1130+
1131+
dtsModifier = ".d.ts",
1132+
tsModifier = ".ts",
1133+
tsxModifier = ".tsx",
1134+
jsModifier = ".js",
1135+
jsxModifier = ".jsx",
1136+
jsonModifier = ".json",
11301137
}
11311138

11321139
export const enum ClassificationTypeNames {

tests/baselines/reference/api/tsserverlibrary.d.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5368,7 +5368,13 @@ declare namespace ts {
53685368
ambientModifier = "declare",
53695369
staticModifier = "static",
53705370
abstractModifier = "abstract",
5371-
optionalModifier = "optional"
5371+
optionalModifier = "optional",
5372+
dtsModifier = ".d.ts",
5373+
tsModifier = ".ts",
5374+
tsxModifier = ".tsx",
5375+
jsModifier = ".js",
5376+
jsxModifier = ".jsx",
5377+
jsonModifier = ".json"
53725378
}
53735379
enum ClassificationTypeNames {
53745380
comment = "comment",

tests/baselines/reference/api/typescript.d.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5368,7 +5368,13 @@ declare namespace ts {
53685368
ambientModifier = "declare",
53695369
staticModifier = "static",
53705370
abstractModifier = "abstract",
5371-
optionalModifier = "optional"
5371+
optionalModifier = "optional",
5372+
dtsModifier = ".d.ts",
5373+
tsModifier = ".ts",
5374+
tsxModifier = ".tsx",
5375+
jsModifier = ".js",
5376+
jsxModifier = ".jsx",
5377+
jsonModifier = ".json"
53725378
}
53735379
enum ClassificationTypeNames {
53745380
comment = "comment",

tests/cases/fourslash/completionsImport_compilerOptionsModule.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,6 @@
2727
verify.completions({ marker: ["b", "c", "d"], excludes: "foo", preferences: { includeCompletionsForModuleExports: true } });
2828
verify.completions({
2929
marker: ["c2", "d2"],
30-
includes: [{ name: "foo", source: "/node_modules/a/index", text: "const foo: 0", kind: "const", hasAction: true, sourceDisplay: "a" }],
30+
includes: [{ name: "foo", source: "/node_modules/a/index", text: "const foo: 0", kind: "const", kindModifiers: "export,declare", hasAction: true, sourceDisplay: "a" }],
3131
preferences: { includeCompletionsForModuleExports: true },
3232
});

tests/cases/fourslash/completionsImport_defaultFalsePositive.ts

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,15 @@ goTo.file("/a.ts");
1414

1515
verify.completions({
1616
marker: "",
17-
includes: [
18-
{ name: "concat", source: "/node_modules/bar/concat", sourceDisplay: "bar/concat", text: "const concat: 0", kind: "const", hasAction: true },
19-
],
17+
includes: {
18+
name: "concat",
19+
source: "/node_modules/bar/concat",
20+
sourceDisplay: "bar/concat",
21+
text: "const concat: 0",
22+
kind: "const",
23+
kindModifiers: "export,declare",
24+
hasAction: true,
25+
},
2026
preferences: { includeCompletionsForModuleExports: true },
2127
});
2228

tests/cases/fourslash/completionsImport_default_anonymous.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,11 @@ goTo.marker("0");
1414
const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true };
1515
verify.completions(
1616
{ marker: "0", excludes: { name: "default", source: "/src/foo-bar" }, preferences },
17-
{ marker: "1", includes: { name: "fooBar", source: "/src/foo-bar", sourceDisplay: "./foo-bar", text: "(property) default: 0", kind: "property", hasAction: true }, preferences }
17+
{
18+
marker: "1",
19+
includes: { name: "fooBar", source: "/src/foo-bar", sourceDisplay: "./foo-bar", text: "(property) default: 0", kind: "property", hasAction: true },
20+
preferences,
21+
},
1822
);
1923
verify.applyCodeActionFromCompletion("1", {
2024
name: "fooBar",

tests/cases/fourslash/completionsImport_matching.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
verify.completions({
1818
marker: "",
1919
includes: ["bdf", "abcdef", "BDF"].map(name =>
20-
({ name, source: "/a", text: `function ${name}(): void`, hasAction: true, kind: "function", sourceDisplay: "./a" })),
20+
({ name, source: "/a", text: `function ${name}(): void`, hasAction: true, kind: "function", kindModifiers: "export", sourceDisplay: "./a" })),
2121
excludes: ["abcde", "dbf"],
2222
preferences: { includeCompletionsForModuleExports: true },
2323
})

tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
verify.completions({
1313
marker: "",
1414
includes: [
15-
{ name: "Test1", source: "/a", sourceDisplay: "./a", text: "function Test1(): void", kind: "function", hasAction: true },
15+
{ name: "Test1", source: "/a", sourceDisplay: "./a", text: "function Test1(): void", kind: "function", kindModifiers: "export", hasAction: true },
1616
{ name: "Test2", text: "(alias) function Test2(): void\nimport Test2", kind: "alias" },
1717
],
1818
excludes: [{ name: "Test2", source: "/a" }],

tests/cases/fourslash/completionsImport_ofAlias_preferShortPath.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
verify.completions({
1919
marker: "",
20-
includes: { name: "foo", source: "/foo/lib/foo", sourceDisplay: "./foo", text: "const foo: 0", kind: "const", hasAction: true },
20+
includes: { name: "foo", source: "/foo/lib/foo", sourceDisplay: "./foo", text: "const foo: 0", kind: "const", kindModifiers: "export", hasAction: true },
2121
excludes: { name: "foo", source: "/foo/index" },
2222
preferences: { includeCompletionsForModuleExports: true },
2323
});

tests/cases/fourslash/completionsJsxAttribute.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
////<div /**/></div>;
1818

1919
const exact: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntry> = [
20-
{ name: "foo", kind: "JSX attribute", text: "(JSX attribute) foo: boolean", documentation: "Doc" },
21-
{ name: "bar", kind: "JSX attribute", text: "(JSX attribute) bar: string" },
20+
{ name: "foo", kind: "JSX attribute", kindModifiers: "declare", text: "(JSX attribute) foo: boolean", documentation: "Doc" },
21+
{ name: "bar", kind: "JSX attribute", kindModifiers: "declare", text: "(JSX attribute) bar: string" },
2222
];
2323
verify.completions({ marker: "", exact });
2424
edit.insert("f");

0 commit comments

Comments
 (0)