Skip to content

Commit 276bfc5

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

14 files changed

+254
-46
lines changed

‎src/services/completions.ts

Lines changed: 54 additions & 28 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",
@@ -8,9 +10,27 @@ namespace ts.Completions {
810
SuggestedClassMembers = "4",
911
GlobalsOrKeywords = "5",
1012
AutoImportSuggestions = "6",
11-
JavascriptIdentifiers = "7"
13+
JavascriptIdentifiers = "7",
14+
DeprecatedLocalDeclarationPriority = "8",
15+
DeprecatedLocationPriority = "9",
16+
DeprecatedOptionalMember = "10",
17+
DeprecatedMemberDeclaredBySpreadAssignment = "11",
18+
DeprecatedSuggestedClassMembers = "12",
19+
DeprecatedGlobalsOrKeywords = "13",
20+
DeprecatedAutoImportSuggestions = "14"
21+
}
22+
23+
enum SortTextId {
24+
LocalDeclarationPriority,
25+
LocationPriority,
26+
OptionalMember,
27+
MemberDeclaredBySpreadAssignment,
28+
SuggestedClassMembers,
29+
GlobalsOrKeywords,
30+
AutoImportSuggestions
1231
}
13-
export type Log = (message: string) => void;
32+
33+
const DeprecatedSortTextStart = SortTextId.AutoImportSuggestions + 2; // for Javascript identifiers since with this change they are preferred over deprecated symbols
1434

1535
/**
1636
* Special values for `CompletionInfo['source']` used to disambiguate
@@ -105,8 +125,8 @@ namespace ts.Completions {
105125
*/
106126
type SymbolOriginInfoMap = Record<number, SymbolOriginInfo>;
107127

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

111131
const enum KeywordCompletionFilters {
112132
None, // No keywords
@@ -205,7 +225,7 @@ namespace ts.Completions {
205225
isJsxIdentifierExpected,
206226
importCompletionNode,
207227
insideJsDocTagTypeExpression,
208-
symbolToSortTextMap,
228+
symbolToSortTextIdMap,
209229
} = completionData;
210230

211231
// Verify if the file is JSX language variant
@@ -238,7 +258,7 @@ namespace ts.Completions {
238258
importCompletionNode,
239259
recommendedCompletion,
240260
symbolToOriginInfoMap,
241-
symbolToSortTextMap
261+
symbolToSortTextIdMap
242262
);
243263
getJSCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target!, entries); // TODO: GH#18217
244264
}
@@ -266,7 +286,7 @@ namespace ts.Completions {
266286
importCompletionNode,
267287
recommendedCompletion,
268288
symbolToOriginInfoMap,
269-
symbolToSortTextMap
289+
symbolToSortTextIdMap
270290
);
271291
}
272292

@@ -566,7 +586,7 @@ namespace ts.Completions {
566586
importCompletionNode?: Node,
567587
recommendedCompletion?: Symbol,
568588
symbolToOriginInfoMap?: SymbolOriginInfoMap,
569-
symbolToSortTextMap?: SymbolSortTextMap,
589+
symbolToSortTextIdMap?: SymbolSortTextIdMap,
570590
): UniqueNameSet {
571591
const start = timestamp();
572592
const variableDeclaration = getVariableDeclaration(location);
@@ -580,14 +600,16 @@ namespace ts.Completions {
580600
const symbol = symbols[i];
581601
const origin = symbolToOriginInfoMap?.[i];
582602
const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind, !!jsxIdentifierExpected);
583-
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextMap && !shouldIncludeSymbol(symbol, symbolToSortTextMap)) {
603+
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextIdMap && !shouldIncludeSymbol(symbol, symbolToSortTextIdMap)) {
584604
continue;
585605
}
586606

587607
const { name, needsConvertPropertyAccess } = info;
608+
const sortTextId = symbolToSortTextIdMap?.[getSymbolId(symbol)] ?? SortTextId.LocationPriority;
609+
const sortText = (isDeprecated(symbol, typeChecker) ? DeprecatedSortTextStart + sortTextId : sortTextId).toString() as SortText;
588610
const entry = createCompletionEntry(
589611
symbol,
590-
symbolToSortTextMap && symbolToSortTextMap[getSymbolId(symbol)] || SortText.LocationPriority,
612+
sortText,
591613
contextToken,
592614
location,
593615
sourceFile,
@@ -623,7 +645,7 @@ namespace ts.Completions {
623645
add: name => uniques.set(name, true),
624646
};
625647

626-
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean {
648+
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextIdMap: SymbolSortTextIdMap): boolean {
627649
if (!isSourceFile(location)) {
628650
// export = /**/ here we want to get all meanings, so any symbol is ok
629651
if (isExportAssignment(location.parent)) {
@@ -645,9 +667,9 @@ namespace ts.Completions {
645667
// Auto Imports are not available for scripts so this conditional is always false
646668
if (!!sourceFile.externalModuleIndicator
647669
&& !compilerOptions.allowUmdGlobalAccess
648-
&& symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords
649-
&& (symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions
650-
|| symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.LocationPriority)) {
670+
&& symbolToSortTextIdMap[getSymbolId(symbol)] === SortTextId.GlobalsOrKeywords
671+
&& (symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.AutoImportSuggestions
672+
|| symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.LocationPriority)) {
651673
return false;
652674
}
653675
// Continue with origin symbol
@@ -669,8 +691,6 @@ namespace ts.Completions {
669691
}
670692
}
671693

672-
673-
674694
function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined {
675695
const entries = getLabelStatementCompletions(node);
676696
if (entries.length) {
@@ -912,7 +932,7 @@ namespace ts.Completions {
912932
readonly previousToken: Node | undefined;
913933
readonly isJsxInitializer: IsJsxInitializer;
914934
readonly insideJsDocTagTypeExpression: boolean;
915-
readonly symbolToSortTextMap: SymbolSortTextMap;
935+
readonly symbolToSortTextIdMap: SymbolSortTextIdMap;
916936
readonly isTypeOnlyLocation: boolean;
917937
/** In JSX tag name and attribute names, identifiers like "my-tag" or "aria-name" is valid identifier. */
918938
readonly isJsxIdentifierExpected: boolean;
@@ -1239,7 +1259,7 @@ namespace ts.Completions {
12391259
// This also gets mutated in nested-functions after the return
12401260
let symbols: Symbol[] = [];
12411261
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
1242-
const symbolToSortTextMap: SymbolSortTextMap = [];
1262+
const symbolToSortTextIdMap: SymbolSortTextIdMap = [];
12431263
const seenPropertySymbols = new Map<SymbolId, true>();
12441264
const isTypeOnly = isTypeOnlyCompletion();
12451265
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
@@ -1295,7 +1315,7 @@ namespace ts.Completions {
12951315
previousToken,
12961316
isJsxInitializer,
12971317
insideJsDocTagTypeExpression,
1298-
symbolToSortTextMap,
1318+
symbolToSortTextIdMap,
12991319
isTypeOnlyLocation: isTypeOnly,
13001320
isJsxIdentifierExpected,
13011321
importCompletionNode,
@@ -1484,7 +1504,7 @@ namespace ts.Completions {
14841504

14851505
function addSymbolSortInfo(symbol: Symbol) {
14861506
if (isStaticProperty(symbol)) {
1487-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority;
1507+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.LocalDeclarationPriority;
14881508
}
14891509
}
14901510

@@ -1601,7 +1621,7 @@ namespace ts.Completions {
16011621
for (const symbol of symbols) {
16021622
if (!typeChecker.isArgumentsSymbol(symbol) &&
16031623
!some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
1604-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.GlobalsOrKeywords;
1624+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.GlobalsOrKeywords;
16051625
}
16061626
}
16071627

@@ -1612,7 +1632,7 @@ namespace ts.Completions {
16121632
for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) {
16131633
symbolToOriginInfoMap[symbols.length] = { kind: SymbolOriginInfoKind.ThisType };
16141634
symbols.push(symbol);
1615-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.SuggestedClassMembers;
1635+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.SuggestedClassMembers;
16161636
}
16171637
}
16181638
}
@@ -1694,7 +1714,7 @@ namespace ts.Completions {
16941714
return false;
16951715
}
16961716

1697-
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextMap` */
1717+
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextIdMap` */
16981718
function collectAutoImports(resolveModuleSpecifiers: boolean) {
16991719
if (!shouldOfferImportCompletions()) return;
17001720
Debug.assert(!detailsEntryId?.data);
@@ -1765,12 +1785,12 @@ namespace ts.Completions {
17651785

17661786
function pushAutoImportSymbol(symbol: Symbol, origin: SymbolOriginInfoResolvedExport | SymbolOriginInfoExport) {
17671787
const symbolId = getSymbolId(symbol);
1768-
if (symbolToSortTextMap[symbolId] === SortText.GlobalsOrKeywords) {
1788+
if (symbolToSortTextIdMap[symbolId] === SortTextId.GlobalsOrKeywords) {
17691789
// If an auto-importable symbol is available as a global, don't add the auto import
17701790
return;
17711791
}
17721792
symbolToOriginInfoMap[symbols.length] = origin;
1773-
symbolToSortTextMap[symbolId] = importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
1793+
symbolToSortTextIdMap[symbolId] = importCompletionNode ? SortTextId.LocationPriority : SortTextId.AutoImportSuggestions;
17741794
symbols.push(symbol);
17751795
}
17761796

@@ -2085,7 +2105,7 @@ namespace ts.Completions {
20852105
localsContainer.locals?.forEach((symbol, name) => {
20862106
symbols.push(symbol);
20872107
if (localsContainer.symbol?.exports?.has(name)) {
2088-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.OptionalMember;
2108+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.OptionalMember;
20892109
}
20902110
});
20912111
return GlobalsSearch.Success;
@@ -2533,7 +2553,8 @@ namespace ts.Completions {
25332553
function setSortTextToOptionalMember() {
25342554
symbols.forEach(m => {
25352555
if (m.flags & SymbolFlags.Optional) {
2536-
symbolToSortTextMap[getSymbolId(m)] = symbolToSortTextMap[getSymbolId(m)] || SortText.OptionalMember;
2556+
const symbolId = getSymbolId(m);
2557+
symbolToSortTextIdMap[symbolId] = symbolToSortTextIdMap[symbolId] ?? SortTextId.OptionalMember;
25372558
}
25382559
});
25392560
}
@@ -2545,7 +2566,7 @@ namespace ts.Completions {
25452566
}
25462567
for (const contextualMemberSymbol of contextualMemberSymbols) {
25472568
if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
2548-
symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
2569+
symbolToSortTextIdMap[getSymbolId(contextualMemberSymbol)] = SortTextId.MemberDeclaredBySpreadAssignment;
25492570
}
25502571
}
25512572
}
@@ -3091,4 +3112,9 @@ namespace ts.Completions {
30913112
addToSeen(seenModules, getSymbolId(sym)) &&
30923113
checker.getExportsOfModule(sym).some(e => symbolCanBeReferencedAtTypeLocation(e, checker, seenModules));
30933114
}
3115+
3116+
function isDeprecated(symbol: Symbol, checker: TypeChecker) {
3117+
const declarations = skipAlias(symbol, checker).declarations;
3118+
return !!length(declarations) && every(declarations, isDeprecatedDeclaration);
3119+
}
30943120
}

‎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/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)