Skip to content

Commit 22eaccb

Browse files
dragomirtitiandragomirtitian
authored
Recover from type reuse errors by falling back to inferred type printing (#58720)
1 parent 59e6620 commit 22eaccb

File tree

5 files changed

+423
-6
lines changed

5 files changed

+423
-6
lines changed

‎src/compiler/checker.ts

Lines changed: 116 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8472,17 +8472,30 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
84728472
cancellationToken.throwIfCancellationRequested();
84738473
}
84748474
let hadError = false;
8475+
const { finalizeBoundary, startRecoveryScope } = createRecoveryBoundary();
84758476
const transformed = visitNode(existing, visitExistingNodeTreeSymbols, isTypeNode);
8476-
if (hadError) {
8477+
if (!finalizeBoundary()) {
84778478
return undefined;
84788479
}
84798480
context.approximateLength += existing.end - existing.pos;
84808481
return transformed;
84818482

84828483
function visitExistingNodeTreeSymbols(node: Node): Node | undefined {
8484+
// If there was an error in a sibling node bail early, the result will be discarded anyway
8485+
if (hadError) return node;
8486+
const recover = startRecoveryScope();
84838487
const onExitNewScope = isNewScopeNode(node) ? onEnterNewScope(node) : undefined;
84848488
const result = visitExistingNodeTreeSymbolsWorker(node);
84858489
onExitNewScope?.();
8490+
8491+
// If there was an error, maybe we can recover by serializing the actual type of the node
8492+
if (hadError) {
8493+
if (isTypeNode(node) && !isTypePredicateNode(node)) {
8494+
recover();
8495+
return serializeExistingTypeNode(context, node);
8496+
}
8497+
return node;
8498+
}
84868499
// We want to clone the subtree, so when we mark it up with __pos and __end in quickfixes,
84878500
// we don't get odd behavior because of reused nodes. We also need to clone to _remove_
84888501
// the position information if the node comes from a different file than the one the node builder
@@ -8492,6 +8505,86 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
84928505
return result === node ? setTextRange(context, factory.cloneNode(result), node) : result;
84938506
}
84948507

8508+
function createRecoveryBoundary() {
8509+
let unreportedErrors: (() => void)[];
8510+
const oldTracker = context.tracker;
8511+
const oldTrackedSymbols = context.trackedSymbols;
8512+
context.trackedSymbols = [];
8513+
const oldEncounteredError = context.encounteredError;
8514+
context.tracker = new SymbolTrackerImpl(context, {
8515+
...oldTracker.inner,
8516+
reportCyclicStructureError() {
8517+
markError(() => oldTracker.reportCyclicStructureError());
8518+
},
8519+
reportInaccessibleThisError() {
8520+
markError(() => oldTracker.reportInaccessibleThisError());
8521+
},
8522+
reportInaccessibleUniqueSymbolError() {
8523+
markError(() => oldTracker.reportInaccessibleUniqueSymbolError());
8524+
},
8525+
reportLikelyUnsafeImportRequiredError(specifier) {
8526+
markError(() => oldTracker.reportLikelyUnsafeImportRequiredError(specifier));
8527+
},
8528+
reportNonSerializableProperty(name) {
8529+
markError(() => oldTracker.reportNonSerializableProperty(name));
8530+
},
8531+
trackSymbol(sym, decl, meaning) {
8532+
const accessibility = isSymbolAccessible(sym, decl, meaning, /*shouldComputeAliasesToMakeVisible*/ false);
8533+
if (accessibility.accessibility !== SymbolAccessibility.Accessible) {
8534+
(context.trackedSymbols ??= []).push([sym, decl, meaning]);
8535+
return true;
8536+
}
8537+
return false;
8538+
},
8539+
moduleResolverHost: context.tracker.moduleResolverHost,
8540+
}, context.tracker.moduleResolverHost);
8541+
8542+
return {
8543+
startRecoveryScope,
8544+
finalizeBoundary,
8545+
};
8546+
8547+
function markError(unreportedError: () => void) {
8548+
hadError = true;
8549+
(unreportedErrors ??= []).push(unreportedError);
8550+
}
8551+
8552+
function startRecoveryScope() {
8553+
const initialTrackedSymbolsTop = context.trackedSymbols?.length ?? 0;
8554+
const unreportedErrorsTop = unreportedErrors?.length ?? 0;
8555+
return () => {
8556+
hadError = false;
8557+
// Reset the tracked symbols to before the error
8558+
if (context.trackedSymbols) {
8559+
context.trackedSymbols.length = initialTrackedSymbolsTop;
8560+
}
8561+
if (unreportedErrors) {
8562+
unreportedErrors.length = unreportedErrorsTop;
8563+
}
8564+
};
8565+
}
8566+
8567+
function finalizeBoundary() {
8568+
context.tracker = oldTracker;
8569+
const newTrackedSymbols = context.trackedSymbols;
8570+
context.trackedSymbols = oldTrackedSymbols;
8571+
context.encounteredError = oldEncounteredError;
8572+
8573+
unreportedErrors?.forEach(fn => fn());
8574+
if (hadError) {
8575+
return false;
8576+
}
8577+
newTrackedSymbols?.forEach(
8578+
([symbol, enclosingDeclaration, meaning]) =>
8579+
context.tracker.trackSymbol(
8580+
symbol,
8581+
enclosingDeclaration,
8582+
meaning,
8583+
),
8584+
);
8585+
return true;
8586+
}
8587+
}
84958588
function onEnterNewScope(node: IntroducesNewScopeNode | ConditionalTypeNode) {
84968589
return enterNewScope(context, node, getParametersInScope(node), getTypeParametersInScope(node));
84978590
}
@@ -8765,13 +8858,30 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
87658858

87668859
function rewriteModuleSpecifier(parent: ImportTypeNode, lit: StringLiteral) {
87678860
if (context.bundled || context.enclosingFile !== getSourceFileOfNode(lit)) {
8768-
const targetFile = getExternalModuleFileFromDeclaration(parent);
8769-
if (targetFile) {
8770-
const newName = getSpecifierForModuleSymbol(targetFile.symbol, context);
8771-
if (newName !== lit.text) {
8772-
return setOriginalNode(factory.createStringLiteral(newName), lit);
8861+
let name = lit.text;
8862+
const nodeSymbol = getNodeLinks(node).resolvedSymbol;
8863+
const meaning = parent.isTypeOf ? SymbolFlags.Value : SymbolFlags.Type;
8864+
const parentSymbol = nodeSymbol
8865+
&& isSymbolAccessible(nodeSymbol, context.enclosingDeclaration, meaning, /*shouldComputeAliasesToMakeVisible*/ false).accessibility === SymbolAccessibility.Accessible
8866+
&& lookupSymbolChain(nodeSymbol, context, meaning, /*yieldModuleSymbol*/ true)[0];
8867+
if (parentSymbol && parentSymbol.flags & SymbolFlags.Module) {
8868+
name = getSpecifierForModuleSymbol(parentSymbol, context);
8869+
}
8870+
else {
8871+
const targetFile = getExternalModuleFileFromDeclaration(parent);
8872+
if (targetFile) {
8873+
name = getSpecifierForModuleSymbol(targetFile.symbol, context);
87738874
}
87748875
}
8876+
if (name.includes("/node_modules/")) {
8877+
context.encounteredError = true;
8878+
if (context.tracker.reportLikelyUnsafeImportRequiredError) {
8879+
context.tracker.reportLikelyUnsafeImportRequiredError(name);
8880+
}
8881+
}
8882+
if (name !== lit.text) {
8883+
return setOriginalNode(factory.createStringLiteral(name), lit);
8884+
}
87758885
}
87768886
return visitNode(lit, visitExistingNodeTreeSymbols, isStringLiteral)!;
87778887
}

‎tests/baselines/reference/declarationEmitUsingTypeAlias2.js

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
//// [tests/cases/compiler/declarationEmitUsingTypeAlias2.ts] ////
2+
3+
//// [inner.d.ts]
4+
export declare type Other = { other: string };
5+
export declare type SomeType = { arg: Other };
6+
7+
//// [other.d.ts]
8+
export declare const shouldLookupName: unique symbol;
9+
export declare const shouldReuseLocalName: unique symbol;
10+
export declare const reuseDepName: unique symbol;
11+
export declare const shouldBeElided: unique symbol;
12+
export declare const isNotAccessibleShouldError: unique symbol;
13+
14+
//// [index.d.ts]
15+
import { Other } from './inner'
16+
import { shouldLookupName, reuseDepName, shouldReuseLocalName, shouldBeElided } from './other'
17+
export declare const goodDeclaration: <T>() => () => {
18+
/** Other Can't be named in index.ts, but the whole conditional type can be resolved */
19+
shouldPrintResult: T extends Other? "O": "N",
20+
/** Symbol shouldBeElided should not be present in index.d.ts, it might be since it's tracked before Other is seen and an error reported */
21+
shouldPrintResult2: T extends typeof shouldBeElided? Other: "N",
22+
/** Specifier should come from module, local path should not be reused */
23+
shouldLookupName: typeof import('./other').shouldLookupName,
24+
/** This is imported in index so local name should be reused */
25+
shouldReuseLocalName: typeof shouldReuseLocalName
26+
/** This is NOT imported in index so import should be created */
27+
reuseDepName: typeof reuseDepName,
28+
}
29+
export { shouldLookupName, shouldReuseLocalName, reuseDepName, shouldBeElided };
30+
31+
//// [package.json]
32+
{
33+
"name": "some-dep",
34+
"exports": {
35+
".": "./dist/index.js"
36+
}
37+
}
38+
39+
//// [index.ts]
40+
import { goodDeclaration, shouldReuseLocalName, shouldBeElided } from "some-dep";
41+
export const bar = goodDeclaration<{}>;
42+
43+
44+
45+
//// [index.js]
46+
"use strict";
47+
Object.defineProperty(exports, "__esModule", { value: true });
48+
exports.bar = void 0;
49+
const some_dep_1 = require("some-dep");
50+
exports.bar = (some_dep_1.goodDeclaration);
51+
52+
53+
//// [index.d.ts]
54+
import { shouldReuseLocalName } from "some-dep";
55+
export declare const bar: () => () => {
56+
shouldPrintResult: "N";
57+
shouldPrintResult2: "N";
58+
shouldLookupName: typeof import("some-dep").shouldLookupName;
59+
shouldReuseLocalName: typeof shouldReuseLocalName;
60+
reuseDepName: typeof import("some-dep").reuseDepName;
61+
};

‎tests/baselines/reference/declarationEmitUsingTypeAlias2.symbols

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
//// [tests/cases/compiler/declarationEmitUsingTypeAlias2.ts] ////
2+
3+
=== node_modules/some-dep/dist/inner.d.ts ===
4+
export declare type Other = { other: string };
5+
>Other : Symbol(Other, Decl(inner.d.ts, 0, 0))
6+
>other : Symbol(other, Decl(inner.d.ts, 0, 29))
7+
8+
export declare type SomeType = { arg: Other };
9+
>SomeType : Symbol(SomeType, Decl(inner.d.ts, 0, 46))
10+
>arg : Symbol(arg, Decl(inner.d.ts, 1, 32))
11+
>Other : Symbol(Other, Decl(inner.d.ts, 0, 0))
12+
13+
=== node_modules/some-dep/dist/other.d.ts ===
14+
export declare const shouldLookupName: unique symbol;
15+
>shouldLookupName : Symbol(shouldLookupName, Decl(other.d.ts, 0, 20))
16+
17+
export declare const shouldReuseLocalName: unique symbol;
18+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(other.d.ts, 1, 20))
19+
20+
export declare const reuseDepName: unique symbol;
21+
>reuseDepName : Symbol(reuseDepName, Decl(other.d.ts, 2, 20))
22+
23+
export declare const shouldBeElided: unique symbol;
24+
>shouldBeElided : Symbol(shouldBeElided, Decl(other.d.ts, 3, 20))
25+
26+
export declare const isNotAccessibleShouldError: unique symbol;
27+
>isNotAccessibleShouldError : Symbol(isNotAccessibleShouldError, Decl(other.d.ts, 4, 20))
28+
29+
=== node_modules/some-dep/dist/index.d.ts ===
30+
import { Other } from './inner'
31+
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))
32+
33+
import { shouldLookupName, reuseDepName, shouldReuseLocalName, shouldBeElided } from './other'
34+
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 1, 8))
35+
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 1, 26))
36+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 1, 40))
37+
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 1, 62))
38+
39+
export declare const goodDeclaration: <T>() => () => {
40+
>goodDeclaration : Symbol(goodDeclaration, Decl(index.d.ts, 2, 20))
41+
>T : Symbol(T, Decl(index.d.ts, 2, 39))
42+
43+
/** Other Can't be named in index.ts, but the whole conditional type can be resolved */
44+
shouldPrintResult: T extends Other? "O": "N",
45+
>shouldPrintResult : Symbol(shouldPrintResult, Decl(index.d.ts, 2, 54))
46+
>T : Symbol(T, Decl(index.d.ts, 2, 39))
47+
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))
48+
49+
/** Symbol shouldBeElided should not be present in index.d.ts, it might be since it's tracked before Other is seen and an error reported */
50+
shouldPrintResult2: T extends typeof shouldBeElided? Other: "N",
51+
>shouldPrintResult2 : Symbol(shouldPrintResult2, Decl(index.d.ts, 4, 47))
52+
>T : Symbol(T, Decl(index.d.ts, 2, 39))
53+
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 1, 62))
54+
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))
55+
56+
/** Specifier should come from module, local path should not be reused */
57+
shouldLookupName: typeof import('./other').shouldLookupName,
58+
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 6, 66))
59+
>shouldLookupName : Symbol(shouldLookupName, Decl(other.d.ts, 0, 20))
60+
61+
/** This is imported in index so local name should be reused */
62+
shouldReuseLocalName: typeof shouldReuseLocalName
63+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 8, 62))
64+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 1, 40))
65+
66+
/** This is NOT imported in index so import should be created */
67+
reuseDepName: typeof reuseDepName,
68+
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 10, 51))
69+
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 1, 26))
70+
}
71+
export { shouldLookupName, shouldReuseLocalName, reuseDepName, shouldBeElided };
72+
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 14, 8))
73+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 14, 26))
74+
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 14, 48))
75+
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 14, 62))
76+
77+
=== src/index.ts ===
78+
import { goodDeclaration, shouldReuseLocalName, shouldBeElided } from "some-dep";
79+
>goodDeclaration : Symbol(goodDeclaration, Decl(index.ts, 0, 8))
80+
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.ts, 0, 25))
81+
>shouldBeElided : Symbol(shouldBeElided, Decl(index.ts, 0, 47))
82+
83+
export const bar = goodDeclaration<{}>;
84+
>bar : Symbol(bar, Decl(index.ts, 1, 12))
85+
>goodDeclaration : Symbol(goodDeclaration, Decl(index.ts, 0, 8))
86+
87+

0 commit comments

Comments
 (0)