vet: Respect PATH

[andrew.p...@gmail.com]

go vet fails to obey the standard PATH environment variable (POSIX, Windows). This makes it unnecessarily cumbersome to use go vet.

Here is an example (Mage) script to scan Go projects for variable shadowing:

func GoVetShadow(args ...string) error {
shadowPath, err := exec.LookPath("shadow")

if err != nil {
return err
}

return GoVet(fmt.Sprintf("-vettool=%s", shadowPath))
}

The shadow tool is unable to recurse over Go projects with the ordinary ./... syntax; It relies on go vet. And even with the x/tools shadow installed and on PATH, go vet nevertheless requires an absolute path to the program.

Please simplify both of these, so that the user can simply run:

go vet -vettool=shadow

Or:

shadow ./...

Which are easier to remember and lighter on the fingers.

Some gophers will point out that we already have a working solution with Mage, and it's conceivable that equivalents are available in related build systems like make (POSIX, GNU, BSD, etc.) or sh (POSIX, bash, zsh, etc.) However, the requirement for the user to provide an absolute path tends to create maintenance hassles, fragile build scripts, many portability problems, and general resource waste. PATH already provides the information; please stop breaking basic things at the process level.

[andrew.p...@gmail.com]

Similar request for gofmt. It should accept the ordinary ./... Go project recursion syntax, instead of the awkward, explicit:

func GoFmt(args ...string) error {
mg.Deps(CollectGoFiles)

for pth := range CollectedGoFiles {
cmd := exec.Command("gofmt")
cmd.Args = append(cmd.Args, args...)
cmd.Args = append(cmd.Args, pth)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

if err := cmd.Run(); err != nil {
return err
}
}

return nil
}

In other words, the user should be able to invoke gofmt on a large Go project with:

gofmt ./...

Looping over files is a simple enough concept for the tool to implement itself, directly. Looping over files in wrapping scripts, such as shell code, invites dragons.

[andrew.p...@gmail.com]

Same request for goimports.

func GoImports(args ...string) error {

mg.Deps(CollectGoFiles)

for pth := range CollectedGoFiles {

cmd := exec.Command("goimports")

cmd.Args = append(cmd.Args, args...)
cmd.Args = append(cmd.Args, pth)
cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr

if err := cmd.Run(); err != nil {
return err
}
}

return nil
}

Instead of the above nonsense, the user should be able to scan a large Go project with:

goimports ./...

Maybe these features have been added in recent Go versions. If so, please comment which specific Go versions finally added basic directory recursion to more of these tools.

[wagner riiffel]

On 5/21/2024 12:48 PM, andrew.p...@gmail.com wrote:
> Please simplify both of these, so that the user can simply run:
>
> go vet -vettool=shadow
>
> Or:
>
> shadow ./...

> In other words, the user should be able to invoke gofmt on a large Go
> project with:
>
> gofmt ./...
>

> Same request for goimports.
>

The "./..." syntax matches packages, not files and that's why tools
which work on files doesn't accept this syntax. The "go" program already
has "fmt" sub-command that do exactly this, run gofmt recursively on all
matched packages sources, so try "go fmt ./...". Analyzers already
accept packages and not files, so "shadow ./..." should work.

I think taking files instead of packages is somewhat legacy, but I found
it's useful, since we moved from GOPATH to go modules resolving packages
is somehow slow, I definitely feel a delay when using programs that
takes packages as an argument, that's very noticeable when you're in a
slow filesystem or storage device, so gofmt not taking 5 seconds (unlike
go fmt) to run is a good feature.

-w