Skip to content

Fixed a ghost error popping up after completions request caching a wrong contextual function type #58378

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

Closed
wants to merge 3 commits into from
Closed

Fixed a ghost error popping up after completions request caching a wrong contextual function type #58378

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2670d96
Fixed a ghost error popping up after completions request caching a wr…
Andarist Apr 30, 2024
b91b8ea
fmt
Andarist Apr 30, 2024
1987106
Merge remote-tracking branch 'origin/main' into fix/out-of-order-comp…
Andarist Jun 23, 2025
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
56 changes: 33 additions & 23 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1923,32 +1923,42 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
}

function runWithoutResolvedSignatureCaching<T>(node: Node | undefined, fn: () => T): T {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This becomes more and more complicated and error-prone. I'm definitely not a fan of this. It builds on top of the existing solution... so at least it shouldn't be significantly worse now. It's just that perhaps there is a completely better way to do it than the existing mechanism. I think that alternatives could be explored separately.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought about using infra introduced in #57421 but that PR is still in a draft. The problem space is also a little bit different - in this PR here we don't need to revert some recent work. In here, we need to temporarily evict some preexisting caches and restore them - we don't need to speculate per se. I assume that both are interested in the same lists of caches though.

const cachedResolvedSignatures = [];
const cachedTypes: (readonly [SymbolLinks, Type | undefined])[] = [];
node = findAncestor(node, isCallLikeOrFunctionLikeExpression);
if (node) {
const cachedResolvedSignatures = [];
const cachedTypes = [];
while (node) {
const nodeLinks = getNodeLinks(node);
cachedResolvedSignatures.push([nodeLinks, nodeLinks.resolvedSignature] as const);
nodeLinks.resolvedSignature = undefined;
if (isFunctionExpressionOrArrowFunction(node)) {
const symbolLinks = getSymbolLinks(getSymbolOfDeclaration(node));
const type = symbolLinks.type;
cachedTypes.push([symbolLinks, type] as const);
symbolLinks.type = undefined;
}
node = findAncestor(node.parent, isCallLikeOrFunctionLikeExpression);
}
const result = fn();
for (const [nodeLinks, resolvedSignature] of cachedResolvedSignatures) {
nodeLinks.resolvedSignature = resolvedSignature;
}
for (const [symbolLinks, type] of cachedTypes) {
symbolLinks.type = type;
if (!node) {
return fn();
}
while (node) {
const nodeLinks = getNodeLinks(node);
cachedResolvedSignatures.push([nodeLinks, nodeLinks.resolvedSignature, nodeLinks.flags] as const);
nodeLinks.resolvedSignature = undefined;
nodeLinks.flags = NodeCheckFlags.None;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The key here is to reset NodeCheckFlags.ContextChecked so contextuallyCheckFunctionExpressionOrObjectLiteralMethod can correctly resolve (and cache) signature.resolvedReturnType multiple times (for requests with inference blocked and for requests without inference blocked). Otherwise, it has to be resolved later on... but without proper contextual information.

So I could just remove that bit and re-add it later if needed. I looked through all NodeCheckFlags and it might also be needed to do the same for NodeCheckFlags.TypeChecked. Although, I'm not 100% sure if that's really the case.

I just tried with a complete flags reset here and it does seem to work... but 🤷‍♂️ I'd probably be in favor of resetting solely what's really-really needed.

if (isFunctionExpressionOrArrowFunction(node)) {
resetCachedSymbolTypes(node);
for (const parameter of node.parameters) {
resetCachedSymbolTypes(parameter);
}
}
return result;
node = findAncestor(node.parent, isCallLikeOrFunctionLikeExpression);
}
const result = fn();
for (const [nodeLinks, resolvedSignature, nodeCheckFlags] of cachedResolvedSignatures) {
nodeLinks.resolvedSignature = resolvedSignature;
nodeLinks.flags = nodeCheckFlags;
}
for (const [symbolLinks, type] of cachedTypes) {
symbolLinks.type = type;
}
return result;

function resetCachedSymbolTypes(declaration: Declaration) {
const symbolLinks = getSymbolLinks(getSymbolOfDeclaration(declaration));
const type = symbolLinks.type;

cachedTypes.push([symbolLinks, type] as const);
symbolLinks.type = undefined;
}
return fn();
}

function runWithInferenceBlockedFromSourceNode<T>(node: Node | undefined, fn: () => T): T {
Expand Down
32 changes: 32 additions & 0 deletions tests/cases/fourslash/noErrorsAfterCompletionsRequestWithinGenericFunction4.ts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/// <reference path="fourslash.ts" />

// @strict: true
// @target: esnext
// @lib: esnext

//// type ObjectFromEntries<T> = T extends readonly [
//// infer Key extends string | number | symbol,
//// infer Value,
//// ][]
//// ? { [key in Key]: Value }
//// : never;
////
//// type KeyValuePairs<T> = {
//// [K in keyof T]: [K, T[K]];
//// }[keyof T];
////
//// declare function mapObjectEntries<
//// const T extends object,
//// const TMapped extends [string | number | symbol, unknown],
//// >(
//// obj: T,
//// mapper: ([a, b]: KeyValuePairs<T>) => TMapped,
//// ): ObjectFromEntries<TMapped[]>;
////
//// mapObjectEntries({ a: 1, b: 2 }, ([x, y]) => ["a/*1*/", y]);

verify.completions({
marker: "1",
exact: ["a"],
});
verify.getSemanticDiagnostics([]);