Skip to content

Add support for completions inside string literals #8428

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
merged 12 commits into from
Jun 8, 2016
2 changes: 1 addition & 1 deletion src/services/breakpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -722,5 +722,5 @@ namespace ts.BreakpointResolver {
return spanInNode(node.parent);
}
}
}
}
}
120 changes: 105 additions & 15 deletions src/services/services.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1045,10 +1045,10 @@ namespace ts {
getCancellationToken?(): HostCancellationToken;
getCurrentDirectory(): string;
getDefaultLibFileName(options: CompilerOptions): string;
log? (s: string): void;
trace? (s: string): void;
error? (s: string): void;
useCaseSensitiveFileNames? (): boolean;
log?(s: string): void;
trace?(s: string): void;
error?(s: string): void;
useCaseSensitiveFileNames?(): boolean;

/*
* LS host can optionally implement this method if it wants to be completely in charge of module name resolution.
Expand Down Expand Up @@ -2480,7 +2480,7 @@ namespace ts {
}

// should be start of dependency list
if (token !== SyntaxKind.OpenBracketToken) {
if (token !== SyntaxKind.OpenBracketToken) {
return true;
}

Expand Down Expand Up @@ -4013,10 +4013,15 @@ namespace ts {
}
}


function getCompletionsAtPosition(fileName: string, position: number): CompletionInfo {
synchronizeHostData();

const sourceFile = getValidSourceFile(fileName);

if (isInString(sourceFile, position)) {
return getStringLiteralCompletionEntries(sourceFile, position);
}

const completionData = getCompletionData(fileName, position);
if (!completionData) {
return undefined;
Expand All @@ -4029,12 +4034,10 @@ namespace ts {
return { isMemberCompletion: false, isNewIdentifierLocation: false, entries: getAllJsDocCompletionEntries() };
}

const sourceFile = getValidSourceFile(fileName);

const entries: CompletionEntry[] = [];
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you start perform checking for characters?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure really. this is how it was before adding the argument. let me look more into it.


if (isSourceFileJavaScript(sourceFile)) {
const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries);
const uniqueNames = getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ false);
addRange(entries, getJavaScriptCompletionEntries(sourceFile, location.pos, uniqueNames));
}
else {
Expand All @@ -4058,7 +4061,7 @@ namespace ts {
}
}

getCompletionEntriesFromSymbols(symbols, entries);
getCompletionEntriesFromSymbols(symbols, entries, location, /*performCharacterChecks*/ true);
}

// Add keywords if this is not a member completion list
Expand Down Expand Up @@ -4108,11 +4111,11 @@ namespace ts {
}));
}

function createCompletionEntry(symbol: Symbol, location: Node): CompletionEntry {
function createCompletionEntry(symbol: Symbol, location: Node, performCharacterChecks: boolean): CompletionEntry {
// Try to get a valid display name for this symbol, if we could not find one, then ignore it.
// We would like to only show things that can be added after a dot, so for instance numeric properties can
// not be accessed with a dot (a.1 <- invalid)
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, program.getCompilerOptions().target, /*performCharacterChecks*/ true, location);
const displayName = getCompletionEntryDisplayNameForSymbol(symbol, program.getCompilerOptions().target, performCharacterChecks, location);
if (!displayName) {
return undefined;
}
Expand All @@ -4133,12 +4136,12 @@ namespace ts {
};
}

function getCompletionEntriesFromSymbols(symbols: Symbol[], entries: CompletionEntry[]): Map<string> {
function getCompletionEntriesFromSymbols(symbols: Symbol[], entries: CompletionEntry[], location: Node, performCharacterChecks: boolean): Map<string> {
const start = new Date().getTime();
const uniqueNames: Map<string> = {};
if (symbols) {
for (const symbol of symbols) {
const entry = createCompletionEntry(symbol, location);
const entry = createCompletionEntry(symbol, location, performCharacterChecks);
if (entry) {
const id = escapeIdentifier(entry.name);
if (!lookUp(uniqueNames, id)) {
Expand All @@ -4152,6 +4155,93 @@ namespace ts {
log("getCompletionsAtPosition: getCompletionEntriesFromSymbols: " + (new Date().getTime() - start));
return uniqueNames;
}

function getStringLiteralCompletionEntries(sourceFile: SourceFile, position: number) {
const node = findPrecedingToken(position, sourceFile);
if (!node || node.kind !== SyntaxKind.StringLiteral) {
return undefined;
}

const argumentInfo = SignatureHelp.getContainingArgumentInfo(node, position, sourceFile);
Copy link
Member

Choose a reason for hiding this comment

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

This seems like a slightly weird sort of dependency. Ideally there would be an API on the type checker like getPossibleContextualTypes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not contextual type per se. but sure. If there is another use case for such API we can push it into the checker, but this is the only one, i would keep it at the use site.

if (argumentInfo) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: could you add comments from the PR description here. so we know what cases are covered

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

// Get string literal completions from specialized signatures of the target
return getStringLiteralCompletionEntriesFromCallExpression(argumentInfo);
}
else if (isElementAccessExpression(node.parent) && node.parent.argumentExpression === node) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why you have to check this "node.parent.argumentExpression === node"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"expression"["index"] want to make sure we are on "index"

// Get all names of properties on the expression
return getStringLiteralCompletionEntriesFromElementAccess(node.parent);
}
else {
// Otherwise, get the completions from the contextual type if one exists
return getStringLiteralCompletionEntriesFromContextualType(<StringLiteral>node);
}
}

function getStringLiteralCompletionEntriesFromCallExpression(argumentInfo: SignatureHelp.ArgumentListInfo) {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can eventually generalize this logic to other types as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i suppose if we do pre-selecting of completion entries based on contextual type.

const typeChecker = program.getTypeChecker();
const candidates: Signature[] = [];
const entries: CompletionEntry[] = [];

typeChecker.getResolvedSignature(argumentInfo.invocation, candidates);

for (const candidate of candidates) {
if (candidate.parameters.length > argumentInfo.argumentIndex) {
const parameter = candidate.parameters[argumentInfo.argumentIndex];
addStringLiteralCompletionsFromType(typeChecker.getTypeAtLocation(parameter.valueDeclaration), entries);
}
}

if (entries.length) {
return { isMemberCompletion: false, isNewIdentifierLocation: true, entries };
}

return undefined;
}

function getStringLiteralCompletionEntriesFromElementAccess(node: ElementAccessExpression) {
const typeChecker = program.getTypeChecker();
const type = typeChecker.getTypeAtLocation(node.expression);
const entries: CompletionEntry[] = [];
if (type) {
getCompletionEntriesFromSymbols(type.getApparentProperties(), entries, node, /*performCharacterChecks*/false);
if (entries.length) {
return { isMemberCompletion: true, isNewIdentifierLocation: true, entries };
}
}
return undefined;
}

function getStringLiteralCompletionEntriesFromContextualType(node: StringLiteral) {
const typeChecker = program.getTypeChecker();
const type = typeChecker.getContextualType(node);
if (type) {
const entries: CompletionEntry[] = [];
addStringLiteralCompletionsFromType(type, entries);
if (entries.length) {
return { isMemberCompletion: false, isNewIdentifierLocation: false, entries };
}
}
return undefined;
}

function addStringLiteralCompletionsFromType(type: Type, result: CompletionEntry[]): void {
if (!type) {
return;
}
if (type.flags & TypeFlags.Union) {
forEach((<UnionType>type).types, t => addStringLiteralCompletionsFromType(t, result));
}
else {
if (type.flags & TypeFlags.StringLiteral) {
result.push({
name: (<StringLiteralType>type).text,
kindModifiers: ScriptElementKindModifier.none,
kind: ScriptElementKind.variableElement,
sortText: "0"
});
}
}
}
}

function getCompletionEntryDetails(fileName: string, position: number, entryName: string): CompletionEntryDetails {
Expand Down Expand Up @@ -4316,7 +4406,7 @@ namespace ts {
// try get the call/construct signature from the type if it matches
let callExpression: CallExpression;
if (location.kind === SyntaxKind.CallExpression || location.kind === SyntaxKind.NewExpression) {
callExpression = <CallExpression> location;
callExpression = <CallExpression>location;
}
else if (isCallExpressionTarget(location) || isNewExpressionTarget(location)) {
callExpression = <CallExpression>location.parent;
Expand Down
Loading