x/tools/imports: ability to customize behavior without needing to fork

[dmitshur]

Currently, the golang.org/x/tools/imports features some internal configuration options to make it possible to adjust it to run (in a limited capacity) in environments where there is no raw filesystem access (for example, App Engine):

// importPathToName returns the package name for the given import path.
var importPathToName = importPathToNameGoPath

// loadExports returns a list exports for a package.
var loadExports = loadExportsGoPath

// findImport searches for a package with the given symbols.
// If no package is found, findImport returns "".
// Declared as a variable rather than a function so goimports can be easily
// extended by adding a file with an init function.
var findImport = findImportGoPath

There's a few code generation files like mkindex.go to help pre-compute static indices of packages for such environments.

However, since those variables are unexported, the only current way to make changes there is to fork the entire golang.org/x/tools/imports package.

It's slightly more convenient in that a new file can be added with init() that overrides the internal variables, but the rest of the package will still be a fork that needs to be maintained and kept up to date.

Feature Request

What do you think about adding a means to override those internal configuration funcs (or provide your own implementations) and pre-computed indices. Perhaps via:

// Or "Context" or "Configuration" or whatever name works best.
type Config struct {
    ImportPathToName func(importPath string) (packageName string)
    LoadExports      func(dir string) map[string]bool
    FindImport       func(pkgName string, symbols map[string]bool) (string, bool, error)
    Stdlib           map[string]string
}

func (c *Config) Process(filename string, src []byte, opt *Options) ([]byte, error) { ... }

// Process formats and adjusts imports for the provided file.
// If opt is nil the defaults are used.
func Process(filename string, src []byte, opt *Options) ([]byte, error) {
    return defaultConfig.Process(filename, src, opt)
}

Or something like that, to make it possible to import golang.org/x/tools/imports and configure it to one's needs, without needing to fork it.

Additionally, it'd be great if the findImport, etc. implementations that currently rely on raw filesystem access and import low level packages like os could be moved into a separate file with a build tag like // +build !nacl to prevent them from being included on systems where those low level packages are not available.

Some context on where I'd like to use this can seen in gopherjs/gopherjs.github.io@40f830c, where I had to fork imports and customize for the GopherJS Playground. Since the code runs in a browser, a static index is used, and unused code importing os and sync is removed.

[dmitshur]

By the way, I think this approach would be bad:

// Or "Context" or "Configuration" or whatever name works best.
type Config struct {
    ImportPathToName func(importPath string) (packageName string)
    LoadExports      func(dir string) map[string]bool
    FindImport       func(pkgName string, symbols map[string]bool) (string, bool, error)
    Stdlib           map[string]string
}

Because it makes internal implementation details a part of the API, making it harder to change.

Perhaps instead of exposing all internal details, imports could use build tags to have a fallback mode where it uses the pre-computed index of standard library for environments without raw filesystem access, and just a way of setting/adding to that pre-computed index.

[dmitshur]

I suspect the best way to resolve this issue might be to decide that "forking imports is the best way to customize its behavior", because it seems very hard to come up with a better way.

[bradfitz]

I'm fine with some Config type, especially if it simplifies our internal Google version a bit.

I'd also want to declare the API as unstable in the package docs so we're free to evolve it as needed for a bit.

[heschi]

Having never seen this issue, I did most of this incidentally when I refactored to create the Resolver interface. I'm not sure that it makes sense to allow users to implement it, though.

[dmitshur]

As an update from my side, I made this feature request 4 years ago only because the GopherJS Playground needed to fork this package. I haven't had any other need for it since then, and the GopherJS Playground fork isn't costly to maintain.

My suggestion from #12696 (comment) still stands. Maybe we should close this issue, since there isn't much demand for it? /cc @heschik

to create the Resolver interface

That exists in golang.org/x/tools/internal/imports as far as I see, not in golang.org/x/tools/imports that this issue is about. I hope it stays that way, to avoid committing to a public API that you can't change.

[heschi]

Yes, I don't think we want to allow users to implement Resolver. Given the lack of other traffic, let's close this.