Skip to content

Commit

Permalink
Support a 'recommended' completion entry (#20020)
Browse files Browse the repository at this point in the history
* Support a 'recommended' completion entry

* Code review

* Restore duplicate comments
  • Loading branch information
Andy authored Dec 1, 2017
1 parent 973cb76 commit fd4d8ab
Show file tree
Hide file tree
Showing 17 changed files with 235 additions and 62 deletions.
45 changes: 20 additions & 25 deletions src/compiler/checker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,7 @@ namespace ts {
return resolveName(location, escapeLeadingUnderscores(name), meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isUse*/ false);
},
getJsxNamespace: () => unescapeLeadingUnderscores(getJsxNamespace()),
getAccessibleSymbolChain,
};

const tupleTypes: GenericType[] = [];
Expand Down Expand Up @@ -763,10 +764,6 @@ namespace ts {
return nodeLinks[nodeId] || (nodeLinks[nodeId] = { flags: 0 });
}

function getObjectFlags(type: Type): ObjectFlags {
return type.flags & TypeFlags.Object ? (<ObjectType>type).objectFlags : 0;
}

function isGlobalSourceFile(node: Node) {
return node.kind === SyntaxKind.SourceFile && !isExternalOrCommonJsModule(<SourceFile>node);
}
Expand Down Expand Up @@ -10452,20 +10449,6 @@ namespace ts {
!hasBaseType(checkClass, getDeclaringClass(p)) : false) ? undefined : checkClass;
}

// Return true if the given type is the constructor type for an abstract class
function isAbstractConstructorType(type: Type) {
if (getObjectFlags(type) & ObjectFlags.Anonymous) {
const symbol = type.symbol;
if (symbol && symbol.flags & SymbolFlags.Class) {
const declaration = getClassLikeDeclarationOfSymbol(symbol);
if (declaration && hasModifier(declaration, ModifierFlags.Abstract)) {
return true;
}
}
}
return false;
}

// Return true if the given type is deeply nested. We consider this to be the case when structural type comparisons
// for 5 or more occurrences or instantiations of the type have been recorded on the given stack. It is possible,
// though highly unlikely, for this test to be true in a situation where a chain of instantiations is not infinitely
Expand Down Expand Up @@ -13767,7 +13750,7 @@ namespace ts {
// the contextual type of an initializer expression is the type annotation of the containing declaration, if present.
function getContextualTypeForInitializerExpression(node: Expression): Type {
const declaration = <VariableLikeDeclaration>node.parent;
if (node === declaration.initializer) {
if (node === declaration.initializer || node.kind === SyntaxKind.EqualsToken) {
const typeNode = getEffectiveTypeAnnotationNode(declaration);
if (typeNode) {
return getTypeFromTypeNode(typeNode);
Expand Down Expand Up @@ -13899,6 +13882,12 @@ namespace ts {
case SyntaxKind.AmpersandAmpersandToken:
case SyntaxKind.CommaToken:
return node === right ? getContextualType(binaryExpression) : undefined;
case SyntaxKind.EqualsEqualsEqualsToken:
case SyntaxKind.EqualsEqualsToken:
case SyntaxKind.ExclamationEqualsEqualsToken:
case SyntaxKind.ExclamationEqualsToken:
// For completions after `x === `
return node === operatorToken ? getTypeOfExpression(binaryExpression.left) : undefined;
default:
return undefined;
}
Expand Down Expand Up @@ -14114,9 +14103,13 @@ namespace ts {
return getContextualTypeForReturnExpression(node);
case SyntaxKind.YieldExpression:
return getContextualTypeForYieldOperand(<YieldExpression>parent);
case SyntaxKind.CallExpression:
case SyntaxKind.NewExpression:
return getContextualTypeForArgument(<CallExpression>parent, node);
if (node.kind === SyntaxKind.NewKeyword) { // for completions after `new `
return getContextualType(parent as NewExpression);
}
// falls through
case SyntaxKind.CallExpression:
return getContextualTypeForArgument(<CallExpression | NewExpression>parent, node);
case SyntaxKind.TypeAssertionExpression:
case SyntaxKind.AsExpression:
return getTypeFromTypeNode((<AssertionExpression>parent).type);
Expand Down Expand Up @@ -14150,6 +14143,12 @@ namespace ts {
case SyntaxKind.JsxOpeningElement:
case SyntaxKind.JsxSelfClosingElement:
return getAttributesTypeFromJsxOpeningLikeElement(<JsxOpeningLikeElement>parent);
case SyntaxKind.CaseClause: {
if (node.kind === SyntaxKind.CaseKeyword) { // for completions after `case `
const switchStatement = (parent as CaseClause).parent.parent;
return getTypeOfExpression(switchStatement.expression);
}
}
}
return undefined;
}
Expand Down Expand Up @@ -22578,10 +22577,6 @@ namespace ts {
return getCheckFlags(s) & CheckFlags.Instantiated ? (<TransientSymbol>s).target : s;
}

function getClassLikeDeclarationOfSymbol(symbol: Symbol): Declaration {
return forEach(symbol.declarations, d => isClassLike(d) ? d : undefined);
}

function getClassOrInterfaceDeclarationsOfSymbol(symbol: Symbol) {
return filter(symbol.declarations, (d: Declaration): d is ClassDeclaration | InterfaceDeclaration =>
d.kind === SyntaxKind.ClassDeclaration || d.kind === SyntaxKind.InterfaceDeclaration);
Expand Down
11 changes: 11 additions & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2818,6 +2818,17 @@ namespace ts {
/* @internal */ getAllPossiblePropertiesOfTypes(type: ReadonlyArray<Type>): Symbol[];
/* @internal */ resolveName(name: string, location: Node, meaning: SymbolFlags): Symbol | undefined;
/* @internal */ getJsxNamespace(): string;

/**
* Note that this will return undefined in the following case:
* // a.ts
* export namespace N { export class C { } }
* // b.ts
* <<enclosingDeclaration>>
* Where `C` is the symbol we're looking for.
* This should be called in a loop climbing parents of the symbol, so we'll get `N`.
*/
/* @internal */ getAccessibleSymbolChain(symbol: Symbol, enclosingDeclaration: Node | undefined, meaning: SymbolFlags, useOnlyExternalAliasing: boolean): Symbol[] | undefined;
}

export enum NodeBuilderFlags {
Expand Down
26 changes: 21 additions & 5 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,6 @@ namespace ts {
return isExternalModule(node) || compilerOptions.isolatedModules || ((getEmitModuleKind(compilerOptions) === ModuleKind.CommonJS) && !!node.commonJsModuleIndicator);
}

/* @internal */
export function isBlockScope(node: Node, parentNode: Node) {
switch (node.kind) {
case SyntaxKind.SourceFile:
Expand Down Expand Up @@ -493,7 +492,6 @@ namespace ts {
return false;
}

/* @internal */
export function isDeclarationWithTypeParameters(node: Node): node is DeclarationWithTypeParameters;
export function isDeclarationWithTypeParameters(node: DeclarationWithTypeParameters): node is DeclarationWithTypeParameters {
switch (node.kind) {
Expand Down Expand Up @@ -523,7 +521,6 @@ namespace ts {
}
}

/* @internal */
export function isAnyImportSyntax(node: Node): node is AnyImportSyntax {
switch (node.kind) {
case SyntaxKind.ImportDeclaration:
Expand Down Expand Up @@ -1805,7 +1802,6 @@ namespace ts {
}
}

/* @internal */
// See GH#16030
export function isAnyDeclarationName(name: Node): boolean {
switch (name.kind) {
Expand Down Expand Up @@ -3115,7 +3111,6 @@ namespace ts {
return flags;
}

/* @internal */
export function getModifierFlagsNoCache(node: Node): ModifierFlags {

let flags = ModifierFlags.None;
Expand Down Expand Up @@ -3677,6 +3672,27 @@ namespace ts {
}
}

// Return true if the given type is the constructor type for an abstract class
export function isAbstractConstructorType(type: Type): boolean {
return !!(getObjectFlags(type) & ObjectFlags.Anonymous) && !!type.symbol && isAbstractConstructorSymbol(type.symbol);
}

export function isAbstractConstructorSymbol(symbol: Symbol): boolean {
if (symbol.flags & SymbolFlags.Class) {
const declaration = getClassLikeDeclarationOfSymbol(symbol);
return !!declaration && hasModifier(declaration, ModifierFlags.Abstract);
}
return false;
}

export function getClassLikeDeclarationOfSymbol(symbol: Symbol): Declaration | undefined {
return find(symbol.declarations, isClassLike);
}

export function getObjectFlags(type: Type): ObjectFlags {
return type.flags & TypeFlags.Object ? (<ObjectType>type).objectFlags : 0;
}

export function typeHasCallOrConstructSignatures(type: Type, checker: TypeChecker) {
return checker.getSignaturesOfType(type, SignatureKind.Call).length !== 0 || checker.getSignaturesOfType(type, SignatureKind.Construct).length !== 0;
}
Expand Down
10 changes: 6 additions & 4 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ namespace FourSlash {
* @param expectedKind the kind of symbol (see ScriptElementKind)
* @param spanIndex the index of the range that the completion item's replacement text span should match
*/
public verifyCompletionListDoesNotContain(entryId: ts.Completions.CompletionEntryIdentifier, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number, options?: ts.GetCompletionsAtPositionOptions) {
public verifyCompletionListDoesNotContain(entryId: ts.Completions.CompletionEntryIdentifier, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number, options?: FourSlashInterface.CompletionsAtOptions) {
let replacementSpan: ts.TextSpan;
if (spanIndex !== undefined) {
replacementSpan = this.getTextSpanForRangeAtIndex(spanIndex);
Expand Down Expand Up @@ -1207,7 +1207,7 @@ Actual: ${stringify(fullActual)}`);
this.raiseError(`verifyReferencesAtPositionListContains failed - could not find the item: ${stringify(missingItem)} in the returned list: (${stringify(references)})`);
}

private getCompletionListAtCaret(options?: ts.GetCompletionsAtPositionOptions): ts.CompletionInfo {
private getCompletionListAtCaret(options?: FourSlashInterface.CompletionsAtOptions): ts.CompletionInfo {
return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition, options);
}

Expand Down Expand Up @@ -1721,7 +1721,7 @@ Actual: ${stringify(fullActual)}`);
const longestNameLength = max(entries, m => m.name.length);
const longestKindLength = max(entries, m => m.kind.length);
entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0);
const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers} ${m.source === undefined ? "" : m.source}`).join("\n");
const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers} ${m.isRecommended ? "recommended " : ""}${m.source === undefined ? "" : m.source}`).join("\n");
Harness.IO.log(membersString);
}

Expand Down Expand Up @@ -3114,6 +3114,7 @@ Actual: ${stringify(fullActual)}`);
}

assert.equal(item.hasAction, hasAction);
assert.equal(item.isRecommended, options && options.isRecommended, "isRecommended");
}

private findFile(indexOrName: string | number) {
Expand Down Expand Up @@ -4552,12 +4553,13 @@ namespace FourSlashInterface {
newContent: string;
}

export interface CompletionsAtOptions {
export interface CompletionsAtOptions extends ts.GetCompletionsAtPositionOptions {
isNewIdentifierLocation?: boolean;
}

export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
sourceDisplay: string;
isRecommended?: true;
}

export interface NewContentOptions {
Expand Down
11 changes: 6 additions & 5 deletions src/server/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,14 +180,15 @@ namespace ts.server {
isGlobalCompletion: false,
isMemberCompletion: false,
isNewIdentifierLocation: false,
entries: response.body.map(entry => {

entries: response.body.map<CompletionEntry>(entry => {
if (entry.replacementSpan !== undefined) {
const { name, kind, kindModifiers, sortText, replacementSpan } = entry;
return { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName) };
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, isRecommended } = entry;
// TODO: GH#241
const res: CompletionEntry = { name, kind, kindModifiers, sortText, replacementSpan: this.decodeSpan(replacementSpan, fileName), hasAction, source, isRecommended };
return res;
}

return entry as { name: string, kind: ScriptElementKind, kindModifiers: string, sortText: string };
return entry as { name: string, kind: ScriptElementKind, kindModifiers: string, sortText: string }; // TODO: GH#18217
})
};
}
Expand Down
11 changes: 9 additions & 2 deletions src/server/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1701,8 +1701,9 @@ namespace ts.server.protocol {
*/
sortText: string;
/**
* An optional span that indicates the text to be replaced by this completion item. If present,
* this span should be used instead of the default one.
* An optional span that indicates the text to be replaced by this completion item.
* If present, this span should be used instead of the default one.
* It will be set if the required span differs from the one generated by the default replacement behavior.
*/
replacementSpan?: TextSpan;
/**
Expand All @@ -1714,6 +1715,12 @@ namespace ts.server.protocol {
* Identifier (not necessarily human-readable) identifying where this completion came from.
*/
source?: string;
/**
* If true, this completion should be highlighted as recommended. There will only be one of these.
* This will be set when we know the user should write an expression with a certain type and that type is an enum or constructable class.
* Then either that enum/class or a namespace containing it will be the recommended symbol.
*/
isRecommended?: true;
}

/**
Expand Down
4 changes: 2 additions & 2 deletions src/server/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1207,10 +1207,10 @@ namespace ts.server {
if (simplifiedResult) {
return mapDefined<CompletionEntry, protocol.CompletionEntry>(completions && completions.entries, entry => {
if (completions.isMemberCompletion || startsWith(entry.name.toLowerCase(), prefix.toLowerCase())) {
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source } = entry;
const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source, isRecommended } = entry;
const convertedSpan = replacementSpan ? this.toLocationTextSpan(replacementSpan, scriptInfo) : undefined;
// Use `hasAction || undefined` to avoid serializing `false`.
return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source };
return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source, isRecommended };
}
}).sort((a, b) => compareStringsCaseSensitiveUI(a.name, b.name));
}
Expand Down
Loading

0 comments on commit fd4d8ab

Please sign in to comment.