Skip to content

Handle undefined location.parent when getting completionEntryDetails #54138

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

Merged
merged 2 commits into from
May 5, 2023

Conversation

MariaSolOs
Copy link
Contributor

Fixes #54106

@MariaSolOs MariaSolOs marked this pull request as draft May 4, 2023 21:10
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label May 4, 2023
@MariaSolOs
Copy link
Contributor Author

Tests are pending because I don't know how to test completionEntryDetails. verify.completions seems to only check completionInfo, and I cannot use the resulting andApplyCodeAction because the entry I want to test doesn't have a description or source (which are needed in the parameter options).

@jakebailey
Copy link
Member

Where is this location coming from? All source files should be bound and have parents, so is this location the SourceFile itself?

@MariaSolOs
Copy link
Contributor Author

Where is this location coming from? All source files should be bound and have parents, so is this location the SourceFile itself?

Correct. So with the example from the bug:

/**/
type lolol = any;
declare const lolol: any;

When explicitly requesting completions in /**/, location will be the source file itself, which causes the error in the line I'm changing.

@MariaSolOs MariaSolOs marked this pull request as ready for review May 4, 2023 21:54
@MariaSolOs
Copy link
Contributor Author

@jakebailey @DanielRosenwasser @andrewbranch any hints on how I can test this with fourslash?

@jakebailey
Copy link
Member

Are the details in https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/completionsObjectLiteralMethod2.ts what you're trying to get at?

@MariaSolOs
Copy link
Contributor Author

Are the details in https://github.com/microsoft/TypeScript/blob/main/tests/cases/fourslash/completionsObjectLiteralMethod2.ts what you're trying to get at?

Hmm no this isn't helping me here. The completion entry remains the same with and without my change, like I need to simulate selecting the actual completion to trigger the failure.

@MariaSolOs
Copy link
Contributor Author

Okay I was able to test this with verify.baselineCompletions(). You were right @iisaduan!

@MariaSolOs MariaSolOs merged commit 42d69cf into microsoft:main May 5, 2023
@MariaSolOs MariaSolOs deleted the completion-undef-parent branch May 5, 2023 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Milestone Bug PRs that fix a bug with a specific milestone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error from completionEntryDetails on entry before definition of merged variable and type alias
4 participants