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

WIP: Make too-many-arguments errors span fn header only #2599

Closed
wants to merge 1 commit into from
Closed

WIP: Make too-many-arguments errors span fn header only #2599

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Mar 31, 2018

The HIR API has no span for just the function header. This commit handles that
by using the entire function span until the start of the body expression. This
isn't perfect because it includes spaces after the header.

Closes #2488

The HIR API has no span for just the function header. This commit handles that
by using the entire function span until the start of the body expression. This
isn't perfect because it includes spaces after the header.

Closes #2488
@ghost
Copy link
Author

ghost commented Mar 31, 2018

I need feedback on this. Is this PR worth merging as is? How can I remove the trailing space from the span?

@oli-obk
Copy link
Contributor

oli-obk commented Mar 31, 2018

I think we should delay fixing this until rust-lang/rust#49154 has been merged. This should allow us to only point to the name of the function.

How can I remove the trailing space from the span?

You'd need to fetch the text snippet, count the trailing whitespace and shorten the span by that count. But I'm fairly confident that the "all idents have spans" PR will get merged in the next few weeks, so we could hold off until then.

Your PR can then be adjusted to make use of the idents.

On the other hand we could just merge the PR now and do that fix later. What do you think?

@ghost
Copy link
Author

ghost commented Apr 1, 2018

Let's wait. I will close this and submit a new PR later.

@ghost ghost closed this Apr 1, 2018
@ghost ghost deleted the issue_2488 branch June 20, 2019 06:36
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant