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.

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

Fix checker initialization crash #46973

Merged
merged 2 commits into from Dec 3, 2021
Merged

Conversation

andrewbranch
Copy link
Member

Fixes #46587

The auto import provider was trying to generate spelling suggestions for IArguments since it runs with noLib: true. Not sure why this happens only sometimes, but (a) there’s no point in trying to come up with spelling suggestions for global symbols, and (b) there’s no point in trying to come up with spelling suggestions for anything inside a non-diagnostics-producing checker.

@andrewbranch andrewbranch changed the title Bug/46587 Fix checker initialization crash Dec 1, 2021
@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Dec 1, 2021
Copy link
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

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

The work we do when producing diagnostics sometimes has side effects, like pushing things into caches, which can affect other calculations down the line, so I'm usually wary against produceDiagnostics checks that aren't immediately around an error call, as it can the a source of reeeeeaaally hard to track bugs where hover info and error info on a symbol don't match (hover data comes from a non-diagnostics producing checkers, while errors do). Specifically here, there's a nested conditional checkResolvedBlockScopedVariable that might toss something into a cache which now we'd avoid doing - it's probably fine until proven otherwise, since everything in the guarded branch is supposed to just be to generate an error message, I'd just keep it in mind in case something weird crops up in the future.

@andrewbranch
Copy link
Member Author

@typescript-bot cherry-pick this to release-4.5

@typescript-bot
Copy link
Collaborator

typescript-bot commented Dec 3, 2021

Heya @andrewbranch, I've started to run the task to cherry-pick this into release-4.5 on this PR at 2064b74. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

Hey @andrewbranch, I've opened #47005 for you.

typescript-bot pushed a commit to typescript-bot/TypeScript that referenced this pull request Dec 3, 2021
Component commits:
0612e18 Fix checker initialization crash

2064b74 Move checks to a place that makes more sense
@andrewbranch andrewbranch merged commit 240ba0a into microsoft:main Dec 3, 2021
@andrewbranch andrewbranch deleted the bug/46587 branch December 3, 2021 22:38
andrewbranch added a commit to typescript-bot/TypeScript that referenced this pull request Dec 3, 2021
Component commits:
0612e18 Fix checker initialization crash

2064b74 Move checks to a place that makes more sense
DanielRosenwasser pushed a commit that referenced this pull request Dec 7, 2021
Component commits:
0612e18 Fix checker initialization crash

2064b74 Move checks to a place that makes more sense

Co-authored-by: Andrew Branch <andrew@wheream.io>
excludeGlobals = false): Symbol | undefined {
return resolveNameHelper(location, name, meaning, nameNotFoundMessage, nameArg, isUse, excludeGlobals, getSymbol);
excludeGlobals = false,
getSpellingSuggstions = true): Symbol | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

I'm obviously very late to this PR but is this argument ironically named? 😛

mprobst pushed a commit to mprobst/TypeScript that referenced this pull request Jan 10, 2022
* Fix checker initialization crash

* Move checks to a place that makes more sense
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team 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.

TS Server fatal error: Cannot read property 'flags' of undefined
4 participants