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

Go modules support #9

Closed
wants to merge 2 commits into from
Closed

Go modules support #9

wants to merge 2 commits into from

Conversation

atombender
Copy link

This uses the new package golang.org/x/tools/go/packages instead of golang.org/x/tools/go/loader to load code, so that we can support checking packages that use Go modules with Go >= 1.11. This fixes #8.

I'm not sure the change in command line usage breaks anything, though the semantics are changed, because go/packages works a bit differently from go/loader. The former now takes a set of files and patterns in order to resolve Go packages. The way I did it, go-sumtype myfile.go still works, and go-sumtype ./mypkg/subpkg/... also works, without having to invoke go list, so this is technically an improvement. If you think this is too magical, it's possible that we could revert to the old behaviour, but I don't think so.

I added a go.mod file for good measure.

@BurntSushi
Copy link
Owner

@atombender Thanks for submitting this! Apologies that it took so long for me to respond, but manual QA on this revealed that this change isn't actually correct. In this PR, you're only looking for sum type decls per package, where as the correct behavior is to look for sum type decls across all given packages first, and only then do exhaustiveness checks with all the decls found. For example, with this PR, if you have a decl in package foo but do case analysis with it in package bar, then this will never analyze that case analysis.

It looks like #10 fixes this, so I'm going to bring that in instead. Thanks again!

@BurntSushi BurntSushi closed this Feb 25, 2019
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.

Update to support Go 1.11 modules
2 participants