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

x/tools/gopls: handle completion after defer and go statements #29313

Closed
stamblerre opened this issue Dec 17, 2018 · 18 comments
Closed

x/tools/gopls: handle completion after defer and go statements #29313

stamblerre opened this issue Dec 17, 2018 · 18 comments
Assignees
Labels
Milestone

Comments

@stamblerre
Copy link
Contributor

@stamblerre stamblerre commented Dec 17, 2018

A defer or go statement is expected to be followed by an *ast.CallExpr. If they are not, the parser treats them as *ast.BadExprs, causing completion to fail. We need to handle this particular case.

Repro:

package main

import "fmt"

func main() {
    defer fmt.<>
}

Triggering a completion here will result in lexical completions, rather than the expected selector completions for package "fmt".

@gopherbot gopherbot added this to the Unreleased milestone Dec 17, 2018
@alindeman

This comment has been minimized.

Copy link

@alindeman alindeman commented Dec 18, 2018

I decided to poke at this a bit, but couldn't reproduce it. Is this a short code snippet you have handy that triggers the issue?

@stamblerre stamblerre self-assigned this Dec 18, 2018
@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Dec 18, 2018

Updated to add a repro. This may be a tough first issue, if this is something you're interested in tackling, just because it will require special handling after a file has been parsed but before it has been type-checked.

@alindeman

This comment has been minimized.

Copy link

@alindeman alindeman commented Dec 18, 2018

@stamblerre Thanks. I'm mostly using anything tagged as lsp as a way to get familiar with the current codebase. I don't want to get in the way yet :) I'll follow along on the CL if you get to it first.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Apr 24, 2019

Change https://golang.org/cl/172974 mentions this issue: internal/lsp: handle completion after defer, go statements

gopherbot pushed a commit to golang/tools that referenced this issue Apr 24, 2019
This change adds support for completion of incomplete selectors after a
defer or go statement. We modify the AST before type-checking it with a
fake *ast.CallExpr.

Updates golang/go#29313

Change-Id: Ic9e8c9c49aa569cd7874791692c70a28c3146251
Reviewed-on: https://go-review.googlesource.com/c/tools/+/172974
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@myitcv

This comment has been minimized.

Copy link
Member

@myitcv myitcv commented May 3, 2019

I might have misunderstood https://go-review.googlesource.com/c/tools/+/172974/, but I still don't see (as of v0.0.0-20190503030157-5cec639030af) completions working for the original example provided in #29313 (comment)

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented May 8, 2019

I'm still working on finishing this because it doesn't work on most cases, but it should work OK for the cases in these tests: https://github.com/golang/tools/blob/master/internal/lsp/testdata/badstmt/badstmt.go.

@myitcv

This comment has been minimized.

Copy link
Member

@myitcv myitcv commented Jun 25, 2019

@stamblerre - not quite sure whether the following is an instance of the same issue? Happy to raise a separate issue if not:

package main

import "playground.com/p"

func main() {
	switch {
	case nil:
		s := new(p.)
	}
}

-- go.mod --
module playground.com

-- p/p.go --
package p

type S struct {
	Name string
	Age  int
}

Attempting completion after s. in main.go does not return any candidates.

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Jun 27, 2019

I'd guess it's actually closer to #31973, but either way, that should definitely work.

@stamblerre stamblerre changed the title x/tools/internal/lsp: handle completion after defer and go statements x/tools/gopls: handle completion after defer and go statements Jul 2, 2019
@taigacute

This comment has been minimized.

Copy link

@taigacute taigacute commented Jul 21, 2019

@stamblerre hi , i got same error without go defer statements.
1

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Jul 21, 2019

@taigacute: I'm not sure I understand the issue here. Do you mind filing a separate issue with logs?

@taigacute

This comment has been minimized.

Copy link

@taigacute taigacute commented Jul 22, 2019

@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Aug 3, 2019

/cc @flowchartsman

This issue has been partially solved, but still needs a bit of work. I believe @flowchartsman will be investigating this.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Sep 9, 2019

Change https://golang.org/cl/194377 mentions this issue: internal/lsp: update to latest version of x/tools

gopherbot pushed a commit to golang/tools that referenced this issue Sep 9, 2019
The fix for golang/go#29313 just went in, so update so people can fetch
it more easily.

Change-Id: I60fda011dfdd62d5de429834a6692f6b21074f0d
Reviewed-on: https://go-review.googlesource.com/c/tools/+/194377
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Sep 9, 2019

This issue should be fixed now. Thanks @muirrn!

@stamblerre stamblerre closed this Sep 9, 2019
@gencer

This comment has been minimized.

Copy link

@gencer gencer commented Sep 9, 2019

@stamblerre, should I do;

go get golang.org/x/tools/gopls@master

or

go get golang.org/x/tools/gopls@latest
@stamblerre

This comment has been minimized.

Copy link
Contributor Author

@stamblerre stamblerre commented Sep 9, 2019

We haven't made a new release yet, so for now, the fix is only on master.

@gencer

This comment has been minimized.

Copy link

@gencer gencer commented Sep 9, 2019

Thanks for the fix. I'll use master branch for now.

@blixenkrone

This comment has been minimized.

Copy link

@blixenkrone blixenkrone commented Sep 10, 2019

Thank you for the fix @stamblerre! Works perfectly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
8 participants
You can’t perform that action at this time.