Skip to content

Add properties priority for completion #32266

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 8 commits into from
Sep 5, 2019
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
69 changes: 61 additions & 8 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@
namespace ts.Completions {
export enum SortText {
LocationPriority = "0",
SuggestedClassMembers = "1",
GlobalsOrKeywords = "2",
AutoImportSuggestions = "3",
JavascriptIdentifiers = "4"
OptionalMember = "1",
MemberDeclaredBySpreadAssignment = "2",
SuggestedClassMembers = "3",
GlobalsOrKeywords = "4",
AutoImportSuggestions = "5",
JavascriptIdentifiers = "6"
}
export type Log = (message: string) => void;

Expand Down Expand Up @@ -1077,6 +1079,7 @@ namespace ts.Completions {
const attrsType = jsxContainer && typeChecker.getContextualType(jsxContainer.attributes);
if (!attrsType) return GlobalsSearch.Continue;
symbols = filterJsxAttributes(getPropertiesForObjectExpression(attrsType, jsxContainer!.attributes, typeChecker), jsxContainer!.attributes.properties);
setSortTextToOptionalMember();
completionKind = CompletionKind.MemberLike;
isNewIdentifierLocation = false;
return GlobalsSearch.Success;
Expand Down Expand Up @@ -1495,6 +1498,8 @@ namespace ts.Completions {
// Add filtered items to the completion list
symbols = filterObjectMembersList(typeMembers, Debug.assertDefined(existingMembers));
}
setSortTextToOptionalMember();

return GlobalsSearch.Success;
}

Expand Down Expand Up @@ -1866,6 +1871,7 @@ namespace ts.Completions {
return contextualMemberSymbols;
}

const membersDeclaredBySpreadAssignment = createMap<true>();
const existingMemberNames = createUnderscoreEscapedMap<boolean>();
for (const m of existingMembers) {
// Ignore omitted expressions for missing members
Expand All @@ -1874,7 +1880,8 @@ namespace ts.Completions {
m.kind !== SyntaxKind.BindingElement &&
m.kind !== SyntaxKind.MethodDeclaration &&
m.kind !== SyntaxKind.GetAccessor &&
m.kind !== SyntaxKind.SetAccessor) {
m.kind !== SyntaxKind.SetAccessor &&
m.kind !== SyntaxKind.SpreadAssignment) {
continue;
}

Expand All @@ -1885,7 +1892,10 @@ namespace ts.Completions {

let existingName: __String | undefined;

if (isBindingElement(m) && m.propertyName) {
if (isSpreadAssignment(m)) {
setMembersDeclaredBySpreadAssignment(m, membersDeclaredBySpreadAssignment);
}
else if (isBindingElement(m) && m.propertyName) {
// include only identifiers in completion list
if (m.propertyName.kind === SyntaxKind.Identifier) {
existingName = m.propertyName.escapedText;
Expand All @@ -1902,7 +1912,43 @@ namespace ts.Completions {
existingMemberNames.set(existingName!, true); // TODO: GH#18217
}

return contextualMemberSymbols.filter(m => !existingMemberNames.get(m.escapedName));
const filteredSymbols = contextualMemberSymbols.filter(m => !existingMemberNames.get(m.escapedName));
setSortTextToMemberDeclaredBySpreadAssignment(membersDeclaredBySpreadAssignment, filteredSymbols);

return filteredSymbols;
}

function setMembersDeclaredBySpreadAssignment(declaration: SpreadAssignment | JsxSpreadAttribute, membersDeclaredBySpreadAssignment: Map<true>) {
const expression = declaration.expression;
const symbol = typeChecker.getSymbolAtLocation(expression);
const type = symbol && typeChecker.getTypeOfSymbolAtLocation(symbol, expression);
const properties = type && (<ObjectType>type).properties;
if (properties) {
properties.forEach(property => {
membersDeclaredBySpreadAssignment.set(property.name, true);
});
}
}

// Set SortText to OptionalMember if it is an optinoal member
function setSortTextToOptionalMember() {
symbols.forEach(m => {
if (m.flags & SymbolFlags.Optional) {
symbolToSortTextMap[getSymbolId(m)] = symbolToSortTextMap[getSymbolId(m)] || SortText.OptionalMember;
}
});
}

// Set SortText to MemberDeclaredBySpreadAssignment if it is fulfilled by spread assignment
function setSortTextToMemberDeclaredBySpreadAssignment(membersDeclaredBySpreadAssignment: Map<true>, contextualMemberSymbols: Symbol[]): void {
if (membersDeclaredBySpreadAssignment.size === 0) {
return;
}
for (const contextualMemberSymbol of contextualMemberSymbols) {
if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
}
}
}

/**
Expand Down Expand Up @@ -1956,6 +2002,7 @@ namespace ts.Completions {
*/
function filterJsxAttributes(symbols: Symbol[], attributes: NodeArray<JsxAttribute | JsxSpreadAttribute>): Symbol[] {
const seenNames = createUnderscoreEscapedMap<boolean>();
const membersDeclaredBySpreadAssignment = createMap<true>();
for (const attr of attributes) {
// If this is the current item we are editing right now, do not filter it out
if (isCurrentlyEditingNode(attr)) {
Expand All @@ -1965,9 +2012,15 @@ namespace ts.Completions {
if (attr.kind === SyntaxKind.JsxAttribute) {
seenNames.set(attr.name.escapedText, true);
}
else if (isJsxSpreadAttribute(attr)) {
setMembersDeclaredBySpreadAssignment(attr, membersDeclaredBySpreadAssignment);
}
}
const filteredSymbols = symbols.filter(a => !seenNames.get(a.escapedName));

setSortTextToMemberDeclaredBySpreadAssignment(membersDeclaredBySpreadAssignment, filteredSymbols);

return symbols.filter(a => !seenNames.get(a.escapedName));
return filteredSymbols;
}

function isCurrentlyEditingNode(node: Node): boolean {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

verify.completions({
marker: "",
exact: ["abc"],
exact: [{ name: 'abc', kind: 'property', kindModifiers: 'declare,optional', sortText: completion.SortText.OptionalMember }],
});
2 changes: 1 addition & 1 deletion tests/cases/fourslash/completionsOptionalMethod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
////declare const x: { m?(): void };
////x./**/

verify.completions({ marker: "", exact: "m" });
verify.completions({ marker: "", exact: "m" });
31 changes: 31 additions & 0 deletions tests/cases/fourslash/completionsPropertiesPriorities.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/// <reference path="fourslash.ts" />
// @strict: true

//// interface I {
//// B?: number;
//// a: number;
//// c?: string;
//// d: string
//// }

//// const foo = {
//// a: 1,
//// B: 2
//// }

//// const i: I = {
//// ...foo,
//// /*a*/
//// }

verify.completions(
{
marker: ['a'],
exact: [
{ name: 'B', kindModifiers: 'optional', sortText: completion.SortText.MemberDeclaredBySpreadAssignment, kind: 'property' },
{ name: 'a', sortText: completion.SortText.MemberDeclaredBySpreadAssignment, kind: 'property' },
{ name: 'c', kindModifiers: 'optional', sortText: completion.SortText.OptionalMember, kind: 'property' },
{ name: 'd', sortText: completion.SortText.LocationPriority, kind: 'property' }
]
}
);
4 changes: 3 additions & 1 deletion tests/cases/fourslash/completionsWithOptionalProperties.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

verify.completions({
marker: "",
includes: ['world']
exact: [
{ name: "world", kind: "property", kindModifiers: "optional", sortText: completion.SortText.OptionalMember }
]
});

10 changes: 6 additions & 4 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,10 +672,12 @@ declare namespace completion {
type Entry = FourSlashInterface.ExpectedCompletionEntryObject;
export const enum SortText {
LocationPriority = "0",
SuggestedClassMembers = "1",
GlobalsOrKeywords = "2",
AutoImportSuggestions = "3",
JavascriptIdentifiers = "4"
OptionalMember = "1",
MemberDeclaredBySpreadAssignment = "2",
SuggestedClassMembers = "3",
GlobalsOrKeywords = "4",
AutoImportSuggestions = "5",
JavascriptIdentifiers = "6"
}
export const globalThisEntry: Entry;
export const undefinedVarEntry: Entry;
Expand Down
10 changes: 8 additions & 2 deletions tests/cases/fourslash/tsxCompletion12.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,13 @@
//// let opt4 = <Opt wrong /*5*/ />;

verify.completions(
{ marker: ["1", "2", "5"], exact: ["propx", "propString", "optional"] },
{ marker: "3", exact: ["propString", "optional"] },
{
marker: ["1", "2", "5"],
exact: ["propx", "propString", { name: "optional", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember }]
},
{
marker: "3",
exact: ["propString", { name: "optional", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember }]
},
{ marker: "4", exact: "propString" },
);
27 changes: 24 additions & 3 deletions tests/cases/fourslash/tsxCompletion13.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,28 @@
//// let opt = <MainButton wrong /*6*/ />;

verify.completions(
{ marker: ["1", "6"], exact: ["onClick", "children", "className", "goTo"] },
{ marker: "2", exact: ["onClick", "className", "goTo"] },
{ marker: ["3", "4", "5"], exact: ["children", "className"] },
{
marker: ["1", "6"],
exact: [
"onClick",
{ name: "children", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember },
{ name: "className", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember },
"goTo"
]
},
{
marker: "2",
exact: [
"onClick",
{ name: "className", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember },
"goTo"
]
},
{
marker: ["3", "4", "5"],
exact: [
{ name: "children", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember },
{ name: "className", kind: "JSX attribute", kindModifiers: "optional", sortText: completion.SortText.OptionalMember }
]
},
);
8 changes: 7 additions & 1 deletion tests/cases/fourslash/tsxCompletion7.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,10 @@
//// let y = { ONE: '' };
//// var x = <div {...y} /**/ />;

verify.completions({ marker: "", exact: ["ONE", "TWO"] });
verify.completions({
marker: "",
exact: [
{ name: "ONE", kind: "JSX attribute", kindModifiers: "declare", sortText: completion.SortText.MemberDeclaredBySpreadAssignment },
{ name: "TWO", kind: "JSX attribute", kindModifiers: "declare", sortText: completion.SortText.LocationPriority }
]
});