Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Recover from type reuse errors by falling back to inferred type printing #58720

Merged
merged 1 commit into from
May 30, 2024
Merged

Recover from type reuse errors by falling back to inferred type printing #58720

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 116 additions & 6 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8472,17 +8472,30 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
cancellationToken.throwIfCancellationRequested();
}
let hadError = false;
const { finalizeBoundary, startRecoveryScope } = createRecoveryBoundary();
const transformed = visitNode(existing, visitExistingNodeTreeSymbols, isTypeNode);
if (hadError) {
if (!finalizeBoundary()) {
return undefined;
}
context.approximateLength += existing.end - existing.pos;
return transformed;

function visitExistingNodeTreeSymbols(node: Node): Node | undefined {
// If there was an error in a sibling node bail early, the result will be discarded anyway
if (hadError) return node;
const recover = startRecoveryScope();
const onExitNewScope = isNewScopeNode(node) ? onEnterNewScope(node) : undefined;
const result = visitExistingNodeTreeSymbolsWorker(node);
onExitNewScope?.();

// If there was an error, maybe we can recover by serializing the actual type of the node
if (hadError) {
if (isTypeNode(node) && !isTypePredicateNode(node)) {
recover();
return serializeExistingTypeNode(context, node);
}
return node;
}
// We want to clone the subtree, so when we mark it up with __pos and __end in quickfixes,
// we don't get odd behavior because of reused nodes. We also need to clone to _remove_
// the position information if the node comes from a different file than the one the node builder
Expand All @@ -8492,6 +8505,86 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
return result === node ? setTextRange(context, factory.cloneNode(result), node) : result;
}

function createRecoveryBoundary() {
let unreportedErrors: (() => void)[];
const oldTracker = context.tracker;
const oldTrackedSymbols = context.trackedSymbols;
context.trackedSymbols = [];
const oldEncounteredError = context.encounteredError;
context.tracker = new SymbolTrackerImpl(context, {
...oldTracker.inner,
reportCyclicStructureError() {
markError(() => oldTracker.reportCyclicStructureError());
},
reportInaccessibleThisError() {
markError(() => oldTracker.reportInaccessibleThisError());
},
reportInaccessibleUniqueSymbolError() {
markError(() => oldTracker.reportInaccessibleUniqueSymbolError());
},
reportLikelyUnsafeImportRequiredError(specifier) {
markError(() => oldTracker.reportLikelyUnsafeImportRequiredError(specifier));
},
reportNonSerializableProperty(name) {
markError(() => oldTracker.reportNonSerializableProperty(name));
},
trackSymbol(sym, decl, meaning) {
const accessibility = isSymbolAccessible(sym, decl, meaning, /*shouldComputeAliasesToMakeVisible*/ false);
if (accessibility.accessibility !== SymbolAccessibility.Accessible) {
(context.trackedSymbols ??= []).push([sym, decl, meaning]);
return true;
}
return false;
},
moduleResolverHost: context.tracker.moduleResolverHost,
}, context.tracker.moduleResolverHost);

return {
startRecoveryScope,
finalizeBoundary,
};

function markError(unreportedError: () => void) {
hadError = true;
(unreportedErrors ??= []).push(unreportedError);
}

function startRecoveryScope() {
const initialTrackedSymbolsTop = context.trackedSymbols?.length ?? 0;
const unreportedErrorsTop = unreportedErrors?.length ?? 0;
return () => {
hadError = false;
// Reset the tracked symbols to before the error
if (context.trackedSymbols) {
context.trackedSymbols.length = initialTrackedSymbolsTop;
}
if (unreportedErrors) {
unreportedErrors.length = unreportedErrorsTop;
}
};
}

function finalizeBoundary() {
context.tracker = oldTracker;
const newTrackedSymbols = context.trackedSymbols;
context.trackedSymbols = oldTrackedSymbols;
context.encounteredError = oldEncounteredError;

unreportedErrors?.forEach(fn => fn());
if (hadError) {
return false;
}
newTrackedSymbols?.forEach(
([symbol, enclosingDeclaration, meaning]) =>
context.tracker.trackSymbol(
symbol,
enclosingDeclaration,
meaning,
),
);
return true;
}
}
function onEnterNewScope(node: IntroducesNewScopeNode | ConditionalTypeNode) {
return enterNewScope(context, node, getParametersInScope(node), getTypeParametersInScope(node));
}
Expand Down Expand Up @@ -8765,13 +8858,30 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {

function rewriteModuleSpecifier(parent: ImportTypeNode, lit: StringLiteral) {
if (context.bundled || context.enclosingFile !== getSourceFileOfNode(lit)) {
const targetFile = getExternalModuleFileFromDeclaration(parent);
if (targetFile) {
const newName = getSpecifierForModuleSymbol(targetFile.symbol, context);
if (newName !== lit.text) {
return setOriginalNode(factory.createStringLiteral(newName), lit);
let name = lit.text;
const nodeSymbol = getNodeLinks(node).resolvedSymbol;
const meaning = parent.isTypeOf ? SymbolFlags.Value : SymbolFlags.Type;
const parentSymbol = nodeSymbol
&& isSymbolAccessible(nodeSymbol, context.enclosingDeclaration, meaning, /*shouldComputeAliasesToMakeVisible*/ false).accessibility === SymbolAccessibility.Accessible
&& lookupSymbolChain(nodeSymbol, context, meaning, /*yieldModuleSymbol*/ true)[0];
if (parentSymbol && parentSymbol.flags & SymbolFlags.Module) {
name = getSpecifierForModuleSymbol(parentSymbol, context);
}
else {
const targetFile = getExternalModuleFileFromDeclaration(parent);
if (targetFile) {
name = getSpecifierForModuleSymbol(targetFile.symbol, context);
}
}
if (name.includes("/node_modules/")) {
context.encounteredError = true;
if (context.tracker.reportLikelyUnsafeImportRequiredError) {
context.tracker.reportLikelyUnsafeImportRequiredError(name);
}
}
if (name !== lit.text) {
return setOriginalNode(factory.createStringLiteral(name), lit);
}
}
return visitNode(lit, visitExistingNodeTreeSymbols, isStringLiteral)!;
}
Expand Down
61 changes: 61 additions & 0 deletions tests/baselines/reference/declarationEmitUsingTypeAlias2.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
//// [tests/cases/compiler/declarationEmitUsingTypeAlias2.ts] ////

//// [inner.d.ts]
export declare type Other = { other: string };
export declare type SomeType = { arg: Other };

//// [other.d.ts]
export declare const shouldLookupName: unique symbol;
export declare const shouldReuseLocalName: unique symbol;
export declare const reuseDepName: unique symbol;
export declare const shouldBeElided: unique symbol;
export declare const isNotAccessibleShouldError: unique symbol;

//// [index.d.ts]
import { Other } from './inner'
import { shouldLookupName, reuseDepName, shouldReuseLocalName, shouldBeElided } from './other'
export declare const goodDeclaration: <T>() => () => {
/** Other Can't be named in index.ts, but the whole conditional type can be resolved */
shouldPrintResult: T extends Other? "O": "N",
/** 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 */
shouldPrintResult2: T extends typeof shouldBeElided? Other: "N",
/** Specifier should come from module, local path should not be reused */
shouldLookupName: typeof import('./other').shouldLookupName,
/** This is imported in index so local name should be reused */
shouldReuseLocalName: typeof shouldReuseLocalName
/** This is NOT imported in index so import should be created */
reuseDepName: typeof reuseDepName,
}
export { shouldLookupName, shouldReuseLocalName, reuseDepName, shouldBeElided };

//// [package.json]
{
"name": "some-dep",
"exports": {
".": "./dist/index.js"
}
}

//// [index.ts]
import { goodDeclaration, shouldReuseLocalName, shouldBeElided } from "some-dep";
export const bar = goodDeclaration<{}>;



//// [index.js]
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = void 0;
const some_dep_1 = require("some-dep");
exports.bar = (some_dep_1.goodDeclaration);


//// [index.d.ts]
import { shouldReuseLocalName } from "some-dep";
export declare const bar: () => () => {
shouldPrintResult: "N";
shouldPrintResult2: "N";
shouldLookupName: typeof import("some-dep").shouldLookupName;
shouldReuseLocalName: typeof shouldReuseLocalName;
reuseDepName: typeof import("some-dep").reuseDepName;
};
87 changes: 87 additions & 0 deletions tests/baselines/reference/declarationEmitUsingTypeAlias2.symbols
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//// [tests/cases/compiler/declarationEmitUsingTypeAlias2.ts] ////

=== node_modules/some-dep/dist/inner.d.ts ===
export declare type Other = { other: string };
>Other : Symbol(Other, Decl(inner.d.ts, 0, 0))
>other : Symbol(other, Decl(inner.d.ts, 0, 29))

export declare type SomeType = { arg: Other };
>SomeType : Symbol(SomeType, Decl(inner.d.ts, 0, 46))
>arg : Symbol(arg, Decl(inner.d.ts, 1, 32))
>Other : Symbol(Other, Decl(inner.d.ts, 0, 0))

=== node_modules/some-dep/dist/other.d.ts ===
export declare const shouldLookupName: unique symbol;
>shouldLookupName : Symbol(shouldLookupName, Decl(other.d.ts, 0, 20))

export declare const shouldReuseLocalName: unique symbol;
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(other.d.ts, 1, 20))

export declare const reuseDepName: unique symbol;
>reuseDepName : Symbol(reuseDepName, Decl(other.d.ts, 2, 20))

export declare const shouldBeElided: unique symbol;
>shouldBeElided : Symbol(shouldBeElided, Decl(other.d.ts, 3, 20))

export declare const isNotAccessibleShouldError: unique symbol;
>isNotAccessibleShouldError : Symbol(isNotAccessibleShouldError, Decl(other.d.ts, 4, 20))

=== node_modules/some-dep/dist/index.d.ts ===
import { Other } from './inner'
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))

import { shouldLookupName, reuseDepName, shouldReuseLocalName, shouldBeElided } from './other'
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 1, 8))
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 1, 26))
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 1, 40))
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 1, 62))

export declare const goodDeclaration: <T>() => () => {
>goodDeclaration : Symbol(goodDeclaration, Decl(index.d.ts, 2, 20))
>T : Symbol(T, Decl(index.d.ts, 2, 39))

/** Other Can't be named in index.ts, but the whole conditional type can be resolved */
shouldPrintResult: T extends Other? "O": "N",
>shouldPrintResult : Symbol(shouldPrintResult, Decl(index.d.ts, 2, 54))
>T : Symbol(T, Decl(index.d.ts, 2, 39))
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))

/** 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 */
shouldPrintResult2: T extends typeof shouldBeElided? Other: "N",
>shouldPrintResult2 : Symbol(shouldPrintResult2, Decl(index.d.ts, 4, 47))
>T : Symbol(T, Decl(index.d.ts, 2, 39))
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 1, 62))
>Other : Symbol(Other, Decl(index.d.ts, 0, 8))

/** Specifier should come from module, local path should not be reused */
shouldLookupName: typeof import('./other').shouldLookupName,
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 6, 66))
>shouldLookupName : Symbol(shouldLookupName, Decl(other.d.ts, 0, 20))

/** This is imported in index so local name should be reused */
shouldReuseLocalName: typeof shouldReuseLocalName
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 8, 62))
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 1, 40))

/** This is NOT imported in index so import should be created */
reuseDepName: typeof reuseDepName,
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 10, 51))
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 1, 26))
}
export { shouldLookupName, shouldReuseLocalName, reuseDepName, shouldBeElided };
>shouldLookupName : Symbol(shouldLookupName, Decl(index.d.ts, 14, 8))
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.d.ts, 14, 26))
>reuseDepName : Symbol(reuseDepName, Decl(index.d.ts, 14, 48))
>shouldBeElided : Symbol(shouldBeElided, Decl(index.d.ts, 14, 62))

=== src/index.ts ===
import { goodDeclaration, shouldReuseLocalName, shouldBeElided } from "some-dep";
>goodDeclaration : Symbol(goodDeclaration, Decl(index.ts, 0, 8))
>shouldReuseLocalName : Symbol(shouldReuseLocalName, Decl(index.ts, 0, 25))
>shouldBeElided : Symbol(shouldBeElided, Decl(index.ts, 0, 47))

export const bar = goodDeclaration<{}>;
>bar : Symbol(bar, Decl(index.ts, 1, 12))
>goodDeclaration : Symbol(goodDeclaration, Decl(index.ts, 0, 8))


Loading