Skip to content

Commit e634ba0

Browse files
a-tarasyuka-tarasyuk
committed
fix(43796): sort deprecated completions lower than others
1 parent 71f338b commit e634ba0

15 files changed

+248
-47
lines changed

‎src/services/completions.ts

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/* @internal */
22
namespace ts.Completions {
3+
export type Log = (message: string) => void;
4+
35
export enum SortText {
46
LocalDeclarationPriority = "0",
57
LocationPriority = "1",
@@ -10,7 +12,17 @@ namespace ts.Completions {
1012
AutoImportSuggestions = "6",
1113
JavascriptIdentifiers = "7"
1214
}
13-
export type Log = (message: string) => void;
15+
16+
enum SortTextId {
17+
LocalDeclarationPriority,
18+
LocationPriority,
19+
OptionalMember,
20+
MemberDeclaredBySpreadAssignment,
21+
SuggestedClassMembers,
22+
GlobalsOrKeywords,
23+
AutoImportSuggestions,
24+
Deprecated
25+
}
1426

1527
/**
1628
* Special values for `CompletionInfo['source']` used to disambiguate
@@ -105,8 +117,8 @@ namespace ts.Completions {
105117
*/
106118
type SymbolOriginInfoMap = Record<number, SymbolOriginInfo>;
107119

108-
/** Map from symbol id -> SortText. */
109-
type SymbolSortTextMap = (SortText | undefined)[];
120+
/** Map from symbol id -> SortTextId. */
121+
type SymbolSortTextIdMap = (SortTextId | undefined)[];
110122

111123
const enum KeywordCompletionFilters {
112124
None, // No keywords
@@ -205,7 +217,7 @@ namespace ts.Completions {
205217
isJsxIdentifierExpected,
206218
importCompletionNode,
207219
insideJsDocTagTypeExpression,
208-
symbolToSortTextMap,
220+
symbolToSortTextIdMap,
209221
} = completionData;
210222

211223
// Verify if the file is JSX language variant
@@ -238,7 +250,7 @@ namespace ts.Completions {
238250
importCompletionNode,
239251
recommendedCompletion,
240252
symbolToOriginInfoMap,
241-
symbolToSortTextMap
253+
symbolToSortTextIdMap
242254
);
243255
getJSCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target!, entries); // TODO: GH#18217
244256
}
@@ -266,7 +278,7 @@ namespace ts.Completions {
266278
importCompletionNode,
267279
recommendedCompletion,
268280
symbolToOriginInfoMap,
269-
symbolToSortTextMap
281+
symbolToSortTextIdMap
270282
);
271283
}
272284

@@ -389,7 +401,7 @@ namespace ts.Completions {
389401

390402
function createCompletionEntry(
391403
symbol: Symbol,
392-
sortText: SortText,
404+
sortText: string,
393405
contextToken: Node | undefined,
394406
location: Node,
395407
sourceFile: SourceFile,
@@ -566,7 +578,7 @@ namespace ts.Completions {
566578
importCompletionNode?: Node,
567579
recommendedCompletion?: Symbol,
568580
symbolToOriginInfoMap?: SymbolOriginInfoMap,
569-
symbolToSortTextMap?: SymbolSortTextMap,
581+
symbolToSortTextIdMap?: SymbolSortTextIdMap,
570582
): UniqueNameSet {
571583
const start = timestamp();
572584
const variableDeclaration = getVariableDeclaration(location);
@@ -580,14 +592,16 @@ namespace ts.Completions {
580592
const symbol = symbols[i];
581593
const origin = symbolToOriginInfoMap?.[i];
582594
const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind, !!jsxIdentifierExpected);
583-
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextMap && !shouldIncludeSymbol(symbol, symbolToSortTextMap)) {
595+
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextIdMap && !shouldIncludeSymbol(symbol, symbolToSortTextIdMap)) {
584596
continue;
585597
}
586598

587599
const { name, needsConvertPropertyAccess } = info;
600+
const sortTextId = symbolToSortTextIdMap && symbolToSortTextIdMap[getSymbolId(symbol)] || SortTextId.LocationPriority;
601+
const sortText = (isDeprecated(symbol, typeChecker) ? sortTextId + SortTextId.Deprecated : sortTextId).toString();
588602
const entry = createCompletionEntry(
589603
symbol,
590-
symbolToSortTextMap && symbolToSortTextMap[getSymbolId(symbol)] || SortText.LocationPriority,
604+
sortText,
591605
contextToken,
592606
location,
593607
sourceFile,
@@ -623,7 +637,7 @@ namespace ts.Completions {
623637
add: name => uniques.set(name, true),
624638
};
625639

626-
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean {
640+
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextIdMap: SymbolSortTextIdMap): boolean {
627641
if (!isSourceFile(location)) {
628642
// export = /**/ here we want to get all meanings, so any symbol is ok
629643
if (isExportAssignment(location.parent)) {
@@ -645,9 +659,9 @@ namespace ts.Completions {
645659
// Auto Imports are not available for scripts so this conditional is always false
646660
if (!!sourceFile.externalModuleIndicator
647661
&& !compilerOptions.allowUmdGlobalAccess
648-
&& symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords
649-
&& (symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions
650-
|| symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.LocationPriority)) {
662+
&& symbolToSortTextIdMap[getSymbolId(symbol)] === SortTextId.GlobalsOrKeywords
663+
&& (symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.AutoImportSuggestions
664+
|| symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.LocationPriority)) {
651665
return false;
652666
}
653667
// Continue with origin symbol
@@ -912,7 +926,7 @@ namespace ts.Completions {
912926
readonly previousToken: Node | undefined;
913927
readonly isJsxInitializer: IsJsxInitializer;
914928
readonly insideJsDocTagTypeExpression: boolean;
915-
readonly symbolToSortTextMap: SymbolSortTextMap;
929+
readonly symbolToSortTextIdMap: SymbolSortTextIdMap;
916930
readonly isTypeOnlyLocation: boolean;
917931
/** In JSX tag name and attribute names, identifiers like "my-tag" or "aria-name" is valid identifier. */
918932
readonly isJsxIdentifierExpected: boolean;
@@ -1239,7 +1253,7 @@ namespace ts.Completions {
12391253
// This also gets mutated in nested-functions after the return
12401254
let symbols: Symbol[] = [];
12411255
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
1242-
const symbolToSortTextMap: SymbolSortTextMap = [];
1256+
const symbolToSortTextIdMap: SymbolSortTextIdMap = [];
12431257
const seenPropertySymbols = new Map<SymbolId, true>();
12441258
const isTypeOnly = isTypeOnlyCompletion();
12451259
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
@@ -1295,7 +1309,7 @@ namespace ts.Completions {
12951309
previousToken,
12961310
isJsxInitializer,
12971311
insideJsDocTagTypeExpression,
1298-
symbolToSortTextMap,
1312+
symbolToSortTextIdMap,
12991313
isTypeOnlyLocation: isTypeOnly,
13001314
isJsxIdentifierExpected,
13011315
importCompletionNode,
@@ -1484,7 +1498,7 @@ namespace ts.Completions {
14841498

14851499
function addSymbolSortInfo(symbol: Symbol) {
14861500
if (isStaticProperty(symbol)) {
1487-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority;
1501+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.LocalDeclarationPriority;
14881502
}
14891503
}
14901504

@@ -1601,7 +1615,7 @@ namespace ts.Completions {
16011615
for (const symbol of symbols) {
16021616
if (!typeChecker.isArgumentsSymbol(symbol) &&
16031617
!some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
1604-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.GlobalsOrKeywords;
1618+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.GlobalsOrKeywords;
16051619
}
16061620
}
16071621

@@ -1612,7 +1626,7 @@ namespace ts.Completions {
16121626
for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) {
16131627
symbolToOriginInfoMap[symbols.length] = { kind: SymbolOriginInfoKind.ThisType };
16141628
symbols.push(symbol);
1615-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.SuggestedClassMembers;
1629+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.SuggestedClassMembers;
16161630
}
16171631
}
16181632
}
@@ -1694,7 +1708,7 @@ namespace ts.Completions {
16941708
return false;
16951709
}
16961710

1697-
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextMap` */
1711+
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextIdMap` */
16981712
function collectAutoImports(resolveModuleSpecifiers: boolean) {
16991713
if (!shouldOfferImportCompletions()) return;
17001714
Debug.assert(!detailsEntryId?.data);
@@ -1765,12 +1779,12 @@ namespace ts.Completions {
17651779

17661780
function pushAutoImportSymbol(symbol: Symbol, origin: SymbolOriginInfoResolvedExport | SymbolOriginInfoExport) {
17671781
const symbolId = getSymbolId(symbol);
1768-
if (symbolToSortTextMap[symbolId] === SortText.GlobalsOrKeywords) {
1782+
if (symbolToSortTextIdMap[symbolId] === SortTextId.GlobalsOrKeywords) {
17691783
// If an auto-importable symbol is available as a global, don't add the auto import
17701784
return;
17711785
}
17721786
symbolToOriginInfoMap[symbols.length] = origin;
1773-
symbolToSortTextMap[symbolId] = importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
1787+
symbolToSortTextIdMap[symbolId] = importCompletionNode ? SortTextId.LocationPriority : SortTextId.AutoImportSuggestions;
17741788
symbols.push(symbol);
17751789
}
17761790

@@ -2085,7 +2099,7 @@ namespace ts.Completions {
20852099
localsContainer.locals?.forEach((symbol, name) => {
20862100
symbols.push(symbol);
20872101
if (localsContainer.symbol?.exports?.has(name)) {
2088-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.OptionalMember;
2102+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.OptionalMember;
20892103
}
20902104
});
20912105
return GlobalsSearch.Success;
@@ -2533,7 +2547,7 @@ namespace ts.Completions {
25332547
function setSortTextToOptionalMember() {
25342548
symbols.forEach(m => {
25352549
if (m.flags & SymbolFlags.Optional) {
2536-
symbolToSortTextMap[getSymbolId(m)] = symbolToSortTextMap[getSymbolId(m)] || SortText.OptionalMember;
2550+
symbolToSortTextIdMap[getSymbolId(m)] = symbolToSortTextIdMap[getSymbolId(m)] || SortTextId.OptionalMember;
25372551
}
25382552
});
25392553
}
@@ -2545,7 +2559,7 @@ namespace ts.Completions {
25452559
}
25462560
for (const contextualMemberSymbol of contextualMemberSymbols) {
25472561
if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
2548-
symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
2562+
symbolToSortTextIdMap[getSymbolId(contextualMemberSymbol)] = SortTextId.MemberDeclaredBySpreadAssignment;
25492563
}
25502564
}
25512565
}
@@ -3091,4 +3105,9 @@ namespace ts.Completions {
30913105
addToSeen(seenModules, getSymbolId(sym)) &&
30923106
checker.getExportsOfModule(sym).some(e => symbolCanBeReferencedAtTypeLocation(e, checker, seenModules));
30933107
}
3108+
3109+
function isDeprecated(symbol: Symbol, checker: TypeChecker) {
3110+
const declarations = skipAlias(symbol, checker).declarations;
3111+
return !!length(declarations) && every(declarations, isDeprecatedDeclaration);
3112+
}
30943113
}

‎src/services/symbolDisplay.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -100,10 +100,6 @@ namespace ts.SymbolDisplay {
100100
return ScriptElementKind.unknown;
101101
}
102102

103-
function isDeprecatedDeclaration(decl: Declaration) {
104-
return !!(getCombinedNodeFlagsAlwaysIncludeJSDoc(decl) & ModifierFlags.Deprecated);
105-
}
106-
107103
function getNormalizedSymbolModifiers(symbol: Symbol) {
108104
if (symbol.declarations && symbol.declarations.length) {
109105
const [declaration, ...declarations] = symbol.declarations;

‎src/services/utilities.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3353,5 +3353,9 @@ namespace ts {
33533353
|| (!!globalCachePath && startsWith(getCanonicalFileName(globalCachePath), toNodeModulesParent));
33543354
}
33553355

3356+
export function isDeprecatedDeclaration(decl: Declaration) {
3357+
return !!(getCombinedNodeFlagsAlwaysIncludeJSDoc(decl) & ModifierFlags.Deprecated);
3358+
}
3359+
33563360
// #endregion
33573361
}

‎tests/baselines/reference/completionsSalsaMethodsOnAssignedFunctionExpressions.baseline

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -302,21 +302,21 @@
302302
"name": "a",
303303
"kind": "warning",
304304
"kindModifiers": "",
305-
"sortText": "7",
305+
"sortText": "14",
306306
"isFromUncheckedFile": true
307307
},
308308
{
309309
"name": "prototype",
310310
"kind": "warning",
311311
"kindModifiers": "",
312-
"sortText": "7",
312+
"sortText": "14",
313313
"isFromUncheckedFile": true
314314
},
315315
{
316316
"name": "m",
317317
"kind": "property",
318318
"kindModifiers": "",
319-
"sortText": "7",
319+
"sortText": "14",
320320
"isFromUncheckedFile": true,
321321
"displayParts": [
322322
{

‎tests/cases/fourslash/completionsWithDeprecatedTag1.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,27 +25,26 @@
2525
verify.completions({
2626
marker: "1",
2727
includes: [
28-
{ name: "Foo", kind: "interface", kindModifiers: "deprecated" }
28+
{ name: "Foo", kind: "interface", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
2929
]
3030
}, {
3131
marker: "2",
3232
includes: [
33-
{ name: "bar", kind: "method", kindModifiers: "deprecated" }
33+
{ name: "bar", kind: "method", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
3434
]
3535
}, {
3636
marker: "3",
3737
includes: [
38-
{ name: "prop", kind: "property", kindModifiers: "deprecated" }
38+
{ name: "prop", kind: "property", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
3939
]
4040
}, {
4141
marker: "4",
4242
includes: [
43-
{ name: "foobar", kind: "function", kindModifiers: "export,deprecated" }
43+
{ name: "foobar", kind: "function", kindModifiers: "export,deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
4444
]
4545
}, {
4646
marker: "5",
4747
includes: [
48-
{ name: "foobar", kind: "alias", kindModifiers: "export,deprecated" }
48+
{ name: "foobar", kind: "alias", kindModifiers: "export,deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
4949
]
50-
}
51-
);
50+
});

‎tests/cases/fourslash/completionsWithDeprecatedTag10.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @Filename: /foo.ts
4+
/////** @deprecated foo */
5+
////export const foo = 0;
6+
7+
// @Filename: /index.ts
8+
/////**/
9+
10+
verify.completions({
11+
marker: "",
12+
includes: [{
13+
name: "foo",
14+
source: "/foo",
15+
sourceDisplay: "./foo",
16+
hasAction: true,
17+
kind: "const",
18+
kindModifiers: "export,deprecated",
19+
sortText: completion.SortText.DeprecatedAutoImportSuggestions
20+
}],
21+
preferences: {
22+
includeCompletionsForModuleExports: true,
23+
},
24+
});

‎tests/cases/fourslash/completionsWithDeprecatedTag2.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99

1010
verify.completions({
1111
marker: "",
12-
includes: [
13-
{ name: "foo", kind: "function", kindModifiers: "deprecated,declare" }
14-
]
12+
includes: [{
13+
name: "foo",
14+
kind: "function",
15+
kindModifiers: "deprecated,declare",
16+
sortText: completion.SortText.DeprecatedLocationPriority
17+
}]
1518
});

‎tests/cases/fourslash/completionsWithDeprecatedTag3.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99

1010
verify.completions({
1111
marker: "",
12-
includes: [
13-
{ name: "foo", kind: "function", kindModifiers: "declare" }
14-
]
12+
includes: [{
13+
name: "foo",
14+
kind: "function",
15+
kindModifiers: "declare",
16+
sortText: completion.SortText.LocationPriority
17+
}]
1518
});

‎tests/cases/fourslash/completionsWithDeprecatedTag4.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/// <reference path="fourslash.ts" />
2+
3+
// @noLib: true
4+
5+
////f({
6+
//// a/**/
7+
//// xyz: ``,
8+
////});
9+
////declare function f(options: {
10+
//// /** @deprecated abc */
11+
//// abc?: number,
12+
//// xyz?: string
13+
////}): void;
14+
15+
verify.completions({
16+
marker: "",
17+
exact: [{
18+
name: "abc",
19+
kind: "property",
20+
kindModifiers: "deprecated,declare,optional",
21+
sortText: completion.SortText.DeprecatedOptionalMember
22+
}],
23+
});

0 commit comments

Comments
 (0)