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.

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

x/tools/gopls: send back ranges for parse errors #29150

Closed
stamblerre opened this issue Dec 7, 2018 · 7 comments
Closed

x/tools/gopls: send back ranges for parse errors #29150

stamblerre opened this issue Dec 7, 2018 · 7 comments
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Milestone
gopls/v0.4.0

Comments

@stamblerre
Copy link
Contributor

We currently only send the starting position of an error in LSP diagnostics. We should determine the end of the error as well. For type errors, we can just get the length of the identifier. We will need another approach for parse errors.
@gopherbot gopherbot added this to the Unreleased milestone Dec 7, 2018
@zmb3
Copy link
Contributor

zmb3 commented Dec 18, 2018

Happy to take this one, but need some pointers on how to get to the identifier given just the token.Position from the error message. Seems we need to convert to a token.Pos, I'm not sure how to convert in that direction.

@stamblerre
Copy link
Contributor Author

stamblerre commented Dec 18, 2018 •
edited
Loading

@zmb3: I saw your reply and started trying to refactor things to make it easier to fix this bug, and then I got a bit carried away and got through about 3/4 of the fix myself. Did you want to fix this yourself or is it cool if I send out my CL? My CL won't fix everything - it handles ranges for types.Errors, but not for scanner.Errors, which are a bit more complicated.

@zmb3
Copy link
Contributor

zmb3 commented Dec 18, 2018

No, please go ahead and send your CL. I’m anxious to see what you came up with 😀

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/154742 mentions this issue: internal/lsp: add ranges to some diagnostics messages

@stamblerre stamblerre self-assigned this Dec 19, 2018
gopherbot pushed a commit to golang/tools that referenced this issue Dec 20, 2018
@stamblerre
internal/lsp: add ranges to some diagnostics messages

Partially verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
We cannot verify signatures from co-authors, and some of the co-authors attributed to this commit require their commits to be signed.
f344c75 
Added a View interface to the source package, which allows for reading
of other files (in the same package or in other packages). We were
already reading files in jump to definition (to handle the lack of
column information in export data), but now we can also read files in
diagnostics, which allows us to determine the end of an identifier so
that we can report ranges in diagnostic messages.

Updates golang/go#29150

Change-Id: I7958d860dea8f41f2df88a467b5e2946bba4d1c5
Reviewed-on: https://go-review.googlesource.com/c/154742
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@stamblerre stamblerre added gopls Issues related to the Go language server, gopls. and removed gopls Issues related to the Go language server, gopls. labels Mar 12, 2019
@stamblerre stamblerre changed the title x/tools/internal/lsp: send back ranges for diagnostics x/tools/internal/lsp: send back ranges for parse errors Jun 5, 2019
@stamblerre stamblerre added the Suggested Issues that may be good for new contributors looking for work to do. label Jun 5, 2019
@stamblerre stamblerre added the NeedsFix The path to resolution is known, but the work has not been done. label Jun 28, 2019
@stamblerre stamblerre changed the title x/tools/internal/lsp: send back ranges for parse errors x/tools/gopls: send back ranges for parse errors Jul 2, 2019
@stamblerre stamblerre added help wanted and removed Suggested Issues that may be good for new contributors looking for work to do. labels Aug 8, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/202542 mentions this issue: internal/lsp: use the AST to get correct ranges

@zikaeroh zikaeroh mentioned this issue Oct 24, 2019
Closed
@stamblerre stamblerre reopened this Nov 7, 2019
@stamblerre
Copy link
Contributor Author

The above CL attempted to address this by using the AST to determine ranges for parse errors. However, this had the unexpected consequence of making the ranges either too large or unintuitive for the user. Most likely, we will want to modify the error ranges based on the type of the error, which will require error codes to allow us to distinguish between the different errors.

@stamblerre stamblerre modified the milestones: Unreleased, gopls unplanned Dec 3, 2019
@stamblerre stamblerre modified the milestones: gopls unplanned, gopls/v1.0.0 Jan 29, 2020
@stamblerre
Copy link
Contributor Author

I don't think that this is actually necessary at this point. Until we have error codes or ranges coming back from go/parser, we can't make progress here. Going to close this issue.

@stamblerre stamblerre modified the milestones: gopls/v1.0.0, gopls/v0.4.0 Jul 22, 2020
@golang golang locked and limited conversation to collaborators Jul 22, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge gopls Issues related to the Go language server, gopls. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
gopls/v0.4.0
Development

No branches or pull requests
3 participants
@stamblerre @zmb3 @gopherbot