Skip to content

Commit 3f9e724

Browse files
a-tarasyukDanielRosenwasser
a-tarasyuk
and
DanielRosenwasser
authored
fix(43796): Sort @deprecated completions lower than others (microsoft#43880)
* fix(43796): sort deprecated completions lower than others * Update src/services/completions.ts Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
1 parent db01e84 commit 3f9e724

14 files changed

+255
-46
lines changed

‎src/services/completions.ts

+55-28
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,28 @@ 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+
// for JavaScript identifiers since they are preferred over deprecated symbols
34+
const DeprecatedSortTextStart = SortTextId.AutoImportSuggestions + 2;
1435

1536
/**
1637
* Special values for `CompletionInfo['source']` used to disambiguate
@@ -105,8 +126,8 @@ namespace ts.Completions {
105126
*/
106127
type SymbolOriginInfoMap = Record<number, SymbolOriginInfo>;
107128

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

111132
const enum KeywordCompletionFilters {
112133
None, // No keywords
@@ -221,7 +242,7 @@ namespace ts.Completions {
221242
isJsxIdentifierExpected,
222243
importCompletionNode,
223244
insideJsDocTagTypeExpression,
224-
symbolToSortTextMap,
245+
symbolToSortTextIdMap,
225246
} = completionData;
226247

227248
// Verify if the file is JSX language variant
@@ -254,7 +275,7 @@ namespace ts.Completions {
254275
importCompletionNode,
255276
recommendedCompletion,
256277
symbolToOriginInfoMap,
257-
symbolToSortTextMap
278+
symbolToSortTextIdMap
258279
);
259280
getJSCompletionEntries(sourceFile, location.pos, uniqueNames, compilerOptions.target!, entries); // TODO: GH#18217
260281
}
@@ -282,7 +303,7 @@ namespace ts.Completions {
282303
importCompletionNode,
283304
recommendedCompletion,
284305
symbolToOriginInfoMap,
285-
symbolToSortTextMap
306+
symbolToSortTextIdMap
286307
);
287308
}
288309

@@ -582,7 +603,7 @@ namespace ts.Completions {
582603
importCompletionNode?: Node,
583604
recommendedCompletion?: Symbol,
584605
symbolToOriginInfoMap?: SymbolOriginInfoMap,
585-
symbolToSortTextMap?: SymbolSortTextMap,
606+
symbolToSortTextIdMap?: SymbolSortTextIdMap,
586607
): UniqueNameSet {
587608
const start = timestamp();
588609
const variableDeclaration = getVariableDeclaration(location);
@@ -596,14 +617,16 @@ namespace ts.Completions {
596617
const symbol = symbols[i];
597618
const origin = symbolToOriginInfoMap?.[i];
598619
const info = getCompletionEntryDisplayNameForSymbol(symbol, target, origin, kind, !!jsxIdentifierExpected);
599-
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextMap && !shouldIncludeSymbol(symbol, symbolToSortTextMap)) {
620+
if (!info || uniques.get(info.name) || kind === CompletionKind.Global && symbolToSortTextIdMap && !shouldIncludeSymbol(symbol, symbolToSortTextIdMap)) {
600621
continue;
601622
}
602623

603624
const { name, needsConvertPropertyAccess } = info;
625+
const sortTextId = symbolToSortTextIdMap?.[getSymbolId(symbol)] ?? SortTextId.LocationPriority;
626+
const sortText = (isDeprecated(symbol, typeChecker) ? DeprecatedSortTextStart + sortTextId : sortTextId).toString() as SortText;
604627
const entry = createCompletionEntry(
605628
symbol,
606-
symbolToSortTextMap && symbolToSortTextMap[getSymbolId(symbol)] || SortText.LocationPriority,
629+
sortText,
607630
contextToken,
608631
location,
609632
sourceFile,
@@ -639,7 +662,7 @@ namespace ts.Completions {
639662
add: name => uniques.set(name, true),
640663
};
641664

642-
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextMap: SymbolSortTextMap): boolean {
665+
function shouldIncludeSymbol(symbol: Symbol, symbolToSortTextIdMap: SymbolSortTextIdMap): boolean {
643666
if (!isSourceFile(location)) {
644667
// export = /**/ here we want to get all meanings, so any symbol is ok
645668
if (isExportAssignment(location.parent)) {
@@ -661,9 +684,9 @@ namespace ts.Completions {
661684
// Auto Imports are not available for scripts so this conditional is always false
662685
if (!!sourceFile.externalModuleIndicator
663686
&& !compilerOptions.allowUmdGlobalAccess
664-
&& symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords
665-
&& (symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions
666-
|| symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.LocationPriority)) {
687+
&& symbolToSortTextIdMap[getSymbolId(symbol)] === SortTextId.GlobalsOrKeywords
688+
&& (symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.AutoImportSuggestions
689+
|| symbolToSortTextIdMap[getSymbolId(symbolOrigin)] === SortTextId.LocationPriority)) {
667690
return false;
668691
}
669692
// Continue with origin symbol
@@ -685,8 +708,6 @@ namespace ts.Completions {
685708
}
686709
}
687710

688-
689-
690711
function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined {
691712
const entries = getLabelStatementCompletions(node);
692713
if (entries.length) {
@@ -930,7 +951,7 @@ namespace ts.Completions {
930951
readonly previousToken: Node | undefined;
931952
readonly isJsxInitializer: IsJsxInitializer;
932953
readonly insideJsDocTagTypeExpression: boolean;
933-
readonly symbolToSortTextMap: SymbolSortTextMap;
954+
readonly symbolToSortTextIdMap: SymbolSortTextIdMap;
934955
readonly isTypeOnlyLocation: boolean;
935956
/** In JSX tag name and attribute names, identifiers like "my-tag" or "aria-name" is valid identifier. */
936957
readonly isJsxIdentifierExpected: boolean;
@@ -1264,7 +1285,7 @@ namespace ts.Completions {
12641285
// This also gets mutated in nested-functions after the return
12651286
let symbols: Symbol[] = [];
12661287
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
1267-
const symbolToSortTextMap: SymbolSortTextMap = [];
1288+
const symbolToSortTextIdMap: SymbolSortTextIdMap = [];
12681289
const seenPropertySymbols = new Map<SymbolId, true>();
12691290
const isTypeOnly = isTypeOnlyCompletion();
12701291
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
@@ -1320,7 +1341,7 @@ namespace ts.Completions {
13201341
previousToken,
13211342
isJsxInitializer,
13221343
insideJsDocTagTypeExpression,
1323-
symbolToSortTextMap,
1344+
symbolToSortTextIdMap,
13241345
isTypeOnlyLocation: isTypeOnly,
13251346
isJsxIdentifierExpected,
13261347
importCompletionNode,
@@ -1509,7 +1530,7 @@ namespace ts.Completions {
15091530

15101531
function addSymbolSortInfo(symbol: Symbol) {
15111532
if (isStaticProperty(symbol)) {
1512-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority;
1533+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.LocalDeclarationPriority;
15131534
}
15141535
}
15151536

@@ -1626,7 +1647,7 @@ namespace ts.Completions {
16261647
for (const symbol of symbols) {
16271648
if (!typeChecker.isArgumentsSymbol(symbol) &&
16281649
!some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
1629-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.GlobalsOrKeywords;
1650+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.GlobalsOrKeywords;
16301651
}
16311652
}
16321653

@@ -1637,7 +1658,7 @@ namespace ts.Completions {
16371658
for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) {
16381659
symbolToOriginInfoMap[symbols.length] = { kind: SymbolOriginInfoKind.ThisType };
16391660
symbols.push(symbol);
1640-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.SuggestedClassMembers;
1661+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.SuggestedClassMembers;
16411662
}
16421663
}
16431664
}
@@ -1719,7 +1740,7 @@ namespace ts.Completions {
17191740
return false;
17201741
}
17211742

1722-
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextMap` */
1743+
/** Mutates `symbols`, `symbolToOriginInfoMap`, and `symbolToSortTextIdMap` */
17231744
function collectAutoImports(resolveModuleSpecifiers: boolean) {
17241745
if (!shouldOfferImportCompletions()) return;
17251746
Debug.assert(!detailsEntryId?.data);
@@ -1790,12 +1811,12 @@ namespace ts.Completions {
17901811

17911812
function pushAutoImportSymbol(symbol: Symbol, origin: SymbolOriginInfoResolvedExport | SymbolOriginInfoExport) {
17921813
const symbolId = getSymbolId(symbol);
1793-
if (symbolToSortTextMap[symbolId] === SortText.GlobalsOrKeywords) {
1814+
if (symbolToSortTextIdMap[symbolId] === SortTextId.GlobalsOrKeywords) {
17941815
// If an auto-importable symbol is available as a global, don't add the auto import
17951816
return;
17961817
}
17971818
symbolToOriginInfoMap[symbols.length] = origin;
1798-
symbolToSortTextMap[symbolId] = importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
1819+
symbolToSortTextIdMap[symbolId] = importCompletionNode ? SortTextId.LocationPriority : SortTextId.AutoImportSuggestions;
17991820
symbols.push(symbol);
18001821
}
18011822

@@ -2110,7 +2131,7 @@ namespace ts.Completions {
21102131
localsContainer.locals?.forEach((symbol, name) => {
21112132
symbols.push(symbol);
21122133
if (localsContainer.symbol?.exports?.has(name)) {
2113-
symbolToSortTextMap[getSymbolId(symbol)] = SortText.OptionalMember;
2134+
symbolToSortTextIdMap[getSymbolId(symbol)] = SortTextId.OptionalMember;
21142135
}
21152136
});
21162137
return GlobalsSearch.Success;
@@ -2558,7 +2579,8 @@ namespace ts.Completions {
25582579
function setSortTextToOptionalMember() {
25592580
symbols.forEach(m => {
25602581
if (m.flags & SymbolFlags.Optional) {
2561-
symbolToSortTextMap[getSymbolId(m)] = symbolToSortTextMap[getSymbolId(m)] || SortText.OptionalMember;
2582+
const symbolId = getSymbolId(m);
2583+
symbolToSortTextIdMap[symbolId] = symbolToSortTextIdMap[symbolId] ?? SortTextId.OptionalMember;
25622584
}
25632585
});
25642586
}
@@ -2570,7 +2592,7 @@ namespace ts.Completions {
25702592
}
25712593
for (const contextualMemberSymbol of contextualMemberSymbols) {
25722594
if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
2573-
symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
2595+
symbolToSortTextIdMap[getSymbolId(contextualMemberSymbol)] = SortTextId.MemberDeclaredBySpreadAssignment;
25742596
}
25752597
}
25762598
}
@@ -3120,4 +3142,9 @@ namespace ts.Completions {
31203142
addToSeen(seenModules, getSymbolId(sym)) &&
31213143
checker.getExportsOfModule(sym).some(e => symbolCanBeReferencedAtTypeLocation(e, checker, seenModules));
31223144
}
3145+
3146+
function isDeprecated(symbol: Symbol, checker: TypeChecker) {
3147+
const declarations = skipAlias(symbol, checker).declarations;
3148+
return !!length(declarations) && every(declarations, isDeprecatedDeclaration);
3149+
}
31233150
}

‎src/services/symbolDisplay.ts

-4
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

+4
Original file line numberDiff line numberDiff line change
@@ -3354,5 +3354,9 @@ namespace ts {
33543354
|| (!!globalCachePath && startsWith(getCanonicalFileName(globalCachePath), toNodeModulesParent));
33553355
}
33563356

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

‎tests/cases/fourslash/completionsWithDeprecatedTag1.ts

+6-7
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

+24
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

+6-3
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

+6-3
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

+23
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)