Skip to content

For path completions, include extension as a kindModifier #28148

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
1 commit merged into from
Oct 30, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
14 changes: 9 additions & 5 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -796,8 +796,8 @@ namespace FourSlash {
}

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

if (actual.insertText !== insertText) {
Expand All @@ -811,8 +811,12 @@ namespace FourSlash {
this.raiseError(`Expected completion replacementSpan to be ${stringify(convertedReplacementSpan)}, got ${stringify(actual.replacementSpan)}`);
}

if (kind !== undefined) assert.equal(actual.kind, kind);
if (typeof expected !== "string" && "kindModifiers" in expected) assert.equal(actual.kindModifiers, expected.kindModifiers);
if (kind !== undefined || kindModifiers !== undefined) {
assert.equal(actual.kind, kind);
if (actual.kindModifiers !== (kindModifiers || "")) {
this.raiseError(`Bad kind modifiers for ${actual.name}: Expected ${kindModifiers || ""}, actual ${actual.kindModifiers}`);
}
}

assert.equal(actual.hasAction, hasAction);
assert.equal(actual.isRecommended, isRecommended);
Expand Down Expand Up @@ -4916,7 +4920,7 @@ namespace FourSlashInterface {
readonly hasAction?: boolean, // If not specified, will assert that this is false.
readonly isRecommended?: boolean; // If not specified, will assert that this is false.
readonly kind?: string, // If not specified, won't assert about this
readonly kindModifiers?: string;
readonly kindModifiers?: string, // Must be paired with 'kind'
readonly text?: string;
readonly documentation?: string;
readonly sourceDisplay?: string;
Expand Down
18 changes: 16 additions & 2 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,23 @@ namespace ts.Completions {
function convertPathCompletions(pathCompletions: ReadonlyArray<PathCompletions.PathCompletion>): CompletionInfo {
const isGlobalCompletion = false; // We don't want the editor to offer any other completions, such as snippets, inside a comment.
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.
const entries = pathCompletions.map(({ name, kind, span }) => ({ name, kind, kindModifiers: ScriptElementKindModifier.none, sortText: "0", replacementSpan: span }));
const entries = pathCompletions.map(({ name, kind, span, extension }): CompletionEntry =>
({ name, kind, kindModifiers: kindModifiersFromExtension(extension), sortText: "0", replacementSpan: span }));
return { isGlobalCompletion, isMemberCompletion: false, isNewIdentifierLocation, entries };
}
function kindModifiersFromExtension(extension: Extension | undefined): ScriptElementKindModifier {
switch (extension) {
case Extension.Dts: return ScriptElementKindModifier.dtsModifier;
case Extension.Js: return ScriptElementKindModifier.jsModifier;
case Extension.Json: return ScriptElementKindModifier.jsonModifier;
case Extension.Jsx: return ScriptElementKindModifier.jsxModifier;
case Extension.Ts: return ScriptElementKindModifier.tsModifier;
case Extension.Tsx: return ScriptElementKindModifier.tsxModifier;
case undefined: return ScriptElementKindModifier.none;
default:
return Debug.assertNever(extension);
}
}

function jsdocCompletionInfo(entries: CompletionEntry[]): CompletionInfo {
return { isGlobalCompletion: false, isMemberCompletion: false, isNewIdentifierLocation: false, entries };
Expand Down Expand Up @@ -638,7 +652,7 @@ namespace ts.Completions {
switch (completion.kind) {
case StringLiteralCompletionKind.Paths: {
const match = find(completion.paths, p => p.name === name);
return match && createCompletionDetails(name, ScriptElementKindModifier.none, match.kind, [textPart(name)]);
return match && createCompletionDetails(name, kindModifiersFromExtension(match.extension), match.kind, [textPart(name)]);
}
case StringLiteralCompletionKind.Properties: {
const match = find(completion.symbols, s => s.name === name);
Expand Down
70 changes: 37 additions & 33 deletions src/services/pathCompletions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,22 @@ namespace ts.Completions.PathCompletions {
export interface NameAndKind {
readonly name: string;
readonly kind: ScriptElementKind.scriptElement | ScriptElementKind.directory | ScriptElementKind.externalModuleName;
readonly extension: Extension | undefined;
}
export interface PathCompletion extends NameAndKind {
readonly span: TextSpan | undefined;
}

function nameAndKind(name: string, kind: NameAndKind["kind"]): NameAndKind {
return { name, kind };
function nameAndKind(name: string, kind: NameAndKind["kind"], extension: Extension | undefined): NameAndKind {
return { name, kind, extension };
}
function directoryResult(name: string): NameAndKind {
return nameAndKind(name, ScriptElementKind.directory, /*extension*/ undefined);
}

function addReplacementSpans(text: string, textStart: number, names: ReadonlyArray<NameAndKind>): ReadonlyArray<PathCompletion> {
const span = getDirectoryFragmentTextSpan(text, textStart);
return names.map(({ name, kind }): PathCompletion => ({ name, kind, span }));
return names.map(({ name, kind, extension }): PathCompletion => ({ name, kind, extension, span }));
}

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

const foundFileName = includeExtensions || fileExtensionIs(filePath, Extension.Json) ? getBaseFileName(filePath) : removeFileExtension(getBaseFileName(filePath));

if (!foundFiles.has(foundFileName)) {
foundFiles.set(foundFileName, true);
}
foundFiles.set(foundFileName, tryGetExtensionFromPath(filePath));
}

forEachKey(foundFiles, foundFile => {
result.push(nameAndKind(foundFile, ScriptElementKind.scriptElement));
foundFiles.forEach((ext, foundFile) => {
result.push(nameAndKind(foundFile, ScriptElementKind.scriptElement, ext));
});
}

Expand All @@ -155,7 +157,7 @@ namespace ts.Completions.PathCompletions {
for (const directory of directories) {
const directoryName = getBaseFileName(normalizePath(directory));
if (directoryName !== "@types") {
result.push(nameAndKind(directoryName, ScriptElementKind.directory));
result.push(directoryResult(directoryName));
}
}
}
Expand Down Expand Up @@ -183,10 +185,10 @@ namespace ts.Completions.PathCompletions {
if (!hasProperty(paths, path)) continue;
const patterns = paths[path];
if (patterns) {
for (const { name, kind } of getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)) {
for (const { name, kind, extension } of getCompletionsForPathMapping(path, patterns, fragment, baseDirectory, fileExtensions, host)) {
// Path mappings may provide a duplicate way to get to something we've already added, so don't add again.
if (!result.some(entry => entry.name === name)) {
result.push(nameAndKind(name, kind));
result.push(nameAndKind(name, kind, extension));
}
}
}
Expand All @@ -200,7 +202,7 @@ namespace ts.Completions.PathCompletions {
* Modules from node_modules (i.e. those listed in package.json)
* This includes all files that are found in node_modules/moduleName/ with acceptable file extensions
*/
function getCompletionEntriesForNonRelativeModules(fragment: string, scriptPath: string, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): NameAndKind[] {
function getCompletionEntriesForNonRelativeModules(fragment: string, scriptPath: string, compilerOptions: CompilerOptions, host: LanguageServiceHost, typeChecker: TypeChecker): ReadonlyArray<NameAndKind> {
const { baseUrl, paths } = compilerOptions;

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

const fragmentDirectory = getFragmentDirectory(fragment);
for (const ambientName of getAmbientModuleCompletions(fragment, fragmentDirectory, typeChecker)) {
result.push(nameAndKind(ambientName, ScriptElementKind.externalModuleName));
result.push(nameAndKind(ambientName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
}

getCompletionEntriesFromTypings(host, compilerOptions, scriptPath, fragmentDirectory, extensionOptions, result);
Expand All @@ -230,7 +232,7 @@ namespace ts.Completions.PathCompletions {
for (const moduleName of enumerateNodeModulesVisibleToScript(host, scriptPath)) {
if (!result.some(entry => entry.name === moduleName)) {
foundGlobal = true;
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName));
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
}
}
}
Expand Down Expand Up @@ -265,7 +267,7 @@ namespace ts.Completions.PathCompletions {
getModulesForPathsPattern(remainingFragment, baseUrl, pattern, fileExtensions, host));

function justPathMappingName(name: string): ReadonlyArray<NameAndKind> {
return startsWith(name, fragment) ? [{ name, kind: ScriptElementKind.directory }] : emptyArray;
return startsWith(name, fragment) ? [directoryResult(name)] : emptyArray;
}
}

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

const matches = tryReadDirectory(host, baseDirectory, fileExtensions, /*exclude*/ undefined, [includeGlob]).map<NameAndKind>(name => ({ name, kind: ScriptElementKind.scriptElement }));
const directories = tryGetDirectories(host, baseDirectory).map(d => combinePaths(baseDirectory, d)).map<NameAndKind>(name => ({ name, kind: ScriptElementKind.directory }));

// Trim away prefix and suffix
return mapDefined<NameAndKind, NameAndKind>(concatenate(matches, directories), ({ name, kind }) => {
const normalizedMatch = normalizePath(name);
const inner = withoutStartAndEnd(normalizedMatch, completePrefix, normalizedSuffix);
return inner !== undefined ? { name: removeLeadingDirectorySeparator(removeFileExtension(inner)), kind } : undefined;
const matches = mapDefined(tryReadDirectory(host, baseDirectory, fileExtensions, /*exclude*/ undefined, [includeGlob]), match => {
const extension = tryGetExtensionFromPath(match);
const name = trimPrefixAndSuffix(match);
return name === undefined ? undefined : nameAndKind(removeFileExtension(name), ScriptElementKind.scriptElement, extension);
});
const directories = mapDefined(tryGetDirectories(host, baseDirectory).map(d => combinePaths(baseDirectory, d)), dir => {
const name = trimPrefixAndSuffix(dir);
return name === undefined ? undefined : directoryResult(name);
});
return [...matches, ...directories];

function trimPrefixAndSuffix(path: string): string | undefined {
const inner = withoutStartAndEnd(normalizePath(path), completePrefix, normalizedSuffix);
return inner === undefined ? undefined : removeLeadingDirectorySeparator(inner);
}
}

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

if (fragmentDirectory === undefined) {
pushResult(packageName);
if (!seen.has(packageName)) {
result.push(nameAndKind(packageName, ScriptElementKind.externalModuleName, /*extension*/ undefined));
seen.set(packageName, true);
}
}
else {
const baseDirectory = combinePaths(directory, typeDirectoryName);
Expand All @@ -393,13 +404,6 @@ namespace ts.Completions.PathCompletions {
}
}
}

function pushResult(moduleName: string) {
if (!seen.has(moduleName)) {
result.push(nameAndKind(moduleName, ScriptElementKind.externalModuleName));
seen.set(moduleName, true);
}
}
}

function findPackageJsons(directory: string, host: LanguageServiceHost): string[] {
Expand Down
9 changes: 8 additions & 1 deletion src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1126,7 +1126,14 @@ namespace ts {
ambientModifier = "declare",
staticModifier = "static",
abstractModifier = "abstract",
optionalModifier = "optional"
optionalModifier = "optional",

dtsModifier = ".d.ts",
tsModifier = ".ts",
tsxModifier = ".tsx",
jsModifier = ".js",
jsxModifier = ".jsx",
jsonModifier = ".json",
}

export const enum ClassificationTypeNames {
Expand Down
8 changes: 7 additions & 1 deletion tests/baselines/reference/api/tsserverlibrary.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5369,7 +5369,13 @@ declare namespace ts {
ambientModifier = "declare",
staticModifier = "static",
abstractModifier = "abstract",
optionalModifier = "optional"
optionalModifier = "optional",
dtsModifier = ".d.ts",
tsModifier = ".ts",
tsxModifier = ".tsx",
jsModifier = ".js",
jsxModifier = ".jsx",
jsonModifier = ".json"
}
enum ClassificationTypeNames {
comment = "comment",
Expand Down
8 changes: 7 additions & 1 deletion tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5369,7 +5369,13 @@ declare namespace ts {
ambientModifier = "declare",
staticModifier = "static",
abstractModifier = "abstract",
optionalModifier = "optional"
optionalModifier = "optional",
dtsModifier = ".d.ts",
tsModifier = ".ts",
tsxModifier = ".tsx",
jsModifier = ".js",
jsxModifier = ".jsx",
jsonModifier = ".json"
}
enum ClassificationTypeNames {
comment = "comment",
Expand Down
16 changes: 8 additions & 8 deletions tests/cases/fourslash/completionEntryForClassMembers2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,18 +189,18 @@
////}

const validInstanceMembersOfBaseClassB: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "protectedMethod", text: "(method) B.protectedMethod(): void" },
{ name: "protectedMethod", text: "(method) B.protectedMethod(): void", kindModifiers: "protected" },
{ name: "getValue", text: "(method) B.getValue(): string | boolean" },
];
const validStaticMembersOfBaseClassB: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "staticMethod", text: "(method) B.staticMethod(): void" },
{ name: "staticMethod", text: "(method) B.staticMethod(): void", kindModifiers: "static" },
];
const privateMembersOfBaseClassB: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "privateMethod", text: "(method) B.privateMethod(): void" },
];
const protectedPropertiesOfBaseClassB0: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "protectedMethod", text: "(method) B0.protectedMethod(): void" },
{ name: "protectedMethod1", text: "(method) B0.protectedMethod1(): void" },
{ name: "protectedMethod", text: "(method) B0.protectedMethod(): void", kindModifiers: "protected" },
{ name: "protectedMethod1", text: "(method) B0.protectedMethod1(): void", kindModifiers: "protected" },
];
const publicPropertiesOfBaseClassB0: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "getValue", text: "(method) B0.getValue(): string | boolean" },
Expand All @@ -214,12 +214,12 @@ const validInstanceMembersOfBaseClassB0_2 : ReadonlyArray<FourSlashInterface.Exp
publicPropertiesOfBaseClassB0[1],
];
const validStaticMembersOfBaseClassB0: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "staticMethod", text: "(method) B0.staticMethod(): void" },
{ name: "staticMethod1", text: "(method) B0.staticMethod1(): void" },
{ name: "staticMethod", text: "(method) B0.staticMethod(): void", kindModifiers: "static" },
{ name: "staticMethod1", text: "(method) B0.staticMethod1(): void", kindModifiers: "static" },
];
const privateMembersOfBaseClassB0: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "privateMethod", text: "(method) B0.privateMethod(): void" },
{ name: "privateMethod1", text: "(method) B0.privateMethod1(): void" },
{ name: "privateMethod", text: "(method) B0.privateMethod(): void", kindModifiers: "private" },
{ name: "privateMethod1", text: "(method) B0.privateMethod1(): void", kindModifiers: "private" },
];
const membersOfI: ReadonlyArray<FourSlashInterface.ExpectedCompletionEntryObject> = [
{ name: "methodOfInterface", text: "(method) I.methodOfInterface(): number" },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
////o["/*prop*/"];

verify.completions(
{ marker: "path", includes: { name: "other", text: "other", kind: "script" }, isNewIdentifierLocation: true },
{ marker: "path", includes: { name: "other", text: "other", kind: "script", kindModifiers: ".ts" }, isNewIdentifierLocation: true },
{ marker: "type", exact: { name: "a", text: "a", kind: "string" } },
{
marker: "prop",
Expand Down
11 changes: 10 additions & 1 deletion tests/cases/fourslash/completionInJsDocQualifiedNames.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,4 +11,13 @@
/////** @type {Foo./**/} */
////const x = 0;

verify.completions({ marker: "", includes: { name: "T", text: "type T = number", documentation: "tee", kind: "type" } });
verify.completions({
marker: "",
includes: {
name: "T",
text: "type T = number",
documentation: "tee",
kind: "type",
kindModifiers: "export,declare",
},
});
Loading