8000 IDE: Cleaner LSPTypechecker::retypecheck() interface. by jvilk-stripe · Pull Request #2304 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

IDE: Cleaner LSPTypechecker::retypecheck() interface. #2304

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 3 commits into from
Dec 6, 2019

Conversation

jvilk-stripe
Copy link
Collaborator

IDE: Cleaner LSPTypechecker::retypecheck() interface. Code actions use this to get a list of errors from a file and extract autocorrects from those errors.

Motivation

This implementation bothered me, so I decided to quickly clean it up.

Test plan

Covered by existing code_action tests.

@jvilk-stripe jvilk-stripe added the IDE Relating to Sorbet's LSP server or VS Code extension label Dec 6, 2019
@jvilk-stripe jvilk-stripe requested a review from a team December 6, 2019 04:45
@ghost ghost requested review from aisamanra and removed request for a team December 6, 2019 04:46
Copy link
Contributor
@aisamanra aisamanra left a comment

Choose a reason for hiding this comment

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

I had one bit of confusion (on me, not on the PR) but otherwise looks good!


TypecheckRun LSPTypechecker::retypecheck(vector<core::FileRef> frefs) const {
LSPFileUpdates updates = getNoopUpdate(move(frefs));
auto workers = WorkerPool::create(0, *config->logger);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is the only line I'm slightly confused about, but I also don't pretend to understand the WorkerPool abstraction. Does this do something side-effecting? Because otherwise it looks like it's unused.

Copy link
Collaborator Author
@jvilk-stripe jvilk-stripe Dec 6, 2019

Choose a reason for hiding this comment

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

embarrassed look

I copied this code from my preemptive slow path changes. It's unused and has no side effects. I'll remove it prior to merging. Thanks for catching!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed. I also switched to using indexed directly instead of getIndexed since I don't want indexedFinalGS trees.

@jvilk-stripe jvilk-stripe merged commit 940aa15 into master 7B48 Dec 6, 2019
@jvilk-stripe jvilk-stripe deleted the jvilk/cleaner-retypecheck-interface branch December 6, 2019 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
IDE Relating to Sorbet's LSP server or VS Code extension
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0