Skip to content

Commit 1cf385d

Browse files
andrewbranchmprobst
andrewbranch
authored and
mprobst
committed
Fix auto import crash on weird JS aliasing (microsoft#46490)
* Fix auto import crash on weird JS aliasing * Comment up the weird test * Fix setting members on union/intersection type, happens later
1 parent f8984ad commit 1cf385d

File tree

5 files changed

+93
-17
lines changed

5 files changed

+93
-17
lines changed

‎src/compiler/checker.ts‎

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3656,8 +3656,8 @@ namespace ts {
36563656
if (exportEquals !== moduleSymbol) {
36573657
const type = getTypeOfSymbol(exportEquals);
36583658
if (shouldTreatPropertiesOfExternalModuleAsExports(type)) {
3659-
getPropertiesOfType(type).forEach(symbol => {
3660-
cb(symbol, symbol.escapedName);
3659+
forEachPropertyOfType(type, (symbol, escapedName) => {
3660+
cb(symbol, escapedName);
36613661
});
36623662
}
36633663
}
@@ -4033,13 +4033,17 @@ namespace ts {
40334033
function getNamedMembers(members: SymbolTable): Symbol[] {
40344034
let result: Symbol[] | undefined;
40354035
members.forEach((symbol, id) => {
4036-
if (!isReservedMemberName(id) && symbolIsValue(symbol)) {
4036+
if (isNamedMember(symbol, id)) {
40374037
(result || (result = [])).push(symbol);
40384038
}
40394039
});
40404040
return result || emptyArray;
40414041
}
40424042

4043+
function isNamedMember(member: Symbol, escapedName: __String) {
4044+
return !isReservedMemberName(escapedName) && symbolIsValue(member);
4045+
}
4046+
40434047
function getNamedOrIndexSignatureMembers(members: SymbolTable): Symbol[] {
40444048
const result = getNamedMembers(members);
40454049
const index = getIndexSymbolFromSymbolTable(members);
@@ -11571,6 +11575,17 @@ namespace ts {
1157111575
getPropertiesOfObjectType(type);
1157211576
}
1157311577

11578+
function forEachPropertyOfType(type: Type, action: (symbol: Symbol, escapedName: __String) => void): void {
11579+
type = getReducedApparentType(type);
11580+
if (type.flags & TypeFlags.StructuredType) {
11581+
resolveStructuredTypeMembers(type as StructuredType).members.forEach((symbol, escapedName) => {
11582+
if (isNamedMember(symbol, escapedName)) {
11583+
action(symbol, escapedName);
11584+
}
11585+
});
11586+
}
11587+
}
11588+
1157411589
function isTypeInvalidDueToUnionDiscriminant(contextualType: Type, obj: ObjectLiteralExpression | JsxAttributes): boolean {
1157511590
const list = obj.properties as NodeArray<ObjectLiteralElementLike | JsxAttributeLike>;
1157611591
return list.some(property => {

‎src/services/codefixes/importFixes.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -268,7 +268,7 @@ namespace ts.codefix {
268268
}
269269

270270
export function getImportCompletionAction(
271-
exportedSymbol: Symbol,
271+
targetSymbol: Symbol,
272272
moduleSymbol: Symbol,
273273
sourceFile: SourceFile,
274274
symbolName: string,
@@ -280,8 +280,8 @@ namespace ts.codefix {
280280
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
281281
const compilerOptions = program.getCompilerOptions();
282282
const exportInfos = pathIsBareSpecifier(stripQuotes(moduleSymbol.name))
283-
? [getSymbolExportInfoForSymbol(exportedSymbol, moduleSymbol, program, host)]
284-
: getAllReExportingModules(sourceFile, exportedSymbol, moduleSymbol, symbolName, host, program, preferences, /*useAutoImportProvider*/ true);
283+
? [getSymbolExportInfoForSymbol(targetSymbol, moduleSymbol, program, host)]
284+
: getAllReExportingModules(sourceFile, targetSymbol, moduleSymbol, symbolName, host, program, preferences, /*useAutoImportProvider*/ true);
285285
const useRequire = shouldUseRequire(sourceFile, program);
286286
const isValidTypeOnlyUseSite = isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
287287
const fix = Debug.checkDefined(getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, symbolName, program, position, isValidTypeOnlyUseSite, useRequire, host, preferences));
@@ -326,7 +326,7 @@ namespace ts.codefix {
326326
}
327327
}
328328

329-
function getAllReExportingModules(importingFile: SourceFile, exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] {
329+
function getAllReExportingModules(importingFile: SourceFile, targetSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] {
330330
const result: SymbolExportInfo[] = [];
331331
const compilerOptions = program.getCompilerOptions();
332332
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
@@ -341,12 +341,12 @@ namespace ts.codefix {
341341
}
342342

343343
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
344-
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions)) === symbolName) && skipAlias(defaultInfo.symbol, checker) === exportedSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
344+
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions)) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
345345
result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson });
346346
}
347347

348348
for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) {
349-
if (exported.name === symbolName && skipAlias(exported, checker) === exportedSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
349+
if (exported.name === symbolName && checker.getMergedSymbol(skipAlias(exported, checker)) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
350350
result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson });
351351
}
352352
}

‎src/services/completions.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1183,9 +1183,9 @@ namespace ts.Completions {
11831183

11841184
const checker = origin.isFromPackageJson ? host.getPackageJsonAutoImportProvider!()!.getTypeChecker() : program.getTypeChecker();
11851185
const { moduleSymbol } = origin;
1186-
const exportedSymbol = checker.getMergedSymbol(skipAlias(symbol.exportSymbol || symbol, checker));
1186+
const targetSymbol = checker.getMergedSymbol(skipAlias(symbol.exportSymbol || symbol, checker));
11871187
const { moduleSpecifier, codeAction } = codefix.getImportCompletionAction(
1188-
exportedSymbol,
1188+
targetSymbol,
11891189
moduleSymbol,
11901190
sourceFile,
11911191
getNameForExportedSymbol(symbol, getEmitScriptTarget(compilerOptions)),

‎src/services/exportInfoMap.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -79,10 +79,14 @@ namespace ts {
7979
}
8080
const isDefault = exportKind === ExportKind.Default;
8181
const namedSymbol = isDefault && getLocalSymbolForExportDefault(symbol) || symbol;
82-
// A re-export merged with an export from a module augmentation can result in `symbol`
83-
// being an external module symbol; the name it is re-exported by will be `symbolTableKey`
84-
// (which comes from the keys of `moduleSymbol.exports`.)
85-
const importedName = isExternalModuleSymbol(namedSymbol)
82+
// 1. A named export must be imported by its key in `moduleSymbol.exports` or `moduleSymbol.members`.
83+
// 2. A re-export merged with an export from a module augmentation can result in `symbol`
84+
// being an external module symbol; the name it is re-exported by will be `symbolTableKey`
85+
// (which comes from the keys of `moduleSymbol.exports`.)
86+
// 3. Otherwise, we have a default/namespace import that can be imported by any name, and
87+
// `symbolTableKey` will be something undesirable like `export=` or `default`, so we try to
88+
// get a better name.
89+
const importedName = exportKind === ExportKind.Named || isExternalModuleSymbol(namedSymbol)
8690
? unescapeLeadingUnderscores(symbolTableKey)
8791
: getNameForExportedSymbol(namedSymbol, scriptTarget);
8892
const moduleName = stripQuotes(moduleSymbol.name);
@@ -321,7 +325,7 @@ namespace ts {
321325
let moduleCount = 0;
322326
forEachExternalModuleToImportFrom(program, host, /*useAutoImportProvider*/ true, (moduleSymbol, moduleFile, program, isFromPackageJson) => {
323327
if (++moduleCount % 100 === 0) cancellationToken?.throwIfCancellationRequested();
324-
const seenExports = new Map<Symbol, true>();
328+
const seenExports = new Map<__String, true>();
325329
const checker = program.getTypeChecker();
326330
const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
327331
// Note: I think we shouldn't actually see resolved module symbols here, but weird merges
@@ -339,7 +343,7 @@ namespace ts {
339343
checker);
340344
}
341345
checker.forEachExportAndPropertyOfModule(moduleSymbol, (exported, key) => {
342-
if (exported !== defaultInfo?.symbol && isImportableSymbol(exported, checker) && addToSeen(seenExports, exported)) {
346+
if (exported !== defaultInfo?.symbol && isImportableSymbol(exported, checker) && addToSeen(seenExports, key)) {
343347
cache.add(
344348
importingFile.path,
345349
exported,

‎tests/cases/fourslash/server/completionsImport_jsModuleExportsAssignment.ts‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
/// <reference path="../fourslash.ts" />
2+
3+
// @Filename: /tsconfig.json
4+
//// { "compilerOptions": { "module": "commonjs", "allowJs": true } }
5+
6+
// @Filename: /third_party/marked/src/defaults.js
7+
//// function getDefaults() {
8+
//// return {
9+
//// baseUrl: null,
10+
//// };
11+
//// }
12+
////
13+
//// function changeDefaults(newDefaults) {
14+
//// module.exports.defaults = newDefaults;
15+
//// }
16+
////
17+
//// module.exports = {
18+
//// defaults: getDefaults(),
19+
//// getDefaults,
20+
//// changeDefaults
21+
//// };
22+
23+
// @Filename: /index.ts
24+
//// /**/
25+
26+
format.setOption("newLineCharacter", "\n")
27+
goTo.marker("");
28+
29+
// Create the exportInfoMap
30+
verify.completions({ marker: "", preferences: { includeCompletionsForModuleExports: true } });
31+
32+
// Create a new program and reuse the exportInfoMap from the last request
33+
edit.insert("d");
34+
verify.completions({
35+
marker: "",
36+
excludes: ["newDefaults"],
37+
includes: [{
38+
name: "defaults",
39+
source: "/third_party/marked/src/defaults",
40+
hasAction: true,
41+
sortText: completion.SortText.AutoImportSuggestions,
42+
}],
43+
preferences: { includeCompletionsForModuleExports: true }
44+
});
45+
46+
verify.applyCodeActionFromCompletion("", {
47+
name: "defaults",
48+
source: "/third_party/marked/src/defaults",
49+
description: `Import 'defaults' from module "./third_party/marked/src/defaults"`,
50+
data: {
51+
exportName: "defaults",
52+
fileName: "/third_party/marked/src/defaults.js",
53+
},
54+
newFileContent: `import { defaults } from "./third_party/marked/src/defaults";
55+
56+
d`
57+
});

0 commit comments

Comments
 (0)