8000 Use callLocs in fewer error messages by jez · Pull Request #5551 · sorbet/sorbet · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
8000

Use callLocs in fewer error messages #5551

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

Jump to bottom
Merged
merged 8 commits into from
Mar 31, 2022
Merged

Use callLocs in fewer error messages #5551

merged 8 commits into from
Mar 31, 2022

Conversation

jez
Copy link
Collaborator
@jez jez commented Mar 31, 2022

Motivation

The args.callLocs() contains the block argument, if one exists. Any time we
use this in an error message, it'll chunder across the whole screen, turning
large swaths of it red, obscuring other errors.

Even if there isn't a block, usually there's something more specific that we can
give.

This change searches for a bunch of places where callLocs is used and stops
using it.

Test plan

Each commit is arguably an independent change, but given that they're so closely
related in spirit I've made only one PR.

You can review by commit to see the changes to tests next to the relevant change
to calls.cc

@jez jez requested a review from a team as a code owner March 31, 2022 07:12
@jez jez requested review from elliottt and removed request for a team March 31, 2022 07:12
jez added 6 commits March 31, 2022 00:16
@jez
Add argsLoc helper, add an initialize autocorrect
be9e4ea
@jez
Autocorrect to delete block if unneeded
f762eff
@jez
Use funLoc for more errors
d045372 
This is important because otherwise if there happens to be a block,
Sorbet will chunder a huge red line over the whole method call including
the block arg if present, which sometimes just makes half the file turn
red.
@jez
Use argLocs for "Not enough arguments"
c78254a
@jez
Change error location for T.must
401b0e1
@jez
Change location for missing required kwarg
0f0abb5
elliottt
elliottt approved these changes Mar 31, 2022
View reviewed changes
Copy link
Collaborator
@elliottt elliottt left a comment

Choose a reason for hiding this comment

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

Looks good! I like the more precise error locations 👍

@jez jez merged commit f0c00b5 into master Mar 31, 2022
@jez jez deleted the jez-calllocs branch March 31, 2022 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@elliottt elliottt elliottt approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jez @elliottt
0