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/types: clearly mark the package as not supporting modules #43170

Open
natefinch opened this issue Dec 14, 2020 · 7 comments
Open

go/types: clearly mark the package as not supporting modules #43170

natefinch opened this issue Dec 14, 2020 · 7 comments

Comments

@natefinch
Copy link
Contributor

@natefinch natefinch commented Dec 14, 2020

What did you do?

Tried to use go/types with code that uses modules

What did you expect to see?

Either that it works or that the documentation would tell me not to expect it to work

What did you see instead?

Can't find import

The Problem

I spent hours trying to get go/types to analyze some code, and none of the documentation said it didn't work with modules. Modules have been out for like 2 years at this point. Please put clear, hard to miss statements in the comments for go/types that it doesn't support code that uses modules.

Also (and probably something that can be fixed faster), please update the documentation on the examples the godoc links to here: https://github.com/golang/example/tree/master/gotypes so that it, too, says that it won't work with modules. No where did I see any indication that it wouldn't work, except when I posted on Twitter.

@mvdan
Copy link
Member

@mvdan mvdan commented Dec 14, 2020

Can you clarify how go/types doesn't work with modules? It's perhaps not trivial to set up correctly, but it certainly works with modules. Over half of the Go tools out there which support analyzing Go code with modules do use go/types.

@mvdan
Copy link
Member

@mvdan mvdan commented Dec 14, 2020

An example of a tool that supports modules and uses go/types directly: https://github.com/burrowers/garble/blob/2e2bd09b5e420455f51f7e1e1cbe46a11a6a7cf3/main.go#L409-L413

Granted that it's a complex file and not a very clear example, but just to prove a point. I agree that better docs would be nice, but they should definitely not say "does not support modules".

@natefinch
Copy link
Contributor Author

@natefinch natefinch commented Dec 14, 2020

hmm... lemme recheck what I was doing. I have an incredibly basic test that fails, but I may be making a simple mistake.

I heard from twitter that go/types didn't work with modules and ... probably should have double checked that assertion :) Lemme get back to you in a bit.

@mvdan
Copy link
Member

@mvdan mvdan commented Dec 14, 2020

I think the reply you got on Twitter is mostly right; most tools which just want to load Go packages should use x/tools/go/packages, which uses go/types under the hood and supports modules. But it's certainly possible to use go/types directly and support modules at the same time. Another way to think about it - if go/types simply did not support modules, how would go/packages support modules and expose go/types.Info etc?

@natefinch
Copy link
Contributor Author

@natefinch natefinch commented Dec 14, 2020

I think I have an idea of the problem I was having. I am running tests that parse go files that exist only for testing purposes (under /testdata). Those testdata files sometimes include 3rd party imports that the main program doesn't have, so they're not in the go.mod or go.sum. I presume this screws up go/types, since it doesn't do the auto-download of imports the way the build tools do.

@natefinch
Copy link
Contributor Author

@natefinch natefinch commented Dec 14, 2020

Previously I was just using go/ast to parse them, and it doesn't care about imports, so these tests worked fine.

@natefinch
Copy link
Contributor Author

@natefinch natefinch commented Dec 14, 2020

Yeah, ok, no, that's not it either.

here's my test:

$ go run main.go
2020/12/14 10:59:15 exec: echo foo
foo
failed to check types on files in ./foo/: foo/foo.go:4:2: could not import github.com/magefile/mage/sh (can't find import: "github.com/magefile/mage/sh")

main.go

package main

import (
	"fmt"
	"go/ast"
	"go/importer"
	"go/parser"
	"go/token"
	"go/types"

	"play.ground/foo"
)

func main() {
	foo.Bar()
	fset := token.NewFileSet()

	path := "./foo/"
	f, err := parser.ParseFile(fset, "foo/foo.go", nil, 0)
	if err != nil {
		panic(err)
	}

	info := &types.Info{
		Defs:  make(map[*ast.Ident]types.Object),
		Uses:  make(map[*ast.Ident]types.Object),
		Types: make(map[ast.Expr]types.TypeAndValue),
	}

	conf := types.Config{Importer: importer.Default()}
	if _, err = conf.Check(path, fset, []*ast.File{f}, info); err != nil {
		fmt.Printf("failed to check types on files in %s: %s\n", path, err)
	}
}

foo/foo.go

package foo

import (
	"github.com/magefile/mage/sh"
)

func Bar() {
	sh.RunV("echo", "foo")
}

go.mod

module play.ground

go 1.15

require github.com/magefile/mage v1.10.0

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
3 participants