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

cmd/fillswitch: not build whole import graph? #17

Open
buyology opened this issue Mar 20, 2018 · 1 comment
Open

cmd/fillswitch: not build whole import graph? #17

buyology opened this issue Mar 20, 2018 · 1 comment
Assignees
Labels

Comments

@buyology
Copy link

buyology commented Mar 20, 2018

First off: Thanks for some great tools. Recently had the pleasure integrating fillstruct into vscode and now considering doing the same with fillswitch :)

While trying it out I noticed that it builds a complete GOPATH import graph, something that fillstruct does not — which makes the command take an order of magnitude longer to execute.

_, rev, _ := importgraph.Build(ctx)

Is there a specific reason for having this in fillswitch and not fillstruct? I naively tried removing it and it worked just fine without.

@davidrjenni davidrjenni self-assigned this Mar 21, 2018
@davidrjenni
Copy link
Owner

Thank you @buyology - that's great to hear.

fillstruct builds the reverse import graph in order to catch as many packages containing types needed for the completion.

Example:

Consider there is package foo defining an interface I.
The type switch statement the user wants to fill is also in package foo and switches over a variable of type I.

package foo

type I interface {
	M() U
}

type U int

func F(i I) {
	switch i.(type) {
		// invoke fillswitch here
	}
}

Also, there is package bar importing package foo and containing a type implementing the interface foo.I.

package bar

import "foo"

type T struct{}

func (T) M() foo.U { return 0 }

By omitting the code section you referred to, the user will not get the completion for type bar.T.

However this is also incomplete, there might be another package (neither importing nor imported by package foo) in the GOPATH containing a type which implements the interface foo.I.
So there is a trade-off between speed and completeness.

If this setting stated in the example above is a corner-case, we might as well omit building the reverse import graph and have a faster tool. What do you think?

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

No branches or pull requests

2 participants