String function is not viable in environments where executable go is unavailable

[dmitshur]

Consider the following (minified) example program that tries to print a value of type font.Metrics:

package main

import (
	"fmt"
	"log"

	"github.com/golang/freetype"
	"github.com/golang/freetype/truetype"
	"github.com/hexops/valast"
	"golang.org/x/image/font/gofont/gomono"
)

func main() {
	f, err := freetype.ParseFont(gomono.TTF)
	if err != nil {
		log.Fatalln(err)
	}

	face := truetype.NewFace(f, &truetype.Options{
		Size:              12,
		GlyphCacheEntries: 0,
		SubPixelsX:        1,
		SubPixelsY:        1,
	})
	fmt.Println("metrics =", valast.String(face.Metrics()))

	// Output:
	// metrics = err: go command required, not found: exec: "go": executable file not found in $PATH: stderr: 
}

(https://play.golang.org/p/6nQBdwaHo5q)

It prints:

metrics = err: go command required, not found: exec: "go": executable file not found in $PATH: stderr: 

When executed in an environment where an executable go is unavailable, such as inside a browser via WebAssembly (GOOS=js GOARCH=wasm), or inside the Go Playground.

It's possible to provide a custom PackagePathToName function via an option, such as:

fmt.Println("metrics =", valast.StringWithOptions(face.Metrics(), &valast.Options{
	PackagePathToName: func(path string) (string, error) { return pathpkg.Base(path), nil }, // TODO: Handle paths like example.com/foo/v2, where the name is 'foo' not 'v2'.
}))

Then it doesn't get the aforementioned error.

Is this intended behavior for String, or should valast know that GOOS=js GOARCH=wasm environment cannot execute a go binary and should automatically use a different PackagePathToName implementation, so that it's possible to use String?

Note that both spew.Dump(face.Metrics()) and goon.Dump(face.Metrics()) print an output without an error inside a browser.

[emidoots]

👋 hey! Nice to hear from you!

:( bummer, this is definitely unintentional. I didn’t think we depended on the go binary at all. I think that what may be happening is our usage of gofumpt has brought this in by accident: https://github.com/hexops/valast/blob/main/valast.go#L113

[dmitshur]

Heya! 😀 Sorry that it's via an issue report! 😅

this is definitely unintentional.

Thanks for confirming.

what may be happening is our usage of gofumpt

From what I can tell, this particular dependence on the go binary comes from DefaultPackagePathToName, which uses golang.org/x/tools/go/packages. When the packages.Load call with its current &packages.Config{Mode: packages.NeedName} config selects the goListDriver driver, that driver uses go list to load information about packages.

[emidoots]

Ahhh, that makes more sense. For context, through reflection we are able to get a types’ package import path - but we aren’t sure how we should refer to it canonically in code (import path != name, as you know) so that is what DefaultPackagePathToName is doing.

I wonder what go-spew and go-goon do in this case? I guess your original comment about using a different default implementation under ‘ GOOS=js GOARCH=wasm’ may be the right path forward!