Skip to content

Fixed an issue with errors not being correctly reported after completion requests in nested calls #54658

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.

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

Conversation

Andarist
Copy link
Contributor

Fixes an accidental 5.1 regression caused by my own #52717 . Even though this particular situation showcased by the test case is a 5.1 regression, the code that I changed here was likely subtly broken before that - there just wasn't any test case that would catch this somehow.

cc @andrewbranch

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jun 15, 2023
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

const containingCallResolvedSignature = containingCall && getNodeLinks(containingCall).resolvedSignature;
if (containingCall) {
getNodeLinks(containingCall).resolvedSignature = undefined;
const cachedSignatures = [];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is important to do this for all signatures on the ancestry path because during completion requests diagnostics are not reported, this is reflected by const reportErrors = !isInferencePartiallyBlocked && !candidatesOutArray; in resolveCall. But if we only restore the result of the inner call (when doing a completion request within its argument) then we might cache the outer signature. That signature might even be correct - but what happens is that when actual diagnostics are requested we don't ever get to checking the inner call because a regular typecheck happens from the top to the bottom and the outer checkCallExpression could just use the cached signature and bail out from reaching the inner checkCallExpression

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

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

Nice catch. Makes sense to me.

@andrewbranch andrewbranch merged commit abf0ef8 into microsoft:main Jun 20, 2023
@Andarist Andarist deleted the fix/no-error-after-completions-in-nested-call branch June 20, 2023 19:41
@Andarist
Copy link
Contributor Author

@andrewbranch since this issue is more likely to be observed in 5.1 - what do you think about cherry-picking this into 5.1? :p

@andrewbranch
Copy link
Member

I think you should start pinging @DanielRosenwasser directly about that stuff 😅 I was just out on vacation so I don’t know what the current patch status is.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants