Exclude keywords from import completions (#28114)

* Exclude keywords from import completions

* Still allow contextual keywords

* Add excludes tests

Author: Andy

src/compiler/utilities.ts

@@ -2586,6 +2586,10 @@ namespace ts {
         return token !== undefined && isNonContextualKeyword(token);
     }
 
+    export function isIdentifierANonContextualKeyword({ originalKeywordKind }: Identifier): boolean {
+        return !!originalKeywordKind && !isContextualKeyword(originalKeywordKind);
+    }
+
     export type TriviaKind = SyntaxKind.SingleLineCommentTrivia
         | SyntaxKind.MultiLineCommentTrivia
         | SyntaxKind.NewLineTrivia

src/services/completions.ts

@@ -1167,14 +1167,16 @@ namespace ts.Completions {
                     // The actual import fix might end up coming from a re-export -- we don't compute that until getting completion details.
                     // This is just to avoid adding duplicate completion entries.
                     //
-                    // If `symbol.parent !== ...`, this comes from an `export * from "foo"` re-export. Those don't create new symbols.
-                    // If `some(...)`, this comes from an `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check).
+                    // If `symbol.parent !== ...`, this is an `export * from "foo"` re-export. Those don't create new symbols.
                     if (typeChecker.getMergedSymbol(symbol.parent!) !== resolvedModuleSymbol
-                        || some(symbol.declarations, d => isExportSpecifier(d) && !d.propertyName && !!d.parent.parent.moduleSpecifier)) {
+                        || some(symbol.declarations, d =>
+                            // If `!!d.name.originalKeywordKind`, this is `export { _break as break };` -- skip this and prefer the keyword completion.
+                            // If `!!d.parent.parent.moduleSpecifier`, this is `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check).
+                            isExportSpecifier(d) && (d.propertyName ? isIdentifierANonContextualKeyword(d.name) : !!d.parent.parent.moduleSpecifier))) {
                         continue;
                     }
 
-                    const isDefaultExport = symbol.name === InternalSymbolName.Default;
+                    const isDefaultExport = symbol.escapedName === InternalSymbolName.Default;
                     if (isDefaultExport) {
                         symbol = getLocalSymbolForExportDefault(symbol) || symbol;
                     }

tests/cases/fourslash/completionsImport_keywords.ts

@@ -0,0 +1,42 @@
+/// <reference path="fourslash.ts" />
+
+// @Filename: /a.ts
+////const _break = 0;
+////export { _break as break };
+////const _implements = 0;
+////export { _implements as implements };
+////const _unique = 0;
+////export { _unique as unique };
+
+// Note: `export const unique = 0;` is legal,
+// but we want to test that we don't block an import completion of 'unique' just because it appears in an ExportSpecifier.
+
+// @Filename: /b.ts
+////br/*break*/
+////im/*implements*/
+////un/*unique*/
+
+const preferences: FourSlashInterface.UserPreferences = { includeCompletionsForModuleExports: true };
+verify.completions(
+    // no reserved words
+    {
+        marker: "break",
+        includes: { name: "break", text: "break", kind: "keyword" },
+        excludes: { name: "break", source: "/a" },
+        preferences,
+    },
+    // no strict mode reserved words
+    {
+        marker: "implements",
+        includes: { name: "implements", text: "implements", kind: "keyword" },
+        excludes: { name: "implements", source: "/a" },
+        preferences,
+    },
+    // yes contextual keywords
+    {
+        marker: "unique",
+        includes: { name: "unique", source: "/a", sourceDisplay: "./a", text: "(alias) const unique: 0\nexport unique", hasAction: true },
+        excludes: { name: "unique", source: undefined },
+        preferences,
+    },
+);