fix(43796): sort deprecated completions lower than others

microsoft · May 6, 2021 · fdec0a5 · fdec0a5
1 parent 5e4fcfb
commit fdec0a5
Show file tree

Hide file tree

Showing 15 changed files with 243 additions and 38 deletions.
diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -8,7 +8,14 @@ namespace ts.Completions {
         SuggestedClassMembers = "4",
         GlobalsOrKeywords = "5",
         AutoImportSuggestions = "6",
-        JavascriptIdentifiers = "7"
+        DeprecatedLocalDeclarationPriority = "7",
+        DeprecatedLocationPriority = "8",
+        DeprecatedOptionalMember = "9",
+        DeprecatedMemberDeclaredBySpreadAssignment = "10",
+        DeprecatedSuggestedClassMembers = "11",
+        DeprecatedGlobalsOrKeywords = "12",
+        DeprecatedAutoImportSuggestions = "13",
+        JavascriptIdentifiers = "14"
     }
     export type Log = (message: string) => void;
 
@@ -585,9 +592,11 @@ namespace ts.Completions {
             }
 
             const { name, needsConvertPropertyAccess } = info;
+            const sortText = symbolToSortTextMap && symbolToSortTextMap[getSymbolId(symbol)]
+                || (isDeprecatedSymbol(symbol, typeChecker) ? SortText.DeprecatedLocationPriority : SortText.LocationPriority);
             const entry = createCompletionEntry(
                 symbol,
-                symbolToSortTextMap && symbolToSortTextMap[getSymbolId(symbol)] || SortText.LocationPriority,
+                sortText,
                 contextToken,
                 location,
                 sourceFile,
@@ -641,13 +650,15 @@ namespace ts.Completions {
                 // module imports then the global keywords will be filtered out so auto
                 // import suggestions will win in the completion
                 const symbolOrigin = skipAlias(symbol, typeChecker);
+                const symbolOriginSortText = symbolToSortTextMap[getSymbolId(symbolOrigin)];
+                const symbolSortText = symbolToSortTextMap[getSymbolId(symbol)];
                 // We only want to filter out the global keywords
                 // Auto Imports are not available for scripts so this conditional is always false
                 if (!!sourceFile.externalModuleIndicator
                     && !compilerOptions.allowUmdGlobalAccess
-                    && symbolToSortTextMap[getSymbolId(symbol)] === SortText.GlobalsOrKeywords
-                    && (symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.AutoImportSuggestions
-                        || symbolToSortTextMap[getSymbolId(symbolOrigin)] === SortText.LocationPriority)) {
+                    && (symbolSortText === SortText.GlobalsOrKeywords || symbolSortText === SortText.DeprecatedGlobalsOrKeywords)
+                    && ((symbolOriginSortText === SortText.AutoImportSuggestions || symbolOriginSortText === SortText.DeprecatedAutoImportSuggestions)
+                        || (symbolOriginSortText === SortText.LocationPriority || symbolOriginSortText === SortText.DeprecatedLocationPriority))) {
                     return false;
                 }
                 // Continue with origin symbol
@@ -669,8 +680,6 @@ namespace ts.Completions {
         }
     }
 
-
-
     function getLabelCompletionAtPosition(node: BreakOrContinueStatement): CompletionInfo | undefined {
         const entries = getLabelStatementCompletions(node);
         if (entries.length) {
@@ -1484,7 +1493,8 @@ namespace ts.Completions {
 
             function addSymbolSortInfo(symbol: Symbol) {
                 if (isStaticProperty(symbol)) {
-                    symbolToSortTextMap[getSymbolId(symbol)] = SortText.LocalDeclarationPriority;
+                    symbolToSortTextMap[getSymbolId(symbol)] =
+                        isDeprecatedSymbol(symbol, typeChecker) ? SortText.DeprecatedLocalDeclarationPriority : SortText.LocalDeclarationPriority;
                 }
             }
 
@@ -1599,9 +1609,9 @@ namespace ts.Completions {
             symbols = typeChecker.getSymbolsInScope(scopeNode, symbolMeanings);
             Debug.assertEachIsDefined(symbols, "getSymbolsInScope() should all be defined");
             for (const symbol of symbols) {
-                if (!typeChecker.isArgumentsSymbol(symbol) &&
-                    !some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
-                    symbolToSortTextMap[getSymbolId(symbol)] = SortText.GlobalsOrKeywords;
+                if (!typeChecker.isArgumentsSymbol(symbol) && !some(symbol.declarations, d => d.getSourceFile() === sourceFile)) {
+                    symbolToSortTextMap[getSymbolId(symbol)] =
+                        isDeprecatedSymbol(symbol, typeChecker) ? SortText.DeprecatedGlobalsOrKeywords : SortText.GlobalsOrKeywords;
                 }
             }
 
@@ -1612,7 +1622,8 @@ namespace ts.Completions {
                     for (const symbol of getPropertiesForCompletion(thisType, typeChecker)) {
                         symbolToOriginInfoMap[symbols.length] = { kind: SymbolOriginInfoKind.ThisType };
                         symbols.push(symbol);
-                        symbolToSortTextMap[getSymbolId(symbol)] = SortText.SuggestedClassMembers;
+                        symbolToSortTextMap[getSymbolId(symbol)] =
+                            isDeprecatedSymbol(symbol, typeChecker) ? SortText.DeprecatedSuggestedClassMembers : SortText.SuggestedClassMembers;
                     }
                 }
             }
@@ -1765,12 +1776,16 @@ namespace ts.Completions {
 
         function pushAutoImportSymbol(symbol: Symbol, origin: SymbolOriginInfoResolvedExport | SymbolOriginInfoExport) {
             const symbolId = getSymbolId(symbol);
-            if (symbolToSortTextMap[symbolId] === SortText.GlobalsOrKeywords) {
+            if (symbolToSortTextMap[symbolId] === SortText.GlobalsOrKeywords
+                || symbolToSortTextMap[symbolId] === SortText.DeprecatedGlobalsOrKeywords) {
                 // If an auto-importable symbol is available as a global, don't add the auto import
                 return;
             }
             symbolToOriginInfoMap[symbols.length] = origin;
-            symbolToSortTextMap[symbolId] = importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
+            symbolToSortTextMap[symbolId] =
+                isDeprecatedSymbol(symbol, typeChecker)
+                    ? importCompletionNode ? SortText.DeprecatedLocationPriority : SortText.DeprecatedAutoImportSuggestions
+                    : importCompletionNode ? SortText.LocationPriority : SortText.AutoImportSuggestions;
             symbols.push(symbol);
         }
 
@@ -2085,7 +2100,8 @@ namespace ts.Completions {
             localsContainer.locals?.forEach((symbol, name) => {
                 symbols.push(symbol);
                 if (localsContainer.symbol?.exports?.has(name)) {
-                    symbolToSortTextMap[getSymbolId(symbol)] = SortText.OptionalMember;
+                    symbolToSortTextMap[getSymbolId(symbol)] =
+                        isDeprecatedSymbol(symbol, typeChecker) ? SortText.DeprecatedOptionalMember : SortText.OptionalMember;
                 }
             });
             return GlobalsSearch.Success;
@@ -2500,7 +2516,8 @@ namespace ts.Completions {
         function setSortTextToOptionalMember() {
             symbols.forEach(m => {
                 if (m.flags & SymbolFlags.Optional) {
-                    symbolToSortTextMap[getSymbolId(m)] = symbolToSortTextMap[getSymbolId(m)] || SortText.OptionalMember;
+                    symbolToSortTextMap[getSymbolId(m)] =
+                        symbolToSortTextMap[getSymbolId(m)] || (isDeprecatedSymbol(m, typeChecker) ? SortText.DeprecatedOptionalMember : SortText.OptionalMember);
                 }
             });
         }
@@ -2512,7 +2529,8 @@ namespace ts.Completions {
             }
             for (const contextualMemberSymbol of contextualMemberSymbols) {
                 if (membersDeclaredBySpreadAssignment.has(contextualMemberSymbol.name)) {
-                    symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] = SortText.MemberDeclaredBySpreadAssignment;
+                    symbolToSortTextMap[getSymbolId(contextualMemberSymbol)] =
+                        isDeprecatedSymbol(contextualMemberSymbol, typeChecker) ? SortText.DeprecatedMemberDeclaredBySpreadAssignment : SortText.MemberDeclaredBySpreadAssignment;
                 }
             }
         }
@@ -3051,4 +3069,9 @@ namespace ts.Completions {
             addToSeen(seenModules, getSymbolId(sym)) &&
             checker.getExportsOfModule(sym).some(e => symbolCanBeReferencedAtTypeLocation(e, checker, seenModules));
     }
+
+    function isDeprecatedSymbol(symbol: Symbol, checker: TypeChecker) {
+        const declarations = skipAlias(symbol, checker).declarations;
+        return !!length(declarations) && every(declarations, isDeprecatedDeclaration);
+    }
 }
diff --git a/src/services/symbolDisplay.ts b/src/services/symbolDisplay.ts
@@ -100,10 +100,6 @@ namespace ts.SymbolDisplay {
         return ScriptElementKind.unknown;
     }
 
-    function isDeprecatedDeclaration(decl: Declaration) {
-        return !!(getCombinedNodeFlagsAlwaysIncludeJSDoc(decl) & ModifierFlags.Deprecated);
-    }
-
     function getNormalizedSymbolModifiers(symbol: Symbol) {
         if (symbol.declarations && symbol.declarations.length) {
             const [declaration, ...declarations] = symbol.declarations;

diff --git a/src/services/utilities.ts b/src/services/utilities.ts
@@ -3352,5 +3352,9 @@ namespace ts {
             || (!!globalCachePath && startsWith(getCanonicalFileName(globalCachePath), toNodeModulesParent));
     }
 
+    export function isDeprecatedDeclaration(decl: Declaration) {
+        return !!(getCombinedNodeFlagsAlwaysIncludeJSDoc(decl) & ModifierFlags.Deprecated);
+    }
+
     // #endregion
 }
diff --git a/tests/baselines/reference/completionsSalsaMethodsOnAssignedFunctionExpressions.baseline b/tests/baselines/reference/completionsSalsaMethodsOnAssignedFunctionExpressions.baseline
@@ -302,21 +302,21 @@
           "name": "a",
           "kind": "warning",
           "kindModifiers": "",
-          "sortText": "7",
+          "sortText": "14",
           "isFromUncheckedFile": true
         },
         {
           "name": "prototype",
           "kind": "warning",
           "kindModifiers": "",
-          "sortText": "7",
+          "sortText": "14",
           "isFromUncheckedFile": true
         },
         {
           "name": "m",
           "kind": "property",
           "kindModifiers": "",
-          "sortText": "7",
+          "sortText": "14",
           "isFromUncheckedFile": true,
           "displayParts": [
             {

diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag1.ts b/tests/cases/fourslash/completionsWithDeprecatedTag1.ts
@@ -25,27 +25,26 @@
 verify.completions({
     marker: "1",
     includes: [
-      { name: "Foo", kind: "interface", kindModifiers: "deprecated" }
+      { name: "Foo", kind: "interface", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
     ]
 }, {
     marker: "2",
     includes: [
-      { name: "bar", kind: "method", kindModifiers: "deprecated" }
+      { name: "bar", kind: "method", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
     ]
 }, {
     marker: "3",
     includes: [
-      { name: "prop", kind: "property", kindModifiers: "deprecated" }
+      { name: "prop", kind: "property", kindModifiers: "deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
     ]
 }, {
     marker: "4",
     includes: [
-      { name: "foobar", kind: "function", kindModifiers: "export,deprecated" }
+      { name: "foobar", kind: "function", kindModifiers: "export,deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
     ]
 }, {
     marker: "5",
     includes: [
-      { name: "foobar", kind: "alias", kindModifiers: "export,deprecated" }
+      { name: "foobar", kind: "alias", kindModifiers: "export,deprecated", sortText: completion.SortText.DeprecatedLocationPriority }
     ]
-}
-);
+});
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag10.ts b/tests/cases/fourslash/completionsWithDeprecatedTag10.ts
@@ -0,0 +1,24 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: /foo.ts
+/////** @deprecated foo */
+////export const foo = 0;
+
+// @Filename: /index.ts
+/////**/
+
+verify.completions({
+    marker: "",
+    includes: [{
+        name: "foo",
+        source: "/foo",
+        sourceDisplay: "./foo",
+        hasAction: true,
+        kind: "const",
+        kindModifiers: "export,deprecated",
+        sortText: completion.SortText.DeprecatedAutoImportSuggestions
+    }],
+    preferences: {
+        includeCompletionsForModuleExports: true,
+    },
+});
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag2.ts b/tests/cases/fourslash/completionsWithDeprecatedTag2.ts
@@ -9,7 +9,10 @@
 
 verify.completions({
     marker: "",
-    includes: [
-        { name: "foo", kind: "function", kindModifiers: "deprecated,declare" }
-    ]
+    includes: [{
+        name: "foo",
+        kind: "function",
+        kindModifiers: "deprecated,declare",
+        sortText: completion.SortText.DeprecatedLocationPriority
+    }]
 });
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag3.ts b/tests/cases/fourslash/completionsWithDeprecatedTag3.ts
@@ -9,7 +9,10 @@
 
 verify.completions({
     marker: "",
-    includes: [
-      { name: "foo", kind: "function", kindModifiers: "declare" }
-    ]
+    includes: [{
+        name: "foo",
+        kind: "function",
+        kindModifiers: "declare",
+        sortText: completion.SortText.LocationPriority
+    }]
 });
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag4.ts b/tests/cases/fourslash/completionsWithDeprecatedTag4.ts
@@ -0,0 +1,23 @@
+/// <reference path="fourslash.ts" />
+
+// @noLib: true
+
+////f({
+////    a/**/
+////    xyz: ``,
+////});
+////declare function f(options: {
+////    /** @deprecated abc */
+////    abc?: number,
+////    xyz?: string
+////}): void;
+
+verify.completions({
+    marker: "",
+    exact: [{
+        name: "abc",
+        kind: "property",
+        kindModifiers: "deprecated,declare,optional",
+        sortText: completion.SortText.DeprecatedOptionalMember
+    }],
+});
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag5.ts b/tests/cases/fourslash/completionsWithDeprecatedTag5.ts
@@ -0,0 +1,24 @@
+/// <reference path="fourslash.ts"/>
+
+////class Foo {
+////    /** @deprecated m */
+////    static m() {}
+////}
+////Foo./**/
+
+verify.completions({
+    marker: "",
+    exact: [
+        {
+            name: "prototype",
+            sortText: completion.SortText.LocationPriority
+        },
+        {
+            name: "m",
+            kind: "method",
+            kindModifiers: "static,deprecated",
+            sortText: completion.SortText.DeprecatedLocalDeclarationPriority
+        },
+        ...completion.functionMembers
+    ]
+});
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag6.ts b/tests/cases/fourslash/completionsWithDeprecatedTag6.ts
@@ -0,0 +1,17 @@
+/// <reference path="fourslash.ts"/>
+
+////module Foo {
+////    /** @deprecated foo */
+////    export var foo: number;
+////}
+////Foo./**/
+
+verify.completions({
+    marker: "",
+    exact: [{
+        name: "foo",
+        kind: "var",
+        kindModifiers: "export,deprecated",
+        sortText: completion.SortText.DeprecatedLocationPriority
+    }]
+});
diff --git a/tests/cases/fourslash/completionsWithDeprecatedTag7.ts b/tests/cases/fourslash/completionsWithDeprecatedTag7.ts
@@ -0,0 +1,27 @@
+/// <reference path="fourslash.ts" />
+
+// @strict: true
+
+////interface I {
+////    /** @deprecated a */
+////    a: number;
+////}
+
+////const foo = {
+////    a: 1
+////}
+
+////const i: I = {
+////    ...foo,
+////    /**/
+////}
+
+verify.completions({
+    marker: "",
+    exact: [{
+        name: "a",
+        sortText: completion.SortText.DeprecatedMemberDeclaredBySpreadAssignment,
+        kind: 'property',
+        kindModifiers: "deprecated"
+    }]
+});