Skip to content

Commit

Permalink
refactor: let exportMapKey accepts bad symbol name (microsoft#54678)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jack-Works authored and snovader committed Sep 23, 2023
1 parent f8fa87b commit 3e0df44
Show file tree
Hide file tree
Showing 30 changed files with 4,713 additions and 4,701 deletions.
2 changes: 1 addition & 1 deletion src/harness/fourslashImpl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2427,7 +2427,7 @@ export class TestState {
baselineFile,
annotations + "\n\n" + stringify(result, (key, value) => {
return key === "exportMapKey"
? value.replace(/\|[0-9]+/g, "|*")
? value.replace(/ \d+ /g, " * ")
: value;
}),
);
Expand Down
3 changes: 2 additions & 1 deletion src/services/codefixes/importFixes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
emptyArray,
every,
ExportKind,
ExportMapInfoKey,
factory,
first,
firstDefined,
Expand Down Expand Up @@ -514,7 +515,7 @@ interface FixAddToExistingImportInfo {
export function getImportCompletionAction(
targetSymbol: Symbol,
moduleSymbol: Symbol,
exportMapKey: string | undefined,
exportMapKey: ExportMapInfoKey | undefined,
sourceFile: SourceFile,
symbolName: string,
isJsxTagName: boolean,
Expand Down
5 changes: 3 additions & 2 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ import {
escapeSnippetText,
every,
ExportKind,
ExportMapInfoKey,
Expression,
ExpressionWithTypeArguments,
factory,
Expand Down Expand Up @@ -485,14 +486,14 @@ interface SymbolOriginInfoExport extends SymbolOriginInfo {
moduleSymbol: Symbol;
isDefaultExport: boolean;
exportName: string;
exportMapKey: string;
exportMapKey: ExportMapInfoKey;
}

interface SymbolOriginInfoResolvedExport extends SymbolOriginInfo {
symbolName: string;
moduleSymbol: Symbol;
exportName: string;
exportMapKey?: string;
exportMapKey?: ExportMapInfoKey;
moduleSpecifier: string;
}

Expand Down
24 changes: 15 additions & 9 deletions src/services/exportInfoMap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,8 @@ export interface ExportInfoMap {
isUsableByFile(importingFile: Path): boolean;
clear(): void;
add(importingFile: Path, symbol: Symbol, key: __String, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, checker: TypeChecker): void;
get(importingFile: Path, key: string): readonly SymbolExportInfo[] | undefined;
search<T>(importingFile: Path, preferCapitalized: boolean, matches: (name: string, targetFlags: SymbolFlags) => boolean, action: (info: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean, key: string) => T | undefined): T | undefined;
get(importingFile: Path, key: ExportMapInfoKey): readonly SymbolExportInfo[] | undefined;
search<T>(importingFile: Path, preferCapitalized: boolean, matches: (name: string, targetFlags: SymbolFlags) => boolean, action: (info: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean, key: ExportMapInfoKey) => T | undefined): T | undefined;
releaseSymbols(): void;
isEmpty(): boolean;
/** @returns Whether the change resulted in the cache being cleared */
Expand All @@ -128,10 +128,11 @@ export interface CacheableExportInfoMapHost {
getGlobalTypingsCacheLocation(): string | undefined;
}

export type ExportMapInfoKey = string & { __exportInfoKey: void; };
/** @internal */
export function createCacheableExportInfoMap(host: CacheableExportInfoMapHost): ExportInfoMap {
let exportInfoId = 1;
const exportInfo = createMultiMap<string, CachedSymbolExportInfo>();
const exportInfo = createMultiMap<ExportMapInfoKey, CachedSymbolExportInfo>();
const symbols = new Map<number, [symbol: Symbol, moduleSymbol: Symbol]>();
/**
* Key: node_modules package name (no @types).
Expand Down Expand Up @@ -267,7 +268,7 @@ export function createCacheableExportInfoMap(host: CacheableExportInfoMapHost):
},
};
if (Debug.isDebugging) {
Object.defineProperty(cache, "__cache", { get: () => exportInfo });
Object.defineProperty(cache, "__cache", { value: exportInfo });
}
return cache;

Expand Down Expand Up @@ -310,14 +311,19 @@ export function createCacheableExportInfoMap(host: CacheableExportInfoMapHost):
};
}

function key(importedName: string, symbol: Symbol, ambientModuleName: string | undefined, checker: TypeChecker): string {
function key(importedName: string, symbol: Symbol, ambientModuleName: string | undefined, checker: TypeChecker) {
const moduleKey = ambientModuleName || "";
return `${importedName}|${getSymbolId(skipAlias(symbol, checker))}|${moduleKey}`;
return `${importedName.length} ${getSymbolId(skipAlias(symbol, checker))} ${importedName} ${moduleKey}` as ExportMapInfoKey;
}

function parseKey(key: string) {
const symbolName = key.substring(0, key.indexOf("|"));
const moduleKey = key.substring(key.lastIndexOf("|") + 1);
function parseKey(key: ExportMapInfoKey) {
const firstSpace = key.indexOf(" ");
const secondSpace = key.indexOf(" ", firstSpace + 1);
const symbolNameLength = parseInt(key.substring(0, firstSpace), 10);

const data = key.substring(secondSpace + 1);
const symbolName = data.substring(0, symbolNameLength);
const moduleKey = data.substring(symbolNameLength + 1);
const ambientModuleName = moduleKey === "" ? undefined : moduleKey;
return { symbolName, ambientModuleName };
}
Expand Down
5 changes: 3 additions & 2 deletions src/services/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
DocumentPositionMapper,
EmitOutput,
ExportInfoMap,
ExportMapInfoKey,
FileReference,
GetEffectiveTypeRootsHost,
HasChangedAutomaticTypeDirectiveNames,
Expand Down Expand Up @@ -1390,7 +1391,7 @@ export interface CompletionEntryDataAutoImport {
* in the case of InternalSymbolName.ExportEquals and InternalSymbolName.Default.
*/
exportName: string;
exportMapKey?: string;
exportMapKey?: ExportMapInfoKey;
moduleSpecifier?: string;
/** The file name declaring the export's module symbol, if it was an external module */
fileName?: string;
Expand All @@ -1401,7 +1402,7 @@ export interface CompletionEntryDataAutoImport {
}

export interface CompletionEntryDataUnresolved extends CompletionEntryDataAutoImport {
exportMapKey: string;
exportMapKey: ExportMapInfoKey;
}

export interface CompletionEntryDataResolved extends CompletionEntryDataAutoImport {
Expand Down
2 changes: 1 addition & 1 deletion src/testRunner/unittests/helpers/tsserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ function sanitizeLog(s: string): string {
s = s.replace(/collectAutoImports: \d+(?:\.\d+)?/g, `collectAutoImports: *`);
s = s.replace(/continuePreviousIncompleteResponse: \d+(?:\.\d+)?/g, `continuePreviousIncompleteResponse: *`);
s = s.replace(/dependencies in \d+(?:\.\d+)?/g, `dependencies in *`);
s = s.replace(/"exportMapKey":\s*"[_$a-zA-Z][_$_$a-zA-Z0-9]*\|\d+\|/g, match => match.replace(/\|\d+\|/, `|*|`));
s = s.replace(/"exportMapKey":\s*"\d+ \d+ /g, match => match.replace(/ \d+ /, ` * `));
return s;
}

Expand Down
7 changes: 5 additions & 2 deletions tests/baselines/reference/api/typescript.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11040,7 +11040,7 @@ declare namespace ts {
* in the case of InternalSymbolName.ExportEquals and InternalSymbolName.Default.
*/
exportName: string;
exportMapKey?: string;
exportMapKey?: ExportMapInfoKey;
moduleSpecifier?: string;
/** The file name declaring the export's module symbol, if it was an external module */
fileName?: string;
Expand All @@ -11050,7 +11050,7 @@ declare namespace ts {
isPackageJsonImport?: true;
}
interface CompletionEntryDataUnresolved extends CompletionEntryDataAutoImport {
exportMapKey: string;
exportMapKey: ExportMapInfoKey;
}
interface CompletionEntryDataResolved extends CompletionEntryDataAutoImport {
moduleSpecifier: string;
Expand Down Expand Up @@ -11365,6 +11365,9 @@ declare namespace ts {
span: TextSpan;
preferences: UserPreferences;
}
type ExportMapInfoKey = string & {
__exportInfoKey: void;
};
/** The classifier is used for syntactic highlighting in editors via the TSServer */
function createClassifier(): Classifier;
interface DocumentHighlights {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
"isImportStatementCompletion": true,
"data": {
"exportName": "foo",
"exportMapKey": "foo|*|",
"exportMapKey": "3 * foo ",
"moduleSpecifier": "./$foo",
"fileName": "/tests/cases/fourslash/$foo.ts"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -558,7 +558,7 @@ Info seq [hh:mm:ss:mss] response:
"source": "/node_modules/@types/node/index",
"data": {
"exportName": "Stats",
"exportMapKey": "Stats|*|",
"exportMapKey": "5 * Stats ",
"fileName": "/node_modules/@types/node/index.d.ts"
}
},
Expand All @@ -572,7 +572,7 @@ Info seq [hh:mm:ss:mss] response:
"isPackageJsonImport": true,
"data": {
"exportName": "Volume",
"exportMapKey": "Volume|*|",
"exportMapKey": "6 * Volume ",
"fileName": "/node_modules/memfs/lib/index.d.ts",
"isPackageJsonImport": true
}
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/tsserver/completions/works.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ Info seq [hh:mm:ss:mss] response:
"source": "/a",
"data": {
"exportName": "foo",
"exportMapKey": "foo|*|",
"exportMapKey": "3 * foo ",
"fileName": "/a.ts"
}
}
Expand All @@ -201,7 +201,7 @@ Info seq [hh:mm:ss:mss] request:
"data": {
"exportName": "foo",
"fileName": "/a.ts",
"exportMapKey": "foo|*|"
"exportMapKey": "3 * foo "
}
}
]
Expand Down Expand Up @@ -302,7 +302,7 @@ Info seq [hh:mm:ss:mss] request:
"data": {
"exportName": "foo",
"fileName": "/a.ts",
"exportMapKey": "foo|*|"
"exportMapKey": "3 * foo "
}
}
]
Expand Down
Loading

0 comments on commit 3e0df44

Please sign in to comment.